From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLVVy-0007cA-Ai for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:54:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLVVs-0006Yy-9b for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:54:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLVVs-0006Ys-1e for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:54:36 -0400 Message-ID: <1379325279.17705.54.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Mon, 16 Sep 2013 12:54:39 +0300 In-Reply-To: <874n9qe1xi.fsf@blackfin.pond.sub.org> References: <1378924006-14057-1-git-send-email-marcel.a@redhat.com> <5231721D.4000109@redhat.com> <87sixagysq.fsf@blackfin.pond.sub.org> <1378982018.2186.23.camel@localhost.localdomain> <874n9qe1xi.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qdev-monitor: Avoid exiting when hot-plugging two devices with the same bootindex value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, gleb@redhat.com, mst@redhat.com, sw@weilnetz.de, qemu-devel@nongnu.org, Paolo Bonzini , afaerber@suse.de On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > Marcel Apfelbaum writes: > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >> Paolo Bonzini 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. > >> > >> > Callers, typically a device's init or realize function, will either > >> > print the error before returning an error code (e.g. -EBUSY for init) or > >> > propagate the error up (for realize). > >> > > >> > Returning/propagating failure will still cause QEMU to exit when the > >> > duplicate bootindexes are found on the command line. > >> > >> I have an unfinished patch in my tree that does exactly that. It's > >> unfinished, because cleanup on error paths needs work. Current state > >> appended with FIXMEs and all. Beware, the FIXMEs may not be correct and > >> are almost certainly not complete. > > Thanks Markus, > > Should I use it as my starting point and finish it or you intend to? > > If you have cycles to spare, you're quite welcome to take this patch and > run with it! I really wanted to take this, but I think we need first to sort out the issues mentioned above and only then follow this path Thanks, Marcel > > You may have noticed that my patch moves the code to add the boot device > path in a few cases. I did this in the hope of simplifying error paths. > Do not hesitate to undo such moves where they turn out not to simplify > anything. > > Here's an issue that exists before my patch: DeviceClass method > unrealize() should clean up everything done by realize(). In > particular, unrealize() needs to remove any entry added to fw_boot_order > by realize() via add_boot_device_path(). Code to do that doesn't exist, > yet. Hot unplug is technically broken for all devices with bootindex. > Impact unknown. Should probably be fixed in a separate patch.