All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, "Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH] pc: add property for Linux setup_data seed
Date: Thu, 4 Aug 2022 14:31:06 +0100	[thread overview]
Message-ID: <YuvKGrUsSAzQEzfL@redhat.com> (raw)
In-Reply-To: <YuvHu3NW592rGFXz@zx2c4.com>

On Thu, Aug 04, 2022 at 03:20:59PM +0200, Jason A. Donenfeld wrote:
> On Thu, Aug 04, 2022 at 03:13:20PM +0200, Paolo Bonzini wrote:
> > Using a property makes it possible to use the normal compat property
> > mechanism instead of ad hoc code; it avoids parameter proliferation
> > in x86_load_linux; and allows shipping the code even if it is
> > disabled by default.
> 
> Strong NACK from me here.
> 
> If this kind of thing is off by default, it's as good as useless. Indeed
> it shouldn't even be a knob at all. Don't do this.

You're misunderstanding the patch. This remains on by default for
 the 7.1 machine type.

The patch is merely exposing a knob so that users can override the
built-in default if they need to. Imagine if we had shipped this
existing code before today's bugs were discovered.  The knob
proposed her would allow users to turn off the broken pieces.
This is a good thing.

> Rather, let's fix the bug. The code as-is -- going back to the 2016 DTB
> addition -- is problematic and needs to be fixed. So let's fix that.
> Trying to cover up the problem with a default-off knob just ensures this
> stuff will never be made to work right.

It isn't covering up the problem, just providing a workaround
option, should another bug be discovered after release. We
still need to fix current discussed problems of course.


With 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:[~2022-08-04 13:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-04 13:13 [PATCH] pc: add property for Linux setup_data seed Paolo Bonzini
2022-08-04 13:20 ` Jason A. Donenfeld
2022-08-04 13:31   ` Daniel P. Berrangé [this message]
2022-08-04 14:31     ` Jason A. Donenfeld
2022-08-04 16:22       ` Michael S. Tsirkin
2022-08-04 21:16       ` Paolo Bonzini
2022-08-04 13:23 ` Daniel P. Berrangé

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=YuvKGrUsSAzQEzfL@redhat.com \
    --to=berrange@redhat.com \
    --cc=Jason@zx2c4.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@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.