From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxXhL-0007gj-FB for qemu-devel@nongnu.org; Fri, 21 Oct 2016 07:09:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxXhK-0003Mu-I0 for qemu-devel@nongnu.org; Fri, 21 Oct 2016 07:09:15 -0400 From: Markus Armbruster References: <1475246744-29302-1-git-send-email-berrange@redhat.com> <1475246744-29302-13-git-send-email-berrange@redhat.com> <87twchjw9z.fsf@dusky.pond.sub.org> <20161020142956.GD27909@redhat.com> Date: Fri, 21 Oct 2016 13:09:07 +0200 In-Reply-To: <20161020142956.GD27909@redhat.com> (Daniel P. Berrange's message of "Thu, 20 Oct 2016 15:29:56 +0100") Message-ID: <87funq2c30.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v14 12/21] option: allow qemu_opts_to_qdict to merge repeated options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= , qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz "Daniel P. Berrange" writes: > On Wed, Oct 12, 2016 at 07:46:00PM +0200, Markus Armbruster wrote: >> "Daniel P. Berrange" writes: >> >> > If given an option string such as >> > >> > size=1024,nodes=10,nodes=4-5,nodes=1-2,policy=bind >> > >> > the qemu_opts_to_qdict() method will currently overwrite >> > the values for repeated option keys, so only the last >> > value is in the returned dict: >> > >> > size=QString("1024") >> > nodes=QString("1-2") >> > policy=QString("bind") >> > >> > With this change the caller can optionally ask for all >> > the repeated values to be stored in a QList. In the >> > above example that would result in 'nodes' being a >> > QList, so the returned dict would contain >> > >> > size=QString("1024") >> > nodes=QList([QString("10"), >> > QString("4-5"), >> > QString("1-2")]) >> > policy=QString("bind") >> > >> > Note that the conversion has no way of knowing whether >> > any given key is expected to be a list upfront - it can >> > only figure that out when seeing the first duplicated >> > key. Thus the caller has to be prepared to deal with the >> > fact that if a key 'foo' is a list, then the returned >> > qdict may contain either a QString or a QList for the >> > key 'foo'. >> > >> > In a third mode, it is possible to ask for repeated >> > options to be reported as an error, rather than silently >> > dropping all but the last one. >> >> To serve as a replacement for the options visitor, this needs to be able >> to behave exactly the same together with a suitably hacked up QObject >> input visitor. Before I dive into the actual patch, let me summarize >> QemuOpts and options visitor behavior. [...] >> Unlike the options visitor, this patch (judging from your description) >> makes a list only when keys are repeated. The QObject visitor will have >> to cope with finding both scalars and lists. When it finds a scalar, >> but needs a list, it'll have to wrap it in a list (PATCH 09, I think). >> When it finds a list, but needs a scalar, it'll have to fish it out of >> the list (where is that?). > > If my code finds a list but wants a scalar, it is reporting an > error. I'm afraid that breaks things like --numa node,cpus=0,cpus=2,mem=512,nodeid=0,nodeid=0 The options visitor inteprets cpus=0,cpus=2 as a list [0,2] (because NumaNodeOptions member @cpus has a list type) and nodeid=0,nodid=0 as integer 0 (because member @nodeid has a scalar type).