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

[Fam Zheng <famz@redhat.com>]

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