On Wed, Jul 1, 2015 at 3:47 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
> On 01/07/2015 16:45, Stefan Hajnoczi wrote:
>> If mirror has more free buffers than IOV_MAX, preadv(2)/pwritev(2)
>> EINVAL failures may be encountered.
>>
>> It is possible to trigger this by setting granularity to a low value
>> like 8192.
>>
>> This patch stops appending chunks once IOV_MAX is reached.
>>
>> The spurious EINVAL failure can be reproduced with a qcow2 image file
>> and the following QMP invocation:
>>
>> qmp.command('drive-mirror', device='virtio0', target='/tmp/r7.s1',
>> granularity=8192, sync='full', mode='absolute-paths',
>> format='raw')
>>
>> While the guest is running dd if=/dev/zero of=/var/tmp/foo oflag=direct
>> bs=4k.
>>
>> Cc: Jeff Cody <jc...@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
>> ---
>> block/mirror.c | 4 ++++
>> trace-events | 1 +
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 048e452..985ad00 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -241,6 +241,10 @@ static uint64_t coroutine_fn
>> mirror_iteration(MirrorBlockJob *s)
>> trace_mirror_break_buf_busy(s, nb_chunks, s->in_flight);
>> break;
>> }
>> + if (IOV_MAX < nb_chunks + added_chunks) {
>
> No Yoda conditions... apart from that,
>
> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
I found it annoying to write it backwards too, but it's for consistency:
if (s->buf_free_count < nb_chunks + added_chunks) {
trace_mirror_break_buf_busy(s, nb_chunks, s->in_flight);
break;
}
if (IOV_MAX < nb_chunks + added_chunks) {
trace_mirror_break_iov_max(s, nb_chunks, added_chunks);
break;
}
It's the same type of check as s->buf_free_count (which isn't modified
by this loop either so it's a yoda conditional).
