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

[Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>]

Marcelo Tosatti wrote:
> On Tue, Nov 30, 2010 at 06:28:55PM +0900, Yoshiaki Tamura wrote:
>> 2010/11/30 Marcelo Tosatti<mtosatti@redhat.com>:
>>> 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<tamura.yoshiaki@lab.ntt.co.jp>
>>>> Signed-off-by: OHMURA Kei<ohmura.kei@lab.ntt.co.jp>
>>>
>>>> +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?

Thanks.  I would fix it.

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

I think you're mentioning about the changes in migration.c?

Yoshi