qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Date: Thu, 06 Sep 2012 12:18:41 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0

Il 06/09/2012 12:07, Kevin Wolf ha scritto:
>> The AIOCB is already invalid at the time the callback is entered, so we
>> could release it before the call.  However, not all implementation of
>> AIO are ready for that and I'm not really in the mood for large scale
>> refactoring...
> 
> But the way, what I'd really want to see in the end is to get rid of
> qemu_aio_flush() and replace it by .bdrv_drain() callbacks in each
> BlockDriver. The way we're doing it today is a layering violation.

That's quite difficult.  Completion of an I/O operation can trigger
another I/O operation on another block device, and so on until we go
back to the first device (think of a hypothetical RAID-5 device).

> Doesn't change anything about this problem, though. So the options that
> we have are:
> 
> 1. Delay the callback using a BH. Doing this in each driver is ugly.
>    But is there actually more than one possible callback in today's
>    coroutine world? I only see bdrv_co_io_em_complete(), which could
>    reenter the coroutine from a BH.

Easy and safe, but it feels a bit like a timebomb.  Also, I'm not
entirely sure of _why_ the bottom half works. :)

> 2. Delay the callback by just calling it later when the cleanup has
>    been completed and .io_flush() can return 0. You say that it's hard
>    to implement for some drivers, except if the AIOCB are leaked until
>    the end of functions like qcow2_create().

... which is what we do in posix-aio-compat.c; nobody screamed so far.

Not really hard, it just has to be assessed for each driver separately.
 We can just do it in gluster and refactor it later.

> 3. Add a delay only later in functions like bdrv_drain_all() that assume
>    that the request has completed. Are there more of this type? AIOCBs
>    are leaked until a bdrv_drain_all() call. Does it work with draining
>    specific BDSes instead of everything?
> 
> Unless I forgot some important point, it almost looks like option 1 is
> the easiest and safest.

I agree with your opinion, but I would feel better if I understood
better why it works.  (2) can be done easily in each driver (no
ugliness) and refactored later.

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]