[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] migration/compress: compress QEMUFile is not writable
From: |
Juan Quintela |
Subject: |
Re: [PATCH 1/2] migration/compress: compress QEMUFile is not writable |
Date: |
Mon, 27 Jan 2020 16:32:01 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Wei Yang (address@hidden) 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/address@hidden/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 <address@hidden>
Thanks, Juan.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 1/2] migration/compress: compress QEMUFile is not writable,
Juan Quintela <=