Re: [Qemu-devel] [PATCH 1/5] RFC: Efficient VM backup for qemu (v1)

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 23.11.2012 10:05, schrieb Dietmar Maurer:
>>>>> My plan was to have something like bs->job->job_type-
>>>>> {before,after}_write.
>>>>>
>>>>>    int coroutine_fn (*before_write)(BlockDriverState *bs,
>>>>>         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
>>>>>         void **cookie);
>>>>>    int coroutine_fn (*after_write)(BlockDriverState *bs,
>>>>>         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
>>>>>         void *cookie);
>>>>
>>>> I don't think that job is the right place. Instead I would put a list
>>>> of filters into
>>>> BDS:
>>>
>>> Well, I can also add it to job_type. Just tell me what you prefer, and I 
>>> will
>>> write the patch.
>
> A block filter shouldn't be tied to a job, I think. We have things like
> blkdebug that are really filters and aren't coupled with a job, and on
> the other hand we want to generalise "block jobs" into just "jobs", so
> adding block specific things to job_type would be a step in the wrong
> direction.
>
> I also think that before_write/after_write isn't a convenient interface,
> it brings back much of the callback-based AIO cruft and passing void*
> isn't nice anyway. It's much nice to have a single .bdrv_co_write
> callback that somewhere in the middle calls the layer below with a
> simple function call.
>
> Also read/write aren't enough, for a full filter interface you
> potentially also need flush, discard and probably most other operations.
>
> This is why I suggested using a regular BlockDriver struct for filters,
> it already has all functions that are needed.

Let me elaborate a bit.

A block backend is a tree of block driver instances (BlockDriverState).
Common examples:

        raw             qcow2           qcow2
         |              /   \           /   \
        file         file   raw      file  qcow2
                             |             /   \
                            file         file  raw
                                                |
                                               file

A less common example:

        raw
         |
      blkdebug
         |
        file

Here, "blkdebug" acts as a filter, i.e. a block driver that can be put
between two adjacent tree nodes.  It injects errors by selectively
failing some bdrv_aio_readv() and bdrv_aio_writev() operations.

Actually, "raw" could also be viewed as a degenerate filter that does
nothing[*], but such a filter isn't particularly useful.

Except perhaps to serve as base for real filters, that do stuff.  To do
stuff in your filter, you'd replace raw's operations with your own.

Hmm, blkdebug implements much fewer operations than raw.  Makes me
wonder whether it works only in special places in the tree now.

[...]


[*] Except occasionally inject bugs when somebody adds new BlockDriver
operations without updating "raw" to forward them.