[1/8] pci: Cleanup configuration for pci-hotplug.c

pci-hotplug.c and the CONFIG_PCI_HOTPLUG variable which controls its
compilation are misnamed.  They're not about PCI hotplug in general, but
rather about the pci_add/pci_del interface which are now deprecated in
favour of the more general device_add/device_del interface.  This patch
therefore renames them to pci-hotplug-old.c and CONFIG_PCI_HOTPLUG_OLD.

CONFIG_PCI_HOTPLUG=y was listed twice in {i386,x86_64}-softmmu.make for no
particular reason, so we clean that up too.  In addition it was included in
ppc64-softmmu.mak for which the old hotplug interface was never used and is
unsuitable, so we remove that too.

Most of pci-hotplug.c was additionaly protected by #ifdef TARGET_I386.  The
small piece which wasn't is only called from the pci_add and pci_del hooks
in hmp-commands.hx, which themselves were protected by #ifdef TARGET_I386.
This patch therefore also removes the #ifdef from pci-hotplug-old.c,
and changes the ifdefs in hmp-commands.hx to use CONFIG_PCI_HOTPLUG_OLD.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 default-configs/i386-softmmu.mak   |    3 +-
 default-configs/ppc64-softmmu.mak  |    2 -
 default-configs/x86_64-softmmu.mak |    3 +-
 hmp-commands.hx                    |    4 +-
 hw/pci/Makefile.objs               |    2 +-
 hw/pci/pci-hotplug-old.c           |  290 +++++++++++++++++++++++++++++++++++
 hw/pci/pci-hotplug.c               |  292 ------------------------------------
 7 files changed, 295 insertions(+), 301 deletions(-)
 create mode 100644 hw/pci/pci-hotplug-old.c
 delete mode 100644 hw/pci/pci-hotplug.c

