All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"mst@redhat.com" <mst@redhat.com>,
	"qemu-trivial@nongnu.org" <qemu-trivial@nongnu.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"McMillan, Erich" <erich.mcmillan@hp.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: PATCH: Increase System Firmware Max Size
Date: Fri, 11 Sep 2020 17:21:38 +0100	[thread overview]
Message-ID: <20200911162138.GL1203593@redhat.com> (raw)
In-Reply-To: <d7d0d37e-4bba-ab82-783d-06463d78d9cf@redhat.com>

On Fri, Sep 11, 2020 at 06:06:27PM +0200, Laszlo Ersek wrote:
> On 09/11/20 17:23, Daniel P. Berrangé wrote:
> 
> > I don't see why we should have this as a hard coded
> > limit that is not runtime configurable.
> > 
> > IOW, why can't we keep our current default and provide a machine type
> > property "firmware_max_size" which users can opt-in to setting if
> > their particular firmware exceeds normal defaults. That won't impact
> > us for migration compat in any way, and lets users have flexibility t
> > do what they want.
> 
> Technically, this is fine, in my opinion.
> 
> My concerns (in distilled form, this time):
> 
> - The change increases maintenance burden.

I think that is so marginal that its lost in the noise compared to
everything else that's done in QEMU. Essentially someone can pick
a value that is so large that it tickles some other problem in QEMU
or their firmware. We're not promising to fix such problems if they
occurs. It'll be perfectly valid to say "dont do that" if someone
sets a value that breaks something else and we don't consider it
worth the time to investigate and fix.

> - The change does not benefit most users of QEMU, as the intended guest
> payload will not available to most of them at all (regardless of
> licensing terms).

I think that's only relevant if the change is imposing a significant
maint burden which needs to be justified. Not the case here IMHO.

> - The existence of the property may entice OVMF users to ask us to
> enlarge the *current* OVMF firmware platform and to pack more stuff in
> it. That is not OK. My counter-proposal ("please contribute a new
> platform DSC/FDF under OvmfPkg, and assume co-reviewership for it")
> would almost certainly not be acted upon.

I don't see this is relevant. If OVMF maintainers want to reject
feature proposals they have the right to do that regardless of what
QEMU sets for max image size. As you say earlier, the existing size
limit is already enourmous compared to what OVMF actually uses, so
if this was a real problem it'd already exist.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: "mst@redhat.com" <mst@redhat.com>,
	"qemu-trivial@nongnu.org" <qemu-trivial@nongnu.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"McMillan, Erich" <erich.mcmillan@hp.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: PATCH: Increase System Firmware Max Size
Date: Fri, 11 Sep 2020 17:21:38 +0100	[thread overview]
Message-ID: <20200911162138.GL1203593@redhat.com> (raw)
In-Reply-To: <d7d0d37e-4bba-ab82-783d-06463d78d9cf@redhat.com>

On Fri, Sep 11, 2020 at 06:06:27PM +0200, Laszlo Ersek wrote:
> On 09/11/20 17:23, Daniel P. Berrangé wrote:
> 
> > I don't see why we should have this as a hard coded
> > limit that is not runtime configurable.
> > 
> > IOW, why can't we keep our current default and provide a machine type
> > property "firmware_max_size" which users can opt-in to setting if
> > their particular firmware exceeds normal defaults. That won't impact
> > us for migration compat in any way, and lets users have flexibility t
> > do what they want.
> 
> Technically, this is fine, in my opinion.
> 
> My concerns (in distilled form, this time):
> 
> - The change increases maintenance burden.

I think that is so marginal that its lost in the noise compared to
everything else that's done in QEMU. Essentially someone can pick
a value that is so large that it tickles some other problem in QEMU
or their firmware. We're not promising to fix such problems if they
occurs. It'll be perfectly valid to say "dont do that" if someone
sets a value that breaks something else and we don't consider it
worth the time to investigate and fix.

> - The change does not benefit most users of QEMU, as the intended guest
> payload will not available to most of them at all (regardless of
> licensing terms).

I think that's only relevant if the change is imposing a significant
maint burden which needs to be justified. Not the case here IMHO.

> - The existence of the property may entice OVMF users to ask us to
> enlarge the *current* OVMF firmware platform and to pack more stuff in
> it. That is not OK. My counter-proposal ("please contribute a new
> platform DSC/FDF under OvmfPkg, and assume co-reviewership for it")
> would almost certainly not be acted upon.

I don't see this is relevant. If OVMF maintainers want to reject
feature proposals they have the right to do that regardless of what
QEMU sets for max image size. As you say earlier, the existing size
limit is already enourmous compared to what OVMF actually uses, so
if this was a real problem it'd already exist.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2020-09-11 16:22 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-11  1:45 PATCH: Increase System Firmware Max Size McMillan, Erich
2020-09-11  7:55 ` Laszlo Ersek
2020-09-11  7:55   ` Laszlo Ersek
2020-09-11  8:34   ` Dr. David Alan Gilbert
2020-09-11  8:34     ` Dr. David Alan Gilbert
2020-09-11 14:53     ` Laszlo Ersek
2020-09-11 14:53       ` Laszlo Ersek
2020-09-11 15:06       ` Dr. David Alan Gilbert
2020-09-11 15:06         ` Dr. David Alan Gilbert
2020-09-11 15:22         ` McMillan, Erich
2020-09-11 15:22           ` McMillan, Erich via
2020-09-11 16:11           ` Laszlo Ersek
2020-09-11 16:11             ` Laszlo Ersek
2020-09-11 15:23         ` Daniel P. Berrangé
2020-09-11 15:23           ` Daniel P. Berrangé
2020-09-11 16:06           ` Laszlo Ersek
2020-09-11 16:06             ` Laszlo Ersek
2020-09-11 16:21             ` Daniel P. Berrangé [this message]
2020-09-11 16:21               ` Daniel P. Berrangé
2020-09-11 16:45               ` Laszlo Ersek
2020-09-11 16:45                 ` Laszlo Ersek
2020-09-11 15:57         ` Laszlo Ersek
2020-09-11 15:57           ` Laszlo Ersek
2020-09-11 16:22           ` Dr. David Alan Gilbert
2020-09-11 16:22             ` Dr. David Alan Gilbert
2020-09-11 16:53             ` Laszlo Ersek
2020-09-11 16:53               ` Laszlo Ersek
2020-09-11 16:59               ` Dr. David Alan Gilbert
2020-09-11 16:59                 ` Dr. David Alan Gilbert
2020-09-11 17:51                 ` McMillan, Erich
2020-09-11 17:51                   ` McMillan, Erich via
2020-09-15 19:09 ` McMillan, Erich
2020-09-15 19:10   ` McMillan, Erich
2020-09-15 19:10     ` McMillan, Erich via
2020-09-16  9:52     ` Laszlo Ersek
2020-09-16  9:52       ` Laszlo Ersek
2020-09-16  9:56       ` Daniel P. Berrangé
2020-09-16  9:56         ` Daniel P. Berrangé
2020-09-16 11:31         ` Laszlo Ersek
2020-09-16 11:31           ` Laszlo Ersek
2020-09-16 11:43           ` Daniel P. Berrangé
2020-09-16 11:43             ` Daniel P. Berrangé
2020-09-16 10:00       ` Laszlo Ersek
2020-09-16 10:00         ` Laszlo Ersek

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=20200911162138.GL1203593@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=erich.mcmillan@hp.com \
    --cc=lersek@redhat.com \
    --cc=mst@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.