Re: [Qemu-devel] [PATCH 1/2] mirror: Rewrite mirror_iteration

[Fam Zheng]

On Wed, 11/18 11:38, Paolo Bonzini wrote:
> 
> 
> On 17/11/2015 12:41, Fam Zheng wrote:
> > +        /* Wait for I/O to this cluster (from a previous iteration) to be
> > +         * done.*/
> > +        while (test_bit(next_chunk, s->in_flight_bitmap)) {
> > +            trace_mirror_yield_in_flight(s, next_sector, s->in_flight);
> > +            s->waiting_for_io = true;
> > +            qemu_coroutine_yield();
> > +            s->waiting_for_io = false;
> > +        }
> > +
> 
> I think this could just "break" if nb_chunks > 0, like
> 
>       if (test_bit(next_chunk, s->in_flight_bitmap)) {
>               if (nb_chunks > 0) {
>                       break;
>               }
>               mirror_wait_for_io(s);
>               /* Now retry.  */
>       } else {
>               hbitmap_next = hbitmap_iter_next(&s->hbi);
>               assert(hbitmap_next == next_sector);
>               nb_chunks++;
>       }
> 
> but it can be done later (the usage of mirror_wait_for_io is a hint :)).
> 
> There's a typo though:
> 
> > +    /* Clear dirty bits before querying the block status, because
> > +     * calling bdrv_reset_dirty_bitmap could yield - if some blocks are 
> > marked
> 
> That's bdrv_get_block_status_above.
> 
> > +     * dirty in this window, we need to know.
> > +     */
> 

Thanks. I'll adopt both suggestions and resend.

Fam