From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52678) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ddFGX-00075a-8a for qemu-devel@nongnu.org; Thu, 03 Aug 2017 08:30:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ddFGV-00067a-Tb for qemu-devel@nongnu.org; Thu, 03 Aug 2017 08:30:13 -0400 Date: Thu, 3 Aug 2017 15:29:23 +0300 From: Manos Pitsidianakis Message-ID: <20170803122923.5wqeper3cqlsatky@postretch> References: <20170731095443.28211-1-el13635@mail.ntua.gr> <20170731095443.28211-5-el13635@mail.ntua.gr> <20170801154703.GD22017@stefanha-x1.localdomain> <20170801164933.c54ocjzr26sxaizy@postretch> <20170802103922.GE5531@stefanha-x1.localdomain> <20170802105704.m2jbxnivl32msbx6@postretch> <20170803080801.GC4456@dhcp-200-186.str.redhat.com> <20170803105359.GB22685@stefanha-x1.localdomain> <20170803111701.GG4456@dhcp-200-186.str.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="d35sbwz5dxtz6v37" Content-Disposition: inline In-Reply-To: <20170803111701.GG4456@dhcp-200-186.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel , Alberto Garcia , qemu-block --d35sbwz5dxtz6v37 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 03, 2017 at 01:17:01PM +0200, Kevin Wolf wrote: >Am 03.08.2017 um 12:53 hat Stefan Hajnoczi geschrieben: >> On Thu, Aug 03, 2017 at 10:08:01AM +0200, Kevin Wolf wrote: >> > Am 02.08.2017 um 12:57 hat Manos Pitsidianakis geschrieben: >> > > On Wed, Aug 02, 2017 at 11:39:22AM +0100, Stefan Hajnoczi wrote: >> > > > On Tue, Aug 01, 2017 at 07:49:33PM +0300, Manos Pitsidianakis wrot= e: >> > > > > On Tue, Aug 01, 2017 at 04:47:03PM +0100, Stefan Hajnoczi wrote: >> > > > > > On Mon, Jul 31, 2017 at 12:54:40PM +0300, Manos Pitsidianakis = wrote: >> > > > > > > ThrottleGroup is converted to an object. This will allow the= future >> > > > > > > throttle block filter drive easy creation and configuration = of throttle >> > > > > > > groups in QMP and cli. >> > > > > > > >> > > > > > > A new QAPI struct, ThrottleLimits, is introduced to provide = a shared >> > > > > > > struct for all throttle configuration needs in QMP. >> > > > > > > >> > > > > > > ThrottleGroups can be created via CLI as >> > > > > > > -object throttle-group,id=3Dfoo,x-iops-total=3D100,x-.. >> > > > > > > where x-* are individual limit properties. Since we can't ad= d non-scalar >> > > > > > > properties in -object this interface must be used instead. H= owever, >> > > > > > > setting these properties must be disabled after initializati= on because >> > > > > > > certain combinations of limits are forbidden and thus config= uration >> > > > > > > changes should be done in one transaction. The individual pr= operties >> > > > > > > will go away when support for non-scalar values in CLI is im= plemented >> > > > > > > and thus are marked as experimental. >> > > > > > > >> > > > > > > ThrottleGroup also has a `limits` property that uses the Thr= ottleLimits >> > > > > > > struct. It can be used to create ThrottleGroups or set the >> > > > > > > configuration in existing groups as follows: >> > > > > > > >> > > > > > > { "execute": "object-add", >> > > > > > > "arguments": { >> > > > > > > "qom-type": "throttle-group", >> > > > > > > "id": "foo", >> > > > > > > "props" : { >> > > > > > > "limits": { >> > > > > > > "iops-total": 100 >> > > > > > > } >> > > > > > > } >> > > > > > > } >> > > > > > > } >> > > > > > > { "execute" : "qom-set", >> > > > > > > "arguments" : { >> > > > > > > "path" : "foo", >> > > > > > > "property" : "limits", >> > > > > > > "value" : { >> > > > > > > "iops-total" : 99 >> > > > > > > } >> > > > > > > } >> > > > > > > } >> > > > > > > >> > > > > > > This also means a group's configuration can be fetched with = qom-get. >> > > > > > > >> > > > > > > ThrottleGroups can be anonymous which means they can't get a= ccessed by >> > > > > > > other users ie they will always be units instead of group (B= ecause they >> > > > > > > have one ThrottleGroupMember). >> > > > > > >> > > > > > blockdev.c automatically assigns -drive id=3D to the group nam= e if >> > > > > > throttling.group=3D wasn't given. So who will use anonymous s= ingle-drive >> > > > > > mode? >> > > > > >> > > > > Manual filter nodes. It's possible to not pass a group name valu= e and the >> > > > > resulting group will be anonymous. Are you suggesting to move th= is change to >> > > > > the throttle filter patch? >> > > > >> > > > What is the use case? Human users will stick to the legacy syntax >> > > > because it's convenient. Management tools will use the filter >> > > > explicitly in the future, and it's easy for them to choose a name. >> > > > >> > > > Unless there is a need for this case I'd prefer to make the group = name >> > > > mandatory. That way there are less code paths to worry about. >> > > >> > > I think Kevin requested this though I don't really remember the use = case. >> > >> > There is no legacy syntax for putting a throttle node anywhere but at >> > the root of a BlockBackend. If you want to throttle e.g. only a specif= ic >> > backing file, you need to manually create a throttle node. >> > >> > (We tend to forget this occasionally, but the work you're doing is not >> > only cleanup just for fun, but it's actually new features that enable >> > new use cases by making everything more flexible.) >> >> It's not clear to me from your reply whether you support anonymous >> throttle groups or not. It is possible to throttle arbitrary nodes in >> the graph either way. > >I think it would be nice for human users to have them, but on second >thought you're right that it's just syntactic sugar for an explicit >-object, so if you think there is any difficulty with supporting >anonymous groups, feel free to drop them. > >Hm, actually... Does this mean that then the whole 'limits' option in >the throttle driver could go away, with all of the parsing code, and the >group name becomes mandatory? That certainly does look tempting. Anonymous groups don't pose any difficulty. The only problem with groups=20 not created with -object or object-add in general is that their limits=20 cannot be set with qom-set afterwards; the throttle node will require a=20 reopen or replacement with a new one. This is a good argument against=20 doing throttle group manipulation in the driver. I don't know if that's=20 very user friendly though. --d35sbwz5dxtz6v37 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEvy2VxhCrsoeMN1aIc2J8L2kN9xAFAlmDFyMACgkQc2J8L2kN 9xD6GxAA2y9Qv3a0+ZlVtdbzikU5/6PA8X+yJ7mAfzM/ZyWyUbZYEvtecPXAOjNK h0WBd1mQQfgmSMJnm1lKejGrxXTMoNkzotyxDHwvXK2mI8cmH12XZS4IWkRCZz0+ fSjrmN8jXDv5VT1oorzk4o5aah9MyNFFcMGUy73+zcGa/Ndclcm0Jo5hOyrgninQ P6YcaQbLgIFYbd1FH7NhXMeRXkJDHSueHxt7CDa+NENFrOSIJ3jTk9CGqYMD8d5s tEBoO1usIsZacnbScev2cgnuBz2SmFgkjYOZa8xD0d6+WFhX9DQcqDEp8bE1Z+ZB IRhyddbLOzxXKu99Oi4LrmzswOXIhdRCw6ozJwEIxJhCTUEYizHS+fIJxEWS+4Bv CZwWkvxk2cAoCRm9DCBddstfKRfgKBQos1hMZawY+lKVR6jlCO4IXneCI7Ml1WOx YYrm50aoYV2VSri11D/kQoi3XUnAT17dWG3sa3+Ix8cTr/i4N9PCnhCWjkRI2rvq wWZ8pRiKFxvUqP/9WW0g8Zyd2gh5AMZFB+iVGJ6VmBOxygeykOJMfnBx1P7Ez1Wi 5uayX5cwPp2r5AaNx1Vb6wIk7trCBf0npLpa0HnPUMKctGZPeHty9XuXFPEDEnLX JFN09FcK5ksAnzoyGv2po90SqHQRCxEi0XunBulimcfCyBZNvx0= =+s7B -----END PGP SIGNATURE----- --d35sbwz5dxtz6v37--