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: aliguori@us.ibm.com, gleb@redhat.com,
	Marcel Apfelbaum <marcel.a@redhat.com>,
	sw@weilnetz.de, qemu-devel@nongnu.org,
	Markus Armbruster <armbru@redhat.com>,
	afaerber@suse.de
Subject: Re: [Qemu-devel] [PATCH] qdev-monitor: Avoid exiting when hot-plugging two devices with the same bootindex value
Date: Mon, 16 Sep 2013 18:59:08 +0300	[thread overview]
Message-ID: <20130916155908.GC4141@redhat.com> (raw)
In-Reply-To: <523724EC.4080105@redhat.com>

On Mon, Sep 16, 2013 at 05:34:04PM +0200, Paolo Bonzini wrote:
> Il 16/09/2013 17:14, Michael S. Tsirkin ha scritto:
> > On Mon, Sep 16, 2013 at 12:11:35PM +0200, Paolo Bonzini wrote:
> >> Il 16/09/2013 11:54, Marcel Apfelbaum ha scritto:
> >>> On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote:
> >>>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> >>>>
> >>>>> On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote:
> >>>>>> Paolo Bonzini <pbonzini@redhat.com> writes:
> >>>>>>
> >>>>>>> Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto:
> >>>>>>>> Qemu is expected to quit if the same boot index value is used by
> >>>>>>>> two devices.
> >>>>>>>> However, hot-plugging a device with a bootindex value already used should
> >>>>>>>> fail with a friendly message rather than quitting a running VM.
> >>>>>>>
> >>>>>>> I think the problem is right where QEMU exits, i.e. in
> >>>>>>> add_boot_device_path.  This function should return an error instead, via
> >>>>>>> an Error ** argument.
> >>>>>>
> >>>>>> Agree.
> >>>
> >>> I understood that the boot order is passed in fw cfg and updated only once at
> >>> "machine done". There is no update of this list after this point.
> >>> Modifying the boot order from monitor does not work at all.
> >>>
> >>> So in order to solve this issue we can:
> >>> 1. Don't allow use of bootindex at hot-plug
> >>> 2. Change the architecture so boot order changing during hot-plug will be possible.
> >>
> >> This is done relatively easily in add_boot_device_path (check the
> >> qdev_hotplug variable and return an error if it is 1).
> > 
> > This refers to 1) here? That's nice but it's not really all that helpful.
> 
> True, but the error propagation changes are the base for both cases, and
> (1) is just 4 lines of code.  So this seems like a plan to me: do the
> error propagation changes and (1), then we can revert those four lines
> when (2) is ready.
> 
> Paolo

I'm fine with applying error propagation, but maybe not (1).

Basically if you accept incorrect code the result often
is that controbutor disappears and it's never completed,
others are discouraged from addressing the problem because
incorrect code created maintainance issues, and
a half-solved problem is a less interesting target
than an unsolved one.

So if a contributor basically says he does not want
to address the complete problem, we should weight
carefully whether we want to be stuck with
half a solution for a long time.

In this case, I don't think it's justified.

-- 
MST

  reply	other threads:[~2013-09-16 15:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-11 18:26 [Qemu-devel] [PATCH] qdev-monitor: Avoid exiting when hot-plugging two devices with the same bootindex value Marcel Apfelbaum
2013-09-11 18:41 ` Marcel Apfelbaum
2013-09-12  7:49 ` Paolo Bonzini
2013-09-12  8:38   ` Marcel Apfelbaum
2013-09-12  9:43   ` Markus Armbruster
2013-09-12 10:33     ` Marcel Apfelbaum
2013-09-12 11:04       ` Markus Armbruster
2013-09-12 11:23         ` Marcel Apfelbaum
2013-09-16  9:54         ` Marcel Apfelbaum
2013-09-16 10:11           ` Paolo Bonzini
2013-09-16 15:14             ` Michael S. Tsirkin
2013-09-16 15:34               ` Paolo Bonzini
2013-09-16 15:59                 ` Michael S. Tsirkin [this message]
2013-09-16 11:03           ` Gleb Natapov

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=20130916155908.GC4141@redhat.com \
    --to=mst@redhat.com \
    --cc=afaerber@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=gleb@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /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).