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: Laszlo Ersek <lersek@redhat.com>,
	"Gabriel L. Somlo" <somlo@cmu.edu>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/
Date: Thu, 17 Mar 2016 15:49:56 +0200	[thread overview]
Message-ID: <20160317153528-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <56EAB17A.1000400@redhat.com>

On Thu, Mar 17, 2016 at 02:30:34PM +0100, Paolo Bonzini wrote:
> I frankly think it's overengineered, but it's already much better and if
> it helps converging to a compromise why not.

Thanks, I'll think of your suggestions over the weekend. We might be able
to simplify things a bit.

> Alternatives to your proposals follow:

I wonder what are the advantages of the alternative though?
It is certainly more code, the rules for users are more
complex to figure out and need more effort for user to follow.


> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
> > 
> > QEMU command line:
> > 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
> > 	   with QEMU hardware
> 
> Alternative: no need to prepend usr/, I think.

I personally dislike telling user "do X". I don't see a reason not to be
friendly and do X. The rare case where users do not want X can be
easily enabled.

> > 	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
> > 		org.qemu/unsupported/ and leaves just XXX,
> > 		for people who want to break^?^?^?^?^?debug QEMU hardware
> 
> Alternative: fail on:
> 
> - a blacklist of etc/* files including etc/system-states,
> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
> etc/boot-menu-wait

We can not predict the future. Future firmware will look for
files under etc/mst. Users using this firmware with
current QEMU will get a nasty surprise where it previously
worked.

Besides, it is way easier to maintain and understand a simple rule than
a blacklist.

> - on all org.qemu/* files
> 
> - iff etc/boot-menu-wait is blacklisted, fail on
> org.seabios/boot-menu-wait too.
> 
> Everything else is passed through.  No hacks required.
>
> > 	C. -fw-cfg opt/FOO accepts any path, for backwards compatibility
> 
> Implicit in my proposed alternative to A.
> 
> > 	D. any other use fails
> 
> Replaced by my alternative to B.  RFQDN is just a best practice, and it
> is not enforced except as proposed in B.  For the same reason, no
> changes are required in the Linux driver.
>
> > OVMF:
> > 	Can use the compatible opt/ovmf/ for now. [snip]
> > 	Long term: Gradually transition OVMF to look up paths in usr/org.uefi/:
> > 	if nothing is found there, look up in opt/ovmf/ for backwards
> > 	compatibility.
> 
> Agreed except it would be org.tianocore.edk2.ovmf/ rather than usr/org.uefi.
>
> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
> stuff usable from both Coreboot and QEMU, e.g.
> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
> QEMU).
> 
> Files that could be moved from etc/ to org.qemu/ correspond to the ones
> that are blacklisted in (B), e.g. etc/system-states ->
> org.qemu/system-states.
> 
> Paolo

I am not sure about moving things into usr/org.qemu.
These are system files, not user-provided ones.
But we can argue about future plans down the road.

-- 
MST

  parent reply	other threads:[~2016-03-17 13:50 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
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 [this message]
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=20160317153528-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).