[Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Igor Mammedov]

Changes since v1:
  * added usb-uas test
  * drop hotplug check in bus_add_child()
  * make SCSI & USB bus as default HotplugHandler
  * fixed dummy HBA hot(un)plug
  * fixed hotunplug on s390x
  * prevent hotplug of non hotpluggable devices
--

Series unifies different hotplug mechanisms to a recent
hotplug-handler API and does shallow conversion of
devices that still use legacy qdev hotplug to it dropping
not used after that legacy hotplug path [29/30].
It also relaces SCSI's own way to do hotplug/unplug with
hotplug-handler callbacks leaving it the only method
perform hotplug tasks.
And the last patch [30/30] allows to unplug of BUS-less
devices using hotplug-handler API.

Converted devices are covered with new hotplug
unit-tests, except of:
 s390x-*: I have no idea how or means to test it, but code
          is close to virtio, so it's converted using
          virtio template
 pvscsi: is broken, so no means to test it

Git tree for testing:
  https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2


Igor Mammedov (36):
  test: virtio-scsi: check if hot-plug/unplug works
  test: virtio-serial: check if hot-plug/unplug works
  test: libqos: add qpci_plug_device_test() and
    qpci_unplug_acpi_device_test()
  test: virtio-rng: check if hot-plug/unplug works
  test: virtio-net: check if hot-plug/unplug works
  test: virtio-blk: check if hot-plug/unplug works
  test: usb: move uhci port test code to libqos/usb.c
  test: usb: add port test to uhci unit test
  test: usb: generic usb device hotplug
  test: usb: usb-storage hotplug test
  test: usb: usb-uas hotplug test
  access BusState.allow_hotplug using wraper qbus_is_hotpluggable()
  qdev: do not allow to instantiate non hotpluggable device with
    device_add
  qdev: HotplugHandler: rename unplug callback to unplug_request
  qdev: HotplugHandler: provide unplug callback
  qdev: add simple/generic unplug callback for HotplugHandler
  qdev: add wrapper to set BUS as HotplugHandler
  qdev: drop hotplug check from bus_add_child()
  target-i386: ICC bus: drop BusState.allow_hotplug
  virtio-pci: drop BusState.allow_hotplug
  virtio-serial: convert to hotplug-handler API
  virtio-mmio: drop useless bus->allow_hotplug = 0
  s390x: drop not used allow_hotplug in event-facility
  s390x: convert s390-virtio to hotplug handler API
  s390x: convert virtio-ccw to hotplug handler API
  scsi: set SCSI BUS itself as default HotplugHandler
  scsi: convert pvscsi HBA to hotplug handler API
  scsi: convert virtio-scsi HBA to hotplug handler API
  scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} fields
  usb-bot: mark device as non hotpluggable
  usb-bot: drop not needed "allow_hotplug = 0"
  usb-storage: drop not needed "allow_hotplug = 0"
  usb: convert usb-ccid to hotplug handler API
  usb: convert usb devices to hotplug handler API
  qdev: drop legacy hotplug fields/methods
  qdev: HotplugHandler: add support for unplugging BUS-less devices

 hw/acpi/piix4.c                |  6 +--
 hw/char/virtio-serial-bus.c    | 20 +++++++---
 hw/core/hotplug.c              | 11 ++++++
 hw/core/qdev.c                 | 85 ++++++++++++++++++++++++++++--------------
 hw/cpu/icc_bus.c               |  8 ----
 hw/i386/acpi-build.c           |  2 +-
 hw/isa/lpc_ich9.c              |  6 +--
 hw/pci-bridge/pci_bridge_dev.c |  2 +-
 hw/pci/pci-hotplug-old.c       |  4 +-
 hw/pci/pcie.c                  |  4 +-
 hw/pci/pcie_port.c             |  2 +-
 hw/pci/shpc.c                  |  4 +-
 hw/s390x/event-facility.c      |  2 -
 hw/s390x/s390-virtio-bus.c     | 12 +++---
 hw/s390x/virtio-ccw.c          | 17 +++++----
 hw/scsi/scsi-bus.c             | 24 ++++--------
 hw/scsi/virtio-scsi.c          | 30 ++++++++++-----
 hw/scsi/vmw_pvscsi.c           | 26 +++++++++----
 hw/usb/bus.c                   |  9 ++++-
 hw/usb/dev-smartcard-reader.c  |  8 +++-
 hw/usb/dev-storage.c           |  4 +-
 hw/virtio/virtio-mmio.c        | 17 +--------
 hw/virtio/virtio-pci.c         |  3 --
 include/hw/hotplug.h           | 16 +++++++-
 include/hw/pci/pcie.h          |  4 +-
 include/hw/pci/shpc.h          |  4 +-
 include/hw/qdev-core.h         | 17 +++++----
 include/hw/scsi/scsi.h         |  2 -
 qdev-monitor.c                 |  5 ++-
 tests/Makefile                 | 13 ++++---
 tests/libqos/pci-pc.c          | 49 ++++++++++++++++++++++++
 tests/libqos/pci.h             |  3 ++
 tests/libqos/usb.c             | 71 +++++++++++++++++++++++++++++++++++
 tests/libqos/usb.h             | 17 +++++++++
 tests/usb-hcd-ehci-test.c      | 50 ++++++++++---------------
 tests/usb-hcd-ohci-test.c      | 10 ++++-
 tests/usb-hcd-uhci-test.c      | 65 +++++++++++++++++++++++++++++++-
 tests/usb-hcd-xhci-test.c      | 68 ++++++++++++++++++++++++++++++++-
 tests/virtio-blk-test.c        | 49 ++++++++++++++++++------
 tests/virtio-net-test.c        | 10 +++++
 tests/virtio-rng-test.c        | 10 +++++
 tests/virtio-scsi-test.c       | 29 ++++++++++++++
 tests/virtio-serial-test.c     | 27 ++++++++++++++
 43 files changed, 624 insertions(+), 201 deletions(-)
 create mode 100644 tests/libqos/usb.c
 create mode 100644 tests/libqos/usb.h

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 01/36] test: virtio-scsi: check if hot-plug/unplug works

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/virtio-scsi-test.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 3230908..41f9602 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -17,14 +17,43 @@ static void pci_nop(void)
 {
 }
 
+static void hotplug(void)
+{
+    QDict *response;
+
+    response = qmp("{\"execute\": \"device_add\","
+                   " \"arguments\": {"
+                   "   \"driver\": \"scsi-hd\","
+                   "   \"id\": \"scsi-hd\","
+                   "   \"drive\": \"drv1\""
+                   "}}");
+
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("{\"execute\": \"device_del\","
+                   " \"arguments\": {"
+                   "   \"id\": \"scsi-hd\""
+                   "}}");
+
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
 
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/scsi/pci/nop", pci_nop);
+    qtest_add_func("/virtio/scsi/pci/hotplug", hotplug);
 
     qtest_start("-drive id=drv0,if=none,file=/dev/null "
+                "-drive id=drv1,if=none,file=/dev/null "
                 "-device virtio-scsi-pci,id=vscsi0 "
                 "-device scsi-hd,bus=vscsi0.0,drive=drv0");
     ret = g_test_run();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 02/36] test: virtio-serial: check if hot-plug/unplug works

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/virtio-serial-test.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/virtio-serial-test.c b/tests/virtio-serial-test.c
index e743875..bf030a6 100644
--- a/tests/virtio-serial-test.c
+++ b/tests/virtio-serial-test.c
@@ -17,12 +17,39 @@ static void pci_nop(void)
 {
 }
 
+static void hotplug(void)
+{
+    QDict *response;
+
+    response = qmp("{\"execute\": \"device_add\","
+                   " \"arguments\": {"
+                   "   \"driver\": \"virtserialport\","
+                   "   \"id\": \"hp-port\""
+                   "}}");
+
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("{\"execute\": \"device_del\","
+                   " \"arguments\": {"
+                   "   \"id\": \"hp-port\""
+                   "}}");
+
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
 
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/serial/pci/nop", pci_nop);
+    qtest_add_func("/virtio/serial/pci/hotplug", hotplug);
 
     qtest_start("-device virtio-serial-pci");
     ret = g_test_run();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 03/36] test: libqos: add qpci_plug_device_test() and qpci_unplug_acpi_device_test()

[Igor Mammedov]

functions will be used for testing hot(un)plug of PCI
devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/libqos/pci-pc.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/pci.h    |  3 +++
 2 files changed, 52 insertions(+)

diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 0609294..6dba0db 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -20,6 +20,9 @@
 
 #include <glib.h>
 
+#define ACPI_PCIHP_ADDR         0xae00
+#define PCI_EJ_BASE             0x0008
+
 typedef struct QPCIBusPC
 {
     QPCIBus bus;
@@ -247,3 +250,49 @@ void qpci_free_pc(QPCIBus *bus)
 
     g_free(s);
 }
+
+void qpci_plug_device_test(const char *driver, const char *id,
+                           uint8_t slot, const char *opts)
+{
+    QDict *response;
+    char *cmd;
+
+    cmd = g_strdup_printf("{'execute': 'device_add',"
+                          " 'arguments': {"
+                          "   'driver': '%s',"
+                          "   'addr': '%d',"
+                          "   %s%s"
+                          "   'id': '%s'"
+                          "}}", driver, slot,
+                          opts ? opts : "", opts ? "," : "",
+                          id);
+    response = qmp(cmd);
+    g_free(cmd);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+}
+
+void qpci_unplug_acpi_device_test(const char *id, uint8_t slot)
+{
+    QDict *response;
+    char *cmd;
+
+    cmd = g_strdup_printf("{'execute': 'device_del',"
+                          " 'arguments': {"
+                          "   'id': '%s'"
+                          "}}", id);
+    response = qmp(cmd);
+    g_free(cmd);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    outb(ACPI_PCIHP_ADDR + PCI_EJ_BASE, 1 << slot);
+
+    response = qmp("");
+    g_assert(response);
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+}
diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
index d51eb9e..dfaee9e 100644
--- a/tests/libqos/pci.h
+++ b/tests/libqos/pci.h
@@ -87,4 +87,7 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
 void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
 void qpci_iounmap(QPCIDevice *dev, void *data);
 
+void qpci_plug_device_test(const char *driver, const char *id,
+                           uint8_t slot, const char *opts);
+void qpci_unplug_acpi_device_test(const char *id, uint8_t slot);
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 04/36] test: virtio-rng: check if hot-plug/unplug works

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  moved code duplication into library
---
 tests/Makefile          |  2 +-
 tests/virtio-rng-test.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index f5de29c..e72c78c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -325,7 +325,7 @@ tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
 tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o
-tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o
+tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y)
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o
 tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o
 tests/virtio-serial-test$(EXESUF): tests/virtio-serial-test.o
diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
index 402c206..41c1cdb 100644
--- a/tests/virtio-rng-test.c
+++ b/tests/virtio-rng-test.c
@@ -11,18 +11,28 @@
 #include <string.h>
 #include "libqtest.h"
 #include "qemu/osdep.h"
+#include "libqos/pci.h"
+
+#define PCI_SLOT_HP             0x06
 
 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void pci_nop(void)
 {
 }
 
+static void hotplug(void)
+{
+    qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
+    qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
 
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/rng/pci/nop", pci_nop);
+    qtest_add_func("/virtio/rng/pci/hotplug", hotplug);
 
     qtest_start("-device virtio-rng-pci");
     ret = g_test_run();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 05/36] test: virtio-net: check if hot-plug/unplug works

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  moved code duplication into library
---
 tests/Makefile          |  2 +-
 tests/virtio-net-test.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index e72c78c..f2ee9d8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -324,7 +324,7 @@ tests/ne2000-test$(EXESUF): tests/ne2000-test.o
 tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
 tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
-tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o
+tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y)
 tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y)
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o
 tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index df99343..ea7478c 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -11,18 +11,28 @@
 #include <string.h>
 #include "libqtest.h"
 #include "qemu/osdep.h"
+#include "libqos/pci.h"
+
+#define PCI_SLOT_HP             0x06
 
 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void pci_nop(void)
 {
 }
 
+static void hotplug(void)
+{
+    qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
+    qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
 
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/net/pci/nop", pci_nop);
+    qtest_add_func("/virtio/net/pci/hotplug", hotplug);
 
     qtest_start("-device virtio-net-pci");
     ret = g_test_run();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 06/36] test: virtio-blk: check if hot-plug/unplug works

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  moved code duplication into library
---
 tests/virtio-blk-test.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 588666c..d93224b 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -45,6 +45,8 @@
 #define PCI_SLOT                0x04
 #define PCI_FN                  0x00
 
+#define PCI_SLOT_HP             0x06
+
 typedef struct QVirtioBlkReq {
     uint32_t type;
     uint32_t ioprio;
@@ -55,7 +57,7 @@ typedef struct QVirtioBlkReq {
 
 static QPCIBus *test_start(void)
 {
-    char cmdline[100];
+    char *cmdline;
     char tmp_path[] = "/tmp/qtest.XXXXXX";
     int fd, ret;
 
@@ -66,11 +68,14 @@ static QPCIBus *test_start(void)
     g_assert_cmpint(ret, ==, 0);
     close(fd);
 
-    snprintf(cmdline, 100, "-drive if=none,id=drive0,file=%s "
-                            "-device virtio-blk-pci,drive=drive0,addr=%x.%x",
-                            tmp_path, PCI_SLOT, PCI_FN);
+    cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s "
+                              "-drive if=none,id=drive1,file=/dev/null "
+                              "-device virtio-blk-pci,id=drv0,drive=drive0,"
+                              "addr=%x.%x",
+                              tmp_path, PCI_SLOT, PCI_FN);
     qtest_start(cmdline);
     unlink(tmp_path);
+    g_free(cmdline);
 
     return qpci_init_pc();
 }
@@ -80,14 +85,14 @@ static void test_end(void)
     qtest_end();
 }
 
-static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus)
+static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus, int slot)
 {
     QVirtioPCIDevice *dev;
 
     dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
     g_assert(dev != NULL);
     g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
-    g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
+    g_assert_cmphex(dev->pdev->devfn, ==, ((slot << 3) | PCI_FN));
 
     qvirtio_pci_device_enable(dev);
     qvirtio_reset(&qvirtio_pci, &dev->vdev);
@@ -147,7 +152,7 @@ static void pci_basic(void)
 
     bus = test_start();
 
-    dev = virtio_blk_init(bus);
+    dev = virtio_blk_init(bus, PCI_SLOT);
 
     /* MSI-X is not enabled */
     addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -293,7 +298,7 @@ static void pci_indirect(void)
 
     bus = test_start();
 
-    dev = virtio_blk_init(bus);
+    dev = virtio_blk_init(bus, PCI_SLOT);
 
     /* MSI-X is not enabled */
     addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -384,7 +389,7 @@ static void pci_config(void)
 
     bus = test_start();
 
-    dev = virtio_blk_init(bus);
+    dev = virtio_blk_init(bus, PCI_SLOT);
 
     /* MSI-X is not enabled */
     addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -426,7 +431,7 @@ static void pci_msix(void)
     bus = test_start();
     alloc = pc_alloc_init();
 
-    dev = virtio_blk_init(bus);
+    dev = virtio_blk_init(bus, PCI_SLOT);
     qpci_msix_enable(dev->pdev);
 
     qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
@@ -536,7 +541,7 @@ static void pci_idx(void)
     bus = test_start();
     alloc = pc_alloc_init();
 
-    dev = virtio_blk_init(bus);
+    dev = virtio_blk_init(bus, PCI_SLOT);
     qpci_msix_enable(dev->pdev);
 
     qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
@@ -640,6 +645,27 @@ static void pci_idx(void)
     test_end();
 }
 
+static void hotplug(void)
+{
+    QPCIBus *bus;
+    QVirtioPCIDevice *dev;
+
+    bus = test_start();
+
+    /* plug secondary disk */
+    qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
+                          "'drive': 'drive1'");
+
+    dev = virtio_blk_init(bus, PCI_SLOT_HP);
+    g_assert(dev);
+    qvirtio_pci_device_disable(dev);
+    g_free(dev);
+
+    /* unplug secondary disk */
+    qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
+    test_end();
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -651,6 +677,7 @@ int main(int argc, char **argv)
     g_test_add_func("/virtio/blk/pci/config", pci_config);
     g_test_add_func("/virtio/blk/pci/msix", pci_msix);
     g_test_add_func("/virtio/blk/pci/idx", pci_idx);
+    g_test_add_func("/virtio/blk/pci/hotplug", hotplug);
 
     ret = g_test_run();
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 07/36] test: usb: move uhci port test code to libqos/usb.c

[Igor Mammedov]

Move code necessary for testing uhci port into library
so it could be used by other USB tests.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/Makefile            |  3 ++-
 tests/libqos/usb.c        | 37 +++++++++++++++++++++++++++++++++++++
 tests/libqos/usb.h        | 14 ++++++++++++++
 tests/usb-hcd-ehci-test.c | 36 +++++-------------------------------
 4 files changed, 58 insertions(+), 32 deletions(-)
 create mode 100644 tests/libqos/usb.c
 create mode 100644 tests/libqos/usb.h

diff --git a/tests/Makefile b/tests/Makefile
index f2ee9d8..f1eff1b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -301,6 +301,7 @@ libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o
+libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
@@ -345,7 +346,7 @@ tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o
 tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o
 tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o
 tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o
-tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-pc-obj-y)
+tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c
new file mode 100644
index 0000000..54865b4
--- /dev/null
+++ b/tests/libqos/usb.c
@@ -0,0 +1,37 @@
+/*
+ * common code shared by usb tests
+ *
+ * Copyright (c) 2014 Red Hat, Inc
+ *
+ * Authors:
+ *     Gerd Hoffmann <kraxel@redhat.com>
+ *     John Snow <jsnow@redhat.com>
+ *     Igor Mammedov <imammedo@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include <glib.h>
+#include <string.h>
+#include "libqtest.h"
+#include "qemu/osdep.h"
+#include "hw/usb/uhci-regs.h"
+#include "libqos/usb.h"
+
+void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc, uint32_t devfn, int bar)
+{
+    hc->dev = qpci_device_find(pcibus, devfn);
+    g_assert(hc->dev != NULL);
+    qpci_device_enable(hc->dev);
+    hc->base = qpci_iomap(hc->dev, bar, NULL);
+    g_assert(hc->base != NULL);
+}
+
+void uhci_port_test(struct qhc *hc, int port, uint16_t expect)
+{
+    void *addr = hc->base + 0x10 + 2 * port;
+    uint16_t value = qpci_io_readw(hc->dev, addr);
+    uint16_t mask = ~(UHCI_PORT_WRITE_CLEAR | UHCI_PORT_RSVD1);
+
+    g_assert((value & mask) == (expect & mask));
+}
diff --git a/tests/libqos/usb.h b/tests/libqos/usb.h
new file mode 100644
index 0000000..7fcd669
--- /dev/null
+++ b/tests/libqos/usb.h
@@ -0,0 +1,14 @@
+#ifndef LIBQOS_USB_H
+#define LIBQOS_USB_H
+
+#include "libqos/pci-pc.h"
+
+struct qhc {
+    QPCIDevice *dev;
+    void *base;
+};
+
+void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc,
+                       uint32_t devfn, int bar);
+void uhci_port_test(struct qhc *hc, int port, uint16_t expect);
+#endif
diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
index c990492..69f8522 100644
--- a/tests/usb-hcd-ehci-test.c
+++ b/tests/usb-hcd-ehci-test.c
@@ -15,11 +15,7 @@
 #include "qemu/osdep.h"
 #include "hw/usb/uhci-regs.h"
 #include "hw/usb/ehci-regs.h"
-
-struct qhc {
-    QPCIDevice *dev;
-    void *base;
-};
+#include "libqos/usb.h"
 
 static QPCIBus *pcibus;
 static struct qhc uhci1;
@@ -29,15 +25,6 @@ static struct qhc ehci1;
 
 /* helpers */
 
