From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48277 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PGrmF-0003ia-4p for qemu-devel@nongnu.org; Fri, 12 Nov 2010 06:26:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PGrmD-0002xr-PC for qemu-devel@nongnu.org; Fri, 12 Nov 2010 06:26:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12419) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PGrmD-0002xW-Gs for qemu-devel@nongnu.org; Fri, 12 Nov 2010 06:26:41 -0500 Date: Fri, 12 Nov 2010 13:26:30 +0200 From: "Michael S. Tsirkin" Message-ID: <20101112112630.GD11354@redhat.com> References: <20101112072411.GG29605@valinux.co.jp> <20101112091857.GH7631@redhat.com> <20101112095952.GA23183@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101112095952.GA23183@valinux.co.jp> Subject: [Qemu-devel] Re: Cannot not unplug cold-plugged devices List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: Cam Macdonell , "qemu-devel@nongnu.org Developers" On Fri, Nov 12, 2010 at 06:59:52PM +0900, Isaku Yamahata wrote: > On Fri, Nov 12, 2010 at 11:18:57AM +0200, Michael S. Tsirkin wrote: > > On Fri, Nov 12, 2010 at 04:24:11PM +0900, Isaku Yamahata wrote: > > > On Thu, Nov 11, 2010 at 11:29:39PM -0700, Cam Macdonell wrote: > > > > Hi, > > > > > > > > I was trying to do a "device_del" on my ivshmem device and it won't > > > > work unless the device is added via hotplug. If the device is > > > > coldplugged (added at startup) then nothing happens. I think I > > > > tracked this behaviour to the patch below. > > > > > > > > Is not allowing coldplugged devices to be unplugged the desired behaviour? > > > > > > Oh, my bad. Does the following patch help? > > > > > > >From 45914b2fc95750b65685dfb98a435f58e38b45ba Mon Sep 17 00:00:00 2001 > > > Message-Id: <45914b2fc95750b65685dfb98a435f58e38b45ba.1289546596.git.yamahata@valinux.co.jp> > > > From: Isaku Yamahata > > > Date: Fri, 12 Nov 2010 16:21:35 +0900 > > > Subject: [PATCH] acpi/piix4: allow pci hotplug for cold plugged device. > > > > > > This patch fixes 5beb8ad503c88a76f2b8106c3b74b4ce485a60e1 > > > reported by Cam Macdonell . > > > Before the change set, cold plugged device can be hot unplugged. > > > This patch unbreaks it. > > > > > > Signed-off-by: Isaku Yamahata > > > > > > > > > --- > > > hw/acpi_piix4.c | 4 +++- > > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c > > > index 66c7885..bca330e 100644 > > > --- a/hw/acpi_piix4.c > > > +++ b/hw/acpi_piix4.c > > > @@ -621,8 +621,10 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, int state) > > > PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, > > > DO_UPCAST(PCIDevice, qdev, qdev)); > > > > > > - if (!dev->qdev.hotplugged) > > > + /* qemu initialization case */ > > > > That comment is pretty obscure :) > > How about the following? > > Reached here during qemu creating a machine before > VM runs. hot plug event shouldn't be triggered > because it's dangerous. > > > > > > > > + if (state && !dev->qdev.hotplugged) { > > > return 0; > > > + } > > > > Do we even need this check at all? > > Hmm, I suppose you don't like the condition more complex. > How about "if (qdev_hotplug)". > An access function to qdev_hotplug in qdev.c would be desirable. No, I am just trying to understand why is hotplug event dangerous. We still get it if we do device add before starting the VM, right? > > > > > > > > > s->pci0_status.up = 0; > > > s->pci0_status.down = 0; > > > -- > > > 1.7.1.1 > > > > > > > > > > > > -- > > > yamahata > > > > -- > yamahata