[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter dri
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver |
Date: |
Tue, 27 Jun 2017 13:42:05 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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
- Re: [Qemu-block] [PATCH RFC v3 2/8] block: Add aio_context field in ThrottleGroupMember, (continued)
[Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Manos Pitsidianakis, 2017/06/23
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Manos Pitsidianakis, 2017/06/26
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Stefan Hajnoczi, 2017/06/26
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Manos Pitsidianakis, 2017/06/26
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Stefan Hajnoczi, 2017/06/27
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Manos Pitsidianakis, 2017/06/27
- Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Stefan Hajnoczi, 2017/06/28
Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Stefan Hajnoczi, 2017/06/26
Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver, Kevin Wolf, 2017/06/28
[Qemu-block] [PATCH RFC v3 1/8] block: move ThrottleGroup membership to ThrottleGroupMember, Manos Pitsidianakis, 2017/06/23