-static void pci_init_one(struct qhc *hc, uint32_t devfn, int bar)
-{
-    hc->dev = qpci_device_find(pcibus, devfn);
-    g_assert(hc->dev != NULL);
-    qpci_device_enable(hc->dev);
-    hc->base = qpci_iomap(hc->dev, bar, NULL);
-    g_assert(hc->base != NULL);
-}
-
 #if 0
 static void uhci_port_update(struct qhc *hc, int port,
                              uint16_t set, uint16_t clear)
@@ -52,19 +39,6 @@ static void uhci_port_update(struct qhc *hc, int port,
 }
 #endif
 
-static void uhci_port_test(struct qhc *hc, int port, uint16_t expect)
-{
-    void *addr = hc->base + 0x10 + 2 * port;
-    uint16_t value = qpci_io_readw(hc->dev, addr);
-    uint16_t mask = ~(UHCI_PORT_WRITE_CLEAR | UHCI_PORT_RSVD1);
-
-#if 0
-    fprintf(stderr, "%s: %d, have 0x%04x, want 0x%04x\n",
-            __func__, port, value & mask, expect & mask);
-#endif
-    g_assert((value & mask) == (expect & mask));
-}
-
 static void ehci_port_test(struct qhc *hc, int port, uint32_t expect)
 {
     void *addr = hc->base + 0x64 + 4 * port;
@@ -88,10 +62,10 @@ static void pci_init(void)
     pcibus = qpci_init_pc();
     g_assert(pcibus != NULL);
 
-    pci_init_one(&uhci1, QPCI_DEVFN(0x1d, 0), 4);
-    pci_init_one(&uhci2, QPCI_DEVFN(0x1d, 1), 4);
-    pci_init_one(&uhci3, QPCI_DEVFN(0x1d, 2), 4);
-    pci_init_one(&ehci1, QPCI_DEVFN(0x1d, 7), 0);
+    qusb_pci_init_one(pcibus, &uhci1, QPCI_DEVFN(0x1d, 0), 4);
+    qusb_pci_init_one(pcibus, &uhci2, QPCI_DEVFN(0x1d, 1), 4);
+    qusb_pci_init_one(pcibus, &uhci3, QPCI_DEVFN(0x1d, 2), 4);
+    qusb_pci_init_one(pcibus, &ehci1, QPCI_DEVFN(0x1d, 7), 0);
 }
 
 static void pci_uhci_port_1(void)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 08/36] test: usb: add port test to uhci unit test

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  split patch from code movment for ehci test
---
 tests/Makefile            |  2 +-
 tests/usb-hcd-uhci-test.c | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index f1eff1b..2374926 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -345,7 +345,7 @@ tests/es1370-test$(EXESUF): tests/es1370-test.o
 tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o
 tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o
 tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o
-tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o
+tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 94e858f..68047b8 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -11,13 +11,23 @@
 #include <string.h>
 #include "libqtest.h"
 #include "qemu/osdep.h"
+#include "libqos/usb.h"
+#include "hw/usb/uhci-regs.h"
 
 
 static void test_uhci_init(void)
 {
-    qtest_start("-device piix3-usb-uhci,id=uhci");
+}
 
-    qtest_end();
+static void test_port_1(void)
+{
+    QPCIBus *pcibus;
+    struct qhc uhci;
+
+    pcibus = qpci_init_pc();
+    g_assert(pcibus != NULL);
+    qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
+    uhci_port_test(&uhci, 0, UHCI_PORT_CCS);
 }
 
 
@@ -28,8 +38,12 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_func("/uhci/pci/init", test_uhci_init);
+    qtest_add_func("/uhci/pci/port1", test_port_1);
 
+    qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
+                " -device usb-tablet,bus=uhci.0,port=1");
     ret = g_test_run();
+    qtest_end();
 
     return ret;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 09/36] test: usb: generic usb device hotplug

[Igor Mammedov]

use usb-tablet as a hotplugged usb device.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Since I'm not familiar with internals of USB,
for OHCI/XHCI test doesn't verify that usb device
has been hotplugged except of checking for error
after hotplug. So port checks similar to that of
ehci/uhci, left as excercise for future.
---
 tests/Makefile            |  4 ++--
 tests/libqos/usb.c        | 34 ++++++++++++++++++++++++++++++++++
 tests/libqos/usb.h        |  3 +++
 tests/usb-hcd-ehci-test.c | 14 ++++++++++++++
 tests/usb-hcd-ohci-test.c | 10 ++++++++--
 tests/usb-hcd-uhci-test.c | 20 ++++++++++++++++++--
 tests/usb-hcd-xhci-test.c | 11 ++++++++---
 7 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 2374926..c00b0e3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -344,10 +344,10 @@ tests/ac97-test$(EXESUF): tests/ac97-test.o
 tests/es1370-test$(EXESUF): tests/es1370-test.o
 tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o
 tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o
-tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o
+tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y)
-tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o
+tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c
index 54865b4..41d89b8 100644
--- a/tests/libqos/usb.c
+++ b/tests/libqos/usb.c
@@ -35,3 +35,37 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect)
 
     g_assert((value & mask) == (expect & mask));
 }
+
+void usb_test_hotplug(const char *hcd_id, const int port,
+                      void (*port_check)(void))
+{
+    QDict *response;
+    char  *cmd;
+
+    cmd = g_strdup_printf("{'execute': 'device_add',"
+                          " 'arguments': {"
+                          "   'driver': 'usb-tablet',"
+                          "   'port': '%d',"
+                          "   'bus': '%s.0',"
+                          "   'id': 'usbdev%d'"
+                          "}}", port, hcd_id, port);
+    response = qmp(cmd);
+    g_free(cmd);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    if (port_check) {
+        port_check();
+    }
+
+    cmd = g_strdup_printf("{'execute': 'device_del',"
+                           " 'arguments': {"
+                           "   'id': 'usbdev%d'"
+                           "}}", port);
+    response = qmp(cmd);
+    g_free(cmd);
+    g_assert(response);
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+}
diff --git a/tests/libqos/usb.h b/tests/libqos/usb.h
index 7fcd669..8fe5687 100644
--- a/tests/libqos/usb.h
+++ b/tests/libqos/usb.h
@@ -11,4 +11,7 @@ struct qhc {
 void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc,
                        uint32_t devfn, int bar);
 void uhci_port_test(struct qhc *hc, int port, uint16_t expect);
+
+void usb_test_hotplug(const char *bus_name, const int port,
+                      void (*port_check)(void));
 #endif
diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
index 69f8522..75073bf 100644
--- a/tests/usb-hcd-ehci-test.c
+++ b/tests/usb-hcd-ehci-test.c
@@ -128,6 +128,19 @@ static void pci_ehci_port_2(void)
     }
 }
 
+static void pci_ehci_port_3_hotplug(void)
+{
+    /* check for presence of hotplugged usb-tablet */
+    g_assert(pcibus != NULL);
+    ehci_port_test(&ehci1, 2, PORTSC_PPOWER | PORTSC_CONNECT);
+}
+
+static void pci_ehci_port_hotplug(void)
+{
+    usb_test_hotplug("ich9-ehci-1", 3, pci_ehci_port_3_hotplug);
+}
+
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -139,6 +152,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ehci/pci/ehci-config", pci_ehci_config);
     qtest_add_func("/ehci/pci/uhci-port-2", pci_uhci_port_2);
     qtest_add_func("/ehci/pci/ehci-port-2", pci_ehci_port_2);
+    qtest_add_func("/ehci/pci/ehci-port-3-hotplug", pci_ehci_port_hotplug);
 
     qtest_start("-machine q35 -device ich9-usb-ehci1,bus=pcie.0,addr=1d.7,"
                 "multifunction=on,id=ich9-ehci-1 "
diff --git a/tests/usb-hcd-ohci-test.c b/tests/usb-hcd-ohci-test.c
index fbc3ffe..1160bde 100644
--- a/tests/usb-hcd-ohci-test.c
+++ b/tests/usb-hcd-ohci-test.c
@@ -11,15 +11,18 @@
 #include <string.h>
 #include "libqtest.h"
 #include "qemu/osdep.h"
+#include "libqos/usb.h"
 
 
 static void test_ohci_init(void)
 {
-    qtest_start("-device pci-ohci,id=ohci");
 
-    qtest_end();
 }
 
+static void test_ohci_hotplug(void)
+{
+    usb_test_hotplug("ohci", 1, NULL);
+}
 
 int main(int argc, char **argv)
 {
@@ -28,8 +31,11 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_func("/ohci/pci/init", test_ohci_init);
+    qtest_add_func("/ohci/pci/hotplug", test_ohci_hotplug);
 
+    qtest_start("-device pci-ohci,id=ohci");
     ret = g_test_run();
+    qtest_end();
 
     return ret;
 }
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 68047b8..da96f98 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -19,17 +19,32 @@ static void test_uhci_init(void)
 {
 }
 
-static void test_port_1(void)
+static void test_port(int port)
 {
     QPCIBus *pcibus;
     struct qhc uhci;
 
+    g_assert(port > 0);
     pcibus = qpci_init_pc();
     g_assert(pcibus != NULL);
     qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
-    uhci_port_test(&uhci, 0, UHCI_PORT_CCS);
+    uhci_port_test(&uhci, port - 1, UHCI_PORT_CCS);
+}
+
+static void test_port_1(void)
+{
+    test_port(1);
 }
 
+static void test_port_2(void)
+{
+    test_port(2);
+}
+
+static void test_uhci_hotplug(void)
+{
+    usb_test_hotplug("uhci", 2, test_port_2);
+}
 
 int main(int argc, char **argv)
 {
@@ -39,6 +54,7 @@ int main(int argc, char **argv)
 
     qtest_add_func("/uhci/pci/init", test_uhci_init);
     qtest_add_func("/uhci/pci/port1", test_port_1);
+    qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug);
 
     qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
                 " -device usb-tablet,bus=uhci.0,port=1");
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index 743e979..d16963d 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -11,15 +11,17 @@
 #include <string.h>
 #include "libqtest.h"
 #include "qemu/osdep.h"
+#include "libqos/usb.h"
 
 
 static void test_xhci_init(void)
 {
-    qtest_start("-device nec-usb-xhci,id=xhci");
-
-    qtest_end();
 }
 
+static void test_xhci_hotplug(void)
+{
+    usb_test_hotplug("xhci", 1, NULL);
+}
 
 int main(int argc, char **argv)
 {
@@ -28,8 +30,11 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_func("/xhci/pci/init", test_xhci_init);
+    qtest_add_func("/xhci/pci/hotplug", test_xhci_hotplug);
 
+    qtest_start("-device nec-usb-xhci,id=xhci");
     ret = g_test_run();
+    qtest_end();
 
     return ret;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 10/36] test: usb: usb-storage hotplug test

[Igor Mammedov]

usb-storage is different from usual usb devices
in that it uses child SCSI bus for underlying storage.
That patch verifies that SCSI bus is hotpluggable for
hotplug operation wouldn't succeed without it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/usb-hcd-uhci-test.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index da96f98..8cf2c5b 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -46,6 +46,35 @@ static void test_uhci_hotplug(void)
     usb_test_hotplug("uhci", 2, test_port_2);
 }
 
+static void test_usb_storage_hotplug(void)
+{
+    QDict *response;
+
+    response = qmp("{'execute': 'device_add',"
+                   " 'arguments': {"
+                   "   'driver': 'usb-storage',"
+                   "   'drive': 'drive0',"
+                   "   'id': 'usbdev0'"
+                   "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("{'execute': 'device_del',"
+                           " 'arguments': {"
+                           "   'id': 'usbdev0'"
+                           "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("");
+    g_assert(response);
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -55,8 +84,10 @@ int main(int argc, char **argv)
     qtest_add_func("/uhci/pci/init", test_uhci_init);
     qtest_add_func("/uhci/pci/port1", test_port_1);
     qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug);
+    qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
 
     qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
+                " -drive id=drive0,if=none,file=/dev/null"
                 " -device usb-tablet,bus=uhci.0,port=1");
     ret = g_test_run();
     qtest_end();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 11/36] test: usb: usb-uas hotplug test

[Igor Mammedov]

checks that it's possible to hotplug usb-uas HBA and
then if it's possible to hot(un)plug scsi-disk to it.
Thest basically covers hot(un)plug on dummy HBAs
without means of hot(un)plug notification of the guest.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/usb-hcd-xhci-test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index d16963d..b1a7dec 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -23,6 +23,63 @@ static void test_xhci_hotplug(void)
     usb_test_hotplug("xhci", 1, NULL);
 }
 
+static void test_usb_uas_hotplug(void)
+{
+    QDict *response;
+
+    response = qmp("{'execute': 'device_add',"
+                   " 'arguments': {"
+                   "   'driver': 'usb-uas',"
+                   "   'id': 'uas'"
+                   "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("{'execute': 'device_add',"
+                   " 'arguments': {"
+                   "   'driver': 'scsi-hd',"
+                   "   'drive': 'drive0',"
+                   "   'id': 'scsi-hd'"
+                   "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    /* TODO:
+        UAS HBA driver in libqos, to check that
+        added disk is visible after BUS rescan
+    */
+
+    response = qmp("{'execute': 'device_del',"
+                           " 'arguments': {"
+                           "   'id': 'scsi-hd'"
+                           "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("");
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+
+
+    response = qmp("{'execute': 'device_del',"
+                           " 'arguments': {"
+                           "   'id': 'uas'"
+                           "}}");
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    response = qmp("");
+    g_assert(response);
+    g_assert(qdict_haskey(response, "event"));
+    g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
+    QDECREF(response);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -31,8 +88,10 @@ int main(int argc, char **argv)
 
     qtest_add_func("/xhci/pci/init", test_xhci_init);
     qtest_add_func("/xhci/pci/hotplug", test_xhci_hotplug);
+    qtest_add_func("/xhci/pci/hotplug/usb-uas", test_usb_uas_hotplug);
 
-    qtest_start("-device nec-usb-xhci,id=xhci");
+    qtest_start("-device nec-usb-xhci,id=xhci"
+                " -drive id=drive0,if=none,file=/dev/null");
     ret = g_test_run();
     qtest_end();
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 12/36] access BusState.allow_hotplug using wraper qbus_is_hotpluggable()

[Igor Mammedov]

it would allow transparently switch detection if Bus
is hotpluggable from allow_hotplug field to hotplug_handler
link and drop allow_hotplug field once all users are
converted to hotplug handler API.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c           | 6 +++---
 hw/i386/acpi-build.c     | 2 +-
 hw/pci/pci-hotplug-old.c | 4 ++--
 include/hw/qdev-core.h   | 5 +++++
 qdev-monitor.c           | 2 +-
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fcb1638..5e5b963 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -86,7 +86,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
     BusChild *kid = g_malloc0(sizeof(*kid));
 
     if (qdev_hotplug) {
-        assert(bus->allow_hotplug);
+        assert(qbus_is_hotpluggable(bus));
     }
 
     kid->index = bus->max_index++;
@@ -213,7 +213,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
+    if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
         error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
         return;
     }
@@ -925,7 +925,7 @@ static bool device_get_hotpluggable(Object *obj, Error **errp)
     DeviceState *dev = DEVICE(obj);
 
     return dc->hotpluggable && (dev->parent_bus == NULL ||
-                                dev->parent_bus->allow_hotplug);
+                                qbus_is_hotpluggable(dev->parent_bus));
 }
 
 static bool device_get_hotplugged(Object *obj, Error **err)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a313321..00be4bb 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -774,7 +774,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
     unsigned *bsel_alloc = opaque;
     unsigned *bus_bsel;
 
-    if (bus->qbus.allow_hotplug) {
+    if (qbus_is_hotpluggable(BUS(bus))) {
         bus_bsel = g_malloc(sizeof *bus_bsel);
 
         *bus_bsel = (*bsel_alloc)++;
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index cf2caeb..f9b7398 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -77,7 +77,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
     }
-    if (!((BusState*)bus)->allow_hotplug) {
+    if (!qbus_is_hotpluggable(BUS(bus))) {
         monitor_printf(mon, "PCI bus doesn't support hotplug\n");
         return NULL;
     }
@@ -224,7 +224,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
     }
-    if (!((BusState*)bus)->allow_hotplug) {
+    if (!qbus_is_hotpluggable(BUS(bus))) {
         monitor_printf(mon, "PCI bus doesn't support hotplug\n");
         return NULL;
     }
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 178fee2..48a96d2 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -368,4 +368,9 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
                              QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
     bus->allow_hotplug = 1;
 }
+
+static inline bool qbus_is_hotpluggable(BusState *bus)
+{
+   return bus->allow_hotplug || bus->hotplug_handler;
+}
 #endif
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 5ec6606..f6db461 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -515,7 +515,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
             return NULL;
         }
     }
-    if (qdev_hotplug && bus && !bus->allow_hotplug) {
+    if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
         qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
         return NULL;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 13/36] qdev: do not allow to instantiate non hotpluggable device with device_add

[Igor Mammedov]

It will allow explicitly mark device as not hotpluggable and
avoid its creation with following error at realize time
and destroying it afterwards anyway. Instead of it will
error out even before instance of device is created.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 qdev-monitor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index f6db461..c721451 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -487,7 +487,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     }
 
     dc = DEVICE_CLASS(oc);
-    if (dc->cannot_instantiate_with_device_add_yet) {
+    if (dc->cannot_instantiate_with_device_add_yet ||
+        (qdev_hotplug && !dc->hotpluggable)) {
         qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver",
                       "pluggable device type");
         return NULL;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 14/36] qdev: HotplugHandler: rename unplug callback to unplug_request

[Igor Mammedov]

'HotplugHandler.unplug' callback is currently used as async
call to issue unplug request for device that implements it.
Renaming 'unplug' callback to 'unplug_request' should help to
avoid confusion about what callback does and would allow to
introduce 'unplug' callback that would perform actual device
removal when guest is ready for it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
Patch is prompted by reviewing https://patchwork.ozlabs.org/patch/383372/

Dedicated 'unplug' callback could be used by bus-less pc-dimm
device. It would allow to call HotplugHandler.unplug callback
from ACPI code when guest calls _EJ0 method and execute board
specific code (PCMachine) to unmap pc-dimm from guest's address
space and perform necessary cleanup. The same applies to CPU
unplug.
---
 hw/acpi/piix4.c                |  6 +++---
 hw/core/hotplug.c              | 10 +++++-----
 hw/core/qdev.c                 |  3 ++-
 hw/isa/lpc_ich9.c              |  6 +++---
 hw/pci-bridge/pci_bridge_dev.c |  2 +-
 hw/pci/pcie.c                  |  4 ++--
 hw/pci/pcie_port.c             |  2 +-
 hw/pci/shpc.c                  |  4 ++--
 include/hw/hotplug.h           | 16 +++++++++-------
 include/hw/pci/pcie.h          |  4 ++--
 include/hw/pci/shpc.h          |  4 ++--
 11 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..0bfa814 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -354,8 +354,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
     }
 }
 
