qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Anthony Liguori <aliguori@us.ibm.com>,
	patches@linaro.org, Michael Tokarev <mjt@tls.msk.ru>,
	qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>
Subject: Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options
Date: Mon, 9 Jul 2012 13:40:09 +0100	[thread overview]
Message-ID: <20120709124009.GL16198@redhat.com> (raw)
In-Reply-To: <4FFAC994.4060804@redhat.com>

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 :|

  parent reply	other threads:[~2012-07-09 12:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-09 11:52 [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options Peter Maydell
2012-07-09 12:07 ` Eric Blake
2012-07-09 12:10   ` Peter Maydell
2012-07-09 12:19     ` Eric Blake
2012-07-09 12:59       ` Markus Armbruster
2012-07-09 12:40   ` Daniel P. Berrange [this message]
2012-07-25 14:53 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120709124009.GL16198@redhat.com \
    --to=berrange@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=blauwirbel@gmail.com \
    --cc=eblake@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).