From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35382) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdJth-0000d0-Id for qemu-devel@nongnu.org; Wed, 01 Apr 2015 10:45:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YdJte-0000hu-8i for qemu-devel@nongnu.org; Wed, 01 Apr 2015 10:45:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38347) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdJte-0000hf-1b for qemu-devel@nongnu.org; Wed, 01 Apr 2015 10:45:34 -0400 Date: Wed, 1 Apr 2015 22:44:51 +0800 From: Fam Zheng Message-ID: <20150401144451.GD2777@fam-t430.nay.redhat.com> References: <3060954d506be499c26d07ba4624dd33cd7b002d.1427732020.git.berto@igalia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3060954d506be499c26d07ba4624dd33cd7b002d.1427732020.git.berto@igalia.com> Subject: Re: [Qemu-devel] [PATCH 4/7] throttle: Add throttle group support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Mon, 03/30 19:19, Alberto Garcia wrote: > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 7873084..d8211b7 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -990,6 +990,8 @@ > # > # @iops_size: #optional an I/O size in bytes (Since 1.7) > # > +# @group: #optional throttle group name (Since 2.3) We should probably elaborate (here, and at other places of @group appearances): @device is used as group name. This is useful since other devices could use this device name as the group name, for exmple the following -drive definition: ... -drive file=foo,id=foo,bps=100 \ -drive file=bar,id=foo,bps=200,group=foo \ ... will put both devices into the same group. Also, as the two bps values don't match, I assume the last one is used? Is there a warning when ignoring a config from previous definitions? Thinking about this, I'd slightly prefer a canonical throttle group definition rather than patching the existing parameters: -object throttle-group,id=tg0,bps=100,iops=200,iops-max=1000 \ -drive file=foo,id=foo,throttle-group=tg0 \ -drive file=bar,id=bar,throttle-group=tg0 \ and error out if "bps=" etc are specified together with "throttle-group=" in -drive. And in QMP, add block_set_io_throttle_group which works together with object-add. What do you think? > +# > # Returns: Nothing on success > # If @device is not a valid block device, DeviceNotFound > # > @@ -1001,7 +1003,7 @@ > '*bps_max': 'int', '*bps_rd_max': 'int', > '*bps_wr_max': 'int', '*iops_max': 'int', > '*iops_rd_max': 'int', '*iops_wr_max': 'int', > - '*iops_size': 'int' } } > + '*iops_size': 'int', '*group': 'str' } } > > ## > # @block-stream: > diff --git a/qemu-options.hx b/qemu-options.hx > index 319d971..7d6b2ec 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -464,6 +464,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, > " [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n" > " [[,iops_max=im]|[[,iops_rd_max=irm][,iops_wr_max=iwm]]]\n" > " [[,iops_size=is]]\n" > + " [[,group=g]]\n" > " use 'file' as a drive image\n", QEMU_ARCH_ALL) > STEXI > @item -drive @var{option}[,@var{option}[,@var{option}[,...]]] > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 3a42ad0..ce897ff 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1730,7 +1730,7 @@ EQMP > > { > .name = "block_set_io_throttle", > - .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,iops_size:l?", > + .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,iops_size:l?,group:s?", > .mhandler.cmd_new = qmp_marshal_input_block_set_io_throttle, > }, > > @@ -1756,6 +1756,7 @@ Arguments: > - "iops_rd_max": read I/O operations max (json-int) > - "iops_wr_max": write I/O operations max (json-int) > - "iops_size": I/O size in bytes when limiting (json-int) Why removing these three? > +- "group": throttle group name (json-string) > > Example: > > -- > 2.1.4 > >