-static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev,
-                                   DeviceState *dev, Error **errp)
+static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                           DeviceState *dev, Error **errp)
 {
     PIIX4PMState *s = PIIX4_PM(hotplug_dev);
 
@@ -615,7 +615,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     dc->cannot_instantiate_with_device_add_yet = true;
     dc->hotpluggable = false;
     hc->plug = piix4_device_plug_cb;
-    hc->unplug = piix4_device_unplug_cb;
+    hc->unplug_request = piix4_device_unplug_request_cb;
     adevc->ospm_status = piix4_ospm_status;
 }
 
diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
index 5573d9d..2ec4736 100644
--- a/hw/core/hotplug.c
+++ b/hw/core/hotplug.c
@@ -23,14 +23,14 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
     }
 }
 
-void hotplug_handler_unplug(HotplugHandler *plug_handler,
-                            DeviceState *plugged_dev,
-                            Error **errp)
+void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
+                                    DeviceState *plugged_dev,
+                                    Error **errp)
 {
     HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
 
-    if (hdc->unplug) {
-        hdc->unplug(plug_handler, plugged_dev, errp);
+    if (hdc->unplug_request) {
+        hdc->unplug_request(plug_handler, plugged_dev, errp);
     }
 }
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5e5b963..c98e5db 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -227,7 +227,8 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     qdev_hot_removed = true;
 
     if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
-        hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
+        hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
+                                       dev, errp);
     } else {
         assert(dc->unplug != NULL);
         if (dc->unplug(dev) < 0) { /* legacy handler */
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 177023b..530b074 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -607,8 +607,8 @@ static void ich9_device_plug_cb(HotplugHandler *hotplug_dev,
     ich9_pm_device_plug_cb(&lpc->pm, dev, errp);
 }
 
-static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev,
-                                  DeviceState *dev, Error **errp)
+static void ich9_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                          DeviceState *dev, Error **errp)
 {
     error_setg(errp, "acpi: device unplug request for not supported device"
                " type: %s", object_get_typename(OBJECT(dev)));
@@ -676,7 +676,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
      */
     dc->cannot_instantiate_with_device_add_yet = true;
     hc->plug = ich9_device_plug_cb;
-    hc->unplug = ich9_device_unplug_cb;
+    hc->unplug_request = ich9_device_unplug_request_cb;
     adevc->ospm_status = ich9_pm_ospm_status;
 }
 
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 92799d0..252ea5e 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -150,7 +150,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &pci_bridge_dev_vmstate;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     hc->plug = shpc_device_hotplug_cb;
-    hc->unplug = shpc_device_hot_unplug_cb;
+    hc->unplug_request = shpc_device_hot_unplug_request_cb;
 }
 
 static const TypeInfo pci_bridge_dev_info = {
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 1babddf..b64a004 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -262,8 +262,8 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                         PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
 }
 
-void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
-                                 Error **errp)
+void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                         DeviceState *dev, Error **errp)
 {
     uint8_t *exp_cap;
 
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index fa24877..40ca8d5 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -154,7 +154,7 @@ static void pcie_slot_class_init(ObjectClass *oc, void *data)
 
     dc->props = pcie_slot_props;
     hc->plug = pcie_cap_slot_hotplug_cb;
-    hc->unplug = pcie_cap_slot_hot_unplug_cb;
+    hc->unplug_request = pcie_cap_slot_hot_unplug_request_cb;
 }
 
 static const TypeInfo pcie_slot_type_info = {
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 1fcb8c4..65b2f51 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -549,8 +549,8 @@ void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
     shpc_interrupt_update(pci_hotplug_dev);
 }
 
-void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
-                               Error **errp)
+void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
 {
     Error *local_err = NULL;
     PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index a6533cb..e397d08 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -47,7 +47,9 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
  *
  * @parent: Opaque parent interface.
  * @plug: plug callback.
- * @unplug: unplug callback.
+ * @unplug_request: unplug request callback.
+ *                  Used as a means to initiate device unplug for devices that
+ *                  require asynchronous unplug handling.
  */
 typedef struct HotplugHandlerClass {
     /* <private> */
@@ -55,7 +57,7 @@ typedef struct HotplugHandlerClass {
 
     /* <public> */
     hotplug_fn plug;
-    hotplug_fn unplug;
+    hotplug_fn unplug_request;
 } HotplugHandlerClass;
 
 /**
@@ -68,11 +70,11 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
                           Error **errp);
 
 /**
- * hotplug_handler_unplug:
+ * hotplug_handler_unplug_request:
  *
- * Call #HotplugHandlerClass.unplug callback of @plug_handler.
+ * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.
  */
-void hotplug_handler_unplug(HotplugHandler *plug_handler,
-                            DeviceState *plugged_dev,
-                            Error **errp);
+void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
+                                    DeviceState *plugged_dev,
+                                    Error **errp);
 #endif
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index d139d58..b48a7a2 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -128,6 +128,6 @@ extern const VMStateDescription vmstate_pcie_device;
 
 void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp);
-void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
-                                 Error **errp);
+void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                         DeviceState *dev, Error **errp);
 #endif /* QEMU_PCIE_H */
diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
index eef1a1a..025bc5b 100644
--- a/include/hw/pci/shpc.h
+++ b/include/hw/pci/shpc.h
@@ -46,8 +46,8 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len);
 
 void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                             Error **errp);
-void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
-                               Error **errp);
+void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp);
 
 extern VMStateInfo shpc_vmstate_info;
 #define SHPC_VMSTATE(_field, _type) \
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 15/36] qdev: HotplugHandler: provide unplug callback

[Igor Mammedov]

it to be called for actual device removal and
will allow to separate request and removal handling
phases of x86-CPU devices and also it's a handler
to be called for synchronously removable devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
unplug handling for bus-less devices will be added
later in this series.

v2:
 - fixup copypast error, s/unplug_request/unplug/
    spotted by Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/core/hotplug.c    | 11 +++++++++++
 hw/core/qdev.c       | 13 +++++++++++--
 include/hw/hotplug.h | 12 ++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
index 2ec4736..4e01074 100644
--- a/hw/core/hotplug.c
+++ b/hw/core/hotplug.c
@@ -34,6 +34,17 @@ void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
     }
 }
 
+void hotplug_handler_unplug(HotplugHandler *plug_handler,
+                            DeviceState *plugged_dev,
+                            Error **errp)
+{
+    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+
+    if (hdc->unplug) {
+        hdc->unplug(plug_handler, plugged_dev, errp);
+    }
+}
+
 static const TypeInfo hotplug_handler_info = {
     .name          = TYPE_HOTPLUG_HANDLER,
     .parent        = TYPE_INTERFACE,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c98e5db..c89d781 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -227,8 +227,17 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     qdev_hot_removed = true;
 
     if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
-        hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
-                                       dev, errp);
+        HotplugHandlerClass *hdc;
+
+        /* If device supports async unplug just request it to be done,
+         * otherwise just remove it synchronously */
+        hdc = HOTPLUG_HANDLER_GET_CLASS(dev->parent_bus->hotplug_handler);
+        if (hdc->unplug_request) {
+            hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
+                                           dev, errp);
+        } else {
+            hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
+        }
     } else {
         assert(dc->unplug != NULL);
         if (dc->unplug(dev) < 0) { /* legacy handler */
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index e397d08..050d2f0 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -50,6 +50,9 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
  * @unplug_request: unplug request callback.
  *                  Used as a means to initiate device unplug for devices that
  *                  require asynchronous unplug handling.
+ * @unplug: unplug callback.
+ *          Used for device removal with devices that implement
+ *          asynchronous and synchronous (suprise) removal.
  */
 typedef struct HotplugHandlerClass {
     /* <private> */
@@ -58,6 +61,7 @@ typedef struct HotplugHandlerClass {
     /* <public> */
     hotplug_fn plug;
     hotplug_fn unplug_request;
+    hotplug_fn unplug;
 } HotplugHandlerClass;
 
 /**
@@ -77,4 +81,12 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
 void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
                                     DeviceState *plugged_dev,
                                     Error **errp);
+/**
+ * hotplug_handler_unplug:
+ *
+ * Calls #HotplugHandlerClass.unplug callback of @plug_handler.
+ */
+void hotplug_handler_unplug(HotplugHandler *plug_handler,
+                            DeviceState *plugged_dev,
+                            Error **errp);
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 16/36] qdev: add simple/generic unplug callback for HotplugHandler

[Igor Mammedov]

it will be used in shallow conversion from legacy hotplug
mechanism and eventually replace all the uses of old mechanism
DeviceClass->unplug = qdev_simple_unplug_cb()

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c         | 5 +++++
 include/hw/qdev-core.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c89d781..1d1b113 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -286,6 +286,11 @@ int qdev_simple_unplug_cb(DeviceState *dev)
     return 0;
 }
 
+void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
+                                  DeviceState *dev, Error **errp)
+{
+    qdev_simple_unplug_cb(dev);
+}
 
 /* Like qdev_init(), but terminate program via error_report() instead of
    returning an error value.  This is okay during machine creation.
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 48a96d2..ba812c5 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -265,6 +265,8 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
 void qdev_unplug(DeviceState *dev, Error **errp);
 int qdev_simple_unplug_cb(DeviceState *dev);
+void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
+                                  DeviceState *dev, Error **errp);
 void qdev_machine_creation_done(void);
 bool qdev_machine_modified(void);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 17/36] qdev: add wrapper to set BUS as HotplugHandler

[Igor Mammedov]

to be used for conversion of SCSI and USB devices,
and would allow to make every HBA/USB host switch
to HotplugHandler API without touching each controller
explicitly.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/core/qdev.c         | 19 +++++++++++++++++++
 include/hw/qdev-core.h | 11 ++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 1d1b113..0de99b2 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -112,6 +112,25 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
     bus_add_child(bus, dev);
 }
 
+static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
+                                              Error **errp)
+{
+
+    object_property_set_link(OBJECT(bus), OBJECT(handler),
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+    bus->allow_hotplug = 1;
+}
+
+void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
+{
+    qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
+}
+
+void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+{
+    qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
+}
+
 /* Create a new device.  This only initializes the device state structure
    and allows properties to be set.  qdev_init should be called to
    initialize the actual device emulation.  */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index ba812c5..48e9579 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -363,13 +363,10 @@ extern int qdev_hotplug;
 
 char *qdev_get_dev_path(DeviceState *dev);
 
-static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
-                                            Error **errp)
-{
-    object_property_set_link(OBJECT(bus), OBJECT(handler),
-                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
-    bus->allow_hotplug = 1;
-}
+void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
+                              Error **errp);
+
+void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
 
 static inline bool qbus_is_hotpluggable(BusState *bus)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 18/36] qdev: drop hotplug check from bus_add_child()

[Igor Mammedov]

check is too restrictive and does not allow
to add childs to just created bus during hotplug
when the bus is part of composite device.

Removing check from bus_add_child() doesn't affect
devices creatable with device_add/del commands since
they have a similar builtin checks and patch will
allow to create complex composite devices during
hotplug.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/core/qdev.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0de99b2..fa86843 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -85,10 +85,6 @@ static void bus_add_child(BusState *bus, DeviceState *child)
     char name[32];
     BusChild *kid = g_malloc0(sizeof(*kid));
 
-    if (qdev_hotplug) {
-        assert(qbus_is_hotpluggable(bus));
-    }
-
     kid->index = bus->max_index++;
     kid->child = child;
     object_ref(OBJECT(kid->child));
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 19/36] target-i386: ICC bus: drop BusState.allow_hotplug

[Igor Mammedov]

since bus_add_child() no longer cares if BUS is hotpluggable
or not, there is no need in setting allow_hotplug field.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/cpu/icc_bus.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 7f44c59..9575fd6 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -24,18 +24,10 @@
 
 /* icc-bridge implementation */
 
-static void icc_bus_init(Object *obj)
-{
-    BusState *b = BUS(obj);
-
-    b->allow_hotplug = true;
-}
-
 static const TypeInfo icc_bus_info = {
     .name = TYPE_ICC_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(ICCBus),
-    .instance_init = icc_bus_init,
 };
 
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 20/36] virtio-pci: drop BusState.allow_hotplug

[Igor Mammedov]

virtio-pci-bus is an internal object of composite
virtio-pci device and it doesn't participate in
-device/device_add hotplug flow, and since it's
not required by bus_add_child() that BUS must
be hotpluggable to be able to add child at runtime,
it's possible to drop not needed 'allow_hotplug'
field.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/virtio/virtio-pci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f560814..9dc35f0 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1543,13 +1543,10 @@ static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtIOPCIProxy *dev)
 {
     DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
     char virtio_bus_name[] = "virtio-bus";
 
     qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_PCI_BUS, qdev,
                         virtio_bus_name);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 1;
 }
 
 static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 21/36] virtio-serial: convert to hotplug-handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/virtio-serial-bus.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3931085..c6870f1 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -904,6 +904,12 @@ static void virtser_port_device_realize(DeviceState *dev, Error **errp)
     }
 
     port->elem.out_num = 0;
+}
+
+static void virtser_port_device_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
 
     QTAILQ_INSERT_TAIL(&port->vser->ports, port, next);
     port->ivq = port->vser->ivqs[port->id];
@@ -912,7 +918,7 @@ static void virtser_port_device_realize(DeviceState *dev, Error **errp)
     add_port(port->vser, port->id);
 
     /* Send an update to the guest about this new port added */
-    virtio_notify_config(vdev);
+    virtio_notify_config(VIRTIO_DEVICE(hotplug_dev));
 }
 
 static void virtser_port_device_unrealize(DeviceState *dev, Error **errp)
@@ -935,7 +941,6 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSerial *vser = VIRTIO_SERIAL(dev);
-    BusState *bus;
     uint32_t i, max_supported_ports;
 
     if (!vser->serial.max_virtserial_ports) {
@@ -957,8 +962,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
     qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
                         dev, vdev->bus_name);
-    bus = BUS(&vser->bus);
-    bus->allow_hotplug = 1;
+    qbus_set_hotplug_handler(BUS(&vser->bus), DEVICE(vser), errp);
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
 
@@ -1021,7 +1025,6 @@ static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
     k->bus_type = TYPE_VIRTIO_SERIAL_BUS;
     k->realize = virtser_port_device_realize;
     k->unrealize = virtser_port_device_unrealize;
-    k->unplug = qdev_simple_unplug_cb;
     k->props = virtser_props;
 }
 
@@ -1064,6 +1067,7 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     QLIST_INIT(&vserdevices.devices);
 
@@ -1077,6 +1081,8 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
     vdc->reset = vser_reset;
     vdc->save = virtio_serial_save_device;
     vdc->load = virtio_serial_load_device;
+    hc->plug = virtser_port_device_plug;
+    hc->unplug = qdev_simple_device_unplug_cb;
 }
 
 static const TypeInfo virtio_device_info = {
@@ -1084,6 +1090,10 @@ static const TypeInfo virtio_device_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOSerial),
     .class_init = virtio_serial_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void virtio_serial_register_types(void)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 22/36] virtio-mmio: drop useless bus->allow_hotplug = 0

[Igor Mammedov]

Bus by default is not hotpluggable.
virtio-mmio-bus and its parent types do not set allow_hotplug
anywhere explicitly, so remove not needed field access
and wrapper along with it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-mmio.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 18c6e5b..2450c13 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -89,9 +89,6 @@ typedef struct {
     VirtioBusState bus;
 } VirtIOMMIOProxy;
 
-static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
-                                VirtIOMMIOProxy *dev);
-
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
@@ -362,7 +359,8 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
     VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
     SysBusDevice *sbd = SYS_BUS_DEVICE(d);
 
-    virtio_mmio_bus_new(&proxy->bus, sizeof(proxy->bus), proxy);
+    qbus_create_inplace(&proxy->bus, sizeof(proxy->bus), TYPE_VIRTIO_MMIO_BUS,
+                        d, NULL);
     sysbus_init_irq(sbd, &proxy->irq);
     memory_region_init_io(&proxy->iomem, OBJECT(d), &virtio_mem_ops, proxy,
                           TYPE_VIRTIO_MMIO, 0x200);
@@ -393,17 +391,6 @@ static const TypeInfo virtio_mmio_info = {
 
 /* virtio-mmio-bus. */
 
-static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
-                                VirtIOMMIOProxy *dev)
-{
-    DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
-
-    qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_MMIO_BUS, qdev, NULL);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 0;
-}
-
 static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *bus_class = BUS_CLASS(klass);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 23/36] s390x: drop not used allow_hotplug in event-facility

[Igor Mammedov]

s390-sclp-event-facility creates s390-sclp-events-bus
and immeadiatly sets its allow_hotplug field to 0,
which is NOP since it's already 0 by default.

Also since BUS is not hotpluggable, it's not possible
to call SCLP_EVENT{ DeviceClass.unplug } callback
from qdev_unplug() making this unreachable code,
so drop it as well.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/event-facility.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 597db34..78da718 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -333,7 +333,6 @@ static int init_event_facility(SCLPEventFacility *event_facility)
     /* Spawn a new bus for SCLP events */
     qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus),
                         TYPE_SCLP_EVENTS_BUS, sdev, NULL);
-    event_facility->sbus.qbus.allow_hotplug = 0;
 
     quiesce = qdev_create(&event_facility->sbus.qbus, "sclpquiesce");
     if (!quiesce) {
@@ -408,7 +407,6 @@ static void event_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->bus_type = TYPE_SCLP_EVENTS_BUS;
-    dc->unplug = qdev_simple_unplug_cb;
     dc->realize = event_realize;
     dc->unrealize = event_unrealize;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 24/36] s390x: convert s390-virtio to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 fix not working unplug
---
 hw/s390x/s390-virtio-bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6b6fb61..b575d6c 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -102,7 +102,7 @@ VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
     bus->next_ring = bus->dev_page + TARGET_PAGE_SIZE;
 
     /* Enable hotplugging */
-    _bus->allow_hotplug = 1;
+    qbus_set_hotplug_handler(_bus, dev, &error_abort);
 
     /* Allocate RAM for VirtIO device pages (descriptors, queues, rings) */
     *ram_size += S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
@@ -600,7 +600,6 @@ static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
 
     dc->init = s390_virtio_busdev_init;
     dc->bus_type = TYPE_S390_VIRTIO_BUS;
-    dc->unplug = qdev_simple_unplug_cb;
     dc->reset = s390_virtio_busdev_reset;
 }
 
@@ -674,8 +673,10 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
 static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->init = s390_virtio_bridge_init;
+    hc->unplug = qdev_simple_device_unplug_cb;
 }
 
 static const TypeInfo s390_virtio_bridge_info = {
@@ -683,6 +684,10 @@ static const TypeInfo s390_virtio_bridge_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusDevice),
     .class_init    = s390_virtio_bridge_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 /* virtio-s390-bus */
@@ -691,13 +696,10 @@ static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
                                 VirtIOS390Device *dev)
 {
     DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
     char virtio_bus_name[] = "virtio-bus";
 
     qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_S390_BUS,
                         qdev, virtio_bus_name);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 1;
 }
 
 static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 25/36] s390x: convert virtio-ccw to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 fix not working unplug
---
 hw/s390x/virtio-ccw.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 33a1d86..09d9500 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -230,7 +230,7 @@ VirtualCssBus *virtual_css_bus_init(void)
     cbus = VIRTUAL_CSS_BUS(bus);
 
     /* Enable hotplugging */
-    bus->allow_hotplug = 1;
+    qbus_set_hotplug_handler(bus, dev, &error_abort);
 
     return cbus;
 }
@@ -1590,7 +1590,8 @@ static int virtio_ccw_busdev_exit(DeviceState *dev)
     return _info->exit(_dev);
 }
 
