Re: [Qemu-devel] [PATCH v0] fsdev: QMP interface for throttling

[Pradeep Jagadeesh]

On 3/20/2017 2:17 PM, Daniel P. Berrange wrote:
> On Mon, Mar 20, 2017 at 09:07:20AM -0400, Pradeep Jagadeesh wrote:
>
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 8819281..e4e3478 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1637,6 +1637,34 @@ ETEXI
> >      },
> >
> >  STEXI
> > address@hidden 9p_passwd @var{device} @var{password}
> > address@hidden 9p_passwd
> > +Set the encrypted device @var{device} password to @var{password}
> > +ETEXI
> > +
> > +    {
> > +        .name       = "fs9p_set_io_throttle",
> > +        .args_type  = 
> > "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
> > +        .params     = "device bps bps_rd bps_wr iops iops_rd iops_wr",
> > +        .help       = "change I/O throttle limits for a block drive",
> > +        .cmd        = hmp_9pfs_set_io_throttle,
> > +    },
> > +
> > +STEXI
> > address@hidden fs9p_set_io_throttle @var{device} @var{bps} @var{bps_rd} 
> > @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> > address@hidden 9pfs_set_io_throttle
> > +Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} 
> > @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> > +ETEXI
> > +
> > +    {
> > +        .name       = "set_password",
> > +        .args_type  = "protocol:s,password:s,connected:s?",
> > +        .params     = "protocol password action-if-connected",
> > +        .help       = "set spice/vnc password",
> > +        .cmd        = hmp_set_password,
> > +    },
> > +
> > +STEXI
>
> This looks all mangled wrt password related stuff
I did not understand the usage of the password.
Do we really need password here?.
I just followed how is it done in block devices.
> > diff --git a/qapi/9pfs.json b/qapi/9pfs.json
> > new file mode 100644
> > index 0000000..c068474
> > --- /dev/null
> > +++ b/qapi/9pfs.json
> > @@ -0,0 +1,169 @@
> > +# -*- Mode: Python -*-
> > +
> > +##
> > +# == QAPI 9p definitions
> > +##
> > +
> > +# QAPI common definitions
> > +{ 'include': 'common.json' }
> > +
> > +##
> > +# @fs9p_set_io_throttle:
> > +#
> > +# Change I/O limits for a 9p/fsdev device.
> > +#
> > +# Since QEMU 2.9, I/0 limits can be enabled on each  fsdev(9pfs) device
>
> Is this supposed to be generic for any fsdev impl, or specific to the
> 9p impl only ? Without knowing more, I'd expect us to really be designing
> something generic, and not tied to just the 9p impl. IOW,  having '9p' in
> the name of the QAPI commands feels wrong - use of 'fsdev' feels like the
> better naming. Same applies to the HMP commands.
Yes its for the fsdev. But other than 9p when the fsdev is used?
I will rename 9p to fsdev.

Regards,
Pradeep
> Regards,
> Daniel