Re: [Qemu-block] [PATCH v2] mirror: double performance of the bulk stage if the disc is full

[Vladimir Sementsov-Ogievskiy]

On 03.08.2016 13:52, Kevin Wolf wrote:
> Am 14.07.2016 um 19:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > Mirror can do up to 16 in-flight requests, but actually on full copy
> > (the whole source disk is non-zero) in-flight is always 1. This happens
> > as the request is not limited in size: the data occupies maximum available
> > capacity of s->buf.
> >
> > The patch limits the size of the request to some artificial constant
> > (1 Mb here), which is not that big or small. This effectively enables
> > back parallelism in mirror code as it was designed.
> >
> > The result is important: the time to migrate 10 Gb disk is reduced from
> > ~350 sec to 170 sec.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> This broke qemu-iotests 109 for raw. Can you please check whether the
> output changes are expected, and send a fix either for the code or for
> the test case?
>
> Kevin

As mirror becomes "more asynchronous" after this patch, in this test we 
have the following behaviour change:

old behaviour:
we create one sync request, which occupies all memory for in-flight 
requests, so it is only one in-flight request. It fails and we see 
BLOCK_JOB_COMPLETED (with zero progress) immediately after BLOCK_JOB_ERROR

new behaviour
several requests are created in parallel, so some of them are finished 
with success and we see BLOCK_JOB_COMPLETED with some progress..

I'm afaraid that this "some progress" may depend on disk speed or other 
environment factors, so simply update 109.out to new value is not good 
solution.

Anyway, I think
+         if (s->ret < 0) {
+             return 0;
+         }

should be added before io request creation in mirror_iteration - to not 
create new async reqs after error. This reduce "some progress", but is 
is still not zero, as some of async req's was started before an error 
has occured. Moreover, some req's was finished with success before this 
error for first chank has occured.

So, the progress of the job in case of error becomes unpredicatable and 
should be filtered in io tests.

--
Best regards,
Vladimir