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

[Kevin Wolf <kwolf@redhat.com>]

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.

> BTW, will such filters work with the new virtio-blk-data-plane?

Not initially, but I think as soon as data plane gets support for image
formats, filters would work as well.

Kevin