-static int virtio_ccw_busdev_unplug(DeviceState *dev)
+static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
 {
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
     SubchDev *sch = _dev->sch;
@@ -1609,7 +1610,6 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev)
     css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, 1, 0);
 
     object_unparent(OBJECT(dev));
-    return 0;
 }
 
 static Property virtio_ccw_properties[] = {
@@ -1624,9 +1624,7 @@ static void virtio_ccw_device_class_init(ObjectClass *klass, void *data)
     dc->props = virtio_ccw_properties;
     dc->init = virtio_ccw_busdev_init;
     dc->exit = virtio_ccw_busdev_exit;
-    dc->unplug = virtio_ccw_busdev_unplug;
     dc->bus_type = TYPE_VIRTUAL_CSS_BUS;
-
 }
 
 static const TypeInfo virtio_ccw_device_info = {
@@ -1650,8 +1648,10 @@ static int virtual_css_bridge_init(SysBusDevice *dev)
 static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->init = virtual_css_bridge_init;
+    hc->unplug = virtio_ccw_busdev_unplug;
 }
 
 static const TypeInfo virtual_css_bridge_info = {
@@ -1659,6 +1659,10 @@ static const TypeInfo virtual_css_bridge_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusDevice),
     .class_init    = virtual_css_bridge_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 /* virtio-ccw-bus */
@@ -1667,13 +1671,10 @@ static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtioCcwDevice *dev)
 {
     DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
     char virtio_bus_name[] = "virtio-bus";
 
     qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_CCW_BUS,
                         qdev, virtio_bus_name);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 1;
 }
 
 static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 26/36] scsi: set SCSI BUS itself as default HotplugHandler

[Igor Mammedov]

That would allow to handle SCSI device unplug
on HBAs without dedicated hot(un)plug handlers
and avoid making such HBAs explicitly hotpluggable.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/scsi/scsi-bus.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 954c607..20fe524 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -23,9 +23,11 @@ static Property scsi_props[] = {
 static void scsi_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->get_dev_path = scsibus_get_dev_path;
     k->get_fw_dev_path = scsibus_get_fw_dev_path;
+    hc->unplug = qdev_simple_device_unplug_cb;
 }
 
 static const TypeInfo scsi_bus_info = {
@@ -33,6 +35,10 @@ static const TypeInfo scsi_bus_info = {
     .parent = TYPE_BUS,
     .instance_size = sizeof(SCSIBus),
     .class_init = scsi_bus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 static int next_scsi_bus;
 
@@ -92,7 +98,7 @@ void scsi_bus_new(SCSIBus *bus, size_t bus_size, DeviceState *host,
     qbus_create_inplace(bus, bus_size, TYPE_SCSI_BUS, host, bus_name);
     bus->busnr = next_scsi_bus++;
     bus->info = info;
-    bus->qbus.allow_hotplug = 1;
+    qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
 }
 
 static void scsi_dma_restart_bh(void *opaque)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 27/36] scsi: convert pvscsi HBA to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
not tested since it's broken, i.e. QEMU hangs in BIOS
with pvscsi device present on CLI

v2:
 * enforce pvscsi specific hotplug-handler
---
 hw/scsi/vmw_pvscsi.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 5734d19..d3a92fb 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -524,17 +524,20 @@ pvscsi_send_msg(PVSCSIState *s, SCSIDevice *dev, uint32_t msg_type)
 }
 
 static void
-pvscsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+pvscsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp)
 {
-    PVSCSIState *s = container_of(bus, PVSCSIState, bus);
-    pvscsi_send_msg(s, dev, PVSCSI_MSG_DEV_ADDED);
+    PVSCSIState *s = PVSCSI(hotplug_dev);
+
+    pvscsi_send_msg(s, SCSI_DEVICE(dev), PVSCSI_MSG_DEV_ADDED);
 }
 
 static void
-pvscsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+pvscsi_hot_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp)
 {
-    PVSCSIState *s = container_of(bus, PVSCSIState, bus);
-    pvscsi_send_msg(s, dev, PVSCSI_MSG_DEV_REMOVED);
+    PVSCSIState *s = PVSCSI(hotplug_dev);
+
+    pvscsi_send_msg(s, SCSI_DEVICE(dev), PVSCSI_MSG_DEV_REMOVED);
+    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
 }
 
 static void
@@ -1057,8 +1060,6 @@ static const struct SCSIBusInfo pvscsi_scsi_info = {
         .get_sg_list = pvscsi_get_sg_list,
         .complete = pvscsi_command_complete,
         .cancel = pvscsi_request_cancelled,
-        .hotplug = pvscsi_hotplug,
-        .hot_unplug = pvscsi_hot_unplug,
 };
 
 static int
@@ -1092,6 +1093,8 @@ pvscsi_init(PCIDevice *pci_dev)
 
     scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
                  &pvscsi_scsi_info, NULL);
+    /* override default SCSI bus hotplug-handler, with pvscsi's one */
+    qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(s), &error_abort);
     pvscsi_reset_state(s);
 
     return 0;
@@ -1187,6 +1190,7 @@ static void pvscsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->init = pvscsi_init;
     k->exit = pvscsi_uninit;
@@ -1199,6 +1203,8 @@ static void pvscsi_class_init(ObjectClass *klass, void *data)
     dc->props = pvscsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     k->config_write = pvscsi_write_config;
+    hc->unplug = pvscsi_hot_unplug;
+    hc->plug = pvscsi_hotplug;
 }
 
 static const TypeInfo pvscsi_info = {
@@ -1206,6 +1212,10 @@ static const TypeInfo pvscsi_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PVSCSIState),
     .class_init    = pvscsi_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 28/36] scsi: convert virtio-scsi HBA to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  * enforce virtio-scsi specific hotplug-handler
---
 hw/scsi/virtio-scsi.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 86aba88..a08e253 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -649,26 +649,29 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
     }
 }
 
-static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                                Error **errp)
 {
-    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
 
     if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
-        virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
+        virtio_scsi_push_event(VIRTIO_SCSI(hotplug_dev), SCSI_DEVICE(dev),
+                               VIRTIO_SCSI_T_TRANSPORT_RESET,
                                VIRTIO_SCSI_EVT_RESET_RESCAN);
     }
 }
 
-static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                                  Error **errp)
 {
-    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
 
     if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
-        virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
+        virtio_scsi_push_event(VIRTIO_SCSI(hotplug_dev), SCSI_DEVICE(dev),
+                               VIRTIO_SCSI_T_TRANSPORT_RESET,
                                VIRTIO_SCSI_EVT_RESET_REMOVED);
     }
+    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
 }
 
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
@@ -680,8 +683,6 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .complete = virtio_scsi_command_complete,
     .cancel = virtio_scsi_request_cancelled,
     .change = virtio_scsi_change,
-    .hotplug = virtio_scsi_hotplug,
-    .hot_unplug = virtio_scsi_hot_unplug,
     .parse_cdb = virtio_scsi_parse_cdb,
     .get_sg_list = virtio_scsi_get_sg_list,
     .save_request = virtio_scsi_save_request,
@@ -736,6 +737,8 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
 
     scsi_bus_new(&s->bus, sizeof(s->bus), dev,
                  &virtio_scsi_scsi_info, vdev->bus_name);
+    /* override default SCSI bus hotplug-handler, with virtio-scsi's one */
+    qbus_set_hotplug_handler(BUS(&s->bus), dev, &error_abort);
 
     if (!dev->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
@@ -785,6 +788,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     dc->props = virtio_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
@@ -793,6 +797,8 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
     vdc->set_config = virtio_scsi_set_config;
     vdc->get_features = virtio_scsi_get_features;
     vdc->reset = virtio_scsi_reset;
+    hc->plug = virtio_scsi_hotplug;
+    hc->unplug = virtio_scsi_hotunplug;
 }
 
 static const TypeInfo virtio_scsi_common_info = {
@@ -808,6 +814,10 @@ static const TypeInfo virtio_scsi_info = {
     .parent = TYPE_VIRTIO_SCSI_COMMON,
     .instance_size = sizeof(VirtIOSCSI),
     .class_init = virtio_scsi_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void virtio_register_types(void)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 29/36] scsi: cleanup not used anymore SCSIBusInfo{hotplug, hot_unplug} fields

[Igor Mammedov]

SCSI subsytem was converted to hotplug handler API and
doesn't use SCSIBusInfo{hotplug, hot_unplug} fields and
related callbacks anymore.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c     | 16 ----------------
 include/hw/scsi/scsi.h |  2 --
 2 files changed, 18 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 20fe524..908f2a0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -208,10 +208,6 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
     }
     dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
                                                      dev);
-
-    if (bus->info->hotplug) {
-        bus->info->hotplug(bus, dev);
-    }
 }
 
 static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
@@ -1924,17 +1920,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static int scsi_qdev_unplug(DeviceState *qdev)
-{
-    SCSIDevice *dev = SCSI_DEVICE(qdev);
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
-
-    if (bus->info->hot_unplug) {
-        bus->info->hot_unplug(bus, dev);
-    }
-    return qdev_simple_unplug_cb(qdev);
-}
-
 static const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1998,7 +1983,6 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_STORAGE, k->categories);
     k->bus_type  = TYPE_SCSI_BUS;
     k->realize   = scsi_qdev_realize;
-    k->unplug    = scsi_qdev_unplug;
     k->unrealize = scsi_qdev_unrealize;
     k->props     = scsi_props;
 }
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 2e3a8f9..8a2581e 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -138,8 +138,6 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
-    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
-    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 30/36] usb-bot: mark device as non hotpluggable

[Igor Mammedov]

usb-bot creates SCSI bus and immediately makes it
non hotpluggable which was making not possible to
hotplug usb-bot since QEMU would abort at
bus_add_child(scsi-hd) time when usb-bot is
realized.

Mark usb-bot as not hotpluggable so that attempt
to hotplug it would error out even before it gets
to device initialization point.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/usb/dev-storage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index bd7cc53..a207507 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -770,9 +770,11 @@ static void usb_msd_class_initfn_storage(ObjectClass *klass, void *data)
 static void usb_msd_class_initfn_bot(ObjectClass *klass, void *data)
 {
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     uc->realize = usb_msd_realize_bot;
     usb_msd_class_initfn_common(klass);
+    dc->hotpluggable = false;
 }
 
 static const TypeInfo msd_info = {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 31/36] usb-bot: drop not needed "allow_hotplug = 0"

[Igor Mammedov]

Drop useless hack that disables hotplug on bus by setting
"allow_hotplug = 0". Even if bus is hotpluggable, It won't
be possible to add another SCSI device to bus since its
realize will fail early with error "no free target"
in scsi_qdev_realize() method.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/usb/dev-storage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index a207507..bccaa3f 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -653,7 +653,6 @@ static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
     usb_desc_init(dev);
     scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
                  &usb_msd_scsi_info_bot, NULL);
-    s->bus.qbus.allow_hotplug = 0;
     usb_msd_handle_reset(dev);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 32/36] usb-storage: drop not needed "allow_hotplug = 0"

[Igor Mammedov]

Drop useless hack that disables hotplug on bus, after backend
storage was added to it, by setting "allow_hotplug = 0". Even
if bus is hotpluggable, It won't be possible to add another
SCSI device to bus since its realize will fail early with
error "no free target" in scsi_qdev_realize() method.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-storage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index bccaa3f..b005783 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -632,7 +632,6 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
-    s->bus.qbus.allow_hotplug = 0;
     usb_msd_handle_reset(dev);
 
     if (bdrv_key_required(bs)) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 33/36] usb: convert usb-ccid to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/usb/dev-smartcard-reader.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index d37ed02..78ce681 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1312,8 +1312,8 @@ static void ccid_realize(USBDevice *dev, Error **errp)
     usb_desc_init(dev);
     qbus_create_inplace(&s->bus, sizeof(s->bus), TYPE_CCID_BUS, DEVICE(dev),
                         NULL);
+    qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(dev), &error_abort);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
-    s->bus.qbus.allow_hotplug = 1;
     s->card = NULL;
     s->migration_state = MIGRATION_NONE;
     s->migration_target_ip = 0;
@@ -1439,6 +1439,7 @@ static void ccid_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     uc->realize        = ccid_realize;
     uc->product_desc   = "QEMU USB CCID";
@@ -1451,6 +1452,7 @@ static void ccid_class_initfn(ObjectClass *klass, void *data)
     dc->vmsd = &ccid_vmstate;
     dc->props = ccid_properties;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    hc->unplug = qdev_simple_device_unplug_cb;
 }
 
 static const TypeInfo ccid_info = {
@@ -1458,6 +1460,10 @@ static const TypeInfo ccid_info = {
     .parent        = TYPE_USB_DEVICE,
     .instance_size = sizeof(USBCCIDState),
     .class_init    = ccid_class_initfn,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void ccid_card_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 34/36] usb: convert usb devices to hotplug handler API

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  replace making each host as HotplugHandler with
  making USB BUS a HotplugHandler. That greatly reduces
  LOC needed for convertion.
---
 hw/usb/bus.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index da1eba9..986b2d8 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -24,10 +24,12 @@ static Property usb_props[] = {
 static void usb_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->print_dev = usb_bus_dev_print;
     k->get_dev_path = usb_get_dev_path;
     k->get_fw_dev_path = usb_get_fw_dev_path;
+    hc->unplug = qdev_simple_device_unplug_cb;
 }
 
 static const TypeInfo usb_bus_info = {
@@ -35,6 +37,10 @@ static const TypeInfo usb_bus_info = {
     .parent = TYPE_BUS,
     .instance_size = sizeof(USBBus),
     .class_init = usb_bus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static int next_usb_bus = 0;
@@ -79,9 +85,9 @@ void usb_bus_new(USBBus *bus, size_t bus_size,
                  USBBusOps *ops, DeviceState *host)
 {
     qbus_create_inplace(bus, bus_size, TYPE_USB_BUS, host, NULL);
+    qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
     bus->ops = ops;
     bus->busnr = next_usb_bus++;
-    bus->qbus.allow_hotplug = 1; /* Yes, we can */
     QTAILQ_INIT(&bus->free);
     QTAILQ_INIT(&bus->used);
     QTAILQ_INSERT_TAIL(&busses, bus, next);
@@ -701,7 +707,6 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_type = TYPE_USB_BUS;
-    k->unplug   = qdev_simple_unplug_cb;
     k->realize  = usb_qdev_realize;
     k->unrealize = usb_qdev_unrealize;
     k->props    = usb_props;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 35/36] qdev: drop legacy hotplug fields/methods

[Igor Mammedov]

It removes not needed anymore BusState.allow_hotplug field and
DeviceClass.unplug callback.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c         | 16 +++-------------
 include/hw/qdev-core.h |  5 +----
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fa86843..bc45a59 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -114,7 +114,6 @@ static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
 
     object_property_set_link(OBJECT(bus), OBJECT(handler),
                              QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
-    bus->allow_hotplug = 1;
 }
 
 void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
@@ -254,10 +253,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
             hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
         }
     } else {
-        assert(dc->unplug != NULL);
-        if (dc->unplug(dev) < 0) { /* legacy handler */
-            error_set(errp, QERR_UNDEFINED_ERROR);
-        }
+        assert(0);
     }
 }
 
@@ -294,17 +290,11 @@ void qbus_reset_all_fn(void *opaque)
 }
 
 /* can be used as ->unplug() callback for the simple cases */
-int qdev_simple_unplug_cb(DeviceState *dev)
-{
-    /* just zap it */
-    object_unparent(OBJECT(dev));
-    return 0;
-}
-
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp)
 {
-    qdev_simple_unplug_cb(dev);
+    /* just zap it */
+    object_unparent(OBJECT(dev));
 }
 
 /* Like qdev_init(), but terminate program via error_report() instead of
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 48e9579..01d7db4 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -126,7 +126,6 @@ typedef struct DeviceClass {
 
     /* Private to qdev / bus.  */
     qdev_initfn init; /* TODO remove, once users are converted to realize */
-    qdev_event unplug;
     qdev_event exit; /* TODO remove, once users are converted to unrealize */
     const char *bus_type;
 } DeviceClass;
@@ -210,7 +209,6 @@ struct BusState {
     Object obj;
     DeviceState *parent;
     const char *name;
-    int allow_hotplug;
     HotplugHandler *hotplug_handler;
     int max_index;
     bool realized;
@@ -264,7 +262,6 @@ void qdev_init_nofail(DeviceState *dev);
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
 void qdev_unplug(DeviceState *dev, Error **errp);
-int qdev_simple_unplug_cb(DeviceState *dev);
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp);
 void qdev_machine_creation_done(void);
@@ -370,6 +367,6 @@ void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
 
 static inline bool qbus_is_hotpluggable(BusState *bus)
 {
-   return bus->allow_hotplug || bus->hotplug_handler;
+   return bus->hotplug_handler;
 }
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  merged in Paolo's suggestion to reduce indentation in patch
---
 hw/core/qdev.c | 61 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index bc45a59..215effb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -223,9 +223,28 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
     dev->alias_required_for_version = required_for_version;
 }
 
+static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+{
+    HotplugHandler *hotplug_ctrl = NULL;
+
+    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+        hotplug_ctrl = dev->parent_bus->hotplug_handler;
+    } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
+        MachineState *machine = MACHINE(qdev_get_machine());
+        MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+        if (mc->get_hotplug_handler) {
+            hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
+        }
+    }
+    return hotplug_ctrl;
+}
+
 void qdev_unplug(DeviceState *dev, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    HotplugHandler *hotplug_ctrl;
+    HotplugHandlerClass *hdc;
 
     if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
         error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
@@ -240,20 +259,18 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 
     qdev_hot_removed = true;
 
-    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
-        HotplugHandlerClass *hdc;
-
-        /* If device supports async unplug just request it to be done,
-         * otherwise just remove it synchronously */
-        hdc = HOTPLUG_HANDLER_GET_CLASS(dev->parent_bus->hotplug_handler);
-        if (hdc->unplug_request) {
-            hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
-                                           dev, errp);
-        } else {
-            hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
-        }
+    hotplug_ctrl = qdev_get_hotplug_handler(dev);
+    /* hotplggable device MUST have HotplugHandler, if it doesn't
+     * then something is very wrong with it */
+    g_assert(hotplug_ctrl);
+
+    /* If device supports async unplug just request it to be done,
+     * otherwise just remove it synchronously */
+    hdc = HOTPLUG_HANDLER_GET_CLASS(hotplug_ctrl);
+    if (hdc->unplug_request) {
+        hotplug_handler_unplug_request(hotplug_ctrl, dev, errp);
     } else {
-        assert(0);
+        hotplug_handler_unplug(hotplug_ctrl, dev, errp);
     }
 }
 
@@ -831,6 +848,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
 {
     DeviceState *dev = DEVICE(obj);
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    HotplugHandler *hotplug_ctrl;
     BusState *bus;
     Error *local_err = NULL;
 
@@ -858,20 +876,9 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
             goto fail;
         }
 
-        if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
-            hotplug_handler_plug(dev->parent_bus->hotplug_handler,
-                                 dev, &local_err);
-        } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
-            HotplugHandler *hotplug_ctrl;
-            MachineState *machine = MACHINE(qdev_get_machine());
-            MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-            if (mc->get_hotplug_handler) {
-                hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
-                if (hotplug_ctrl) {
-                    hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
-                }
-            }
+        hotplug_ctrl = qdev_get_hotplug_handler(dev);
+        if (hotplug_ctrl) {
+            hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
         }
 
         if (local_err != NULL) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 09:28:05 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> Changes since v1:
