| Message ID | 1485266747-4694-2-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <paul.durrant@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > 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 >
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); }
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 <paul.durrant@citrix.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> v2: - Fix style issue --- hw/i386/xen/xen_platform.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)