qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: pbonzini@redhat.com, "Gabriel L. Somlo" <somlo@cmu.edu>,
	qemu-devel@nongnu.org, armbru@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/
Date: Wed, 16 Mar 2016 20:43:19 +0200	[thread overview]
Message-ID: <20160316204150-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <56E9A75D.60603@redhat.com>

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" <mst@redhat.com> 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 <mst@redhat.com>
> >>>
> >>> 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". So change it to must then?

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

-- 
MST

  reply	other threads:[~2016-03-16 18:43 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15 14:47 [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/ Michael S. Tsirkin
2016-03-16 16:29 ` Markus Armbruster
2016-03-16 16:50   ` Michael S. Tsirkin
2016-03-16 18:15     ` Gabriel L. Somlo
2016-03-16 18:35       ` Laszlo Ersek
2016-03-16 18:43         ` Michael S. Tsirkin [this message]
2016-03-16 19:15           ` Laszlo Ersek
2016-03-16 20:22             ` Michael S. Tsirkin
2016-03-16 20:24             ` Michael S. Tsirkin
2016-03-16 20:31         ` Michael S. Tsirkin
2016-03-17  8:49           ` Laszlo Ersek
2016-03-17  9:40             ` Paolo Bonzini
2016-03-17 11:32               ` Michael S. Tsirkin
2016-03-17 13:12                 ` Paolo Bonzini
2016-03-17 13:15                   ` Michael S. Tsirkin
2016-03-17  8:42         ` Gerd Hoffmann
2016-03-17  9:43           ` Laszlo Ersek
2016-03-17 10:22             ` Gerd Hoffmann
2016-03-17 13:28               ` Laszlo Ersek
2016-03-17 13:35                 ` Michael S. Tsirkin
2016-03-17 13:37                   ` Paolo Bonzini
2016-03-17 16:59                 ` Gerd Hoffmann
2016-03-17 13:23             ` Michael S. Tsirkin
2016-03-17  9:49           ` Laszlo Ersek
2016-03-17 10:09 ` Markus Armbruster
2016-03-17 13:13   ` Michael S. Tsirkin
2016-03-17 13:30     ` Paolo Bonzini
2016-03-17 13:49       ` Laszlo Ersek
2016-03-17 13:49       ` Michael S. Tsirkin
2016-03-17 13:55         ` Paolo Bonzini
2016-03-17 14:17           ` Michael S. Tsirkin
2016-03-17 14:50             ` Paolo Bonzini
2016-03-17 15:40               ` Michael S. Tsirkin
2016-03-17 17:17       ` Gerd Hoffmann
2016-03-17 19:35         ` Paolo Bonzini
2016-03-17 19:55           ` Michael S. Tsirkin

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=20160316204150-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=somlo@cmu.edu \
    /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).