>   * added usb-uas test
>   * drop hotplug check in bus_add_child()
>   * make SCSI & USB bus as default HotplugHandler
>   * fixed dummy HBA hot(un)plug
>   * fixed hotunplug on s390x
>   * prevent hotplug of non hotpluggable devices
> --
> 
> Series unifies different hotplug mechanisms to a recent
> hotplug-handler API and does shallow conversion of
> devices that still use legacy qdev hotplug to it dropping
> not used after that legacy hotplug path [29/30].
> It also relaces SCSI's own way to do hotplug/unplug with
> hotplug-handler callbacks leaving it the only method
> perform hotplug tasks.
> And the last patch [30/30] allows to unplug of BUS-less
> devices using hotplug-handler API.
> 
> Converted devices are covered with new hotplug
> unit-tests, except of:
>  s390x-*: I have no idea how or means to test it, but code
>           is close to virtio, so it's converted using
>           virtio template
>  pvscsi: is broken, so no means to test it
> 
> Git tree for testing:
>   https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2

I've played around with attaching/detaching virtio devices on the
s390-virtio and the s390-virtio-ccw machines. Everything seemed to work
as before.

I'll still take a look at the s390-related patches.

Re: [Qemu-devel] [PATCH v2 23/36] s390x: drop not used allow_hotplug in event-facility

[Cornelia Huck]

On Fri, 26 Sep 2014 09:28:28 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> s390-sclp-event-facility creates s390-sclp-events-bus
> and immeadiatly sets its allow_hotplug field to 0,
> which is NOP since it's already 0 by default.
> 
> Also since BUS is not hotpluggable, it's not possible
> to call SCLP_EVENT{ DeviceClass.unplug } callback
> from qdev_unplug() making this unreachable code,
> so drop it as well.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/s390x/event-facility.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [PATCH v2 24/36] s390x: convert s390-virtio to hotplug handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 09:28:29 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>  fix not working unplug

While the current code for s390-virtio implements unplug, the transport
per se does not really support unplug (and never has), only hotplugging
of devices. I removed the ->unplug callback and qemu now behaves like
I'd expect: I can add devices with device_add, but device_del leaves
them in place. That would be the way to go for me.

> ---
>  hw/s390x/s390-virtio-bus.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Re: [Qemu-devel] [PATCH v2 25/36] s390x: convert virtio-ccw to hotplug handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 09:28:30 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>  fix not working unplug
> ---
>  hw/s390x/virtio-ccw.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 33a1d86..09d9500 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c

> @@ -1624,9 +1624,7 @@ static void virtio_ccw_device_class_init(ObjectClass *klass, void *data)
>      dc->props = virtio_ccw_properties;
>      dc->init = virtio_ccw_busdev_init;
>      dc->exit = virtio_ccw_busdev_exit;
> -    dc->unplug = virtio_ccw_busdev_unplug;
>      dc->bus_type = TYPE_VIRTUAL_CSS_BUS;
> -

unrelated whitespace change (but I don't really mind)

>  }
> 
>  static const TypeInfo virtio_ccw_device_info = {

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [PATCH v2 24/36] s390x: convert s390-virtio to hotplug handler API

[Igor Mammedov]

On Fri, 26 Sep 2014 13:15:16 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Fri, 26 Sep 2014 09:28:29 +0000
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >  fix not working unplug
> 
> While the current code for s390-virtio implements unplug, the transport
> per se does not really support unplug (and never has), only hotplugging
> of devices. I removed the ->unplug callback and qemu now behaves like
> I'd expect: I can add devices with device_add, but device_del leaves
> them in place. That would be the way to go for me.
Would a path to remove unplug on top of this one do or you'd prefer
me to respin this patch without unplug?

> 
> > ---
> >  hw/s390x/s390-virtio-bus.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Paolo Bonzini]

Il 26/09/2014 11:28, Igor Mammedov ha scritto:
> Changes since v1:
>   * added usb-uas test
>   * drop hotplug check in bus_add_child()
>   * make SCSI & USB bus as default HotplugHandler
>   * fixed dummy HBA hot(un)plug
>   * fixed hotunplug on s390x
>   * prevent hotplug of non hotpluggable devices
> --
> 
> Series unifies different hotplug mechanisms to a recent
> hotplug-handler API and does shallow conversion of
> devices that still use legacy qdev hotplug to it dropping
> not used after that legacy hotplug path [29/30].
> It also relaces SCSI's own way to do hotplug/unplug with
> hotplug-handler callbacks leaving it the only method
> perform hotplug tasks.
> And the last patch [30/30] allows to unplug of BUS-less
> devices using hotplug-handler API.
> 
> Converted devices are covered with new hotplug
> unit-tests, except of:
>  s390x-*: I have no idea how or means to test it, but code
>           is close to virtio, so it's converted using
>           virtio template
>  pvscsi: is broken, so no means to test it
> 
> Git tree for testing:
>   https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2
> 
> 
> Igor Mammedov (36):
>   test: virtio-scsi: check if hot-plug/unplug works
>   test: virtio-serial: check if hot-plug/unplug works
>   test: libqos: add qpci_plug_device_test() and
>     qpci_unplug_acpi_device_test()
>   test: virtio-rng: check if hot-plug/unplug works
>   test: virtio-net: check if hot-plug/unplug works
>   test: virtio-blk: check if hot-plug/unplug works
>   test: usb: move uhci port test code to libqos/usb.c
>   test: usb: add port test to uhci unit test
>   test: usb: generic usb device hotplug
>   test: usb: usb-storage hotplug test
>   test: usb: usb-uas hotplug test
>   access BusState.allow_hotplug using wraper qbus_is_hotpluggable()
>   qdev: do not allow to instantiate non hotpluggable device with
>     device_add
>   qdev: HotplugHandler: rename unplug callback to unplug_request
>   qdev: HotplugHandler: provide unplug callback
>   qdev: add simple/generic unplug callback for HotplugHandler
>   qdev: add wrapper to set BUS as HotplugHandler
>   qdev: drop hotplug check from bus_add_child()
>   target-i386: ICC bus: drop BusState.allow_hotplug
>   virtio-pci: drop BusState.allow_hotplug
>   virtio-serial: convert to hotplug-handler API
>   virtio-mmio: drop useless bus->allow_hotplug = 0
>   s390x: drop not used allow_hotplug in event-facility
>   s390x: convert s390-virtio to hotplug handler API
>   s390x: convert virtio-ccw to hotplug handler API
>   scsi: set SCSI BUS itself as default HotplugHandler
>   scsi: convert pvscsi HBA to hotplug handler API
>   scsi: convert virtio-scsi HBA to hotplug handler API
>   scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} fields
>   usb-bot: mark device as non hotpluggable
>   usb-bot: drop not needed "allow_hotplug = 0"
>   usb-storage: drop not needed "allow_hotplug = 0"
>   usb: convert usb-ccid to hotplug handler API
>   usb: convert usb devices to hotplug handler API
>   qdev: drop legacy hotplug fields/methods
>   qdev: HotplugHandler: add support for unplugging BUS-less devices
> 
>  hw/acpi/piix4.c                |  6 +--
>  hw/char/virtio-serial-bus.c    | 20 +++++++---
>  hw/core/hotplug.c              | 11 ++++++
>  hw/core/qdev.c                 | 85 ++++++++++++++++++++++++++++--------------
>  hw/cpu/icc_bus.c               |  8 ----
>  hw/i386/acpi-build.c           |  2 +-
>  hw/isa/lpc_ich9.c              |  6 +--
>  hw/pci-bridge/pci_bridge_dev.c |  2 +-
>  hw/pci/pci-hotplug-old.c       |  4 +-
>  hw/pci/pcie.c                  |  4 +-
>  hw/pci/pcie_port.c             |  2 +-
>  hw/pci/shpc.c                  |  4 +-
>  hw/s390x/event-facility.c      |  2 -
>  hw/s390x/s390-virtio-bus.c     | 12 +++---
>  hw/s390x/virtio-ccw.c          | 17 +++++----
>  hw/scsi/scsi-bus.c             | 24 ++++--------
>  hw/scsi/virtio-scsi.c          | 30 ++++++++++-----
>  hw/scsi/vmw_pvscsi.c           | 26 +++++++++----
>  hw/usb/bus.c                   |  9 ++++-
>  hw/usb/dev-smartcard-reader.c  |  8 +++-
>  hw/usb/dev-storage.c           |  4 +-
>  hw/virtio/virtio-mmio.c        | 17 +--------
>  hw/virtio/virtio-pci.c         |  3 --
>  include/hw/hotplug.h           | 16 +++++++-
>  include/hw/pci/pcie.h          |  4 +-
>  include/hw/pci/shpc.h          |  4 +-
>  include/hw/qdev-core.h         | 17 +++++----
>  include/hw/scsi/scsi.h         |  2 -
>  qdev-monitor.c                 |  5 ++-
>  tests/Makefile                 | 13 ++++---
>  tests/libqos/pci-pc.c          | 49 ++++++++++++++++++++++++
>  tests/libqos/pci.h             |  3 ++
>  tests/libqos/usb.c             | 71 +++++++++++++++++++++++++++++++++++
>  tests/libqos/usb.h             | 17 +++++++++
>  tests/usb-hcd-ehci-test.c      | 50 ++++++++++---------------
>  tests/usb-hcd-ohci-test.c      | 10 ++++-
>  tests/usb-hcd-uhci-test.c      | 65 +++++++++++++++++++++++++++++++-
>  tests/usb-hcd-xhci-test.c      | 68 ++++++++++++++++++++++++++++++++-
>  tests/virtio-blk-test.c        | 49 ++++++++++++++++++------
>  tests/virtio-net-test.c        | 10 +++++
>  tests/virtio-rng-test.c        | 10 +++++
>  tests/virtio-scsi-test.c       | 29 ++++++++++++++
>  tests/virtio-serial-test.c     | 27 ++++++++++++++
>  43 files changed, 624 insertions(+), 201 deletions(-)
>  create mode 100644 tests/libqos/usb.c
>  create mode 100644 tests/libqos/usb.h
> 

For the qtest and SCSI parts:

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 24/36] s390x: convert s390-virtio to hotplug handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 13:31:21 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 26 Sep 2014 13:15:16 +0200
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > On Fri, 26 Sep 2014 09:28:29 +0000
> > Igor Mammedov <imammedo@redhat.com> wrote:
> > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > v2:
> > >  fix not working unplug
> > 
> > While the current code for s390-virtio implements unplug, the transport
> > per se does not really support unplug (and never has), only hotplugging
> > of devices. I removed the ->unplug callback and qemu now behaves like
> > I'd expect: I can add devices with device_add, but device_del leaves
> > them in place. That would be the way to go for me.
> Would a path to remove unplug on top of this one do or you'd prefer
> me to respin this patch without unplug?

Whatever works best for you. I don't think it's worth adding an extra
patch except if it saves you a respin of the whole series.

[Qemu-devel] [PATCH v3 24/36] s390x: convert s390-virtio to hotplug handler API

[Igor Mammedov]

beside of conversion, patch drops present unplug
handling, effectively disabling hot-unplug of
s390-virtio devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
 disable unplug, Cornelia Huck <cornelia.huck@de.ibm.com>
v2:
 fix not working unplug
---
 hw/s390x/s390-virtio-bus.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6b6fb61..7fa47d4 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -102,7 +102,7 @@ VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
     bus->next_ring = bus->dev_page + TARGET_PAGE_SIZE;
 
     /* Enable hotplugging */
-    _bus->allow_hotplug = 1;
+    qbus_set_hotplug_handler(_bus, dev, &error_abort);
 
     /* Allocate RAM for VirtIO device pages (descriptors, queues, rings) */
     *ram_size += S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
@@ -600,7 +600,6 @@ static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
 
     dc->init = s390_virtio_busdev_init;
     dc->bus_type = TYPE_S390_VIRTIO_BUS;
-    dc->unplug = qdev_simple_unplug_cb;
     dc->reset = s390_virtio_busdev_reset;
 }
 
@@ -674,6 +673,7 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
 static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->init = s390_virtio_bridge_init;
 }
@@ -683,6 +683,10 @@ static const TypeInfo s390_virtio_bridge_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusDevice),
     .class_init    = s390_virtio_bridge_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 /* virtio-s390-bus */
@@ -691,13 +695,10 @@ static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
                                 VirtIOS390Device *dev)
 {
     DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
     char virtio_bus_name[] = "virtio-bus";
 
     qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_S390_BUS,
                         qdev, virtio_bus_name);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 1;
 }
 
 static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 24/36] s390x: convert s390-virtio to hotplug handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 12:08:16 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> beside of conversion, patch drops present unplug
> handling, effectively disabling hot-unplug of
> s390-virtio devices.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v3:
>  disable unplug, Cornelia Huck <cornelia.huck@de.ibm.com>
> v2:
>  fix not working unplug
> ---
>  hw/s390x/s390-virtio-bus.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 6b6fb61..7fa47d4 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c

> @@ -674,6 +673,7 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
>  static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
>  {
>      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);

This line is not needed (and gives an error for me).

> 
>      k->init = s390_virtio_bridge_init;
>  }

Re: [Qemu-devel] [PATCH v3 24/36] s390x: convert s390-virtio to hotplug handler API

[Igor Mammedov]

On Fri, 26 Sep 2014 14:15:48 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Fri, 26 Sep 2014 12:08:16 +0000
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > beside of conversion, patch drops present unplug
> > handling, effectively disabling hot-unplug of
> > s390-virtio devices.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v3:
> >  disable unplug, Cornelia Huck <cornelia.huck@de.ibm.com>
> > v2:
> >  fix not working unplug
> > ---
> >  hw/s390x/s390-virtio-bus.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> > index 6b6fb61..7fa47d4 100644
> > --- a/hw/s390x/s390-virtio-bus.c
> > +++ b/hw/s390x/s390-virtio-bus.c
> 
> > @@ -674,6 +673,7 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
> >  static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
> >  {
> >      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> 
> This line is not needed (and gives an error for me).
Ops, sorry.
I'll fix it.

> 
> > 
> >      k->init = s390_virtio_bridge_init;
> >  }
> 
> 

[Qemu-devel] [PATCH v4 24/36] s390x: convert s390-virtio to hotplug handler API

[Igor Mammedov]

beside of conversion, patch drops present unplug
handling, effectively disabling hot-unplug of
s390-virtio devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v4:
 fix build break due to unused variable
v3:
 disable unplug, Cornelia Huck <cornelia.huck@de.ibm.com>
v2:
 fix not working unplug
---
 hw/s390x/s390-virtio-bus.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6b6fb61..d710038 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -102,7 +102,7 @@ VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
     bus->next_ring = bus->dev_page + TARGET_PAGE_SIZE;
 
     /* Enable hotplugging */
-    _bus->allow_hotplug = 1;
+    qbus_set_hotplug_handler(_bus, dev, &error_abort);
 
     /* Allocate RAM for VirtIO device pages (descriptors, queues, rings) */
     *ram_size += S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
@@ -600,7 +600,6 @@ static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
 
     dc->init = s390_virtio_busdev_init;
     dc->bus_type = TYPE_S390_VIRTIO_BUS;
-    dc->unplug = qdev_simple_unplug_cb;
     dc->reset = s390_virtio_busdev_reset;
 }
 
@@ -683,6 +682,10 @@ static const TypeInfo s390_virtio_bridge_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusDevice),
     .class_init    = s390_virtio_bridge_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 /* virtio-s390-bus */
@@ -691,13 +694,10 @@ static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
                                 VirtIOS390Device *dev)
 {
     DeviceState *qdev = DEVICE(dev);
-    BusState *qbus;
     char virtio_bus_name[] = "virtio-bus";
 
     qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_S390_BUS,
                         qdev, virtio_bus_name);
-    qbus = BUS(bus);
-    qbus->allow_hotplug = 1;
 }
 
 static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 24/36] s390x: convert s390-virtio to hotplug handler API

[Cornelia Huck]

On Fri, 26 Sep 2014 12:37:30 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> beside of conversion, patch drops present unplug
> handling, effectively disabling hot-unplug of
> s390-virtio devices.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v4:
>  fix build break due to unused variable
> v3:
>  disable unplug, Cornelia Huck <cornelia.huck@de.ibm.com>
> v2:
>  fix not working unplug
> ---
>  hw/s390x/s390-virtio-bus.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Gerd Hoffmann]

  Hi,

> Series unifies different hotplug mechanisms to a recent
> hotplug-handler API and does shallow conversion of
> devices that still use legacy qdev hotplug to it dropping
> not used after that legacy hotplug path [29/30].
> It also relaces SCSI's own way to do hotplug/unplug with
> hotplug-handler callbacks leaving it the only method
> perform hotplug tasks.
> And the last patch [30/30] allows to unplug of BUS-less
> devices using hotplug-handler API.

USB bits look good to me now.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Michael S. Tsirkin]

On Fri, Sep 26, 2014 at 09:28:05AM +0000, Igor Mammedov wrote:
> Changes since v1:
>   * added usb-uas test
>   * drop hotplug check in bus_add_child()
>   * make SCSI & USB bus as default HotplugHandler
>   * fixed dummy HBA hot(un)plug
>   * fixed hotunplug on s390x
>   * prevent hotplug of non hotpluggable devices

For pci, virtio, acpi bits:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Which tree should this go in through? afaerber's?
In that case, it might be a good idea to Cc him ...

