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

[Laszlo Ersek <lersek@redhat.com>]

On 01/13/14 11:03, Qiao Nuohan wrote:
> Sorry for responsing late.
> 
> On 01/07/2014 07:38 PM, Laszlo Ersek wrote:

>>> >  +    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;
>>> >  +    }

>> - 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.

Yes that's what I meant.

> 
>>
>> - 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?

I meant that lseek() would seek over an unwritten portion of the file.
If that portion had any kind of data written into it earlier, then that
data would now likely turn into garbage (lose meaning, become truncated
etc.) It wouldn't be corrupted or anything like that, it would just
become a leftover with potential to cause misinterpretation.

But, since we have O_TRUNC at open() time, we're seeking past the end of
the file, and this sought-over portion will read back as zeroes (and the
file might become "sparse", dependent on the filesystem and the size of
the range sought-over).

Seeking past the end of the file is explicitly allowed by POSIX:

    The lseek() function shall allow the file offset to be set beyond
    the end of the existing data in the file. If data is later written
    at this point, subsequent reads of data in the gap shall return
    bytes with the value 0 until data is actually written into the gap.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/lseek.html

So this is fine.

Thanks
Laszlo