> > Yup, it's already not too bad. I haven't looked into it in much
> > detail, but I'd like to reduce it even a bit more. In particular, the
> > backup_info field in the BlockDriverState feels wrong to me. In the
> > long term the generic block layer shouldn't know at all what a backup
> > is, and baking it into BDS couples it very tightly.
> 
> 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:

typedef struct BlockFilter {
    void *opaque;
    int cluster_size;
    int coroutine_fn (before_read)(BlockDriverState *bs,
        int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
        BdrvRequestFlags flags, void **cookie);
    int coroutine_fn (after_read)(BlockDriverState *bs,
        int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
        BdrvRequestFlags flags, void *cookie);
    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);
} BlockFilter;

struct BlockDriverState {
   ...
    QLIST_HEAD(, BlockFilters) filters;
};

Would that work for you?


Reply via email to