Re: [Qemu-devel] [PATCH v6 06/11] dump: add API to write dump header

[Qiao Nuohan <qiaonuohan@cn.fujitsu.com>]

Sorry for responsing late.

On 01/07/2014 07:38 PM, Laszlo Ersek wrote:
> The following fields in "dh" are left zero-filled:
> - timestamp
> - total_ram_blocks
> - device_blocks
> - written_blocks
> - current_cpu
>
> I guess we'll either overwrite them later or it's OK to leave them all
> zeroed.

Yes, they are leaved all zeroed here. Tools, like crash will get exact data
from dumped memory.

>
> Also... is it OK to write these fields to the file in host native byte
> order? What happens if an i686 / x86_64 target is emulated on a BE host?

I will add convert work in v7.

>
>> >  +
>> >  +    /* write sub header */
>> >  +    size = sizeof(KdumpSubHeader32);
>> >  +    kh = g_malloc0(size);
>> >  +
>> >  +    /* 64bit max_mapnr_64 */
>> >  +    kh->max_mapnr_64 = s->max_mapnr;
>> >  +    kh->phys_base = PHYS_BASE;
>> >  +    kh->dump_level = DUMP_LEVEL;
>> >  +
>> >  +    kh->offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size + size;
>> >  +    kh->note_size = s->note_size;
>> >  +
>> >  +    if (write_buffer(s->fd, s->flag_flatten, dh->block_size, kh, size)<  0) {
>> >  +        ret = -1;
>> >  +        goto out;
>> >  +    }
> - Same question about endianness as above.
>
> - Again, many fields left zeroed in "kh", but I guess that's OK.
>
> - I would prefer if you repeated the multiplication by
> DISKDUMP_HEADER_BLOCKS verbatim in the "offset" write_buffer() argument.

write_buffer(s->fd, s->flag_flatten, DISKDUMP_HEADER_BLOCKS * dh->block_size,
kh, size) ?

Yes, I should change it.

>
> - When this write_buffer() is directed to a regular file in non-flat
> mode, then the file might become sparse (you jump over a range of
> offsets with lseek() in write_buffer()). If the output has been opened
> by qemu itself (ie."file:....", in qmp_dump_guest_memory()), then due
> to the O_TRUNC we can't seek over preexistent data (and keep garbage in
> the file). When libvirt pre-opens the file (to send over the fd later),
> in doCoreDump(), it also passes O_TRUNC. OK.
>

Do you mean because of O_TRUNC，seek will exceed the end of the file that may 
cause some problem?

-- 
Regards
Qiao Nuohan