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