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

[Qiao Nuohan]

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