Re: [Qemu-devel] [PULL 00/20] acpi,pc,pci fixes and enhancements

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 18 Feb 2014 12:45:29 +0000
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> On Tue, 18 Feb 2014, Paolo Bonzini wrote:
> > Il 18/02/2014 13:16, Stefano Stabellini ha scritto:
> > > It looks like that this series breaks disk unplug
> > > (hw/ide/piix.c:pci_piix3_xen_ide_unplug).
> > > 
> > > I bisected it and the problem is caused by:
> > > 
> > > commit 5e95494380ecf83c97d28f72134ab45e0cace8f9
> > > Author: Igor Mammedov <imammedo@redhat.com>
> > > Date:   Wed Feb 5 16:36:52 2014 +0100
> > > 
> > >     hw/pci: switch to a generic hotplug handling for PCIDevice
> > > 
> > >     make qdev_unplug()/device_set_realized() to call hotplug handler's
> > >     plug/unplug methods if available and remove not needed anymore
> > >     hot(un)plug handling from PCIDevice.
> > > 
> > >     In case if hotplug handler is not available, revert to the legacy
> > >     hotplug method for compatibility with not yet converted buses.
> > > 
> > >     Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > >     Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > >     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > 
> > 
> > What exactly breaks?
> 
> Disk unplug: hw/ide/piix.c:pci_piix3_xen_ide_unplug (see the beginning
> of the email :-P).
> It is called by hw/xen/xen_platform.c:platform_fixed_ioport_writew, in
> response to the guest writing to a magic ioport specifically to unplug
> the emulated disk.
> With this patch after the guest boots I can still access both xvda and
> sda for the same disk, leading to fs corruptions.
> 
Could you try with following debug patch?

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 64b66e0..84aa8be 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -214,6 +214,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
         return;
     }
 
+    fprintf(stderr, "dc->hotpluggable %d\n", dc->hotpluggable);
     if (!dc->hotpluggable) {
         error_set(errp, QERR_DEVICE_NO_HOTPLUG,
                   object_get_typename(OBJECT(dev)));
@@ -223,8 +224,12 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     qdev_hot_removed = true;
 
     if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+        fprintf(stderr, "bus name: %s, hotplug_handler: %s\n",
+                dev->parent_bus->name,
+                object_get_typename(OBJECT(dev->parent_bus->hotplug_handler)));
         hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
     } else {
+        fprintf(stderr, "legacy unplug: %p\n", dc->unplug);
         assert(dc->unplug != NULL);
         if (dc->unplug(dev) < 0) { /* legacy handler */
             error_set(errp, QERR_UNDEFINED_ERROR);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1acd2b2..74b0cac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -238,6 +238,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (pci_enabled && acpi_enabled) {
         i2c_bus *smbus;
 
+       fprintf(stderr, "create piix4_pm_init\n");
         smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,