qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Corey Minyard <cminyard@mvista.com>,
	qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] vl.c: disallow command line fw cfg without opt/
Date: Tue, 15 Mar 2016 16:25:38 +0200	[thread overview]
Message-ID: <20160315161755-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <56E818BD.2080906@redhat.com>

On Tue, Mar 15, 2016 at 03:14:21PM +0100, Paolo Bonzini wrote:
> 
> 
> On 15/03/2016 14:55, Michael S. Tsirkin wrote:
> > Allowing arbitary file names on command line is setting us up for
> > failure: future guests will look for a specific QEMU-specified name and
> > will get confused finding a user file there.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Too bad for the user.

Why, because we trapped the user by allowing this
and violating our own compatibility rules?
Because user did not notice the warning?
Do you look at all warnings all your software prints?
What if user uses some tool to build the command line?

consider mem-path - for years we printed a warning if
it's not on hugetlbfs. It worked and users just ignored it.

> I think we have already gone through this discussion.

So now Corey basically is prevented from sorting sanely
because command line might not start with opt/

This is interfering with development instead of enabling it.

If it is really super important for development, add some
option "x-unsupported-fw-cfg" for developers to play with.
Don't allow such things by default, it's a trap.


> Paolo
> 
> > ---
> >  vl.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/vl.c b/vl.c
> > index 7a28982..5654af6 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2321,6 +2321,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
> >      if (strncmp(name, "opt/", 4) != 0) {
> > -        error_report("warning: externally provided fw_cfg item names "
> > -                     "should be prefixed with \"opt/\"");
> > +        error_report("externally provided fw_cfg item names "
> > +                     "must be prefixed with \"opt/\"");
> > +        return -1;
> >      }
> >      if (nonempty_str(str)) {
> >          size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
> > 

  parent reply	other threads:[~2016-03-15 14:25 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15 13:55 [Qemu-devel] [PATCH] vl.c: disallow command line fw cfg without opt/ Michael S. Tsirkin
2016-03-15 14:14 ` Paolo Bonzini
2016-03-15 14:25   ` Gerd Hoffmann
2016-03-15 14:27     ` Michael S. Tsirkin
2016-03-15 14:25   ` Michael S. Tsirkin [this message]
2016-03-15 14:46     ` Gerd Hoffmann
2016-03-15 14:54       ` Michael S. Tsirkin
2016-03-15 15:03         ` Gerd Hoffmann
2016-03-15 15:05           ` Michael S. Tsirkin
2016-03-15 14:54     ` Paolo Bonzini
2016-03-15 14:58       ` Michael S. Tsirkin
2016-03-17  1:31         ` Corey Minyard

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=20160315161755-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=cminyard@mvista.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --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).