> --
> 
> Series unifies different hotplug mechanisms to a recent
> hotplug-handler API and does shallow conversion of
> devices that still use legacy qdev hotplug to it dropping
> not used after that legacy hotplug path [29/30].
> It also relaces SCSI's own way to do hotplug/unplug with
> hotplug-handler callbacks leaving it the only method
> perform hotplug tasks.
> And the last patch [30/30] allows to unplug of BUS-less
> devices using hotplug-handler API.
> 
> Converted devices are covered with new hotplug
> unit-tests, except of:
>  s390x-*: I have no idea how or means to test it, but code
>           is close to virtio, so it's converted using
>           virtio template
>  pvscsi: is broken, so no means to test it
> 
> Git tree for testing:
>   https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2
> 
> 
> Igor Mammedov (36):
>   test: virtio-scsi: check if hot-plug/unplug works
>   test: virtio-serial: check if hot-plug/unplug works
>   test: libqos: add qpci_plug_device_test() and
>     qpci_unplug_acpi_device_test()
>   test: virtio-rng: check if hot-plug/unplug works
>   test: virtio-net: check if hot-plug/unplug works
>   test: virtio-blk: check if hot-plug/unplug works
>   test: usb: move uhci port test code to libqos/usb.c
>   test: usb: add port test to uhci unit test
>   test: usb: generic usb device hotplug
>   test: usb: usb-storage hotplug test
>   test: usb: usb-uas hotplug test
>   access BusState.allow_hotplug using wraper qbus_is_hotpluggable()
>   qdev: do not allow to instantiate non hotpluggable device with
>     device_add
>   qdev: HotplugHandler: rename unplug callback to unplug_request
>   qdev: HotplugHandler: provide unplug callback
>   qdev: add simple/generic unplug callback for HotplugHandler
>   qdev: add wrapper to set BUS as HotplugHandler
>   qdev: drop hotplug check from bus_add_child()
>   target-i386: ICC bus: drop BusState.allow_hotplug
>   virtio-pci: drop BusState.allow_hotplug
>   virtio-serial: convert to hotplug-handler API
>   virtio-mmio: drop useless bus->allow_hotplug = 0
>   s390x: drop not used allow_hotplug in event-facility
>   s390x: convert s390-virtio to hotplug handler API
>   s390x: convert virtio-ccw to hotplug handler API
>   scsi: set SCSI BUS itself as default HotplugHandler
>   scsi: convert pvscsi HBA to hotplug handler API
>   scsi: convert virtio-scsi HBA to hotplug handler API
>   scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} fields
>   usb-bot: mark device as non hotpluggable
>   usb-bot: drop not needed "allow_hotplug = 0"
>   usb-storage: drop not needed "allow_hotplug = 0"
>   usb: convert usb-ccid to hotplug handler API
>   usb: convert usb devices to hotplug handler API
>   qdev: drop legacy hotplug fields/methods
>   qdev: HotplugHandler: add support for unplugging BUS-less devices
> 
>  hw/acpi/piix4.c                |  6 +--
>  hw/char/virtio-serial-bus.c    | 20 +++++++---
>  hw/core/hotplug.c              | 11 ++++++
>  hw/core/qdev.c                 | 85 ++++++++++++++++++++++++++++--------------
>  hw/cpu/icc_bus.c               |  8 ----
>  hw/i386/acpi-build.c           |  2 +-
>  hw/isa/lpc_ich9.c              |  6 +--
>  hw/pci-bridge/pci_bridge_dev.c |  2 +-
>  hw/pci/pci-hotplug-old.c       |  4 +-
>  hw/pci/pcie.c                  |  4 +-
>  hw/pci/pcie_port.c             |  2 +-
>  hw/pci/shpc.c                  |  4 +-
>  hw/s390x/event-facility.c      |  2 -
>  hw/s390x/s390-virtio-bus.c     | 12 +++---
>  hw/s390x/virtio-ccw.c          | 17 +++++----
>  hw/scsi/scsi-bus.c             | 24 ++++--------
>  hw/scsi/virtio-scsi.c          | 30 ++++++++++-----
>  hw/scsi/vmw_pvscsi.c           | 26 +++++++++----
>  hw/usb/bus.c                   |  9 ++++-
>  hw/usb/dev-smartcard-reader.c  |  8 +++-
>  hw/usb/dev-storage.c           |  4 +-
>  hw/virtio/virtio-mmio.c        | 17 +--------
>  hw/virtio/virtio-pci.c         |  3 --
>  include/hw/hotplug.h           | 16 +++++++-
>  include/hw/pci/pcie.h          |  4 +-
>  include/hw/pci/shpc.h          |  4 +-
>  include/hw/qdev-core.h         | 17 +++++----
>  include/hw/scsi/scsi.h         |  2 -
>  qdev-monitor.c                 |  5 ++-
>  tests/Makefile                 | 13 ++++---
>  tests/libqos/pci-pc.c          | 49 ++++++++++++++++++++++++
>  tests/libqos/pci.h             |  3 ++
>  tests/libqos/usb.c             | 71 +++++++++++++++++++++++++++++++++++
>  tests/libqos/usb.h             | 17 +++++++++
>  tests/usb-hcd-ehci-test.c      | 50 ++++++++++---------------
>  tests/usb-hcd-ohci-test.c      | 10 ++++-
>  tests/usb-hcd-uhci-test.c      | 65 +++++++++++++++++++++++++++++++-
>  tests/usb-hcd-xhci-test.c      | 68 ++++++++++++++++++++++++++++++++-
>  tests/virtio-blk-test.c        | 49 ++++++++++++++++++------
>  tests/virtio-net-test.c        | 10 +++++
>  tests/virtio-rng-test.c        | 10 +++++
>  tests/virtio-scsi-test.c       | 29 ++++++++++++++
>  tests/virtio-serial-test.c     | 27 ++++++++++++++
>  43 files changed, 624 insertions(+), 201 deletions(-)
>  create mode 100644 tests/libqos/usb.c
>  create mode 100644 tests/libqos/usb.h
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Igor Mammedov]

On Mon, 29 Sep 2014 13:16:08 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Sep 26, 2014 at 09:28:05AM +0000, Igor Mammedov wrote:
> > Changes since v1:
> >   * added usb-uas test
> >   * drop hotplug check in bus_add_child()
> >   * make SCSI & USB bus as default HotplugHandler
> >   * fixed dummy HBA hot(un)plug
> >   * fixed hotunplug on s390x
> >   * prevent hotplug of non hotpluggable devices
> 
> For pci, virtio, acpi bits:
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Which tree should this go in through? afaerber's?
> In that case, it might be a good idea to Cc him ...
I'm not sure through which tree it should go,
it's not QOM or CPU that's why Andreas wasn't even CCed.

Could you take it through your tree?

> 
> > --
> > 
> > Series unifies different hotplug mechanisms to a recent
> > hotplug-handler API and does shallow conversion of
> > devices that still use legacy qdev hotplug to it dropping
> > not used after that legacy hotplug path [29/30].
> > It also relaces SCSI's own way to do hotplug/unplug with
> > hotplug-handler callbacks leaving it the only method
> > perform hotplug tasks.
> > And the last patch [30/30] allows to unplug of BUS-less
> > devices using hotplug-handler API.
> > 
> > Converted devices are covered with new hotplug
> > unit-tests, except of:
> >  s390x-*: I have no idea how or means to test it, but code
> >           is close to virtio, so it's converted using
> >           virtio template
> >  pvscsi: is broken, so no means to test it
> > 
> > Git tree for testing:
> >   https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2
> > 
> > 
> > Igor Mammedov (36):
> >   test: virtio-scsi: check if hot-plug/unplug works
> >   test: virtio-serial: check if hot-plug/unplug works
> >   test: libqos: add qpci_plug_device_test() and
> >     qpci_unplug_acpi_device_test()
> >   test: virtio-rng: check if hot-plug/unplug works
> >   test: virtio-net: check if hot-plug/unplug works
> >   test: virtio-blk: check if hot-plug/unplug works
> >   test: usb: move uhci port test code to libqos/usb.c
> >   test: usb: add port test to uhci unit test
> >   test: usb: generic usb device hotplug
> >   test: usb: usb-storage hotplug test
> >   test: usb: usb-uas hotplug test
> >   access BusState.allow_hotplug using wraper qbus_is_hotpluggable()
> >   qdev: do not allow to instantiate non hotpluggable device with
> >     device_add
> >   qdev: HotplugHandler: rename unplug callback to unplug_request
> >   qdev: HotplugHandler: provide unplug callback
> >   qdev: add simple/generic unplug callback for HotplugHandler
> >   qdev: add wrapper to set BUS as HotplugHandler
> >   qdev: drop hotplug check from bus_add_child()
> >   target-i386: ICC bus: drop BusState.allow_hotplug
> >   virtio-pci: drop BusState.allow_hotplug
> >   virtio-serial: convert to hotplug-handler API
> >   virtio-mmio: drop useless bus->allow_hotplug = 0
> >   s390x: drop not used allow_hotplug in event-facility
> >   s390x: convert s390-virtio to hotplug handler API
> >   s390x: convert virtio-ccw to hotplug handler API
> >   scsi: set SCSI BUS itself as default HotplugHandler
> >   scsi: convert pvscsi HBA to hotplug handler API
> >   scsi: convert virtio-scsi HBA to hotplug handler API
> >   scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug}
> > fields usb-bot: mark device as non hotpluggable
> >   usb-bot: drop not needed "allow_hotplug = 0"
> >   usb-storage: drop not needed "allow_hotplug = 0"
> >   usb: convert usb-ccid to hotplug handler API
> >   usb: convert usb devices to hotplug handler API
> >   qdev: drop legacy hotplug fields/methods
> >   qdev: HotplugHandler: add support for unplugging BUS-less devices
> > 
> >  hw/acpi/piix4.c                |  6 +--
> >  hw/char/virtio-serial-bus.c    | 20 +++++++---
> >  hw/core/hotplug.c              | 11 ++++++
> >  hw/core/qdev.c                 | 85
> > ++++++++++++++++++++++++++++--------------
> > hw/cpu/icc_bus.c               |  8 ----
> > hw/i386/acpi-build.c           |  2 +-
> > hw/isa/lpc_ich9.c              |  6 +--
> > hw/pci-bridge/pci_bridge_dev.c |  2 +-
> > hw/pci/pci-hotplug-old.c       |  4 +-
> > hw/pci/pcie.c                  |  4 +-
> > hw/pci/pcie_port.c             |  2 +-
> > hw/pci/shpc.c                  |  4 +-
> > hw/s390x/event-facility.c      |  2 -
> > hw/s390x/s390-virtio-bus.c     | 12 +++---
> > hw/s390x/virtio-ccw.c          | 17 +++++----
> > hw/scsi/scsi-bus.c             | 24 ++++--------
> > hw/scsi/virtio-scsi.c          | 30 ++++++++++-----
> > hw/scsi/vmw_pvscsi.c           | 26 +++++++++----
> > hw/usb/bus.c                   |  9 ++++-
> > hw/usb/dev-smartcard-reader.c  |  8 +++-
> > hw/usb/dev-storage.c           |  4 +-
> > hw/virtio/virtio-mmio.c        | 17 +--------
> > hw/virtio/virtio-pci.c         |  3 --
> > include/hw/hotplug.h           | 16 +++++++-
> > include/hw/pci/pcie.h          |  4 +-
> > include/hw/pci/shpc.h          |  4 +-
> > include/hw/qdev-core.h         | 17 +++++----
> > include/hw/scsi/scsi.h         |  2 -
> > qdev-monitor.c                 |  5 ++-
> > tests/Makefile                 | 13 ++++---
> > tests/libqos/pci-pc.c          | 49 ++++++++++++++++++++++++
> > tests/libqos/pci.h             |  3 ++
> > tests/libqos/usb.c             | 71
> > +++++++++++++++++++++++++++++++++++ tests/libqos/usb.h
> > | 17 +++++++++ tests/usb-hcd-ehci-test.c      | 50
> > ++++++++++--------------- tests/usb-hcd-ohci-test.c      | 10 ++++-
> > tests/usb-hcd-uhci-test.c      | 65
> > +++++++++++++++++++++++++++++++- tests/usb-hcd-xhci-test.c      |
> > 68 ++++++++++++++++++++++++++++++++- tests/virtio-blk-test.c
> > | 49 ++++++++++++++++++------ tests/virtio-net-test.c        | 10
> > +++++ tests/virtio-rng-test.c        | 10 +++++
> > tests/virtio-scsi-test.c       | 29 ++++++++++++++
> > tests/virtio-serial-test.c     | 27 ++++++++++++++ 43 files
> > changed, 624 insertions(+), 201 deletions(-) create mode 100644
> > tests/libqos/usb.c create mode 100644 tests/libqos/usb.h
> > 
> > -- 
> > 1.8.3.1
> 

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Michael S. Tsirkin]

On Mon, Sep 29, 2014 at 12:28:04PM +0200, Igor Mammedov wrote:
> On Mon, 29 Sep 2014 13:16:08 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Fri, Sep 26, 2014 at 09:28:05AM +0000, Igor Mammedov wrote:
> > > Changes since v1:
> > >   * added usb-uas test
> > >   * drop hotplug check in bus_add_child()
> > >   * make SCSI & USB bus as default HotplugHandler
> > >   * fixed dummy HBA hot(un)plug
> > >   * fixed hotunplug on s390x
> > >   * prevent hotplug of non hotpluggable devices
> > 
> > For pci, virtio, acpi bits:
> > 
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Which tree should this go in through? afaerber's?
> > In that case, it might be a good idea to Cc him ...
> I'm not sure through which tree it should go,
> it's not QOM or CPU that's why Andreas wasn't even CCed.
> 
> Could you take it through your tree?

As usual with cross-tree changes, selecting a tree is not
obvious, but Andreas was handling most core/qdev patches.
So sure, I can merge these if I get his ack.

> > 
> > > --
> > > 
> > > Series unifies different hotplug mechanisms to a recent
> > > hotplug-handler API and does shallow conversion of
> > > devices that still use legacy qdev hotplug to it dropping
> > > not used after that legacy hotplug path [29/30].
> > > It also relaces SCSI's own way to do hotplug/unplug with
> > > hotplug-handler callbacks leaving it the only method
> > > perform hotplug tasks.
> > > And the last patch [30/30] allows to unplug of BUS-less
> > > devices using hotplug-handler API.
> > > 
> > > Converted devices are covered with new hotplug
> > > unit-tests, except of:
> > >  s390x-*: I have no idea how or means to test it, but code
> > >           is close to virtio, so it's converted using
> > >           virtio template
> > >  pvscsi: is broken, so no means to test it
> > > 
> > > Git tree for testing:
> > >   https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2
> > > 
> > > 
> > > Igor Mammedov (36):
> > >   test: virtio-scsi: check if hot-plug/unplug works
> > >   test: virtio-serial: check if hot-plug/unplug works
> > >   test: libqos: add qpci_plug_device_test() and
> > >     qpci_unplug_acpi_device_test()
> > >   test: virtio-rng: check if hot-plug/unplug works
> > >   test: virtio-net: check if hot-plug/unplug works
> > >   test: virtio-blk: check if hot-plug/unplug works
> > >   test: usb: move uhci port test code to libqos/usb.c
> > >   test: usb: add port test to uhci unit test
> > >   test: usb: generic usb device hotplug
> > >   test: usb: usb-storage hotplug test
> > >   test: usb: usb-uas hotplug test
> > >   access BusState.allow_hotplug using wraper qbus_is_hotpluggable()
> > >   qdev: do not allow to instantiate non hotpluggable device with
> > >     device_add
> > >   qdev: HotplugHandler: rename unplug callback to unplug_request
> > >   qdev: HotplugHandler: provide unplug callback
> > >   qdev: add simple/generic unplug callback for HotplugHandler
> > >   qdev: add wrapper to set BUS as HotplugHandler
> > >   qdev: drop hotplug check from bus_add_child()
> > >   target-i386: ICC bus: drop BusState.allow_hotplug
> > >   virtio-pci: drop BusState.allow_hotplug
> > >   virtio-serial: convert to hotplug-handler API
> > >   virtio-mmio: drop useless bus->allow_hotplug = 0
> > >   s390x: drop not used allow_hotplug in event-facility
> > >   s390x: convert s390-virtio to hotplug handler API
> > >   s390x: convert virtio-ccw to hotplug handler API
> > >   scsi: set SCSI BUS itself as default HotplugHandler
> > >   scsi: convert pvscsi HBA to hotplug handler API
> > >   scsi: convert virtio-scsi HBA to hotplug handler API
> > >   scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug}
> > > fields usb-bot: mark device as non hotpluggable
> > >   usb-bot: drop not needed "allow_hotplug = 0"
> > >   usb-storage: drop not needed "allow_hotplug = 0"
> > >   usb: convert usb-ccid to hotplug handler API
> > >   usb: convert usb devices to hotplug handler API
> > >   qdev: drop legacy hotplug fields/methods
> > >   qdev: HotplugHandler: add support for unplugging BUS-less devices
> > > 
> > >  hw/acpi/piix4.c                |  6 +--
> > >  hw/char/virtio-serial-bus.c    | 20 +++++++---
> > >  hw/core/hotplug.c              | 11 ++++++
> > >  hw/core/qdev.c                 | 85
> > > ++++++++++++++++++++++++++++--------------
> > > hw/cpu/icc_bus.c               |  8 ----
> > > hw/i386/acpi-build.c           |  2 +-
> > > hw/isa/lpc_ich9.c              |  6 +--
> > > hw/pci-bridge/pci_bridge_dev.c |  2 +-
> > > hw/pci/pci-hotplug-old.c       |  4 +-
> > > hw/pci/pcie.c                  |  4 +-
> > > hw/pci/pcie_port.c             |  2 +-
> > > hw/pci/shpc.c                  |  4 +-
> > > hw/s390x/event-facility.c      |  2 -
> > > hw/s390x/s390-virtio-bus.c     | 12 +++---
> > > hw/s390x/virtio-ccw.c          | 17 +++++----
> > > hw/scsi/scsi-bus.c             | 24 ++++--------
> > > hw/scsi/virtio-scsi.c          | 30 ++++++++++-----
> > > hw/scsi/vmw_pvscsi.c           | 26 +++++++++----
> > > hw/usb/bus.c                   |  9 ++++-
> > > hw/usb/dev-smartcard-reader.c  |  8 +++-
> > > hw/usb/dev-storage.c           |  4 +-
> > > hw/virtio/virtio-mmio.c        | 17 +--------
> > > hw/virtio/virtio-pci.c         |  3 --
> > > include/hw/hotplug.h           | 16 +++++++-
> > > include/hw/pci/pcie.h          |  4 +-
> > > include/hw/pci/shpc.h          |  4 +-
> > > include/hw/qdev-core.h         | 17 +++++----
> > > include/hw/scsi/scsi.h         |  2 -
> > > qdev-monitor.c                 |  5 ++-
> > > tests/Makefile                 | 13 ++++---
> > > tests/libqos/pci-pc.c          | 49 ++++++++++++++++++++++++
> > > tests/libqos/pci.h             |  3 ++
> > > tests/libqos/usb.c             | 71
> > > +++++++++++++++++++++++++++++++++++ tests/libqos/usb.h
> > > | 17 +++++++++ tests/usb-hcd-ehci-test.c      | 50
> > > ++++++++++--------------- tests/usb-hcd-ohci-test.c      | 10 ++++-
> > > tests/usb-hcd-uhci-test.c      | 65
> > > +++++++++++++++++++++++++++++++- tests/usb-hcd-xhci-test.c      |
> > > 68 ++++++++++++++++++++++++++++++++- tests/virtio-blk-test.c
> > > | 49 ++++++++++++++++++------ tests/virtio-net-test.c        | 10
> > > +++++ tests/virtio-rng-test.c        | 10 +++++
> > > tests/virtio-scsi-test.c       | 29 ++++++++++++++
> > > tests/virtio-serial-test.c     | 27 ++++++++++++++ 43 files
> > > changed, 624 insertions(+), 201 deletions(-) create mode 100644
> > > tests/libqos/usb.c create mode 100644 tests/libqos/usb.h
> > > 
> > > -- 
> > > 1.8.3.1
> > 

Re: [Qemu-devel] [PATCH v2 13/36] qdev: do not allow to instantiate non hotpluggable device with device_add

[Paolo Bonzini]

Il 26/09/2014 11:28, Igor Mammedov ha scritto:
> It will allow explicitly mark device as not hotpluggable and
> avoid its creation with following error at realize time
> and destroying it afterwards anyway. Instead of it will
> error out even before instance of device is created.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  qdev-monitor.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f6db461..c721451 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -487,7 +487,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      }
>  
>      dc = DEVICE_CLASS(oc);
> -    if (dc->cannot_instantiate_with_device_add_yet) {
> +    if (dc->cannot_instantiate_with_device_add_yet ||
> +        (qdev_hotplug && !dc->hotpluggable)) {
>          qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver",
>                        "pluggable device type");
>          return NULL;
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 17/36] qdev: add wrapper to set BUS as HotplugHandler

[Paolo Bonzini]

