Kevin Wolf <kw...@redhat.com> 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.