On Mon, Jun 26, 2017 at 07:01:18PM +0300, Manos Pitsidianakis wrote: > On Mon, Jun 26, 2017 at 03:30:55PM +0100, Stefan Hajnoczi wrote: > > > +static BlockDriver bdrv_throttle = { > > > + .format_name = "throttle", > > > + .protocol_name = "throttle", > > > + .instance_size = sizeof(ThrottleGroupMember), > > > + > > > + .bdrv_file_open = throttle_open, > > > + .bdrv_close = throttle_close, > > > + .bdrv_co_flush = throttle_co_flush, > > > + > > > + .bdrv_child_perm = bdrv_filter_default_perms, > > > + > > > + .bdrv_getlength = throttle_getlength, > > > + > > > + .bdrv_co_preadv = throttle_co_preadv, > > > + .bdrv_co_pwritev = throttle_co_pwritev, > > > + > > > + .bdrv_co_pwrite_zeroes = throttle_co_pwrite_zeroes, > > > + .bdrv_co_pdiscard = throttle_co_pdiscard, > > > + > > > + .bdrv_recurse_is_first_non_filter = > > > bdrv_recurse_is_first_non_filter, > > > + > > > + .bdrv_attach_aio_context = throttle_attach_aio_context, > > > + .bdrv_detach_aio_context = throttle_detach_aio_context, > > > + > > > + .bdrv_reopen_prepare = throttle_reopen_prepare, > > > + .bdrv_reopen_commit = throttle_reopen_commit, > > > + .bdrv_reopen_abort = throttle_reopen_abort, > > > + > > > + .is_filter = true, > > > +}; > > > > Missing: > > bdrv_co_get_block_status() > > bdrv_truncate() > > bdrv_get_info() > > bdrv_probe_blocksizes() > > bdrv_probe_geometry() > > bdrv_media_changed() > > bdrv_eject() > > bdrv_lock_medium() > > bdrv_co_ioctl() > > > > See block/raw-format.c. > > > > I think most of these could be modified in block.c or block/io.c to > > automatically call bs->file's function if drv doesn't implement them. > > This way all block drivers would transparently pass them through by > > default and block/raw-format.c code could be eliminated. > > Are these truly necessary? Because other filter drivers (ie quorum, > blkverify) don't implement them.
Both quorum and blkverify are rarely used. This explains why the issue hasn't been found yet. These are the callbacks I identified which do not automatically forward to bs->file. Therefore the throttle driver will break these features when bs->file supports them. That's why I suggest forwarding to bs->file in block.c. Then individual drivers do not have to implement these callbacks just to forward to bs->file. And if the driver wishes to prohibit a feature, it can implement the callback and return -ENOTSUP. You can send this fix as a separate patch series, independent of the throttle driver. Once it has been merged the throttle driver will gain support for these features. Stefan
signature.asc
Description: PGP signature