From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agHxi-0000fB-By for qemu-devel@nongnu.org; Wed, 16 Mar 2016 16:22:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agHxf-0002CF-4y for qemu-devel@nongnu.org; Wed, 16 Mar 2016 16:22:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47695) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agHxe-0002C9-Ty for qemu-devel@nongnu.org; Wed, 16 Mar 2016 16:22:31 -0400 Date: Wed, 16 Mar 2016 22:22:26 +0200 From: "Michael S. Tsirkin" Message-ID: <20160316221923-mutt-send-email-mst@redhat.com> References: <20160316181541.GG12454@HEDWIG.INI.CMU.EDU> <56E9A75D.60603@redhat.com> <20160316204150-mutt-send-email-mst@redhat.com> <56E9B0BB.8040700@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E9B0BB.8040700@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: pbonzini@redhat.com, "Gabriel L. Somlo" , qemu-devel@nongnu.org, armbru@redhat.com, kraxel@redhat.com On Wed, Mar 16, 2016 at 08:15:07PM +0100, Laszlo Ersek wrote: > On 03/16/16 19:43, Michael S. Tsirkin wrote: > > On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote: > >> On 03/16/16 19:15, Gabriel L. Somlo wrote: > >>> On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote: > >>>> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote: > >>>>> "Michael S. Tsirkin" writes: > >>>>> > >>>>>> 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. > >>>>>> > >>>>>> We do warn but people are conditioned to ignore warnings by now, > >>>>>> so at best that will help users debug problem, not avoid it. > >>>>>> > >>>>>> Disable this by default, so distros don't get to deal with it, > >>>>>> but leave an option for developers to configure this in, > >>>>>> with scary warnings so people only do it if they know > >>>>>> what they are doing. > >>>>>> > >>>>>> Signed-off-by: Michael S. Tsirkin > >>>>> > >>>>> I'm having a hard time to see the point. > >>>> > >>>> Frankly, I am having a hard time to see the point of exposing fw cfg to > >>>> users at all. It was designed as an internal interface between QEMU PC > >>>> hardware and firmware. As a PC maintainer, I do not like it that users > >>>> get to poke at PC internals. > >>>> > >>>> So it is yet another way to pass binaries to Linux guests. Don't we > >>>> have enough of these? But Gerd likes it for some reason, and merged it. > >>>> OK. > >>> > >>> As the author of the feature, I feel I should jump back in and clarify: > >>> > >>> It's a way to pass arbitrary blobs to any type of guest, with two > >>> important properties: 1. asynchronous, and 2. out-of-band. When I > >>> started looking, all existing methods involved either having the host > >>> start polling for the guest to bring up qga and be ready to accept an > >>> out-of-band connection (i.e., *not* asynchronous), or have the guest > >>> mount some special cdrom or floppy image prepared by the host (i.e., > >>> *not* out of band). > >>> > >>> fw_cfg is both asynchronous and out-of-band, so it appeared to be the > >>> perfect choice. > >>> > >>>> But please find a way to make sure it does not conflict with its current > >>>> usage in PC. Asking that all files have an "opt/" prefix is one way > >>>> but only if it is enforced. > >>> > >>> Enforcing the "opt/" prefix was clearly on the table when I submitted > >>> the feature (and totally acceptable for my own needs). At the time, however, > >>> most of the advice I received on the list was to leave it as a warning > >>> only (i.e., "mechanism, not policy"), especially since other respondents > >>> expressed interest in passing in non-"/opt" blobs for easier development > >>> and debugging of alternative firmware (such as OVMF, iirc). > >>> > >>> Having a mis-use of this feature become "institutionalized" over time was > >>> seen as a low/negligible risk at the time. Do we have any new reasons > >>> to worry about it ? > >> > >> OVMF uses this feature for a few flags. They are all called > >> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which > >> shouldn't be surprising since I seem to have reviewed every patch for > >> that file): > >> > >>> NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/" > >>> when using the "-fw_cfg" command line option, to avoid conflicting with > >>> item names used internally by QEMU. For instance: > >>> > >>> -fw_cfg name=opt/my_item_name,file=./my_blob.bin > >>> > >>> Similarly, QEMU developers *SHOULD NOT* use item names prefixed with > >>> "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file(). > >> > >> It says "should", not "must". > > > > should means "might be ok". > > Yes, if there is a pressing reason. And even then, "you have been warned". > > > So change it to must then? > > I didn't suggest that. > > (For consistency, your patch should be attempting to modify the code and > the docs together -- but this doesn't mean that I agree with the patch.) > > >> I liked (and like) the "mechanism, not > >> policy" thing. Letting developers pass in whatever they want, for > >> development / debugging / testing purposes, is a plus to me. > >> > >> Thanks > >> Laszlo > > > > Could you flesh out the development / debugging / testing and > > how is inserting files in PC namespace useful for that? > > I don't know -- which is part of the argument. Lack of a ready example > doesn't imply (to me) that the possibility should be excluded. > > As Paolo said, I believe we've been through this discussion. I have no > new arguments; the old ones are valid to me still. I won't try to > influence this discussion any further; I only chimed in because OVMF was > mentioned (and because I noticed that the docs would get out of sync > with the code). > > Thanks > Laszlo By the way, these are developer docs. User docs do not say anything: -fw_cfg [name=]name,file=file Add named fw_cfg entry from file. name determines the name of the entry in the fw_cfg file directory exposed to the guest. -fw_cfg [name=]name,string=str Add named fw_cfg entry from string. -- MST