Re: [Qemu-devel] [PATCH] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

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 <address@hidden>
> 
> 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 
> > */
> >