From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39350) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WT2Ue-0005cK-N6 for qemu-devel@nongnu.org; Thu, 27 Mar 2014 01:04:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WT2UY-0008Mw-M1 for qemu-devel@nongnu.org; Thu, 27 Mar 2014 01:04:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34943) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WT2UY-0008Ml-Dd for qemu-devel@nongnu.org; Thu, 27 Mar 2014 01:04:38 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2R54a0H030792 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 Mar 2014 01:04:37 -0400 Date: Thu, 27 Mar 2014 13:04:33 +0800 From: Amos Kong Message-ID: <20140327050433.GC7531@amosk.info> References: <1394073416-12578-1-git-send-email-akong@redhat.com> <1394073416-12578-3-git-send-email-akong@redhat.com> <5318E743.7070609@redhat.com> <878usms5a6.fsf@blackfin.pond.sub.org> <20140320140312.GA24041@amosk.info> <87fvm55cex.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87fvm55cex.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: pbonzini@redhat.com, libvirt-list@redhat.com, qemu-devel@nongnu.org, jyang@redhat.com, lcapitulino@redhat.com On Wed, Mar 26, 2014 at 02:15:18PM +0100, Markus Armbruster wrote: > Amos Kong writes: > > > On Fri, Mar 07, 2014 at 10:54:09AM +0100, Markus Armbruster wrote: > >> Eric Blake writes: > >> > >> > On 03/05/2014 07:36 PM, Amos Kong wrote: > >> >> vm_config_groups[] only contains part of the options which have > >> >> argument, and all options which have no argument aren't added > >> >> to vm_config_groups[]. Current query-command-line-options only > >> >> checks options from vm_config_groups[], so some options will > >> >> be lost. > >> >> > >> >> We have macro in qemu-options.hx to generate a table that > >> >> contains all the options. This patch tries to query options > >> >> from the table. > >> >> > >> >> Then we won't lose the legacy options that weren't added to > >> >> vm_config_groups[] (eg: -vnc, -smbios). The options that have > >> >> no argument will also be returned (eg: -enable-fips) > >> >> > >> >> Some options that have argument have a NULL desc list, some > >> >> options don't have argument, and "parameters" is mandatory > >> >> in the past. So we add a new field "argument" to present > >> >> if the option takes unspecified arguments. > >> > > >> > I like Markus' suggestion of naming the new field > >> > 'unspecified-parameters' rather than 'argument'. > > > > Hi Markus, > > > >> Looking again, there are more problems. > >> > >> 1. Non-parameter argument vs. parameter argument taking unspecified > >> parameters > >> > >> Example: -device takes unspecified parameters. -cdrom doesn't take > >> parameters, it takes a file name. Yet, the command reports the same > >> for both: "parameters": [], "argument": true. > >> > >> Looks like we need a tri-state: option takes no argument, QemuOpts > >> argument, or other argument. > > > > '-cdrom' is the 'other argument' == 'Non-parameter argument'? > > Let me clarify my terminology: > > * A "parameter argument" is an option argument of the form KEY=VALUE,... > Since parameter arguments should always be parsed with QemuOpts[*], I > use the term "QemuOpts argument" interchangeably. > > * A "non-parameter argument" or "other argument" is an option argument > that doesn't use this form. > > Does that answer your question? Got it, thanks. > > We can use a enum state. > > I'm not sure I got that. > > > | { 'enum': 'ArgumentStateType', > > | 'data': ['no-argument', 'unspecified-parameters-argument', > > | 'non-parameter-argument'] > > | } {'enum': 'ArgumentStateType', 'data': ['no-argument', 'qemuopts-argument', 'non-param-argument'] } no-argument: -enable-kvm qemuopts-argument: -boot order=c,menu=on non-param-argument: -cdrom file I don't know if it's the tri-state you suggested in previous reply. > > | > > | { 'type': 'CommandLineOptionInfo', > > | 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'], > > | '*argument-state': 'ArgumentStateType' } } > > > >> parameters is [] unless it's a QemuOpts argument. Then it lists the > >> recognized parameters. > > > > How about balloon? we should treat as 'taking unspecified parameters'? > > > > "-balloon none disable balloon device\n" > > "-balloon virtio[,addr=str]\n" > > > > I think we can only check this from help message in qemu-options.hx, > > is it a stable/acceptable method? > > -balloon is yet another sugar option: > > * "-balloon none" does nothing. It could suppress a default balloon > device, if such a thing existed. > > * "-balloon virtio,KEY=VALUE..." is sugar for "-device > virtio-balloon,KEY=VALUE...". Keys other than "addr" are > undocumented. > > The actual option argument parsing is ad hoc, not QemuOpts. > > I sure hope something like this would not pass review today. > > My advice to tools introspecting the command line is to avoid sugared > options, unless the desugaring encapsulates something they don't want to > know. > > > We introduce query-command-line-options command to avoid libvirt to > > check qemu commandline information by scanning qemu's help message, > > it's not strict & stable. > > > >> 2. Our dear friend -drive is more complicated than you might think > >> > >> We special-case it to report the union of drive_config_groups[], > >> which contains qemu_legacy_drive_opts, qemu_common_drive_opts and > >> qemu_drive_opts. The latter accepts unspecified parameters. > > > > I'm confused here. But there is only one option (-drive), we should > > return merged desc table (legacy + common). > > > > Desc table of qemu_drive_opts is NULL, then -drive can also take > > unspecified parameters? > > Yes: driver-specific parameters. > > -drive takes currently takes unspecified parameters (the driver-specific > parameters) in addition to a number of specified parameters (the common > and legacy parameters). > > >> I believe qemu_drive_opts is actually not used by the (complex!) code > >> parsing the argument of -drive. > >> > >> Nevertheless, said code accepts more than just qemu_legacy_drive_opts > >> and qemu_common_drive_opts, namely driver-specific parameters. > >> > >> Until we define those properly in a schema, I guess the best we can > >> do is add one more case: option takes QemuOpts argument, but > >> parameters is not exhaustive. > > > > > > Thanks, Amos > > > [*] Leftovers still parsed by other means, if any, should be converted > to QemuOpts. -- Amos.