diff mbox

[12/15] qdev: using devtree lock to protect device's accessing

Message ID 1344407156-25562-13-git-send-email-qemulist@gmail.com
State New
Headers show

Commit Message

pingfan liu Aug. 8, 2012, 6:25 a.m. UTC 
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

lock:
  qemu_device_tree_mutex

competitors:
  --device_del(destruction of device will be postphoned until unplug
    ack from guest),
  --pci hot-unplug
  --iteration (qdev_reset_all)
  --device_add

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 hw/pci-hotplug.c  |    4 ++++
 hw/qdev-monitor.c |   17 ++++++++++++++++-
 hw/qdev.c         |    2 ++
 3 files changed, 22 insertions(+), 1 deletions(-)

Comments

Peter Maydell Aug. 8, 2012, 9:33 a.m. UTC | #1 
On 8 August 2012 07:25, Liu Ping Fan <qemulist@gmail.com> wrote:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>
> lock:
>   qemu_device_tree_mutex

Looking at where it's used, this doesn't seem to have anything to do
with device trees (ie dtb, see www.devicetree.org) : poorly named lock?

-- PMM
diff mbox

Patch

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index e7fb780..33a9dfe 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -265,9 +265,11 @@  static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
         return -1;
     }
 
+    qemu_lock_devtree();
     d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
     if (!d) {
         monitor_printf(mon, "slot %d empty\n", slot);
+        qemu_unlock_devtree();
         return -1;
     }
 
@@ -275,9 +277,11 @@  static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
     if (error_is_set(&local_err)) {
         monitor_printf(mon, "%s\n", error_get_pretty(local_err));
         error_free(local_err);
+        qemu_unlock_devtree();
         return -1;
     }
 
+    qemu_unlock_devtree();
     return 0;
 }
 
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 7915b45..2d47fe0 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -429,14 +429,18 @@  DeviceState *qdev_device_add(QemuOpts *opts)
 
     /* find bus */
     path = qemu_opt_get(opts, "bus");
+
+    qemu_lock_devtree();
     if (path != NULL) {
         bus = qbus_find(path);
         if (!bus) {
+            qemu_unlock_devtree();
             return NULL;
         }
         if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) != 0) {
             qerror_report(QERR_BAD_BUS_FOR_DEVICE,
                           driver, object_get_typename(OBJECT(bus)));
+            qemu_unlock_devtree();
             return NULL;
         }
     } else {
@@ -444,11 +448,13 @@  DeviceState *qdev_device_add(QemuOpts *opts)
         if (!bus) {
             qerror_report(QERR_NO_BUS_FOR_DEVICE,
                           driver, k->bus_type);
+            qemu_unlock_devtree();
             return NULL;
         }
     }
     if (qdev_hotplug && !bus->allow_hotplug) {
         qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
+        qemu_unlock_devtree();
         return NULL;
     }
 
@@ -466,6 +472,7 @@  DeviceState *qdev_device_add(QemuOpts *opts)
     }
     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
         qdev_free(qdev);
+        qemu_unlock_devtree();
         return NULL;
     }
     if (qdev->id) {
@@ -478,6 +485,8 @@  DeviceState *qdev_device_add(QemuOpts *opts)
                                   OBJECT(qdev), NULL);
         g_free(name);
     }        
+    qemu_unlock_devtree();
+
     if (qdev_init(qdev) < 0) {
         qerror_report(QERR_DEVICE_INIT_FAILED, driver);
         return NULL;
@@ -600,13 +609,19 @@  void qmp_device_del(const char *id, Error **errp)
 {
     DeviceState *dev;
 
+    /* protect against unplug ack from guest, where we really remove device
+     * from system
+     */
+    qemu_lock_devtree();
     dev = qdev_find_recursive(sysbus_get_default(), id);
     if (NULL == dev) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+        qemu_unlock_devtree();
         return;
     }
-
+    /* Just remove from system, and drop refcnt there*/
     qdev_unplug(dev, errp);
+    qemu_unlock_devtree();
 }
 
 void qdev_machine_init(void)
diff --git a/hw/qdev.c b/hw/qdev.c
index af54467..17525fe 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -230,7 +230,9 @@  static int qbus_reset_one(BusState *bus, void *opaque)
 
 void qdev_reset_all(DeviceState *dev)
 {
+    qemu_lock_devtree();
     qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL);
+    qemu_unlock_devtree();
 }
 
 void qbus_reset_all_fn(void *opaque)