Am 05.07.2017 um 14:43 hat Eric Blake geschrieben: > On 07/04/2017 04:44 AM, Kevin Wolf wrote: > > Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben: > >> The following functions fail if bs->drv does not implement them: > >> > > >> @@ -511,6 +513,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, > >> HDGeometry *geo) > >> > >> if (drv && drv->bdrv_probe_geometry) { > >> return drv->bdrv_probe_geometry(bs, geo); > >> + } else if (drv && bs->file && bs->file->bs) { > >> + return bdrv_probe_geometry(bs->file->bs, geo); > >> } > >> > >> return -ENOTSUP; > > > > The probed geometry would refer to the physical image file, not to the > > actually presented image. So propagating this to bs->file is wrong for > > image formats. > > > > Possibly checking .is_filter in addition would be enough. > > In fact, this made me think: if we define .is_filter as an enum instead > of a bool, we could implement 0 for no filter, 1 for file filter, and 2 > for backing filter. Then instead of having to have 2 separate default > bdrv_get_block_status_for_*() generic functions in patch 4, we could > instead patch bdrv_co_get_block_status() to check .is_filter, and > automatically fall back to bs->file or bs->backing according to the enum > value stored in .is_filter.
After reviewing the rest of the series, I'm not completely convinced of using .is_filter any more. If you look at the patch for the raw format driver, it actually turns out that even though we would intuitively call it a filter, it has different properties than for example a throttle filter and passing through function calls to bs->file makes sense for a different set of functions. Kevin
pgp7SWFgOr71m.pgp
Description: PGP signature