Il 26/09/2014 11:28, Igor Mammedov ha scritto:
> to be used for conversion of SCSI and USB devices,
> and would allow to make every HBA/USB host switch
> to HotplugHandler API without touching each controller
> explicitly.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/core/qdev.c         | 19 +++++++++++++++++++
>  include/hw/qdev-core.h | 11 ++++-------
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1d1b113..0de99b2 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -112,6 +112,25 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>      bus_add_child(bus, dev);
>  }
>  
> +static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
> +                                              Error **errp)
> +{
> +
> +    object_property_set_link(OBJECT(bus), OBJECT(handler),
> +                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
> +    bus->allow_hotplug = 1;
> +}
> +
> +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
> +{
> +    qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
> +}
> +
> +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
> +{
> +    qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
> +}
> +
>  /* Create a new device.  This only initializes the device state structure
>     and allows properties to be set.  qdev_init should be called to
>     initialize the actual device emulation.  */
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index ba812c5..48e9579 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -363,13 +363,10 @@ extern int qdev_hotplug;
>  
>  char *qdev_get_dev_path(DeviceState *dev);
>  
> -static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
> -                                            Error **errp)
> -{
> -    object_property_set_link(OBJECT(bus), OBJECT(handler),
> -                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
> -    bus->allow_hotplug = 1;
> -}
> +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
> +                              Error **errp);
> +
> +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
>  
>  static inline bool qbus_is_hotpluggable(BusState *bus)
>  {
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 19/36] target-i386: ICC bus: drop BusState.allow_hotplug

[Paolo Bonzini]

Il 26/09/2014 11:28, Igor Mammedov ha scritto:
> since bus_add_child() no longer cares if BUS is hotpluggable
> or not, there is no need in setting allow_hotplug field.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/cpu/icc_bus.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 7f44c59..9575fd6 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -24,18 +24,10 @@
>  
>  /* icc-bridge implementation */
>  
> -static void icc_bus_init(Object *obj)
> -{
> -    BusState *b = BUS(obj);
> -
> -    b->allow_hotplug = true;
> -}
> -
>  static const TypeInfo icc_bus_info = {
>      .name = TYPE_ICC_BUS,
>      .parent = TYPE_BUS,
>      .instance_size = sizeof(ICCBus),
> -    .instance_init = icc_bus_init,
>  };
>  
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 18/36] qdev: drop hotplug check from bus_add_child()

[Paolo Bonzini]

Il 26/09/2014 11:28, Igor Mammedov ha scritto:
> check is too restrictive and does not allow
> to add childs to just created bus during hotplug
> when the bus is part of composite device.
> 
> Removing check from bus_add_child() doesn't affect
> devices creatable with device_add/del commands since
> they have a similar builtin checks and patch will
> allow to create complex composite devices during
> hotplug.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/core/qdev.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 0de99b2..fa86843 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -85,10 +85,6 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>      char name[32];
>      BusChild *kid = g_malloc0(sizeof(*kid));
>  
> -    if (qdev_hotplug) {
> -        assert(qbus_is_hotpluggable(bus));
> -    }
> -
>      kid->index = bus->max_index++;
>      kid->child = child;
>      object_ref(OBJECT(kid->child));
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 14/36] qdev: HotplugHandler: rename unplug callback to unplug_request

[Michael S. Tsirkin]

On Fri, Sep 26, 2014 at 09:28:19AM +0000, Igor Mammedov wrote:
> 'HotplugHandler.unplug' callback is currently used as async
> call to issue unplug request for device that implements it.
> Renaming 'unplug' callback to 'unplug_request' should help to
> avoid confusion about what callback does and would allow to
> introduce 'unplug' callback that would perform actual device
> removal when guest is ready for it.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> Patch is prompted by reviewing https://patchwork.ozlabs.org/patch/383372/
> 
> Dedicated 'unplug' callback could be used by bus-less pc-dimm
> device. It would allow to call HotplugHandler.unplug callback
> from ACPI code when guest calls _EJ0 method and execute board
> specific code (PCMachine) to unmap pc-dimm from guest's address
> space and perform necessary cleanup. The same applies to CPU
> unplug.
> ---
>  hw/acpi/piix4.c                |  6 +++---
>  hw/core/hotplug.c              | 10 +++++-----
>  hw/core/qdev.c                 |  3 ++-
>  hw/isa/lpc_ich9.c              |  6 +++---
>  hw/pci-bridge/pci_bridge_dev.c |  2 +-
>  hw/pci/pcie.c                  |  4 ++--
>  hw/pci/pcie_port.c             |  2 +-
>  hw/pci/shpc.c                  |  4 ++--
>  include/hw/hotplug.h           | 16 +++++++++-------
>  include/hw/pci/pcie.h          |  4 ++--
>  include/hw/pci/shpc.h          |  4 ++--
>  11 files changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..0bfa814 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -354,8 +354,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
>      }
>  }
>  
> -static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev,
> -                                   DeviceState *dev, Error **errp)
> +static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                           DeviceState *dev, Error **errp)
>  {
>      PIIX4PMState *s = PIIX4_PM(hotplug_dev);
>  
> @@ -615,7 +615,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
>      dc->cannot_instantiate_with_device_add_yet = true;
>      dc->hotpluggable = false;
>      hc->plug = piix4_device_plug_cb;
> -    hc->unplug = piix4_device_unplug_cb;
> +    hc->unplug_request = piix4_device_unplug_request_cb;
>      adevc->ospm_status = piix4_ospm_status;
>  }
>  
> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> index 5573d9d..2ec4736 100644
> --- a/hw/core/hotplug.c
> +++ b/hw/core/hotplug.c
> @@ -23,14 +23,14 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
>      }
>  }
>  
> -void hotplug_handler_unplug(HotplugHandler *plug_handler,
> -                            DeviceState *plugged_dev,
> -                            Error **errp)
> +void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
> +                                    DeviceState *plugged_dev,
> +                                    Error **errp)
>  {
>      HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
>  
> -    if (hdc->unplug) {
> -        hdc->unplug(plug_handler, plugged_dev, errp);
> +    if (hdc->unplug_request) {
> +        hdc->unplug_request(plug_handler, plugged_dev, errp);
>      }
>  }
>  
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 5e5b963..c98e5db 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -227,7 +227,8 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>      qdev_hot_removed = true;
>  
>      if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> -        hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
> +        hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
> +                                       dev, errp);
>      } else {
>          assert(dc->unplug != NULL);
>          if (dc->unplug(dev) < 0) { /* legacy handler */
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 177023b..530b074 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -607,8 +607,8 @@ static void ich9_device_plug_cb(HotplugHandler *hotplug_dev,
>      ich9_pm_device_plug_cb(&lpc->pm, dev, errp);
>  }
>  
> -static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev,
> -                                  DeviceState *dev, Error **errp)
> +static void ich9_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                          DeviceState *dev, Error **errp)
>  {
>      error_setg(errp, "acpi: device unplug request for not supported device"
>                 " type: %s", object_get_typename(OBJECT(dev)));
> @@ -676,7 +676,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
>       */
>      dc->cannot_instantiate_with_device_add_yet = true;
>      hc->plug = ich9_device_plug_cb;
> -    hc->unplug = ich9_device_unplug_cb;
> +    hc->unplug_request = ich9_device_unplug_request_cb;
>      adevc->ospm_status = ich9_pm_ospm_status;
>  }
>  
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index 92799d0..252ea5e 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -150,7 +150,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
>      dc->vmsd = &pci_bridge_dev_vmstate;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>      hc->plug = shpc_device_hotplug_cb;
> -    hc->unplug = shpc_device_hot_unplug_cb;
> +    hc->unplug_request = shpc_device_hot_unplug_request_cb;
>  }
>  
>  static const TypeInfo pci_bridge_dev_info = {
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 1babddf..b64a004 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -262,8 +262,8 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>                          PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
>  }
>  
> -void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> -                                 Error **errp)
> +void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                         DeviceState *dev, Error **errp)
>  {
>      uint8_t *exp_cap;
>  
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index fa24877..40ca8d5 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -154,7 +154,7 @@ static void pcie_slot_class_init(ObjectClass *oc, void *data)
>  
>      dc->props = pcie_slot_props;
>      hc->plug = pcie_cap_slot_hotplug_cb;
> -    hc->unplug = pcie_cap_slot_hot_unplug_cb;
> +    hc->unplug_request = pcie_cap_slot_hot_unplug_request_cb;
>  }
>  
>  static const TypeInfo pcie_slot_type_info = {
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 1fcb8c4..65b2f51 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -549,8 +549,8 @@ void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>      shpc_interrupt_update(pci_hotplug_dev);
>  }
>  
> -void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> -                               Error **errp)
> +void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                       DeviceState *dev, Error **errp)
>  {
>      Error *local_err = NULL;
>      PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> index a6533cb..e397d08 100644
> --- a/include/hw/hotplug.h
> +++ b/include/hw/hotplug.h
> @@ -47,7 +47,9 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
>   *
>   * @parent: Opaque parent interface.
>   * @plug: plug callback.
> - * @unplug: unplug callback.
> + * @unplug_request: unplug request callback.
> + *                  Used as a means to initiate device unplug for devices that
> + *                  require asynchronous unplug handling.
>   */
>  typedef struct HotplugHandlerClass {
>      /* <private> */
> @@ -55,7 +57,7 @@ typedef struct HotplugHandlerClass {
>  
>      /* <public> */
>      hotplug_fn plug;
> -    hotplug_fn unplug;
> +    hotplug_fn unplug_request;
>  } HotplugHandlerClass;
>  
>  /**
> @@ -68,11 +70,11 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
>                            Error **errp);
>  
>  /**
> - * hotplug_handler_unplug:
> + * hotplug_handler_unplug_request:
>   *
> - * Call #HotplugHandlerClass.unplug callback of @plug_handler.
> + * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.
>   */
> -void hotplug_handler_unplug(HotplugHandler *plug_handler,
> -                            DeviceState *plugged_dev,
> -                            Error **errp);
> +void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
> +                                    DeviceState *plugged_dev,
> +                                    Error **errp);
>  #endif
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index d139d58..b48a7a2 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -128,6 +128,6 @@ extern const VMStateDescription vmstate_pcie_device;
>  
>  void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                Error **errp);
> -void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> -                                 Error **errp);
> +void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                         DeviceState *dev, Error **errp);
>  #endif /* QEMU_PCIE_H */
> diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
> index eef1a1a..025bc5b 100644
> --- a/include/hw/pci/shpc.h
> +++ b/include/hw/pci/shpc.h
> @@ -46,8 +46,8 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len);
>  
>  void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>                              Error **errp);
> -void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> -                               Error **errp);
> +void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                       DeviceState *dev, Error **errp);
>  
>  extern VMStateInfo shpc_vmstate_info;
>  #define SHPC_VMSTATE(_field, _type) \
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Bharata B Rao]

On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   merged in Paolo's suggestion to reduce indentation in patch
> ---
>  hw/core/qdev.c | 61 ++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index bc45a59..215effb 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -223,9 +223,28 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>      dev->alias_required_for_version = required_for_version;
>  }
> 
> +static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +{
> +    HotplugHandler *hotplug_ctrl = NULL;
> +
> +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> +    } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> +        MachineState *machine = MACHINE(qdev_get_machine());
> +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> +
> +        if (mc->get_hotplug_handler) {
> +            hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> +        }
> +    }
> +    return hotplug_ctrl;
> +}
> +
>  void qdev_unplug(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +    HotplugHandler *hotplug_ctrl;
> +    HotplugHandlerClass *hdc;
> 
>      if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);

Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on any bus.
I was under the impression that this patch helps in unplugging such
devices. However I do see some problems.

While trying to unplug a PowerPC CPU that has earlier been added using
device_add, qdev-monitor.c:qmp_device_del() fails to find/locate the
device since the device isn't on any bus and hence qdev_unplug() won't
be called for such a device. I thought I could make "main_system_bus" as
the parent bus of this CPU device but that wouldn't help during unplugging.
With that qmp_device_del() can find the device, but qdev_uplug() fails at the
above if condition since "main_system_bus" isn't hotpluggable.

So how should we deal with such devices which don't have any parent bus ?

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Igor Mammedov]

On Wed, 1 Oct 2014 14:27:19 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >   merged in Paolo's suggestion to reduce indentation in patch
> > ---
> >  hw/core/qdev.c | 61 ++++++++++++++++++++++++++++++++--------------------------
> >  1 file changed, 34 insertions(+), 27 deletions(-)
> > 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index bc45a59..215effb 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -223,9 +223,28 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> >      dev->alias_required_for_version = required_for_version;
> >  }
> > 
> > +static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> > +{
> > +    HotplugHandler *hotplug_ctrl = NULL;
> > +
> > +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> > +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> > +    } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> > +        MachineState *machine = MACHINE(qdev_get_machine());
> > +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> > +
> > +        if (mc->get_hotplug_handler) {
> > +            hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> > +        }
> > +    }
> > +    return hotplug_ctrl;
> > +}
> > +
> >  void qdev_unplug(DeviceState *dev, Error **errp)
> >  {
> >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > +    HotplugHandler *hotplug_ctrl;
> > +    HotplugHandlerClass *hdc;
> > 
> >      if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
> >          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> 
> Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on any bus.
> I was under the impression that this patch helps in unplugging such
> devices. However I do see some problems.
Indeed, this patch goes only half way for supporting unplug of bussless
devices (i.e. it only fetches unplug handler without addressing device
lookup issue)

> 
> While trying to unplug a PowerPC CPU that has earlier been added using
> device_add, qdev-monitor.c:qmp_device_del() fails to find/locate the
> device since the device isn't on any bus and hence qdev_unplug() won't
> be called for such a device. I thought I could make "main_system_bus" as
> the parent bus of this CPU device but that wouldn't help during unplugging.
> With that qmp_device_del() can find the device, but qdev_uplug() fails at the
> above if condition since "main_system_bus" isn't hotpluggable.
> 
> So how should we deal with such devices which don't have any parent bus ?
There is no need to invent non existing BUS. We should fix lookup issue
instead of it. Doing something along this lines:
https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html

Above of cause assumes that device is placed in 'peripherals' tree.
Does above patch fixes issue?

> 
> Regards,
> Bharata.
> 

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Bharata B Rao]

On Wed, Oct 01, 2014 at 11:57:37AM +0200, Igor Mammedov wrote:
> On Wed, 1 Oct 2014 14:27:19 +0530
> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> 
> > On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > v2:
> > >   merged in Paolo's suggestion to reduce indentation in patch
> > > ---
> > >  hw/core/qdev.c | 61 ++++++++++++++++++++++++++++++++--------------------------
> > >  1 file changed, 34 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > > index bc45a59..215effb 100644
> > > --- a/hw/core/qdev.c
> > > +++ b/hw/core/qdev.c
> > > @@ -223,9 +223,28 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> > >      dev->alias_required_for_version = required_for_version;
> > >  }
> > > 
> > > +static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> > > +{
> > > +    HotplugHandler *hotplug_ctrl = NULL;
> > > +
> > > +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> > > +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> > > +    } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> > > +        MachineState *machine = MACHINE(qdev_get_machine());
> > > +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > +
> > > +        if (mc->get_hotplug_handler) {
> > > +            hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> > > +        }
> > > +    }
> > > +    return hotplug_ctrl;
> > > +}
> > > +
> > >  void qdev_unplug(DeviceState *dev, Error **errp)
> > >  {
> > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > +    HotplugHandler *hotplug_ctrl;
> > > +    HotplugHandlerClass *hdc;
> > > 
> > >      if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
> > >          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> > 
> > Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on any bus.
> > I was under the impression that this patch helps in unplugging such
> > devices. However I do see some problems.
> Indeed, this patch goes only half way for supporting unplug of bussless
> devices (i.e. it only fetches unplug handler without addressing device
> lookup issue)
> 
> > 
> > While trying to unplug a PowerPC CPU that has earlier been added using
> > device_add, qdev-monitor.c:qmp_device_del() fails to find/locate the
> > device since the device isn't on any bus and hence qdev_unplug() won't
> > be called for such a device. I thought I could make "main_system_bus" as
> > the parent bus of this CPU device but that wouldn't help during unplugging.
> > With that qmp_device_del() can find the device, but qdev_uplug() fails at the
> > above if condition since "main_system_bus" isn't hotpluggable.
> > 
> > So how should we deal with such devices which don't have any parent bus ?
> There is no need to invent non existing BUS. We should fix lookup issue
> instead of it. Doing something along this lines:
> https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html
> 
> Above of cause assumes that device is placed in 'peripherals' tree.
> Does above patch fixes issue?

Yes it does fix the issue. Thanks.

However device_del command completion isn't showing my CPU device. I guess
the patch you mentioned should take care of looking up in the peripherals
tree in monitor.c:device_del_bus_completion() too ?

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Igor Mammedov]

On Wed, 1 Oct 2014 16:22:23 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Wed, Oct 01, 2014 at 11:57:37AM +0200, Igor Mammedov wrote:
> > On Wed, 1 Oct 2014 14:27:19 +0530
> > Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> > 
> > > On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > v2:
> > > >   merged in Paolo's suggestion to reduce indentation in patch
> > > > ---
> > > >  hw/core/qdev.c | 61
> > > > ++++++++++++++++++++++++++++++++-------------------------- 1
> > > > file changed, 34 insertions(+), 27 deletions(-)
> > > > 
> > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > > > index bc45a59..215effb 100644
> > > > --- a/hw/core/qdev.c
> > > > +++ b/hw/core/qdev.c
> > > > @@ -223,9 +223,28 @@ void
> > > > qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> > > > dev->alias_required_for_version = required_for_version; }
> > > > 
> > > > +static HotplugHandler *qdev_get_hotplug_handler(DeviceState
> > > > *dev) +{
> > > > +    HotplugHandler *hotplug_ctrl = NULL;
> > > > +
> > > > +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> > > > +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> > > > +    } else if (object_dynamic_cast(qdev_get_machine(),
> > > > TYPE_MACHINE)) {
> > > > +        MachineState *machine = MACHINE(qdev_get_machine());
> > > > +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > > +
> > > > +        if (mc->get_hotplug_handler) {
> > > > +            hotplug_ctrl = mc->get_hotplug_handler(machine,
> > > > dev);
> > > > +        }
> > > > +    }
> > > > +    return hotplug_ctrl;
> > > > +}
> > > > +
> > > >  void qdev_unplug(DeviceState *dev, Error **errp)
> > > >  {
> > > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > > +    HotplugHandler *hotplug_ctrl;
> > > > +    HotplugHandlerClass *hdc;
> > > > 
> > > >      if (dev->parent_bus
> > > > && !qbus_is_hotpluggable(dev->parent_bus)) { error_set(errp,
> > > > QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> > > 
> > > Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on
> > > any bus. I was under the impression that this patch helps in
> > > unplugging such devices. However I do see some problems.
> > Indeed, this patch goes only half way for supporting unplug of
> > bussless devices (i.e. it only fetches unplug handler without
> > addressing device lookup issue)
> > 
> > > 
> > > While trying to unplug a PowerPC CPU that has earlier been added
> > > using device_add, qdev-monitor.c:qmp_device_del() fails to
> > > find/locate the device since the device isn't on any bus and
> > > hence qdev_unplug() won't be called for such a device. I thought
> > > I could make "main_system_bus" as the parent bus of this CPU
> > > device but that wouldn't help during unplugging. With that
> > > qmp_device_del() can find the device, but qdev_uplug() fails at
> > > the above if condition since "main_system_bus" isn't hotpluggable.
> > > 
> > > So how should we deal with such devices which don't have any
> > > parent bus ?
> > There is no need to invent non existing BUS. We should fix lookup
> > issue instead of it. Doing something along this lines:
> > https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html
> > 
> > Above of cause assumes that device is placed in 'peripherals' tree.
> > Does above patch fixes issue?
> 
> Yes it does fix the issue. Thanks.
Thanks for confirming, I'll try to rewrite above path to a more
acceptable form and post it as follow up to this series, hopefully
tomorrow.

Andreas,
 is it acceptable or should I respin fixedup 36/36 instead?

> 
> However device_del command completion isn't showing my CPU device. I
> guess the patch you mentioned should take care of looking up in the
> peripherals tree in monitor.c:device_del_bus_completion() too ?
device_del completion doesn't work yet.
there was patches on the list to this end /search for 'completion'
keyword in subject/

> 
> Regards,
> Bharata.
> 
> 

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Andreas Färber]

Am 01.10.2014 um 14:43 schrieb Igor Mammedov:
> On Wed, 1 Oct 2014 16:22:23 +0530
> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> 
>> On Wed, Oct 01, 2014 at 11:57:37AM +0200, Igor Mammedov wrote:
>>> On Wed, 1 Oct 2014 14:27:19 +0530
>>> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
>>>
>>>> On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
>>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>>> ---
>>>>> v2:
>>>>>   merged in Paolo's suggestion to reduce indentation in patch
>>>>> ---
>>>>>  hw/core/qdev.c | 61
>>>>> ++++++++++++++++++++++++++++++++-------------------------- 1
>>>>> file changed, 34 insertions(+), 27 deletions(-)
>>>>>
>>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>>> index bc45a59..215effb 100644
>>>>> --- a/hw/core/qdev.c
>>>>> +++ b/hw/core/qdev.c
>>>>> @@ -223,9 +223,28 @@ void
>>>>> qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>>>>> dev->alias_required_for_version = required_for_version; }
>>>>>
>>>>> +static HotplugHandler *qdev_get_hotplug_handler(DeviceState
>>>>> *dev) +{
>>>>> +    HotplugHandler *hotplug_ctrl = NULL;
>>>>> +
>>>>> +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
>>>>> +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
>>>>> +    } else if (object_dynamic_cast(qdev_get_machine(),
>>>>> TYPE_MACHINE)) {
>>>>> +        MachineState *machine = MACHINE(qdev_get_machine());
>>>>> +        MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>>> +
>>>>> +        if (mc->get_hotplug_handler) {
>>>>> +            hotplug_ctrl = mc->get_hotplug_handler(machine,
>>>>> dev);
>>>>> +        }
>>>>> +    }
>>>>> +    return hotplug_ctrl;
>>>>> +}
>>>>> +
>>>>>  void qdev_unplug(DeviceState *dev, Error **errp)
>>>>>  {
>>>>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>>>> +    HotplugHandler *hotplug_ctrl;
>>>>> +    HotplugHandlerClass *hdc;
>>>>>
>>>>>      if (dev->parent_bus
>>>>> && !qbus_is_hotpluggable(dev->parent_bus)) { error_set(errp,
>>>>> QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>>>>
>>>> Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on
>>>> any bus. I was under the impression that this patch helps in
>>>> unplugging such devices. However I do see some problems.
>>> Indeed, this patch goes only half way for supporting unplug of
>>> bussless devices (i.e. it only fetches unplug handler without
>>> addressing device lookup issue)
>>>
>>>>
>>>> While trying to unplug a PowerPC CPU that has earlier been added
>>>> using device_add, qdev-monitor.c:qmp_device_del() fails to
>>>> find/locate the device since the device isn't on any bus and
>>>> hence qdev_unplug() won't be called for such a device. I thought
>>>> I could make "main_system_bus" as the parent bus of this CPU
>>>> device but that wouldn't help during unplugging. With that
>>>> qmp_device_del() can find the device, but qdev_uplug() fails at
>>>> the above if condition since "main_system_bus" isn't hotpluggable.
>>>>
>>>> So how should we deal with such devices which don't have any
>>>> parent bus ?
>>> There is no need to invent non existing BUS. We should fix lookup
>>> issue instead of it. Doing something along this lines:
>>> https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html
>>>
>>> Above of cause assumes that device is placed in 'peripherals' tree.
>>> Does above patch fixes issue?
>>
>> Yes it does fix the issue. Thanks.
> Thanks for confirming, I'll try to rewrite above path to a more
> acceptable form and post it as follow up to this series, hopefully
> tomorrow.
> 
> Andreas,
>  is it acceptable or should I respin fixedup 36/36 instead?

As I started looking into this series I'd appreciate not respinning the
full series if not necessary. :) Whether you do a 37/36 v2 or a 36/36 v3
is up to you guys.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Igor Mammedov]

On Wed, 01 Oct 2014 14:46:03 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 01.10.2014 um 14:43 schrieb Igor Mammedov:
> > On Wed, 1 Oct 2014 16:22:23 +0530
> > Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> > 
> >> On Wed, Oct 01, 2014 at 11:57:37AM +0200, Igor Mammedov wrote:
> >>> On Wed, 1 Oct 2014 14:27:19 +0530
> >>> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> >>>
> >>>> On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> >>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >>>>> ---
> >>>>> v2:
> >>>>>   merged in Paolo's suggestion to reduce indentation in patch
> >>>>> ---
> >>>>>  hw/core/qdev.c | 61
> >>>>> ++++++++++++++++++++++++++++++++-------------------------- 1
> >>>>> file changed, 34 insertions(+), 27 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >>>>> index bc45a59..215effb 100644
> >>>>> --- a/hw/core/qdev.c
> >>>>> +++ b/hw/core/qdev.c
> >>>>> @@ -223,9 +223,28 @@ void
> >>>>> qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> >>>>> dev->alias_required_for_version = required_for_version; }
> >>>>>
> >>>>> +static HotplugHandler *qdev_get_hotplug_handler(DeviceState
> >>>>> *dev) +{
> >>>>> +    HotplugHandler *hotplug_ctrl = NULL;
> >>>>> +
> >>>>> +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> >>>>> +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> >>>>> +    } else if (object_dynamic_cast(qdev_get_machine(),
> >>>>> TYPE_MACHINE)) {
> >>>>> +        MachineState *machine = MACHINE(qdev_get_machine());
> >>>>> +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> >>>>> +
> >>>>> +        if (mc->get_hotplug_handler) {
> >>>>> +            hotplug_ctrl = mc->get_hotplug_handler(machine,
> >>>>> dev);
> >>>>> +        }
> >>>>> +    }
> >>>>> +    return hotplug_ctrl;
> >>>>> +}
> >>>>> +
> >>>>>  void qdev_unplug(DeviceState *dev, Error **errp)
> >>>>>  {
> >>>>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> >>>>> +    HotplugHandler *hotplug_ctrl;
> >>>>> +    HotplugHandlerClass *hdc;
> >>>>>
> >>>>>      if (dev->parent_bus
> >>>>> && !qbus_is_hotpluggable(dev->parent_bus)) { error_set(errp,
> >>>>> QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> >>>>
> >>>> Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on
> >>>> any bus. I was under the impression that this patch helps in
> >>>> unplugging such devices. However I do see some problems.
> >>> Indeed, this patch goes only half way for supporting unplug of
> >>> bussless devices (i.e. it only fetches unplug handler without
> >>> addressing device lookup issue)
> >>>
> >>>>
> >>>> While trying to unplug a PowerPC CPU that has earlier been added
> >>>> using device_add, qdev-monitor.c:qmp_device_del() fails to
> >>>> find/locate the device since the device isn't on any bus and
> >>>> hence qdev_unplug() won't be called for such a device. I thought
> >>>> I could make "main_system_bus" as the parent bus of this CPU
> >>>> device but that wouldn't help during unplugging. With that
> >>>> qmp_device_del() can find the device, but qdev_uplug() fails at
> >>>> the above if condition since "main_system_bus" isn't
> >>>> hotpluggable.
> >>>>
> >>>> So how should we deal with such devices which don't have any
> >>>> parent bus ?
> >>> There is no need to invent non existing BUS. We should fix lookup
> >>> issue instead of it. Doing something along this lines:
> >>> https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html
> >>>
> >>> Above of cause assumes that device is placed in 'peripherals'
> >>> tree. Does above patch fixes issue?
> >>
> >> Yes it does fix the issue. Thanks.
> > Thanks for confirming, I'll try to rewrite above path to a more
> > acceptable form and post it as follow up to this series, hopefully
> > tomorrow.
> > 
> > Andreas,
> >  is it acceptable or should I respin fixedup 36/36 instead?
> 
> As I started looking into this series I'd appreciate not respinning
> the full series if not necessary. :) Whether you do a 37/36 v2 or a
> 36/36 v3 is up to you guys.
I'll add 37/36 to fix lookup issue.


> 
> Thanks,
> Andreas
> 

Thanks,
Igor

[Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Igor Mammedov]

device_add puts every device with 'id' inside of 'peripheral'
container using id's value as the last component name.
Use it by replacing recursive search on sysbus with path
lookup in 'peripheral' container, which could handle both
BUS and BUS-less device cases.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 qdev-monitor.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index c721451..754437b 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -686,15 +686,20 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
 void qmp_device_del(const char *id, Error **errp)
 {
-    DeviceState *dev;
+    Object *obj;
+    char *root_path = object_get_canonical_path(qdev_get_peripheral());
+    char *path = g_strdup_printf("%s/%s", root_path, id);
 
-    dev = qdev_find_recursive(sysbus_get_default(), id);
-    if (!dev) {
+    g_free(root_path);
+    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
+    g_free(path);
+
+    if (!obj) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, id);
         return;
     }
 
-    qdev_unplug(dev, errp);
+    qdev_unplug(DEVICE(obj), errp);
 }
 
 void qdev_machine_init(void)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Zhu Guihua]

On Thu, 2014-10-02 at 10:08 +0000, Igor Mammedov wrote:
> device_add puts every device with 'id' inside of 'peripheral'
> container using id's value as the last component name.
> Use it by replacing recursive search on sysbus with path
> lookup in 'peripheral' container, which could handle both
> BUS and BUS-less device cases.
> 

If I want to delete device without id inside of 'peripheral-anon'
container, the command 'device_del' does not work. 
My suggestion is deleting device by the last component name, is this
feasiable?
Thanks.

Zhu

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  qdev-monitor.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index c721451..754437b 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -686,15 +686,20 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  
>  void qmp_device_del(const char *id, Error **errp)
>  {
> -    DeviceState *dev;
> +    Object *obj;
> +    char *root_path = object_get_canonical_path(qdev_get_peripheral());
> +    char *path = g_strdup_printf("%s/%s", root_path, id);
>  
> -    dev = qdev_find_recursive(sysbus_get_default(), id);
> -    if (!dev) {
> +    g_free(root_path);
> +    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> +    g_free(path);
> +
> +    if (!obj) {
>          error_set(errp, QERR_DEVICE_NOT_FOUND, id);
>          return;
>      }
>  
> -    qdev_unplug(dev, errp);
> +    qdev_unplug(DEVICE(obj), errp);
>  }
>  
>  void qdev_machine_init(void)

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Igor Mammedov]

On Tue, 7 Oct 2014 19:59:51 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> On Thu, 2014-10-02 at 10:08 +0000, Igor Mammedov wrote:
> > device_add puts every device with 'id' inside of 'peripheral'
> > container using id's value as the last component name.
> > Use it by replacing recursive search on sysbus with path
> > lookup in 'peripheral' container, which could handle both
> > BUS and BUS-less device cases.
> > 
> 
> If I want to delete device without id inside of 'peripheral-anon'
> container, the command 'device_del' does not work. 
> My suggestion is deleting device by the last component name, is this
> feasiable?
So far device_del was designed to work only with id-ed devices.

What's a use-case for unplugging unnamed device from peripheral-anon?

> Thanks.
> 
> Zhu
> 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  qdev-monitor.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index c721451..754437b 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -686,15 +686,20 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >  
> >  void qmp_device_del(const char *id, Error **errp)
> >  {
> > -    DeviceState *dev;
> > +    Object *obj;
> > +    char *root_path = object_get_canonical_path(qdev_get_peripheral());
> > +    char *path = g_strdup_printf("%s/%s", root_path, id);
> >  
> > -    dev = qdev_find_recursive(sysbus_get_default(), id);
> > -    if (!dev) {
> > +    g_free(root_path);
> > +    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> > +    g_free(path);
> > +
> > +    if (!obj) {
> >          error_set(errp, QERR_DEVICE_NOT_FOUND, id);
> >          return;
> >      }
> >  
> > -    qdev_unplug(dev, errp);
> > +    qdev_unplug(DEVICE(obj), errp);
> >  }
> >  
> >  void qdev_machine_init(void)
> 
> 

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Andreas Färber]

Am 07.10.2014 um 14:10 schrieb Igor Mammedov:
> On Tue, 7 Oct 2014 19:59:51 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> 
>> On Thu, 2014-10-02 at 10:08 +0000, Igor Mammedov wrote:
>>> device_add puts every device with 'id' inside of 'peripheral'
>>> container using id's value as the last component name.
>>> Use it by replacing recursive search on sysbus with path
>>> lookup in 'peripheral' container, which could handle both
>>> BUS and BUS-less device cases.
>>>
>>
>> If I want to delete device without id inside of 'peripheral-anon'
>> container, the command 'device_del' does not work. 
>> My suggestion is deleting device by the last component name, is this
>> feasiable?
> So far device_del was designed to work only with id-ed devices.
> 
> What's a use-case for unplugging unnamed device from peripheral-anon?

I can think of use cases where you may want to balloon memory or CPUs.

But that seems orthogonal to this series.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Igor Mammedov]

On Tue, 07 Oct 2014 15:23:45 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 07.10.2014 um 14:10 schrieb Igor Mammedov:
> > On Tue, 7 Oct 2014 19:59:51 +0800
> > Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> > 
> >> On Thu, 2014-10-02 at 10:08 +0000, Igor Mammedov wrote:
> >>> device_add puts every device with 'id' inside of 'peripheral'
> >>> container using id's value as the last component name.
> >>> Use it by replacing recursive search on sysbus with path
> >>> lookup in 'peripheral' container, which could handle both
> >>> BUS and BUS-less device cases.
> >>>
> >>
> >> If I want to delete device without id inside of 'peripheral-anon'
> >> container, the command 'device_del' does not work. 
> >> My suggestion is deleting device by the last component name, is this
> >> feasiable?
> > So far device_del was designed to work only with id-ed devices.
> > 
> > What's a use-case for unplugging unnamed device from peripheral-anon?
> 
> I can think of use cases where you may want to balloon memory or CPUs.
yep currently initial CPUs are created without dev->id and even without
device_add help.
However if/when it's switched to device_add we can make them use
auto-generated IDs so they would go into peripheral section.
That would let us keep peripheral-anon for devices that shouldn't
be unplugged.

> 
> But that seems orthogonal to this series.
> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Zhu Guihua]

On Tue, 2014-10-07 at 15:53 +0200, Igor Mammedov wrote:
> On Tue, 07 Oct 2014 15:23:45 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 07.10.2014 um 14:10 schrieb Igor Mammedov:
> > > On Tue, 7 Oct 2014 19:59:51 +0800
> > > Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> > > 
> > >> On Thu, 2014-10-02 at 10:08 +0000, Igor Mammedov wrote:
> > >>> device_add puts every device with 'id' inside of 'peripheral'
> > >>> container using id's value as the last component name.
> > >>> Use it by replacing recursive search on sysbus with path
> > >>> lookup in 'peripheral' container, which could handle both
> > >>> BUS and BUS-less device cases.
> > >>>
> > >>
> > >> If I want to delete device without id inside of 'peripheral-anon'
> > >> container, the command 'device_del' does not work. 
> > >> My suggestion is deleting device by the last component name, is this
> > >> feasiable?
> > > So far device_del was designed to work only with id-ed devices.
> > > 
> > > What's a use-case for unplugging unnamed device from peripheral-anon?
> > 
> > I can think of use cases where you may want to balloon memory or CPUs.
> yep currently initial CPUs are created without dev->id and even without
> device_add help.
> However if/when it's switched to device_add we can make them use
> auto-generated IDs so they would go into peripheral section.
> That would let us keep peripheral-anon for devices that shouldn't
> be unplugged.

when device_add pc-dimm, only 'memdev' property is necessary, but the
'id' property is optional. 

So I execute the command as followings:
object_add memory-backend-ram,id=ram0,size=128M
device_add pc-dimm,memdev=ram0

Now it is impossible to delete the pc-dimm, because it has no id, and it
is inside of 'peripheral-anon' container. 

Regards,
Zhu

> 
> > 
> > But that seems orthogonal to this series.
> > 
> > Regards,
> > Andreas
> > 
> 

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Paolo Bonzini]

Il 08/10/2014 05:49, Zhu Guihua ha scritto:
> when device_add pc-dimm, only 'memdev' property is necessary, but the
> 'id' property is optional. 
> 
> So I execute the command as followings:
> object_add memory-backend-ram,id=ram0,size=128M
> device_add pc-dimm,memdev=ram0
> 
> Now it is impossible to delete the pc-dimm, because it has no id, and it
> is inside of 'peripheral-anon' container. 

Sure; but that was an explicit choice when you issued device_add.

Paolo

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Zhu Guihua]

On Wed, 2014-10-08 at 10:01 +0200, Paolo Bonzini wrote:
> Il 08/10/2014 05:49, Zhu Guihua ha scritto:
> > when device_add pc-dimm, only 'memdev' property is necessary, but the
> > 'id' property is optional. 
> > 
> > So I execute the command as followings:
> > object_add memory-backend-ram,id=ram0,size=128M
> > device_add pc-dimm,memdev=ram0
> > 
> > Now it is impossible to delete the pc-dimm, because it has no id, and it
> > is inside of 'peripheral-anon' container. 
> 
> Sure; but that was an explicit choice when you issued device_add.

Thanks for your patient explanation。
And I think it is better to make memory-devices without dev->id use
auto-generated IDs.

Regards,
Zhu
> 
> Paolo

Re: [Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container

[Markus Armbruster]

Zhu Guihua <zhugh.fnst@cn.fujitsu.com> writes:

> On Wed, 2014-10-08 at 10:01 +0200, Paolo Bonzini wrote:
>> Il 08/10/2014 05:49, Zhu Guihua ha scritto:
>> > when device_add pc-dimm, only 'memdev' property is necessary, but the
>> > 'id' property is optional. 
>> > 
>> > So I execute the command as followings:
>> > object_add memory-backend-ram,id=ram0,size=128M
>> > device_add pc-dimm,memdev=ram0
>> > 
>> > Now it is impossible to delete the pc-dimm, because it has no id, and it
>> > is inside of 'peripheral-anon' container. 
>> 
>> Sure; but that was an explicit choice when you issued device_add.
>
> Thanks for your patient explanation。
> And I think it is better to make memory-devices without dev->id use
> auto-generated IDs.

Auto-generated IDs have been discussed and rejected several times.  At
the very least, clashes with user-specified IDs must be impossible.

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Andreas Färber]

Hi,

All 37 patches should be applied now, in their latest version, with
Reviewed-bys and my Sob...
https://github.com/afaerber/qemu-cpu/commits/qom-next

This is a series size that I would ask to split in the future.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

[Igor Mammedov]

On Mon, 13 Oct 2014 19:33:20 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Hi,
> 
> All 37 patches should be applied now, in their latest version, with
> Reviewed-bys and my Sob...
> https://github.com/afaerber/qemu-cpu/commits/qom-next
> 
> This is a series size that I would ask to split in the future.
Sure, it could have been split into 2 separate series testsuite and
conversion itself. 
I'll try to make my series smaller in future.

> 
> Thanks,
> Andreas
> 

Thanks,
  Igor