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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/02/2014 15:25, Stefano Stabellini ha scritto:
> On Tue, 18 Feb 2014, Paolo Bonzini wrote:
>> Il 18/02/2014 13:45, Stefano Stabellini ha scritto:
>>> 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.
>>
>> Ok, the last paragraph is what I was missing.
>>
>> So this is dc->unplug for the PIIX3 IDE device.  Because PCI declares a
>> hotplug handler, dc->unplug is not called anymore.
>>
>> But unlike other dc->unplug callbacks, pci_piix3_xen_ide_unplug doesn't free
>> the device, it just drops the disks underneath.  I think the simplest solution
>> is to _not_ make it a dc->unplug callback at all, and call
>> pci_piix3_xen_ide_unplug from unplug_disks instead of qdev_unplug.
>> qdev_unplug means "ask guest to start unplug", which is not what Xen wants to
>> do here.
>
> Yes, you are right, pci_piix3_xen_ide_unplug is not called anymore.
> Calling it directly from unplug_disks fixes the issue:
>
>
> ---
>
> Call pci_piix3_xen_ide_unplug from unplug_disks
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 0eda301..40757eb 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -167,7 +167,7 @@ static int pci_piix_ide_initfn(PCIDevice *dev)
>      return 0;
>  }
>
> -static int pci_piix3_xen_ide_unplug(DeviceState *dev)
> +int pci_piix3_xen_ide_unplug(DeviceState *dev)
>  {
>      PCIIDEState *pci_ide;
>      DriveInfo *di;
> @@ -266,7 +266,6 @@ static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
>      k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
>      k->class_id = PCI_CLASS_STORAGE_IDE;
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> -    dc->unplug = pci_piix3_xen_ide_unplug;
>  }
>
>  static const TypeInfo piix3_ide_xen_info = {
> diff --git a/hw/xen/xen_platform.c b/hw/xen/xen_platform.c
> index 70875e4..1d9d0e9 100644
> --- a/hw/xen/xen_platform.c
> +++ b/hw/xen/xen_platform.c
> @@ -27,6 +27,7 @@
>
>  #include "hw/hw.h"
>  #include "hw/i386/pc.h"
> +#include "hw/ide.h"
>  #include "hw/pci/pci.h"
>  #include "hw/irq.h"
>  #include "hw/xen/xen_common.h"
> @@ -110,7 +111,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void *o)
>      if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
>              PCI_CLASS_STORAGE_IDE
>              && strcmp(d->name, "xen-pci-passthrough") != 0) {
> -        qdev_unplug(DEVICE(d), NULL);
> +        pci_piix3_xen_ide_unplug(DEVICE(d));
>      }
>  }
>
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index 507e6d3..bc8bd32 100644
> --- a/include/hw/ide.h
> +++ b/include/hw/ide.h
> @@ -17,6 +17,7 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
>  PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>  PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>  PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> +int pci_piix3_xen_ide_unplug(DeviceState *dev);
>  void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>
>  /* ide-mmio.c */
>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo