"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Wei Yang (richardw.y...@linux.intel.com) wrote:
>> We open a file with empty_ops for compress QEMUFile, which means this is
>> not writable.
>
> That explanation sounds reasonable; but I'm confused by the history of
> this; the code was added by Liang Li in :
>
> b3be289 qemu-file: Fix qemu_put_compression_data flaw
>
> ( https://www.mail-archive.com/qemu-devel@nongnu.org/msg368974.html )
>
> with almost exactly the opposite argument; can we figure out why?
Comment of that patch is wrong, code agrees with this patch.
The patch that went in does:
- return 0;
+ if (!qemu_file_is_writable(f)) {
+ return -1;
+ }
+ qemu_fflush(f);
+ blen = IO_BUF_SIZE - sizeof(int32_t);
+ if (blen < compressBound(size)) {
+ return -1;
+ }
i.e. it move from return 0 to return -1 if we are not able to write, or
if we are not able to get enough space.
>> @@ -756,14 +753,7 @@ ssize_t qemu_put_compression_data(QEMUFile *f, z_stream
>> *stream,
>> ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
>>
>> if (blen < compressBound(size)) {
>> - if (!qemu_file_is_writable(f)) {
>> - return -1;
>> - }
>> - qemu_fflush(f);
>> - blen = IO_BUF_SIZE - sizeof(int32_t);
>> - if (blen < compressBound(size)) {
>> - return -1;
>> - }
>> + return -1;
>> }
This moves from trying some things to just return -1.
And patch is ok. compression file has a file with empty_os, so
qemu_fflush() will not help at all.
Reviewed-by: Juan Quintela <quint...@redhat.com>
Thanks, Juan.
