Re: [Qemu-devel] [PATCH v2 1/3] xen-platform: re-structure unplug_disks

[Stefano Stabellini]

On Tue, 24 Jan 2017, Paul Durrant wrote:
> The current code is poorly structured and potentially leads to multiple
> config space reads when one is sufficient. Also the UNPLUG_ALL_IDE_DISKS
> flag is mis-named since it also results in SCSI disks being unplugged.
> 
> This patch renames the flag and re-structures the code to be more
> efficient, and readable.
> 
> Signed-off-by: Paul Durrant <address@hidden>

Reviewed-by: Stefano Stabellini <address@hidden>

> ---
> Cc: Stefano Stabellini <address@hidden>
> Cc: Anthony Perard <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Richard Henderson <address@hidden>
> Cc: Eduardo Habkost <address@hidden>
> Cc: "Michael S. Tsirkin" <address@hidden>
> 
> v2:
> - Fix style issue
> ---
>  hw/i386/xen/xen_platform.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index 2e1e543..f50915f 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -88,7 +88,7 @@ static void log_writeb(PCIXenPlatformState *s, char val)
>  }
>  
>  /* Xen Platform, Fixed IOPort */
> -#define UNPLUG_ALL_IDE_DISKS 1
> +#define UNPLUG_ALL_DISKS 1
>  #define UNPLUG_ALL_NICS 2
>  #define UNPLUG_AUX_IDE_DISKS 4
>  
> @@ -110,14 +110,21 @@ static void pci_unplug_nics(PCIBus *bus)
>  static void unplug_disks(PCIBus *b, PCIDevice *d, void *o)
>  {
>      /* We have to ignore passthrough devices */
> -    if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
> -            PCI_CLASS_STORAGE_IDE
> -            && strcmp(d->name, "xen-pci-passthrough") != 0) {
> +    if (!strcmp(d->name, "xen-pci-passthrough")) {
> +        return;
> +    }
> +
> +    switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
> +    case PCI_CLASS_STORAGE_IDE:
>          pci_piix3_xen_ide_unplug(DEVICE(d));
> -    } else if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
> -            PCI_CLASS_STORAGE_SCSI
> -            && strcmp(d->name, "xen-pci-passthrough") != 0) {
> +        break;
> +
> +    case PCI_CLASS_STORAGE_SCSI:
>          object_unparent(OBJECT(d));
> +        break;
> +
> +    default:
> +        break;
>      }
>  }
>  
> @@ -134,9 +141,9 @@ static void platform_fixed_ioport_writew(void *opaque, 
> uint32_t addr, uint32_t v
>      case 0: {
>          PCIDevice *pci_dev = PCI_DEVICE(s);
>          /* Unplug devices.  Value is a bitmask of which devices to
> -           unplug, with bit 0 the IDE devices, bit 1 the network
> +           unplug, with bit 0 the disk devices, bit 1 the network
>             devices, and bit 2 the non-primary-master IDE devices. */
> -        if (val & UNPLUG_ALL_IDE_DISKS) {
> +        if (val & UNPLUG_ALL_DISKS) {
>              DPRINTF("unplug disks\n");
>              pci_unplug_disks(pci_dev->bus);
>          }
> -- 
> 2.1.4
>