"Li, Liang Z" <liang.z...@intel.com> wrote:
>> > There are some flaws in qemu_put_compression_data, this patch tries to
>> > fix it. Now it can be used by other code.
>> >
>> > Signed-off-by: Liang Li <liang.z...@intel.com>
>> > ---
>> > migration/qemu-file.c | 10 +++++++++-
>> > 1 file changed, 9 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c index
>> > 0bbd257..ef9cd4a 100644
>> > --- a/migration/qemu-file.c
>> > +++ b/migration/qemu-file.c
>> > @@ -616,7 +616,9 @@ ssize_t qemu_put_compression_data(QEMUFile *f,
>> const uint8_t *p, size_t size,
>> > ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
>> >
>> > if (blen < compressBound(size)) {
>> > - return 0;
>> > + if (f->ops->writev_buffer || f->ops->put_buffer) {
>> > + qemu_fflush(f);
>> > + }
>> > }
>>
>> With your change, when we arrive here:
>>
>> - blen could still be smaller that compressBound(size), you need to
>> recheck
>> - blen could have changed, but you don't take that in account for the
>> following caller.
>>
>> So, I think code has a bug?
>
> Yes, there is a bug, I should consider the case QEMUFile with empty ops.
> The right code should be like:
>
> if (blen < compressBound(size)) {
> if (f->ops->writev_buffer || f->ops->put_buffer) {
> qemu_fflush(f);
> } else {
> return 0;
> }
> }
> ....
>
> It is enough?
No. We need something like:
if (blen < compressBound(size)) {
if (!f->ops->writev_buffer && !f->ops->put_buffer) {
return 0;
}
qemu_fflush(f);
blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
if (blen < compressBound(size)) {
return 0;
}
}
No?
>
> Liang
>
>
>
>>
>> Later, Juan.
