From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58936) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoDGM-0004sn-LS for qemu-devel@nongnu.org; Mon, 09 Jul 2012 08:40:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SoDGF-0000bH-Mm for qemu-devel@nongnu.org; Mon, 09 Jul 2012 08:40:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9020) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoDGF-0000al-Eu for qemu-devel@nongnu.org; Mon, 09 Jul 2012 08:40:19 -0400 Date: Mon, 9 Jul 2012 13:40:09 +0100 From: "Daniel P. Berrange" Message-ID: <20120709124009.GL16198@redhat.com> References: <1341834742-28654-1-git-send-email-peter.maydell@linaro.org> <4FFAC994.4060804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4FFAC994.4060804@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Peter Maydell , Anthony Liguori , patches@linaro.org, Michael Tokarev , qemu-devel@nongnu.org, Blue Swirl On Mon, Jul 09, 2012 at 06:07:48AM -0600, Eric Blake wrote: > On 07/09/2012 05:52 AM, Peter Maydell wrote: > > For command line options which permit '?' meaning 'please list the > > permitted values', add support for 'help' as a synonym, by abstracting > > the check out into a helper function. > > > > Update the documentation to use 'help' rather than '?', since '?' > > is a shell metacharacter and thus prone to fail confusingly if there > > is a single character filename in the current working directory and > > the '?' has not been escaped. It's therefore better to steer users > > towards 'help', though '?' is retained for backwards compatibility. > > I like the idea, but this may cause grief for libvirt, which basically does: > > const char *help; // = output of 'qemu -h' > if (strstr(help, "-device driver,?")) { > /* Cram together all device-related queries into one invocation; > * the output format makes it possible to distinguish what we > * need. With qemu 0.13.0 and later, unrecognized '-device > * bogus,?' cause an error in isolation, but are silently ignored > * in combination with '-device ?'. Upstream qemu 0.12.x doesn't > * understand '-device name,?', and always exits with status 1 for > * the simpler '-device ?', so this function is really only useful > * if -help includes "device driver,?". */ > cmd = virCommandNewArgList(qemu, > "-device", "?", > "-device", "pci-assign,?", > "-device", "virtio-blk-pci,?", > "-device", "virtio-net-pci,?", > "-device", "scsi-disk,?", > NULL); > } > > That is, we are filtering based on the explicit presence of a literal > '?' in the help output to determine whether we can further filter based > on '-device device,?' queries without confusing qemu or libvirt; > changing the 'help' output means that old libvirt with new qemu won't > run the extra queries (as if it had been targeting qemu 0.12.x), even > though the older spelling of the query would still work. > > If that is not a concern (that is, if you are willing to state that use > of newer qemu is intended to be coupled with newer libvirt that has been > taught to use 'help' instead of '?'), then this patch is probably fine. > The alternative is to update 'qemu -h' output to mention both forms, > rather than completely eradicating the ? form from documentation. Regardless of what QEMU does here, I think we should change libvirt's handling of this arg. I think we can probably just skip that check, and run 'qemu -device XXX,?' unconditionally and then handle any error that we get back from old QEMU. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|