[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 8/9] dump: Add API to write dump pages
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5 8/9] dump: Add API to write dump pages |
Date: |
Tue, 16 Jul 2013 10:43:58 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Jul 09, 2013 at 03:30:13PM +0800, Qiao Nuohan wrote:
> if test "$seccomp" != "no" ; then
> @@ -3872,6 +3914,14 @@ if test "$glx" = "yes" ; then
> echo "GLX_LIBS=$glx_libs" >> $config_host_mak
> fi
>
> +if test "$lzo" = "yes" ; then
> + echo "CONFIG_LZO=y" >> $config_host_mak
> +fi
> +
> +if test "$snappy" = "yes" ; then
> + echo "CONFIG_SNAPPY=y" >> $config_host_mak
> +fi
Please also include a run-time check so QEMU can produce an error when a
user chooses a compression algorithm which is not built in. For
example, the user should get a clear error when they select Snappy but
QEMU was built without Snappy support.
> +static size_t get_len_buf_out(size_t page_size, uint32_t flag_compress)
> +{
> + size_t len_buf_out_zlib, len_buf_out_lzo, len_buf_out_snappy;
> + size_t len_buf_out;
> +
> + /* init buf_out */
> + len_buf_out_zlib = len_buf_out_lzo = len_buf_out_snappy = 0;
> +
> + /* buf size for zlib */
> + len_buf_out_zlib = compressBound(page_size);
> +
> + /* buf size for lzo */
> +#ifdef CONFIG_LZO
> + if (flag_compress & DUMP_DH_COMPRESSED_LZO) {
> + if (lzo_init() != LZO_E_OK) {
> + /* return 0 to indicate lzo is unavailable */
> + return 0;
> + }
> + }
> +
> + len_buf_out_lzo = page_size + page_size / 16 + 64 + 3;
Please introduce constants for these magic numbers. I don't know what
they mean.
> +/*
> + * check if the page is all 0
> + */
> +static inline bool is_zero_page(unsigned char *buf, long page_size)
QEMU has an optimized buffer_is_zero() function which you can use
instead.
> +
> +static int write_dump_pages(DumpState *s)
> +{
> + int ret = 0;
> + DataCache page_desc, page_data;
> + size_t len_buf_out, size_out;
> + unsigned char *buf_out = NULL;
> + off_t offset_desc, offset_data;
> + PageDesc pd, pd_zero;
> + uint64_t pfn_start, pfn_end, pfn;
> + unsigned char buf[s->page_size];
> + MemoryMapping *memory_mapping;
> + bool zero_page;
> +
> + prepare_data_cache(&page_desc, s);
> + prepare_data_cache(&page_data, s);
> +
> + /* prepare buffer to store compressed data */
> + len_buf_out = get_len_buf_out(s->page_size, s->flag_compress);
> + if (len_buf_out == 0) {
> + dump_error(s, "dump: failed to get length of output buffer.\n");
> + goto out;
This goto jumps over the declaration of wrkmem. The g_free(wrkmem)
below will result in undefined behavior! Please define wrkmem above and
initialize it to NULL.
> @@ -130,6 +139,13 @@ typedef struct DataCache {
> off_t offset; /* offset of the file */
> } DataCache;
>
> +typedef struct PageDesc {
> + off_t offset; /* the offset of the page data*/
The guest may be 32-bit or 64-bit, independently of the QEMU host
wordsize. Is off_t correct when running a 64-bit guest on a 32-bit
host?
I guess you are assuming off_t == uint64_t here?
- [Qemu-devel] [PATCH v5 2/9] dump: Add API to write header of flatten format, (continued)
- [Qemu-devel] [PATCH v5 2/9] dump: Add API to write header of flatten format, Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 3/9] dump: Add API to write vmcore, Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 7/9] dump: Add APIs to operate DataCache, Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 4/9] dump: Add API to write elf notes to buffer, Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 6/9] dump: Add API to write dump_bitmap, Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 9/9] dump: Make kdump-compressed format available for 'dump-guest-memory', Qiao Nuohan, 2013/07/09
- [Qemu-devel] [PATCH v5 8/9] dump: Add API to write dump pages, Qiao Nuohan, 2013/07/09
- Re: [Qemu-devel] [PATCH v5 8/9] dump: Add API to write dump pages,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH v5 5/9] dump: add API to write dump header, Qiao Nuohan, 2013/07/09
- Re: [Qemu-devel] [PATCH v5 0/9] Make 'dump-guest-memory' dump in kdump-compressed format, Qiao Nuohan, 2013/07/09