[Qemu-devel] [patch 0/2] pci hotplug v2

[Qemu-devel] [patch 0/2] pci hotplug v2

[Marcelo Tosatti]

Addressing comments (modulo the replies).

[Qemu-devel] [patch 1/2] pci device registration helpers

[Marcelo Tosatti]

[-- Attachment #1: pci-helper-parser --]
[-- Type: text/plain, Size: 3252 bytes --]

This code parses full PCI device addresses.  It then rejects domains
other than zero, because these are not supported in QEMU.

From: Markus Armbruster <armbru@pond.sub.org>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: trunk/hw/pci.c
===================================================================
--- trunk.orig/hw/pci.c
+++ trunk/hw/pci.c
@@ -26,6 +26,7 @@
 #include "console.h"
 #include "net.h"
 #include "virtio-net.h"
+#include "sysemu.h"
 
 //#define DEBUG_PCI
 
@@ -158,6 +159,89 @@ static int pci_set_default_subsystem_id(
     return 0;
 }
 
+/*
+ * Parse [[<domain>:]<bus>:]<slot>, return -1 on error
+ */
+static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
+{
+    const char *p;
+    char *e;
+    unsigned long val;
+    unsigned long dom = 0, bus = 0;
+    unsigned slot = 0;
+
+    p = addr;
+    val = strtoul(p, &e, 16);
+    if (e == p)
+	return -1;
+    if (*e == ':') {
+	bus = val;
+	p = e + 1;
+	val = strtoul(p, &e, 16);
+	if (e == p)
+	    return -1;
+	if (*e == ':') {
+	    dom = bus;
+	    bus = val;
+	    p = e + 1;
+	    val = strtoul(p, &e, 16);
+	    if (e == p)
+		return -1;
+	}
+    }
+
+    if (dom > 0xffff || bus > 0xff || val > 0x1f) {
+        fprintf(stderr, "PCI device address %s out of range", addr);
+	 return -1;
+    }
+    slot = val;
+
+    if (*e)
+	return -1;
+
+    /* Note: QEMU doesn't implement domains other than 0 */
+    if (dom != 0 || pci_find_bus(bus) == NULL) {
+	fprintf(stderr, "PCI device address %s not supported", addr);
+	return -1;
+    }
+
+    *domp = dom;
+    *busp = bus;
+    *slotp = slot;
+    return 0;
+}
+
+int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
+{
+    char devaddr[32];
+
+    if (!get_param_value(devaddr, sizeof(devaddr), "pci_addr", addr)) {
+        term_printf("Missing pci_addr=\n");
+        return -1;
+    }
+
+    return pci_parse_devaddr(devaddr, domp, busp, slotp);
+}
+
+int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
+{
+    char devaddr[32];
+
+    if (!get_param_value(devaddr, sizeof(devaddr), "pci_addr", addr)) {
+        term_printf("Missing pci_addr=\n");
+        return -1;
+    }
+
+    if (!strcmp(devaddr, "auto")) {
+        *domp = *busp = 0;
+        *slotp = -1;
+        /* want to support dom/bus auto-assign at some point */
+        return 0;
+    }
+
+    return pci_parse_devaddr(devaddr, domp, busp, slotp);
+}
+
 /* -1 for devfn means auto assign */
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
                                int instance_size, int devfn,
Index: trunk/hw/pci.h
===================================================================
--- trunk.orig/hw/pci.h
+++ trunk/hw/pci.h
@@ -232,6 +232,9 @@ void pci_for_each_device(int bus_num, vo
 PCIBus *pci_find_bus(int bus_num);
 PCIDevice *pci_find_device(int bus_num, int slot, int function);
 
+int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
+int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
+
 void pci_info(void);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
                         pci_map_irq_fn map_irq, const char *name);

[Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Marcelo Tosatti]

[-- Attachment #1: dev-and-disk-hotadd --]
[-- Type: text/plain, Size: 14424 bytes --]

Add monitor command to hot-add PCI devices (nic and storage).
    
Syntax is:
    
pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
    
It returns the domain, bus and slot for the newly added device on success.
    
It is possible to attach a disk to a device after PCI initialization via
the drive_add command. If so, a manual scan of the SCSI bus on the guest
is necessary.
    
Save QEMUMachine necessary for drive_init.
    
Add monitor command to hot-remove devices, remove device data on _EJ0 notification.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


Index: trunk/Makefile.target
===================================================================
--- trunk.orig/Makefile.target
+++ trunk/Makefile.target
@@ -580,6 +580,7 @@ OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND
 OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
 OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
+OBJS += device-hotplug.o pci-hotplug.o
 CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
 endif
 ifeq ($(TARGET_BASE_ARCH), ppc)
Index: trunk/hw/boards.h
===================================================================
--- trunk.orig/hw/boards.h
+++ trunk/hw/boards.h
@@ -25,6 +25,8 @@ typedef struct QEMUMachine {
 int qemu_register_machine(QEMUMachine *m);
 void register_machines(void);
 
+extern QEMUMachine *current_machine;
+
 /* Axis ETRAX.  */
 extern QEMUMachine bareetraxfs_machine;
 extern QEMUMachine axisdev88_machine;
Index: trunk/hw/device-hotplug.c
===================================================================
--- /dev/null
+++ trunk/hw/device-hotplug.c
@@ -0,0 +1,61 @@
+#include "hw.h"
+#include "boards.h"
+#include "net.h"
+#include "block_int.h"
+#include "sysemu.h"
+
+int add_init_drive(const char *opts)
+{
+    int drive_opt_idx, drive_idx;
+    int ret = -1;
+
+    drive_opt_idx = drive_add(NULL, "%s", opts);
+    if (!drive_opt_idx)
+        return ret;
+
+    drive_idx = drive_init(&drives_opt[drive_opt_idx], 0, current_machine);
+    if (drive_idx == -1) {
+        drive_remove(drive_opt_idx);
+        return ret;
+    }
+
+    return drive_idx;
+}
+
+void destroy_nic(dev_match_fn *match_fn, void *arg)
+{
+    int i;
+    NICInfo *nic;
+
+    for (i = 0; i < MAX_NICS; i++)
+        nic = &nd_table[i];
+        if (nic->used) {
+            if (nic->private && match_fn(nic->private, arg)) {
+                if (nic->vlan) {
+                    VLANClientState *vc;
+                    vc = qemu_find_vlan_client(nic->vlan, nic->private);
+                    if (vc)
+                        qemu_del_vlan_client(vc);
+                }
+                net_client_uninit(nic);
+            }
+        }
+}
+
+void destroy_bdrvs(dev_match_fn *match_fn, void *arg)
+{
+    int i;
+    struct BlockDriverState *bs;
+
+    for (i = 0; i <= MAX_DRIVES; i++) {
+        bs = drives_table[i].bdrv;
+        if (bs) {
+            if (bs->private && match_fn(bs->private, arg)) {
+                drive_uninit(bs);
+                bdrv_delete(bs);
+            }
+        }
+    }
+}
+
+
Index: trunk/monitor.c
===================================================================
--- trunk.orig/monitor.c
+++ trunk/monitor.c
@@ -1515,6 +1515,20 @@ static const term_cmd_t term_cmds[] = {
       "", "cancel the current VM migration" },
     { "migrate_set_speed", "s", do_migrate_set_speed,
       "value", "set maximum speed (in bytes) for migrations" },
+#if defined(TARGET_I386)
+    { "drive_add", "ss", drive_hot_add, "pci_addr=[[<domain>:]<bus>:]<slot>\n"
+                                         "[file=file][,if=type][,bus=n]\n"
+                                        "[,unit=m][,media=d][index=i]\n"
+                                        "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
+                                        "[snapshot=on|off][,cache=on|off]",
+                                        "add drive to PCI storage controller" },
+    { "pci_add", "sss", pci_device_hot_add, "pci_addr=auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", "hot-add PCI device" },
+    { "pci_del", "s", pci_device_hot_remove, "pci_addr=[[<domain>:]<bus>:]<slot>", "hot remove PCI device" },
+    { "host_net_add", "ss", net_host_device_add,
+      "[tap,user,socket,vde] options", "add host VLAN client" },
+    { "host_net_remove", "is", net_host_device_remove,
+      "vlan_id name", "remove host VLAN client" },
+#endif
     { "balloon", "i", do_balloon,
       "target", "request VM to change it's memory allocation (in MB)" },
     { "set_link", "ss", do_set_link, "name [up|down]" },
Index: trunk/sysemu.h
===================================================================
--- trunk.orig/sysemu.h
+++ trunk/sysemu.h
@@ -170,6 +170,20 @@ extern int drive_init(struct drive_opt *
 void qemu_system_hot_add_init(void);
 void qemu_system_device_hot_add(int pcibus, int slot, int state);
 
+/* device-hotplug */
+
+typedef int (dev_match_fn)(void *dev_private, void *arg);
+
+int add_init_drive(const char *opts);
+void destroy_nic(dev_match_fn *match_fn, void *arg);
+void destroy_bdrvs(dev_match_fn *match_fn, void *arg);
+
+/* pci-hotplug */
+void pci_device_hot_add(const char *pci_addr, const char *type, const char *opts);
+void drive_hot_add(const char *pci_addr, const char *opts);
+void pci_device_hot_remove(const char *pci_addr);
+void pci_device_hot_remove_success(int pcibus, int slot);
+
 /* serial ports */
 
 #define MAX_SERIAL_PORTS 4
Index: trunk/vl.c
===================================================================
--- trunk.orig/vl.c
+++ trunk/vl.c
@@ -3419,6 +3419,7 @@ static void qemu_bh_update_timeout(int *
 /* machine registration */
 
 static QEMUMachine *first_machine = NULL;
+QEMUMachine *current_machine = NULL;
 
 int qemu_register_machine(QEMUMachine *m)
 {
@@ -5603,6 +5604,8 @@ int main(int argc, char **argv, char **e
     machine->init(ram_size, vga_ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
+    current_machine = machine;
+
     /* Set KVM's vcpu state to qemu's initial CPUState. */
     if (kvm_enabled()) {
         int ret;
Index: trunk/hw/acpi.c
===================================================================
--- trunk.orig/hw/acpi.c
+++ trunk/hw/acpi.c
@@ -679,8 +679,12 @@ static uint32_t pciej_read(void *opaque,
 
 static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
 {
+#if defined (TARGET_I386)
     int slot = ffs(val) - 1;
 
+    pci_device_hot_remove_success(0, slot);
+#endif
+
 #if defined(DEBUG)
     printf("pciej write %lx <== %d\n", addr, val);
 #endif
Index: trunk/net.c
===================================================================
--- trunk.orig/net.c
+++ trunk/net.c
@@ -1748,6 +1748,62 @@ void net_client_uninit(NICInfo *nd)
     free((void *)nd->model);
 }
 
+static int net_host_check_device(const char *device)
+{
+    int i;
+    const char *valid_param_list[] = { "tap", "socket"
+#ifdef CONFIG_SLIRP
+                                       ,"user"
+#endif
+#ifdef CONFIG_VDE
+                                       ,"vde"
+#endif
+    };
+    for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
+        if (!strncmp(valid_param_list[i], device,
+                     strlen(valid_param_list[i])))
+            return 1;
+    }
+
+    return 0;
+}
+
+void net_host_device_add(const char *device, const char *opts)
+{
+    if (!net_host_check_device(device)) {
+        term_printf("invalid host network device %s\n", device);
+        return;
+    }
+    net_client_init(device, opts);
+}
+
+void net_host_device_remove(int vlan_id, const char *device)
+{
+    VLANState *vlan;
+    VLANClientState *vc;
+
+    if (!net_host_check_device(device)) {
+        term_printf("invalid host network device %s\n", device);
+        return;
+    }
+
+    vlan = qemu_find_vlan(vlan_id);
+    if (!vlan) {
+        term_printf("can't find vlan %d\n", vlan_id);
+        return;
+    }
+
+   for(vc = vlan->first_client; vc != NULL; vc = vc->next)
+        if (!strcmp(vc->name, device))
+            break;
+
+    if (!vc) {
+        term_printf("can't find device %s\n", device);
+        return;
+    }
+    qemu_del_vlan_client(vc);
+}
+
 int net_client_parse(const char *str)
 {
     const char *p;
Index: trunk/net.h
===================================================================
--- trunk.orig/net.h
+++ trunk/net.h
@@ -102,6 +102,8 @@ void net_slirp_redir(const char *redir_s
 void net_cleanup(void);
 int slirp_is_inited(void);
 void net_client_check(void);
+void net_host_device_add(const char *device, const char *opts);
+void net_host_device_remove(int vlan_id, const char *device);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
Index: trunk/hw/pci-hotplug.c
===================================================================
--- /dev/null
+++ trunk/hw/pci-hotplug.c
@@ -0,0 +1,189 @@
+#include "hw.h"
+#include "boards.h"
+#include "pci.h"
+#include "net.h"
+#include "sysemu.h"
+#include "pc.h"
+#include "console.h"
+#include "block_int.h"
+#include "virtio-blk.h"
+
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+static PCIDevice *qemu_pci_hot_add_nic(PCIBus *pci_bus, const char *opts)
+{
+    int ret;
+
+    ret = net_client_init ("nic", opts);
+    if (ret < 0 || !nd_table[ret].model)
+        return NULL;
+    return pci_nic_init (pci_bus, &nd_table[ret], -1, "rtl8139");
+}
+
+void drive_hot_add(const char *pci_addr, const char *opts)
+{
+    int dom, pci_bus;
+    unsigned slot;
+    int drive_idx, type, bus;
+    int success = 0;
+    PCIDevice *dev;
+
+    if (pci_read_devaddr(pci_addr, &dom, &pci_bus, &slot))
+        return;
+
+    dev = pci_find_device(pci_bus, slot, 0);
+    if (!dev) {
+        term_printf("no pci device with address %s\n", pci_addr);
+        return;
+    }
+
+    drive_idx = add_init_drive(opts);
+    if (drive_idx < 0)
+        return;
+    type = drives_table[drive_idx].type;
+    bus = drive_get_max_bus (type);
+
+    switch (type) {
+    case IF_SCSI:
+        success = 1;
+        lsi_scsi_attach (dev, drives_table[drive_idx].bdrv,
+                         drives_table[drive_idx].unit);
+        break;
+    default:
+        term_printf("Can't hot-add drive to type %d\n", type);
+    }
+
+    if (success)
+        term_printf("OK bus %d, unit %d\n", drives_table[drive_idx].bus,
+                                            drives_table[drive_idx].unit);
+    return;
+}
+
+static PCIDevice *qemu_pci_hot_add_storage(PCIBus *pci_bus, const char *opts)
+{
+    void *opaque = NULL;
+    int type = -1, drive_idx = -1;
+    char buf[128];
+
+    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 {
+        term_printf("no if= specified\n");
+        return NULL;
+    }
+
+    if (get_param_value(buf, sizeof(buf), "file", opts)) {
+        drive_idx = add_init_drive(opts);
+        if (drive_idx < 0)
+            return NULL;
+    } else if (type == IF_VIRTIO) {
+        term_printf("virtio requires a backing file/device.\n");
+        return NULL;
+    }
+
+    switch (type) {
+    case IF_SCSI:
+        opaque = lsi_scsi_init (pci_bus, -1);
+        if (opaque && drive_idx >= 0)
+            lsi_scsi_attach (opaque, drives_table[drive_idx].bdrv,
+                             drives_table[drive_idx].unit);
+        break;
+    case IF_VIRTIO:
+        opaque = virtio_blk_init (pci_bus, drives_table[drive_idx].bdrv);
+        break;
+    default:
+        term_printf ("type %s not a hotpluggable PCI device.\n", buf);
+    }
+
+    return opaque;
+}
+
+void pci_device_hot_add(const char *pci_addr, const char *type, const char *opts)
+{
+    PCIDevice *dev = NULL;
+    PCIBus *pci_bus;
+    int dom, bus;
+    unsigned slot;
+
+    if (pci_assign_devaddr(pci_addr, &dom, &bus, &slot))
+        return;
+
+    pci_bus = pci_find_bus(bus);
+    if (!pci_bus) {
+        term_printf("Can't find pci_bus %d\n", bus);
+        return;
+    }
+
+    if (strcmp(type, "nic") == 0)
+        dev = qemu_pci_hot_add_nic(pci_bus, opts);
+    else if (strcmp(type, "storage") == 0)
+        dev = qemu_pci_hot_add_storage(pci_bus, opts);
+    else
+        term_printf("invalid type: %s\n", type);
+
+    if (dev) {
+        qemu_system_device_hot_add(bus, PCI_SLOT(dev->devfn), 1);
+        term_printf("OK domain %d, bus %d, slot %d, function %d\n",
+                    0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
+                    PCI_FUNC(dev->devfn));
+    } else
+        term_printf("failed to add %s\n", opts);
+}
+#endif
+
+void pci_device_hot_remove(const char *pci_addr)
+{
+    PCIDevice *d;
+    int dom, bus;
+    unsigned slot;
+
+    if (pci_read_devaddr(pci_addr, &dom, &bus, &slot))
+        return;
+
+    d = pci_find_device(bus, slot, 0);
+    if (!d) {
+        term_printf("slot %d empty\n", slot);
+        return;
+    }
+
+    qemu_system_device_hot_add(bus, slot, 0);
+}
+
+static int pci_match_fn(void *dev_private, void *arg)
+{
+    PCIDevice *dev = dev_private;
+    PCIDevice *match = arg;
+
+    return (dev == match);
+}
+
+/*
+ * OS has executed _EJ0 method, we now can remove the device
+ */
+void pci_device_hot_remove_success(int pcibus, int slot)
+{
+    PCIDevice *d = pci_find_device(pcibus, slot, 0);
+    int class_code;
+
+    if (!d) {
+        term_printf("invalid slot %d\n", slot);
+        return;
+    }
+
+    class_code = d->config_read(d, PCI_CLASS_DEVICE+1, 1);
+
+    switch(class_code) {
+    case PCI_BASE_CLASS_STORAGE:
+        destroy_bdrvs(pci_match_fn, d);
+        break;
+    case PCI_BASE_CLASS_NETWORK:
+        destroy_nic(pci_match_fn, d);
+        break;
+    }
+
+    pci_unregister_device(d);
+}
+
Index: trunk/hw/pci.h
===================================================================
--- trunk.orig/hw/pci.h
+++ trunk/hw/pci.h
@@ -14,6 +14,9 @@ extern target_phys_addr_t pci_mem_base;
 
 /* Device classes and subclasses */
 
+#define PCI_BASE_CLASS_STORAGE           0x01
+#define PCI_BASE_CLASS_NETWORK           0x02
+
 #define PCI_CLASS_STORAGE_SCSI           0x0100
 #define PCI_CLASS_STORAGE_IDE            0x0101
 #define PCI_CLASS_STORAGE_OTHER          0x0180

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Avi Kivity]

Marcelo Tosatti wrote:
> Add monitor command to hot-add PCI devices (nic and storage).
>     
> Syntax is:
>     
> pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
>     
> It returns the domain, bus and slot for the newly added device on success

Instead of returning something, how about

  pci_add pci_addr=...,name=blah

and then

  pci_del blah

'info pci' could also output the name so that it could be associated 
with the bus address.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Marcelo Tosatti]

On Sun, Feb 08, 2009 at 12:08:34PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Add monitor command to hot-add PCI devices (nic and storage).
>>     Syntax is:
>>     pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
>>     It returns the domain, bus and slot for the newly added device on 
>> success
>
> Instead of returning something, how about
>
>  pci_add pci_addr=...,name=blah
>
> and then
>
>  pci_del blah
>
> 'info pci' could also output the name so that it could be associated  
> with the bus address.

Sure that can be done. Like an alias. So you identify <domain,bus,slot>
with a name. I can see it simplifies direct management of hotplug on the
monitor.

Markus, Daniel, any comments on that?

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Markus Armbruster]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Sun, Feb 08, 2009 at 12:08:34PM +0200, Avi Kivity wrote:
>> Marcelo Tosatti wrote:
>>> Add monitor command to hot-add PCI devices (nic and storage).
>>>     Syntax is:
>>>     pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
>>>     It returns the domain, bus and slot for the newly added device on 
>>> success
>>
>> Instead of returning something, how about
>>
>>  pci_add pci_addr=...,name=blah
>>
>> and then
>>
>>  pci_del blah
>>
>> 'info pci' could also output the name so that it could be associated  
>> with the bus address.
>
> Sure that can be done. Like an alias. So you identify <domain,bus,slot>
> with a name. I can see it simplifies direct management of hotplug on the
> monitor.
>
> Markus, Daniel, any comments on that?

No objections from me, but Dan is the man to tell us what simplifies
management software and what doesn't.

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Daniel P. Berrange]

On Sun, Feb 08, 2009 at 03:28:45PM -0200, Marcelo Tosatti wrote:
> On Sun, Feb 08, 2009 at 12:08:34PM +0200, Avi Kivity wrote:
> > Marcelo Tosatti wrote:
> >> Add monitor command to hot-add PCI devices (nic and storage).
> >>     Syntax is:
> >>     pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
> >>     It returns the domain, bus and slot for the newly added device on 
> >> success
> >
> > Instead of returning something, how about
> >
> >  pci_add pci_addr=...,name=blah
> >
> > and then
> >
> >  pci_del blah
> >
> > 'info pci' could also output the name so that it could be associated  
> > with the bus address.
> 
> Sure that can be done. Like an alias. So you identify <domain,bus,slot>
> with a name. I can see it simplifies direct management of hotplug on the
> monitor.

Also bear in mind that we need the ability to hot unplug devices which
were specified on the command line argv. For these we currently have
neither the pci <domain,bus,slot>, nor are given any 'name'. Markus
has previously suggested allowing <domain,bus,slot> to be specified on
the cli which would let us manually assign & use that info for unplug.

A device type specific unique naming would be useful too though. In
changeset r6220,  Mark McLoughlin provided the ability to give every
single NIC a unique name. I'd like to be able to use that name for
unplug. 

For disks, meanwhile we generally know the logic bus,unit,index info as
provided to -drive / pci_add disk, which can be used to unplug.

So, I'd suggest that pci_del allow either a <domain,bus,slot> or some
form of unique name, or device type specific unique identifier. Ideally
so we can still just as easily use unplug for devices added on the cli,
as for those added via the monitor.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Marcelo Tosatti]

On Mon, Feb 09, 2009 at 08:41:39AM +0000, Daniel P. Berrange wrote:
> On Sun, Feb 08, 2009 at 03:28:45PM -0200, Marcelo Tosatti wrote:
> > On Sun, Feb 08, 2009 at 12:08:34PM +0200, Avi Kivity wrote:
> > > Marcelo Tosatti wrote:
> > >> Add monitor command to hot-add PCI devices (nic and storage).
> > >>     Syntax is:
> > >>     pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
> > >>     It returns the domain, bus and slot for the newly added device on 
> > >> success
> > >
> > > Instead of returning something, how about
> > >
> > >  pci_add pci_addr=...,name=blah
> > >
> > > and then
> > >
> > >  pci_del blah
> > >
> > > 'info pci' could also output the name so that it could be associated  
> > > with the bus address.
> > 
> > Sure that can be done. Like an alias. So you identify <domain,bus,slot>
> > with a name. I can see it simplifies direct management of hotplug on the
> > monitor.
> 
> Also bear in mind that we need the ability to hot unplug devices which
> were specified on the command line argv. For these we currently have
> neither the pci <domain,bus,slot>, nor are given any 'name'. 

Any reason you can't discover <domain,bus,slot> from "info pci" ?
(domain/bus are always zero at the moment).

> Markus has previously suggested allowing <domain,bus,slot> to be
> specified on the cli which would let us manually assign & use that
> info for unplug.

> A device type specific unique naming would be useful too though. In
> changeset r6220,  Mark McLoughlin provided the ability to give every
> single NIC a unique name. I'd like to be able to use that name for
> unplug. 

The NIC identifiers are not specific to PCI. So if you specify a
non-PCI nic with the cli, you should not expect "pci_del name" to work.
Now I see where you're getting at with the private discussion around
"dev_add/dev_add".

Ok, simply adding a PCI specific identifier can be messy (since it will
clash with net identifier, for example).

So what is needed is, perhaps, a unique identifier
across all devices in the system? (hum, QEMUDevice from
http://bellard.org/qemu/patches/qemu-config2.diff).

> For disks, meanwhile we generally know the logic bus,unit,index info as
> provided to -drive / pci_add disk, which can be used to unplug.
> 
> So, I'd suggest that pci_del allow either a <domain,bus,slot>

It does.

> or some form of unique name, or device type specific unique
> identifier. Ideally so we can still just as easily use unplug for
> devices added on the cli, as for those added via the monitor.

info pci should work.

Thanks.

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Daniel P. Berrange]

On Mon, Feb 09, 2009 at 08:41:25PM -0200, Marcelo Tosatti wrote:
> On Mon, Feb 09, 2009 at 08:41:39AM +0000, Daniel P. Berrange wrote:
> > 
> > Also bear in mind that we need the ability to hot unplug devices which
> > were specified on the command line argv. For these we currently have
> > neither the pci <domain,bus,slot>, nor are given any 'name'. 
> 
> Any reason you can't discover <domain,bus,slot> from "info pci" ?
> (domain/bus are always zero at the moment).

'info pci' just gives you a list of PCI devices, and no direct means
to correlate these to devices listed in your command line argv.

eg, I launch QEMU with 3 nics

  qemu \
  ....more args...
  -net tap  -net nic,mac=01:02:03:04:05:01 \
  -net tap  -net nic,mac=01:02:03:04:05:02 \
  -net tap  -net nic,mac=01:02:03:04:05:03 \
  ....more args...

Then info pci shows this:

  ...snip...
  Bus  0, device   3, function 0:
    Ethernet controller: PCI device 10ec:8029
      IRQ 11.
      BAR0: I/O at 0xc100 [0xc1ff].
  Bus  0, device   4, function 0:
    Ethernet controller: PCI device 10ec:8029
      IRQ 9.
      BAR0: I/O at 0xc200 [0xc2ff].
  Bus  0, device   5, function 0:
    Ethernet controller: PCI device 10ec:8029
      IRQ 11.
      BAR0: I/O at 0xc300 [0xc3ff].
  ...snip...

I want to remove NIC with mac adress 01:02:03:04:05:02, but there is
no way to correlate this 'info pci' data to the NIC I wish to remove.
They may happen to appear in the same sorted order immediately after
startup, but if you've hot added & removed a couple of devices already,
then the order from 'info pci' can't easily map back to argv, unless
you very carefully tracked all changes.

> > Markus has previously suggested allowing <domain,bus,slot> to be
> > specified on the cli which would let us manually assign & use that
> > info for unplug.
> 
> > A device type specific unique naming would be useful too though. In
> > changeset r6220,  Mark McLoughlin provided the ability to give every
> > single NIC a unique name. I'd like to be able to use that name for
> > unplug. 
> 
> The NIC identifiers are not specific to PCI. So if you specify a
> non-PCI nic with the cli, you should not expect "pci_del name" to work.
> Now I see where you're getting at with the private discussion around
> "dev_add/dev_add".

Yes, if the device already had a unqiue identifier, it some ways it
would be easier to just be able todo  'nic_del 01:02:03:04:05:02'
Or for a SCSI disk 'drive_del bus=scsi,index=3'. QEMU ought to have
enough info internally to be able to map this to the PCI address it
needs to remove.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [patch 2/2] qemu: PCI device, disk and host network hot-add / hot-remove

[Paul Brook]

> Then info pci shows this:
>
>   ...snip...
>   Bus  0, device   3, function 0:
>     Ethernet controller: PCI device 10ec:8029
>       IRQ 11.
>       BAR0: I/O at 0xc100 [0xc1ff].

Sounds like the real problem is that info pci and info net don't contain 
enough information.

> Yes, if the device already had a unqiue identifier, it some ways it
> would be easier to just be able todo  'nic_del 01:02:03:04:05:02'
> Or for a SCSI disk 'drive_del bus=scsi,index=3'. QEMU ought to have
> enough info internally to be able to map this to the PCI address it
> needs to remove.

It may be reasonably to assume a 1:1 mapping between network interfaces and 
devices (for current devices at least). However the same is not true for 
disks. Adding/removing host adapters, and adding/removing disks should be 
separate operaions. The same strategy could also be used for network devices 
(you create a PCI NIC, then you connect that NIC to a vlan). We're probably 
going to want this separation anyway for machine config files.

Paul

Re: [Qemu-devel] [patch 0/2] pci hotplug v2

[Marcelo Tosatti]

[-- Attachment #1: Type: text/plain, Size: 125 bytes --]

On Fri, Feb 06, 2009 at 03:44:06PM -0200, Marcelo Tosatti wrote:
> Addressing comments (modulo the replies).

Now for real.


[-- Attachment #2: pci-helpers --]
[-- Type: text/plain, Size: 1810 bytes --]

qemu: add pci helper functions

Add pci_find_bus/pci_find_device to be used by PCI hotplug.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: trunk/hw/pci.c
===================================================================
--- trunk.orig/hw/pci.c
+++ trunk/hw/pci.c
@@ -713,6 +713,26 @@ static void pci_bridge_write_config(PCID
     pci_default_write_config(d, address, val, len);
 }
 
+PCIBus *pci_find_bus(int bus_num)
+{
+    PCIBus *bus = first_bus;
+
+    while (bus && bus->bus_num != bus_num)
+        bus = bus->next;
+
+    return bus;
+}
+
+PCIDevice *pci_find_device(int bus_num, int slot, int function)
+{
+    PCIBus *bus = pci_find_bus(bus_num);
+
+    if (!bus)
+        return NULL;
+
+    return bus->devices[PCI_DEVFN(slot, function)];
+}
+
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
                         pci_map_irq_fn map_irq, const char *name)
 {
Index: trunk/hw/pci.h
===================================================================
--- trunk.orig/hw/pci.h
+++ trunk/hw/pci.h
@@ -8,6 +8,10 @@
 
 extern target_phys_addr_t pci_mem_base;
 
+#define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
+#define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
+#define PCI_FUNC(devfn)         ((devfn) & 0x07)
+
 /* Device classes and subclasses */
 
 #define PCI_CLASS_STORAGE_SCSI           0x0100
@@ -222,6 +226,8 @@ void pci_data_write(void *opaque, uint32
 uint32_t pci_data_read(void *opaque, uint32_t addr, int len);
 int pci_bus_num(PCIBus *s);
 void pci_for_each_device(int bus_num, void (*fn)(PCIDevice *d));
+PCIBus *pci_find_bus(int bus_num);
+PCIDevice *pci_find_device(int bus_num, int slot, int function);
 
 void pci_info(void);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,

[-- Attachment #3: dev-and-disk-hotadd --]
[-- Type: text/plain, Size: 14490 bytes --]

qemu: PCI device, disk and host network hot-add / hot-remove
    
Add monitor command to hot-add PCI devices (nic and storage).
    
Syntax is:
    
pci_add pci_addr=[[<domain>:]<bus>:]<slot> nic|storage params
    
It returns the domain, bus and slot for the newly added device on success.
    
It is possible to attach a disk to a device after PCI initialization via
the drive_add command. If so, a manual scan of the SCSI bus on the guest
is necessary.
    
Save QEMUMachine necessary for drive_init.
    
Add monitor command to hot-remove devices, remove device data on _EJ0 notification.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


Index: trunk/Makefile.target
===================================================================
--- trunk.orig/Makefile.target
+++ trunk/Makefile.target
@@ -580,6 +580,7 @@ OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND
 OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
 OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
+OBJS += device-hotplug.o pci-hotplug.o
 CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
 endif
 ifeq ($(TARGET_BASE_ARCH), ppc)
Index: trunk/hw/boards.h
===================================================================
--- trunk.orig/hw/boards.h
+++ trunk/hw/boards.h
@@ -25,6 +25,8 @@ typedef struct QEMUMachine {
 int qemu_register_machine(QEMUMachine *m);
 void register_machines(void);
 
+extern QEMUMachine *current_machine;
+
 /* Axis ETRAX.  */
 extern QEMUMachine bareetraxfs_machine;
 extern QEMUMachine axisdev88_machine;
Index: trunk/hw/device-hotplug.c
===================================================================
--- /dev/null
+++ trunk/hw/device-hotplug.c
@@ -0,0 +1,61 @@
+#include "hw.h"
+#include "boards.h"
+#include "net.h"
+#include "block_int.h"
+#include "sysemu.h"
+
+int add_init_drive(const char *opts)
+{
+    int drive_opt_idx, drive_idx;
+    int ret = -1;
+
+    drive_opt_idx = drive_add(NULL, "%s", opts);
+    if (!drive_opt_idx)
+        return ret;
+
+    drive_idx = drive_init(&drives_opt[drive_opt_idx], 0, current_machine);
+    if (drive_idx == -1) {
+        drive_remove(drive_opt_idx);
+        return ret;
+    }
+
+    return drive_idx;
+}
+
+void destroy_nic(dev_match_fn *match_fn, void *arg)
+{
+    int i;
+    NICInfo *nic;
+
+    for (i = 0; i < MAX_NICS; i++)
+        nic = &nd_table[i];
+        if (nic->used) {
+            if (nic->private && match_fn(nic->private, arg)) {
+                if (nic->vlan) {
+                    VLANClientState *vc;
+                    vc = qemu_find_vlan_client(nic->vlan, nic->private);
+                    if (vc)
+                        qemu_del_vlan_client(vc);
+                }
+                net_client_uninit(nic);
+            }
+        }
+}
+
+void destroy_bdrvs(dev_match_fn *match_fn, void *arg)
+{
+    int i;
+    struct BlockDriverState *bs;
+
+    for (i = 0; i <= MAX_DRIVES; i++) {
+        bs = drives_table[i].bdrv;
+        if (bs) {
+            if (bs->private && match_fn(bs->private, arg)) {
+                drive_uninit(bs);
+                bdrv_delete(bs);
+            }
+        }
+    }
+}
+
+
Index: trunk/monitor.c
===================================================================
--- trunk.orig/monitor.c
+++ trunk/monitor.c
@@ -1515,6 +1515,20 @@ static const term_cmd_t term_cmds[] = {
       "", "cancel the current VM migration" },
     { "migrate_set_speed", "s", do_migrate_set_speed,
       "value", "set maximum speed (in bytes) for migrations" },
+#if defined(TARGET_I386)
+    { "drive_add", "ss", drive_hot_add, "pci_addr=[[<domain>:]<bus>:]<slot>\n"
+                                         "[file=file][,if=type][,bus=n]\n"
+                                        "[,unit=m][,media=d][index=i]\n"
+                                        "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
+                                        "[snapshot=on|off][,cache=on|off]",
+                                        "add drive to PCI storage controller" },
+    { "pci_add", "sss", pci_device_hot_add, "pci_addr=auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", "hot-add PCI device" },
+    { "pci_del", "s", pci_device_hot_remove, "pci_addr=[[<domain>:]<bus>:]<slot>", "hot remove PCI device" },
+    { "host_net_add", "ss", net_host_device_add,
+      "[tap,user,socket,vde] options", "add host VLAN client" },
+    { "host_net_remove", "is", net_host_device_remove,
+      "vlan_id name", "remove host VLAN client" },
+#endif
     { "balloon", "i", do_balloon,
       "target", "request VM to change it's memory allocation (in MB)" },
     { "set_link", "ss", do_set_link, "name [up|down]" },
Index: trunk/sysemu.h
===================================================================
--- trunk.orig/sysemu.h
+++ trunk/sysemu.h
@@ -170,6 +170,20 @@ extern int drive_init(struct drive_opt *
 void qemu_system_hot_add_init(void);
 void qemu_system_device_hot_add(int pcibus, int slot, int state);
 
+/* device-hotplug */
+
+typedef int (dev_match_fn)(void *dev_private, void *arg);
+
+int add_init_drive(const char *opts);
+void destroy_nic(dev_match_fn *match_fn, void *arg);
+void destroy_bdrvs(dev_match_fn *match_fn, void *arg);
+
+/* pci-hotplug */
+void pci_device_hot_add(const char *pci_addr, const char *type, const char *opts);
+void drive_hot_add(const char *pci_addr, const char *opts);
+void pci_device_hot_remove(const char *pci_addr);
+void pci_device_hot_remove_success(int pcibus, int slot);
+
 /* serial ports */
 
 #define MAX_SERIAL_PORTS 4
Index: trunk/vl.c
===================================================================
--- trunk.orig/vl.c
+++ trunk/vl.c
@@ -3419,6 +3419,7 @@ static void qemu_bh_update_timeout(int *
 /* machine registration */
 
 static QEMUMachine *first_machine = NULL;
+QEMUMachine *current_machine = NULL;
 
 int qemu_register_machine(QEMUMachine *m)
 {
@@ -5603,6 +5604,8 @@ int main(int argc, char **argv, char **e
     machine->init(ram_size, vga_ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
+    current_machine = machine;
+
     /* Set KVM's vcpu state to qemu's initial CPUState. */
     if (kvm_enabled()) {
         int ret;
Index: trunk/hw/acpi.c
===================================================================
--- trunk.orig/hw/acpi.c
+++ trunk/hw/acpi.c
@@ -679,8 +679,12 @@ static uint32_t pciej_read(void *opaque,
 
 static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
 {
+#if defined (TARGET_I386)
     int slot = ffs(val) - 1;
 
+    pci_device_hot_remove_success(0, slot);
+#endif
+
 #if defined(DEBUG)
     printf("pciej write %lx <== %d\n", addr, val);
 #endif
Index: trunk/net.c
===================================================================
--- trunk.orig/net.c
+++ trunk/net.c
@@ -1748,6 +1748,62 @@ void net_client_uninit(NICInfo *nd)
     free((void *)nd->model);
 }
 
+static int net_host_check_device(const char *device)
+{
+    int i;
+    const char *valid_param_list[] = { "tap", "socket"
+#ifdef CONFIG_SLIRP
+                                       ,"user"
+#endif
+#ifdef CONFIG_VDE
+                                       ,"vde"
+#endif
+    };
+    for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
+        if (!strncmp(valid_param_list[i], device,
+                     strlen(valid_param_list[i])))
+            return 1;
+    }
+
+    return 0;
+}
+
+void net_host_device_add(const char *device, const char *opts)
+{
+    if (!net_host_check_device(device)) {
+        term_printf("invalid host network device %s\n", device);
+        return;
+    }
+    net_client_init(device, opts);
+}
+
+void net_host_device_remove(int vlan_id, const char *device)
+{
+    VLANState *vlan;
+    VLANClientState *vc;
+
+    if (!net_host_check_device(device)) {
+        term_printf("invalid host network device %s\n", device);
+        return;
+    }
+
+    vlan = qemu_find_vlan(vlan_id);
+    if (!vlan) {
+        term_printf("can't find vlan %d\n", vlan_id);
+        return;
+    }
+
+   for(vc = vlan->first_client; vc != NULL; vc = vc->next)
+        if (!strcmp(vc->name, device))
+            break;
+
+    if (!vc) {
+        term_printf("can't find device %s\n", device);
+        return;
+    }
+    qemu_del_vlan_client(vc);
+}
+
 int net_client_parse(const char *str)
 {
     const char *p;
Index: trunk/net.h
===================================================================
--- trunk.orig/net.h
+++ trunk/net.h
@@ -102,6 +102,8 @@ void net_slirp_redir(const char *redir_s
 void net_cleanup(void);
 int slirp_is_inited(void);
 void net_client_check(void);
+void net_host_device_add(const char *device, const char *opts);
+void net_host_device_remove(int vlan_id, const char *device);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
Index: trunk/hw/pci-hotplug.c
===================================================================
--- /dev/null
+++ trunk/hw/pci-hotplug.c
@@ -0,0 +1,189 @@
+#include "hw.h"
+#include "boards.h"
+#include "pci.h"
+#include "net.h"
+#include "sysemu.h"
+#include "pc.h"
+#include "console.h"
+#include "block_int.h"
+#include "virtio-blk.h"
+
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+static PCIDevice *qemu_pci_hot_add_nic(PCIBus *pci_bus, const char *opts)
+{
+    int ret;
+
+    ret = net_client_init ("nic", opts);
+    if (ret < 0 || !nd_table[ret].model)
+        return NULL;
+    return pci_nic_init (pci_bus, &nd_table[ret], -1, "rtl8139");
+}
+
+void drive_hot_add(const char *pci_addr, const char *opts)
+{
+    int dom, pci_bus;
+    unsigned slot;
+    int drive_idx, type, bus;
+    int success = 0;
+    PCIDevice *dev;
+
+    if (pci_read_devaddr(pci_addr, &dom, &pci_bus, &slot))
+        return;
+
+    dev = pci_find_device(pci_bus, slot, 0);
+    if (!dev) {
+        term_printf("no pci device with address %s\n", pci_addr);
+        return;
+    }
+
+    drive_idx = add_init_drive(opts);
+    if (drive_idx < 0)
+        return;
+    type = drives_table[drive_idx].type;
+    bus = drive_get_max_bus (type);
+
+    switch (type) {
+    case IF_SCSI:
+        success = 1;
+        lsi_scsi_attach (dev, drives_table[drive_idx].bdrv,
+                         drives_table[drive_idx].unit);
+        break;
+    default:
+        term_printf("Can't hot-add drive to type %d\n", type);
+    }
+
+    if (success)
+        term_printf("OK bus %d, unit %d\n", drives_table[drive_idx].bus,
+                                            drives_table[drive_idx].unit);
+    return;
+}
+
+static PCIDevice *qemu_pci_hot_add_storage(PCIBus *pci_bus, const char *opts)
+{
+    void *opaque = NULL;
+    int type = -1, drive_idx = -1;
+    char buf[128];
+
+    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 {
+        term_printf("no if= specified\n");
+        return NULL;
+    }
+
+    if (get_param_value(buf, sizeof(buf), "file", opts)) {
+        drive_idx = add_init_drive(opts);
+        if (drive_idx < 0)
+            return NULL;
+    } else if (type == IF_VIRTIO) {
+        term_printf("virtio requires a backing file/device.\n");
+        return NULL;
+    }
+
+    switch (type) {
+    case IF_SCSI:
+        opaque = lsi_scsi_init (pci_bus, -1);
+        if (opaque && drive_idx >= 0)
+            lsi_scsi_attach (opaque, drives_table[drive_idx].bdrv,
+                             drives_table[drive_idx].unit);
+        break;
+    case IF_VIRTIO:
+        opaque = virtio_blk_init (pci_bus, drives_table[drive_idx].bdrv);
+        break;
+    default:
+        term_printf ("type %s not a hotpluggable PCI device.\n", buf);
+    }
+
+    return opaque;
+}
+
+void pci_device_hot_add(const char *pci_addr, const char *type, const char *opts)
+{
+    PCIDevice *dev = NULL;
+    PCIBus *pci_bus;
+    int dom, bus;
+    unsigned slot;
+
+    if (pci_assign_devaddr(pci_addr, &dom, &bus, &slot))
+        return;
+
+    pci_bus = pci_find_bus(bus);
+    if (!pci_bus) {
+        term_printf("Can't find pci_bus %d\n", bus);
+        return;
+    }
+
+    if (strcmp(type, "nic") == 0)
+        dev = qemu_pci_hot_add_nic(pci_bus, opts);
+    else if (strcmp(type, "storage") == 0)
+        dev = qemu_pci_hot_add_storage(pci_bus, opts);
+    else
+        term_printf("invalid type: %s\n", type);
+
+    if (dev) {
+        qemu_system_device_hot_add(bus, PCI_SLOT(dev->devfn), 1);
+        term_printf("OK domain %d, bus %d, slot %d, function %d\n",
+                    0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
+                    PCI_FUNC(dev->devfn));
+    } else
+        term_printf("failed to add %s\n", opts);
+}
+#endif
+
+void pci_device_hot_remove(const char *pci_addr)
+{
+    PCIDevice *d;
+    int dom, bus;
+    unsigned slot;
+
+    if (pci_read_devaddr(pci_addr, &dom, &bus, &slot))
+        return;
+
+    d = pci_find_device(bus, slot, 0);
+    if (!d) {
+        term_printf("slot %d empty\n", slot);
+        return;
+    }
+
+    qemu_system_device_hot_add(bus, slot, 0);
+}
+
+static int pci_match_fn(void *dev_private, void *arg)
+{
+    PCIDevice *dev = dev_private;
+    PCIDevice *match = arg;
+
+    return (dev == match);
+}
+
+/*
+ * OS has executed _EJ0 method, we now can remove the device
+ */
+void pci_device_hot_remove_success(int pcibus, int slot)
+{
+    PCIDevice *d = pci_find_device(pcibus, slot, 0);
+    int class_code;
+
+    if (!d) {
+        term_printf("invalid slot %d\n", slot);
+        return;
+    }
+
+    class_code = d->config_read(d, PCI_CLASS_DEVICE+1, 1);
+
+    switch(class_code) {
+    case PCI_BASE_CLASS_STORAGE:
+        destroy_bdrvs(pci_match_fn, d);
+        break;
+    case PCI_BASE_CLASS_NETWORK:
+        destroy_nic(pci_match_fn, d);
+        break;
+    }
+
+    pci_unregister_device(d);
+}
+
Index: trunk/hw/pci.h
===================================================================
--- trunk.orig/hw/pci.h
+++ trunk/hw/pci.h
@@ -14,6 +14,9 @@ extern target_phys_addr_t pci_mem_base;
 
 /* Device classes and subclasses */
 
+#define PCI_BASE_CLASS_STORAGE           0x01
+#define PCI_BASE_CLASS_NETWORK           0x02
+
 #define PCI_CLASS_STORAGE_SCSI           0x0100
 #define PCI_CLASS_STORAGE_IDE            0x0101
 #define PCI_CLASS_STORAGE_OTHER          0x0180