Re: [Qemu-devel] Re: [PATCH 09/21] Introduce event-tap.

[Marcelo Tosatti]

On Tue, Nov 30, 2010 at 06:28:55PM +0900, Yoshiaki Tamura wrote:
> 2010/11/30 Marcelo Tosatti <address@hidden>:
> > On Thu, Nov 25, 2010 at 03:06:48PM +0900, Yoshiaki Tamura wrote:
> >> event-tap controls when to start FT transaction, and provides proxy
> >> functions to called from net/block devices.  While FT transaction, it
> >> queues up net/block requests, and flush them when the transaction gets
> >> completed.
> >>
> >> Signed-off-by: Yoshiaki Tamura <address@hidden>
> >> Signed-off-by: OHMURA Kei <address@hidden>
> >
> >> +static void event_tap_alloc_blk_req(EventTapBlkReq *blk_req,
> >> +                                    BlockDriverState *bs, BlockRequest 
> >> *reqs,
> >> +                                    int num_reqs, 
> >> BlockDriverCompletionFunc *cb,
> >> +                                    void *opaque, bool is_multiwrite)
> >> +{
> >> +    int i;
> >> +
> >> +    blk_req->num_reqs = num_reqs;
> >> +    blk_req->num_cbs = num_reqs;
> >> +    blk_req->device_name = qemu_strdup(bs->device_name);
> >> +    blk_req->is_multiwrite = is_multiwrite;
> >> +
> >> +    for (i = 0; i < num_reqs; i++) {
> >> +        blk_req->reqs[i].sector = reqs[i].sector;
> >> +        blk_req->reqs[i].nb_sectors = reqs[i].nb_sectors;
> >> +        blk_req->reqs[i].qiov = reqs[i].qiov;
> >> +        blk_req->reqs[i].cb = cb;
> >> +        blk_req->reqs[i].opaque = opaque;
> >> +        blk_req->cb[i] = reqs[i].cb;
> >> +        blk_req->opaque[i] = reqs[i].opaque;
> >> +    }
> >> +}
> >
> > bdrv_aio_flush should also be logged, so that guest initiated flush is
> > respected on replay.
> 
> In the current implementation w/o flush logging, there might be
> order inversion after replay?
> 
> Yoshi

Yes, since a vcpu is allowed to continue after synchronization is
scheduled via a bh. For virtio-blk, for example:

1) bdrv_aio_write, event queued.
2) bdrv_aio_flush
3) bdrv_aio_write, event queued.

On replay, there is no flush between the two writes.

Why can't synchronization be done from event-tap itself, synchronously,
to avoid this kind of problem?

The way you hook synchronization into savevm seems unclean. Perhaps
better separation between standard savevm path and FT savevm would make
it cleaner.