Re: [Qemu-devel] [PATCH] 9pfs: add support for IO limits to 9p-local driver

[Greg Kurz <groug@kaod.org>]

[-- Attachment #1: Type: text/plain, Size: 3040 bytes --]

On Wed, 7 Sep 2016 16:36:12 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 09/07/2016 07:39 AM, Pradeep wrote:
> > Uses throttling APIs to limit I/O bandwidth and number of operations on the 
> > devices which use 9p-local driver.
> > 
> > Signed-off-by: Pradeep <pradeep.jagadeesh@huawei.com>
> >  fsdev/file-op-9p.h      |   3 +
> >  fsdev/qemu-fsdev-opts.c |  52 +++++++++++++
> >  hw/9pfs/9p-local.c      |  18 ++++-
> >  hw/9pfs/9p-throttle.c   | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/9pfs/9p-throttle.h   |  55 +++++++++++++
> >  hw/9pfs/9p.c            |   7 ++
> >  hw/9pfs/Makefile.objs   |   1 +
> >  7 files changed, 333 insertions(+), 2 deletions(-)
> >  create mode 100644 hw/9pfs/9p-throttle.c
> >  create mode 100644 hw/9pfs/9p-throttle.h
> > 
> > Hi All,
> > 
> > This adds the support for the 9p-local driver.
> > For now this functionality can be enabled only through qemu cli options.
> > QMP interface and support to other drivers need further extensions.
> > To make it simple for other drivers, the throttle code has been put in
> > separate files.  
> 
> CLI-only extensions mean that we can't use it during hotplug.  Are you
> sure that blockdev-add won't automatically enable these options, given
> that it tries to convert the dictionary passed through QMP back into the
> QemuOpts form that matches command line parsing?  Also, aren't these

9p-local is fsdev, not blockdev: it has its own QemuOpts and cannot be
passed to blockdev-add (and BTW, there isn't any fsdev-add either).

> limits very similar to what is already available in the throttle driver?

Yes, IIUC this patch tries to implement similar throttling concepts to the
read/write operations in fsdev.

More details on the motivation here:

https://lists.gnu.org/archive/html/qemu-discuss/2016-04/msg00064.html

> Why are we repeating them at the 9p driver, instead of putting a
> throttle group BDS in the mix?
> 

Not sure if it is possible to mix fsdev and blockdev... to be honest, I
really don't know how one would achieve that :)

> > +++ b/fsdev/qemu-fsdev-opts.c
> > @@ -37,6 +37,58 @@ static QemuOptsList qemu_fsdev_opts = {
> >          }, {
> >              .name = "sock_fd",
> >              .type = QEMU_OPT_NUMBER,
> > +        }, {
> > +            .name = "bps",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "total bytes burst",
> > +        }, {
> > +            .name = "bps_rd",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "total bytes read burst",
> > +        }, {  
> 
> 
> At the very least, if we must have multiple throttle implementations,
> can they at least share code so that we don't forget to update one when
> adding another option to the other?
> 

This is experimental work. If it turns out that fsdev and blockdev
throttling are close enough, I agree that they should probably share
code. But at this point, I guess they should remain distinct.

Cheers.

--
Greg

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]