On Thu, May 09, 2013 at 10:31:05AM +1000, David Gibson wrote:
> pci-hotplug.c and the CONFIG_PCI_HOTPLUG variable which controls its
> compilation are misnamed.  They're not about PCI hotplug in general, but
> rather about the pci_add/pci_del interface which are now deprecated in
> favour of the more general device_add/device_del interface.  This patch
> therefore renames them to pci-hotplug-old.c and CONFIG_PCI_HOTPLUG_OLD.
> 
> CONFIG_PCI_HOTPLUG=y was listed twice in {i386,x86_64}-softmmu.make for no
> particular reason, so we clean that up too.  In addition it was included in
> ppc64-softmmu.mak for which the old hotplug interface was never used and is
> unsuitable, so we remove that too.
> 
> Most of pci-hotplug.c was additionaly protected by #ifdef TARGET_I386.  The
> small piece which wasn't is only called from the pci_add and pci_del hooks
> in hmp-commands.hx, which themselves were protected by #ifdef TARGET_I386.
> This patch therefore also removes the #ifdef from pci-hotplug-old.c,
> and changes the ifdefs in hmp-commands.hx to use CONFIG_PCI_HOTPLUG_OLD.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  default-configs/i386-softmmu.mak   |    3 +-
>  default-configs/ppc64-softmmu.mak  |    2 -
>  default-configs/x86_64-softmmu.mak |    3 +-
>  hmp-commands.hx                    |    4 +-
>  hw/pci/Makefile.objs               |    2 +-
>  hw/pci/pci-hotplug-old.c           |  290 +++++++++++++++++++++++++++++++++++
>  hw/pci/pci-hotplug.c               |  292 ------------------------------------
>  7 files changed, 295 insertions(+), 301 deletions(-)
>  create mode 100644 hw/pci/pci-hotplug-old.c
>  delete mode 100644 hw/pci/pci-hotplug.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 03deca2..4a0fc9c 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_TPM_TIS=$(CONFIG_TPM)
> -CONFIG_PCI_HOTPLUG=y
> +CONFIG_PCI_HOTPLUG_OLD=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_PCI_HOTPLUG=y
>  CONFIG_WDT_IB700=y
>  CONFIG_PC_SYSFW=y
>  CONFIG_XEN_I386=$(CONFIG_XEN)
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index 884ea8a..d7140c4 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -44,7 +44,5 @@ CONFIG_XILINX_ETHLITE=y
>  CONFIG_OPENPIC=y
>  CONFIG_PSERIES=$(CONFIG_FDT)
>  CONFIG_E500=$(CONFIG_FDT)
> -# For pSeries
> -CONFIG_PCI_HOTPLUG=y
>  # For PReP
>  CONFIG_MC146818RTC=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 599b630..10bb0c6 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_TPM_TIS=$(CONFIG_TPM)
> -CONFIG_PCI_HOTPLUG=y
> +CONFIG_PCI_HOTPLUG_OLD=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_PCI_HOTPLUG=y
>  CONFIG_WDT_IB700=y
>  CONFIG_PC_SYSFW=y
>  CONFIG_XEN_I386=$(CONFIG_XEN)
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9cea415..1d88320 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1077,7 +1077,7 @@ STEXI
>  Add drive to PCI storage controller.
>  ETEXI
>  
> -#if defined(TARGET_I386)
> +#if defined(CONFIG_PCI_HOTPLUG_OLD)
>      {
>          .name       = "pci_add",
>          .args_type  = "pci_addr:s,type:s,opts:s?",
> @@ -1093,7 +1093,7 @@ STEXI
>  Hot-add PCI device.
>  ETEXI
>  
> -#if defined(TARGET_I386)
> +#if defined(CONFIG_PCI_HOTPLUG_OLD)
>      {
>          .name       = "pci_del",
>          .args_type  = "pci_addr:s",
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> index a7fb9d0..2ad32b6 100644
> --- a/hw/pci/Makefile.objs
> +++ b/hw/pci/Makefile.objs
> @@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
>  common-obj-$(CONFIG_NO_PCI) += pci-stub.o
>  common-obj-$(CONFIG_ALL) += pci-stub.o
>  
> -obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
> +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> new file mode 100644
> index 0000000..724a80b
> --- /dev/null
> +++ b/hw/pci/pci-hotplug-old.c

Please use git format-patch -M so renames are
properly shown. I can't see whether anything changed here.

> @@ -0,0 +1,290 @@
> +/*
> + * QEMU PCI hotplug support

I don't particularly mind whether it's called pci-hotplug or
pci-hotplug-old, but I'm prepared to go with you on this.
However, what we really should do is fix the comment
to match reality.

I think this needs two commits:
- fix comment
- rename

to make format-patch -M detect this as a rename

> + *
> + * Copyright (c) 2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +#include "hw/pci/pci.h"
> +#include "net/net.h"
> +#include "hw/i386/pc.h"
> +#include "monitor/monitor.h"
> +#include "hw/scsi/scsi.h"
> +#include "hw/virtio/virtio-blk.h"
> +#include "qemu/config-file.h"
> +#include "sysemu/blockdev.h"
> +#include "qapi/error.h"
> +
> +static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
> +                                       const char *devaddr,
> +                                       const char *opts_str)
> +{
> +    Error *local_err = NULL;
> +    QemuOpts *opts;
> +    PCIBus *bus;
> +    int ret, devfn;
> +
> +    bus = pci_get_bus_devfn(&devfn, devaddr);
> +    if (!bus) {
> +        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> +        return NULL;
> +    }
> +    if (!((BusState*)bus)->allow_hotplug) {
> +        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> +        return NULL;
> +    }
> +
> +    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
> +    if (!opts) {
> +        return NULL;
> +    }
> +
> +    qemu_opt_set(opts, "type", "nic");
> +
> +    ret = net_client_init(opts, 0, &local_err);
> +    if (error_is_set(&local_err)) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +        return NULL;
> +    }
> +    if (nd_table[ret].devaddr) {
> +        monitor_printf(mon, "Parameter addr not supported\n");
> +        return NULL;
> +    }
> +    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
> +}
> +
> +static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
> +                        DriveInfo *dinfo, int printinfo)
> +{
> +    SCSIBus *scsibus;
> +    SCSIDevice *scsidev;
> +
> +    scsibus = (SCSIBus *)
> +        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
> +                            TYPE_SCSI_BUS);
> +    if (!scsibus) {
> +	error_report("Device is not a SCSI adapter");
> +	return -1;
> +    }
> +
> +    /*
> +     * drive_init() tries to find a default for dinfo->unit.  Doesn't
> +     * work at all for hotplug though as we assign the device to a
> +     * specific bus instead of the first bus with spare scsi ids.
> +     *
> +     * Ditch the calculated value and reload from option string (if
> +     * specified).
> +     */
> +    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
> +    dinfo->bus = scsibus->busnr;
> +    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> +                                        false, -1, NULL);
> +    if (!scsidev) {
> +        return -1;
> +    }
> +    dinfo->unit = scsidev->id;
> +
> +    if (printinfo)
> +        monitor_printf(mon, "OK bus %d, unit %d\n",
> +                       scsibus->busnr, scsidev->id);
> +    return 0;
> +}
> +
> +int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> +{
> +    int dom, pci_bus;
> +    unsigned slot;
> +    PCIDevice *dev;
> +    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> +
> +    switch (dinfo->type) {
> +    case IF_SCSI:
> +        if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
> +            goto err;
> +        }
> +        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> +                              PCI_DEVFN(slot, 0));
> +        if (!dev) {
> +            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> +            goto err;
> +        }
> +        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
> +            goto err;
> +        }
> +        break;
> +    default:
> +        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
> +        goto err;
> +    }
> +
> +    return 0;
> +err:
> +    return -1;
> +}
> +
> +static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> +                                           const char *devaddr,
> +                                           const char *opts)
> +{
> +    PCIDevice *dev;
> +    DriveInfo *dinfo = NULL;
> +    int type = -1;
> +    char buf[128];
> +    PCIBus *bus;
> +    int devfn;
> +
> +    if (get_param_value(buf, sizeof(buf), "if", opts)) {
> +        if (!strcmp(buf, "scsi"))
> +            type = IF_SCSI;
> +        else if (!strcmp(buf, "virtio")) {
> +            type = IF_VIRTIO;
> +        } else {
> +            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
> +            return NULL;
> +        }
> +    } else {
> +        monitor_printf(mon, "no if= specified\n");
> +        return NULL;
> +    }
> +
> +    if (get_param_value(buf, sizeof(buf), "file", opts)) {
> +        dinfo = add_init_drive(opts);
> +        if (!dinfo)
> +            return NULL;
> +        if (dinfo->devaddr) {
> +            monitor_printf(mon, "Parameter addr not supported\n");
> +            return NULL;
> +        }
> +    } else {
> +        dinfo = NULL;
> +    }
> +
> +    bus = pci_get_bus_devfn(&devfn, devaddr);
> +    if (!bus) {
> +        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> +        return NULL;
> +    }
> +    if (!((BusState*)bus)->allow_hotplug) {
> +        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> +        return NULL;
> +    }
> +
> +    switch (type) {
> +    case IF_SCSI:
> +        dev = pci_create(bus, devfn, "lsi53c895a");
> +        if (qdev_init(&dev->qdev) < 0)
> +            dev = NULL;
> +        if (dev && dinfo) {
> +            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
> +                qdev_unplug(&dev->qdev, NULL);
> +                dev = NULL;
> +            }
> +        }
> +        break;
> +    case IF_VIRTIO:
> +        if (!dinfo) {
> +            monitor_printf(mon, "virtio requires a backing file/device.\n");
> +            return NULL;
> +        }
> +        dev = pci_create(bus, devfn, "virtio-blk-pci");
> +        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> +            qdev_free(&dev->qdev);
> +            dev = NULL;
> +            break;
> +        }
> +        if (qdev_init(&dev->qdev) < 0)
> +            dev = NULL;
> +        break;
> +    default:
> +        dev = NULL;
> +    }
> +    return dev;
> +}
> +
> +void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> +{
> +    PCIDevice *dev = NULL;
> +    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> +    const char *type = qdict_get_str(qdict, "type");
> +    const char *opts = qdict_get_try_str(qdict, "opts");
> +
> +    /* strip legacy tag */
> +    if (!strncmp(pci_addr, "pci_addr=", 9)) {
> +        pci_addr += 9;
> +    }
> +
> +    if (!opts) {
> +        opts = "";
> +    }
> +
> +    if (!strcmp(pci_addr, "auto"))
> +        pci_addr = NULL;
> +
> +    if (strcmp(type, "nic") == 0) {
> +        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
> +    } else if (strcmp(type, "storage") == 0) {
> +        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
> +    } else {
> +        monitor_printf(mon, "invalid type: %s\n", type);
> +    }
> +
> +    if (dev) {
> +        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> +                       pci_find_domain(dev->bus),
> +                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
> +                       PCI_FUNC(dev->devfn));
> +    } else
> +        monitor_printf(mon, "failed to add %s\n", opts);
> +}
> +
> +static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> +{
> +    PCIDevice *d;
> +    int dom, bus;
> +    unsigned slot;
> +    Error *local_err = NULL;
> +
> +    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> +        return -1;
> +    }
> +
> +    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> +    if (!d) {
> +        monitor_printf(mon, "slot %d empty\n", slot);
> +        return -1;
> +    }
> +
> +    qdev_unplug(&d->qdev, &local_err);
> +    if (error_is_set(&local_err)) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
> +        error_free(local_err);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
> +{
> +    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
> +}
> diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
> deleted file mode 100644
> index 12287d1..0000000
> --- a/hw/pci/pci-hotplug.c
> +++ /dev/null
> @@ -1,292 +0,0 @@
> -/*
> - * QEMU PCI hotplug support
> - *
> - * Copyright (c) 2004 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#include "hw/hw.h"
> -#include "hw/boards.h"
> -#include "hw/pci/pci.h"
> -#include "net/net.h"
> -#include "hw/i386/pc.h"
> -#include "monitor/monitor.h"
> -#include "hw/scsi/scsi.h"
> -#include "hw/virtio/virtio-blk.h"
> -#include "qemu/config-file.h"
> -#include "sysemu/blockdev.h"
> -#include "qapi/error.h"
> -
> -#if defined(TARGET_I386)
> -static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
> -                                       const char *devaddr,
> -                                       const char *opts_str)
> -{
> -    Error *local_err = NULL;
> -    QemuOpts *opts;
> -    PCIBus *bus;
> -    int ret, devfn;
> -
> -    bus = pci_get_bus_devfn(&devfn, devaddr);
> -    if (!bus) {
> -        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> -        return NULL;
> -    }
> -    if (!((BusState*)bus)->allow_hotplug) {
> -        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> -        return NULL;
> -    }
> -
> -    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
> -    if (!opts) {
> -        return NULL;
> -    }
> -
> -    qemu_opt_set(opts, "type", "nic");
> -
> -    ret = net_client_init(opts, 0, &local_err);
> -    if (error_is_set(&local_err)) {
> -        qerror_report_err(local_err);
> -        error_free(local_err);
> -        return NULL;
> -    }
> -    if (nd_table[ret].devaddr) {
> -        monitor_printf(mon, "Parameter addr not supported\n");
> -        return NULL;
> -    }
> -    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
> -}
> -
> -static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
> -                        DriveInfo *dinfo, int printinfo)
> -{
> -    SCSIBus *scsibus;
> -    SCSIDevice *scsidev;
> -
> -    scsibus = (SCSIBus *)
> -        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
> -                            TYPE_SCSI_BUS);
> -    if (!scsibus) {
> -	error_report("Device is not a SCSI adapter");
> -	return -1;
> -    }
> -
> -    /*
> -     * drive_init() tries to find a default for dinfo->unit.  Doesn't
> -     * work at all for hotplug though as we assign the device to a
> -     * specific bus instead of the first bus with spare scsi ids.
> -     *
> -     * Ditch the calculated value and reload from option string (if
> -     * specified).
> -     */
> -    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
> -    dinfo->bus = scsibus->busnr;
> -    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> -                                        false, -1, NULL);
> -    if (!scsidev) {
> -        return -1;
> -    }
> -    dinfo->unit = scsidev->id;
> -
> -    if (printinfo)
> -        monitor_printf(mon, "OK bus %d, unit %d\n",
> -                       scsibus->busnr, scsidev->id);
> -    return 0;
> -}
> -
> -int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> -{
> -    int dom, pci_bus;
> -    unsigned slot;
> -    PCIDevice *dev;
> -    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> -
> -    switch (dinfo->type) {
> -    case IF_SCSI:
> -        if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
> -            goto err;
> -        }
> -        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> -                              PCI_DEVFN(slot, 0));
> -        if (!dev) {
> -            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> -            goto err;
> -        }
> -        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
> -            goto err;
> -        }
> -        break;
> -    default:
> -        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
> -        goto err;
> -    }
> -
> -    return 0;
> -err:
> -    return -1;
> -}
> -
> -static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> -                                           const char *devaddr,
> -                                           const char *opts)
> -{
> -    PCIDevice *dev;
> -    DriveInfo *dinfo = NULL;
> -    int type = -1;
> -    char buf[128];
> -    PCIBus *bus;
> -    int devfn;
> -
> -    if (get_param_value(buf, sizeof(buf), "if", opts)) {
> -        if (!strcmp(buf, "scsi"))
> -            type = IF_SCSI;
> -        else if (!strcmp(buf, "virtio")) {
> -            type = IF_VIRTIO;
> -        } else {
> -            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
> -            return NULL;
> -        }
> -    } else {
> -        monitor_printf(mon, "no if= specified\n");
> -        return NULL;
> -    }
> -
> -    if (get_param_value(buf, sizeof(buf), "file", opts)) {
> -        dinfo = add_init_drive(opts);
> -        if (!dinfo)
> -            return NULL;
> -        if (dinfo->devaddr) {
> -            monitor_printf(mon, "Parameter addr not supported\n");
> -            return NULL;
> -        }
> -    } else {
> -        dinfo = NULL;
> -    }
> -
> -    bus = pci_get_bus_devfn(&devfn, devaddr);
> -    if (!bus) {
> -        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> -        return NULL;
> -    }
> -    if (!((BusState*)bus)->allow_hotplug) {
> -        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> -        return NULL;
> -    }
> -
> -    switch (type) {
> -    case IF_SCSI:
> -        dev = pci_create(bus, devfn, "lsi53c895a");
> -        if (qdev_init(&dev->qdev) < 0)
> -            dev = NULL;
> -        if (dev && dinfo) {
> -            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
> -                qdev_unplug(&dev->qdev, NULL);
> -                dev = NULL;
> -            }
> -        }
> -        break;
> -    case IF_VIRTIO:
> -        if (!dinfo) {
> -            monitor_printf(mon, "virtio requires a backing file/device.\n");
> -            return NULL;
> -        }
> -        dev = pci_create(bus, devfn, "virtio-blk-pci");
> -        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> -            qdev_free(&dev->qdev);
> -            dev = NULL;
> -            break;
> -        }
> -        if (qdev_init(&dev->qdev) < 0)
> -            dev = NULL;
> -        break;
> -    default:
> -        dev = NULL;
> -    }
> -    return dev;
> -}
> -
> -void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> -{
> -    PCIDevice *dev = NULL;
> -    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> -    const char *type = qdict_get_str(qdict, "type");
> -    const char *opts = qdict_get_try_str(qdict, "opts");
> -
> -    /* strip legacy tag */
> -    if (!strncmp(pci_addr, "pci_addr=", 9)) {
> -        pci_addr += 9;
> -    }
> -
> -    if (!opts) {
> -        opts = "";
> -    }
> -
> -    if (!strcmp(pci_addr, "auto"))
> -        pci_addr = NULL;
> -
> -    if (strcmp(type, "nic") == 0) {
> -        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
> -    } else if (strcmp(type, "storage") == 0) {
> -        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
> -    } else {
> -        monitor_printf(mon, "invalid type: %s\n", type);
> -    }
> -
> -    if (dev) {
> -        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> -                       pci_find_domain(dev->bus),
> -                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
> -                       PCI_FUNC(dev->devfn));
> -    } else
> -        monitor_printf(mon, "failed to add %s\n", opts);
> -}
> -#endif
> -
> -static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> -{
> -    PCIDevice *d;
> -    int dom, bus;
> -    unsigned slot;
> -    Error *local_err = NULL;
> -
> -    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> -        return -1;
> -    }
> -
> -    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> -    if (!d) {
> -        monitor_printf(mon, "slot %d empty\n", slot);
> -        return -1;
> -    }
> -
> -    qdev_unplug(&d->qdev, &local_err);
> -    if (error_is_set(&local_err)) {
> -        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
> -        error_free(local_err);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
> -{
> -    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
> -}
> -- 
> 1.7.10.4

On Thu, May 23, 2013 at 02:11:35PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:05AM +1000, David Gibson wrote:
[snip]
> > diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> > new file mode 100644
> > index 0000000..724a80b
> > --- /dev/null
> > +++ b/hw/pci/pci-hotplug-old.c
> 
> Please use git format-patch -M so renames are
> properly shown. I can't see whether anything changed here.

Ah, sorry.  Forgot that wasn't the default.

> > @@ -0,0 +1,290 @@
> > +/*
> > + * QEMU PCI hotplug support
> 
> I don't particularly mind whether it's called pci-hotplug or
> pci-hotplug-old, but I'm prepared to go with you on this.
> However, what we really should do is fix the comment
> to match reality.

Good point.

> I think this needs two commits:
> - fix comment
> - rename
> 
> to make format-patch -M detect this as a rename

Hrm, I think it should detect the rename, even with a change to the
comment.   But I'll check to make sure, and split if necessary.

Il 09/05/2013 02:31, David Gibson ha scritto:
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> index a7fb9d0..2ad32b6 100644
> --- a/hw/pci/Makefile.objs
> +++ b/hw/pci/Makefile.objs
> @@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
>  common-obj-$(CONFIG_NO_PCI) += pci-stub.o
>  common-obj-$(CONFIG_ALL) += pci-stub.o
>  
> -obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
> +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o

With these changes, pci-hotplug-old.o can be compiled just once.  Please
s/^obj-/common-obj-/.

Thanks,

Paolo

On Thu, May 23, 2013 at 02:11:35PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:05AM +1000, David Gibson wrote:
[snip]
> > diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> > index a7fb9d0..2ad32b6 100644
> > --- a/hw/pci/Makefile.objs
> > +++ b/hw/pci/Makefile.objs
> > @@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
> >  common-obj-$(CONFIG_NO_PCI) += pci-stub.o
> >  common-obj-$(CONFIG_ALL) += pci-stub.o
> >  
> > -obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
> > +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
> > diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> > new file mode 100644
> > index 0000000..724a80b
> > --- /dev/null
> > +++ b/hw/pci/pci-hotplug-old.c
> 
> Please use git format-patch -M so renames are
> properly shown. I can't see whether anything changed here.
> 
> > @@ -0,0 +1,290 @@
> > +/*
> > + * QEMU PCI hotplug support
> 
> I don't particularly mind whether it's called pci-hotplug or
> pci-hotplug-old, but I'm prepared to go with you on this.
> However, what we really should do is fix the comment
> to match reality.

Good point.  Comment updated in my tree.

> I think this needs two commits:
> - fix comment
> - rename
> 
> to make format-patch -M detect this as a rename

I verified that even with the comment change and rename in the same
patch, git format-patch -M detects the rename.

On Thu, May 23, 2013 at 04:54:17PM +0200, Paolo Bonzini wrote:
> Il 09/05/2013 02:31, David Gibson ha scritto:
> > diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> > index a7fb9d0..2ad32b6 100644
> > --- a/hw/pci/Makefile.objs
> > +++ b/hw/pci/Makefile.objs
> > @@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
> >  common-obj-$(CONFIG_NO_PCI) += pci-stub.o
> >  common-obj-$(CONFIG_ALL) += pci-stub.o
> >  
> > -obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
> > +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
> 
> With these changes, pci-hotplug-old.o can be compiled just once.  Please
> s/^obj-/common-obj-/.

Done, thanks.