From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afpuo-0004yS-OS for qemu-devel@nongnu.org; Tue, 15 Mar 2016 10:25:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afpun-0003pe-PX for qemu-devel@nongnu.org; Tue, 15 Mar 2016 10:25:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60707) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afpun-0003pa-KL for qemu-devel@nongnu.org; Tue, 15 Mar 2016 10:25:41 -0400 Date: Tue, 15 Mar 2016 16:25:38 +0200 From: "Michael S. Tsirkin" Message-ID: <20160315161755-mutt-send-email-mst@redhat.com> References: <1458050002-27741-1-git-send-email-mst@redhat.com> <56E818BD.2080906@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E818BD.2080906@redhat.com> Subject: Re: [Qemu-devel] [PATCH] vl.c: disallow command line fw cfg without opt/ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Corey Minyard , qemu-devel@nongnu.org, Gerd Hoffmann 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 > > 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 */ > >