[Qemu-devel] [PATCH v2 0/2] Drop pci_add and pci_del from QMP

[Qemu-devel] [PATCH v2 0/2] Drop pci_add and pci_del from QMP

[Markus Armbruster]

See PATCH 1/1 for rationale.

v2: Cover pci_del, better commit message, rebased (no conflicts)

Markus Armbruster (2):
  Revert "PCI: Convert pci_device_hot_add() to QObject"
  Revert "monitor: Convert do_pci_device_hot_remove() to QObject"

 hw/pci-hotplug.c |   51 +++++++--------------------------------------------
 qemu-monitor.hx  |    6 ++----
 sysemu.h         |    6 ++----
 3 files changed, 11 insertions(+), 52 deletions(-)

[Qemu-devel] [PATCH v2 1/2] Revert "PCI: Convert pci_device_hot_add() to QObject"

[Markus Armbruster]

Short story: We don't want pci_add in QMP.  Long story follows.

pci_add can do two things:

* Hot plug a PCI NIC.  device_add is more general.

* Hot plug a PCI disk controller, and a drive connected to it.

  The controller is either virtio-blk-pci (if=virtio) or lsi53c895a
  (if=scsi).  With the latter, the drive is optional.  Use drive_add to
  hotplug additional SCSI drives.  Except drive_add is not available in
  QMP.

  device_add is more general for controllers and the guest part of
  drives.  I'm working on a more general alternative for the host part
  of drives.

Why am I proposing to remove pci_add from QMP before its replacement is
ready?  I want it out sooner rather than later, because it isn't fully
functional (errors and drive_add are missing), and we do not plan to
complete the job.  In other words, it's not really usable over QMP now,
and it's not what we want for QMP anyway.  Since we don't want it to be
used over QMP, we should take it out, not leave it around as a trap for
the uninitiated.

Dan Berrange confirmed that libvirt has no need for pci_add & friends
over QMP.

This reverts commit 7a344f7ac7bb651d0556a933ed8060d3a9e5d949.

Conflicts:

	hw/pci-hotplug.c
	sysemu.h

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-hotplug.c |   46 +++++-----------------------------------------
 qemu-monitor.hx  |    3 +--
 sysemu.h         |    3 +--
 3 files changed, 7 insertions(+), 45 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index cc45c50..22a7ce4 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -33,7 +33,6 @@
 #include "scsi.h"
 #include "virtio-blk.h"
 #include "qemu-config.h"
-#include "qemu-objects.h"
 
 #if defined(TARGET_I386)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -224,36 +223,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     return dev;
 }
 
-void pci_device_hot_add_print(Monitor *mon, const QObject *data)
-{
-    QDict *qdict;
-
-    assert(qobject_type(data) == QTYPE_QDICT);
-    qdict = qobject_to_qdict(data);
-
-    monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                   (int) qdict_get_int(qdict, "domain"),
-                   (int) qdict_get_int(qdict, "bus"),
-                   (int) qdict_get_int(qdict, "slot"),
-                   (int) qdict_get_int(qdict, "function"));
-
-}
-
-/**
- * pci_device_hot_add(): Hot add a PCI device
- *
- * Return a QDict with the following device information:
- *
- * - "domain": domain number
- * - "bus": bus number
- * - "slot": slot number
- * - "function": function number
- *
- * Example:
- *
- * { "domain": 0, "bus": 0, "slot": 5, "function": 0 }
- */
-int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void pci_device_hot_add(Monitor *mon, const QDict *qdict)
 {
     PCIDevice *dev = NULL;
     const char *pci_addr = qdict_get_str(qdict, "pci_addr");
@@ -278,20 +248,14 @@ int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
         dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
     } else {
         monitor_printf(mon, "invalid type: %s\n", type);
-        return -1;
     }
 
     if (dev) {
-        *ret_data =
-        qobject_from_jsonf("{ 'domain': 0, 'bus': %d, 'slot': %d, "
-                           "'function': %d }", pci_bus_num(dev->bus),
-                           PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
-    } else {
+        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
+                       0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
+                       PCI_FUNC(dev->devfn));
+    } else
         monitor_printf(mon, "failed to add %s\n", opts);
-        return -1;
-    }
-
-    return 0;
 }
 #endif
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index a8f194c..fba4c3f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -858,8 +858,7 @@ ETEXI
         .args_type  = "pci_addr:s,type:s,opts:s?",
         .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
         .help       = "hot-add PCI device",
-        .user_print = pci_device_hot_add_print,
-        .mhandler.cmd_new = pci_device_hot_add,
+        .mhandler.cmd = pci_device_hot_add,
     },
 #endif
 
diff --git a/sysemu.h b/sysemu.h
index fa921df..47975b5 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -201,8 +201,7 @@ extern DriveInfo *drive_init(QemuOpts *arg, void *machine, int *fatal_error);
 DriveInfo *add_init_drive(const char *opts);
 
 /* pci-hotplug */
-void pci_device_hot_add_print(Monitor *mon, const QObject *data);
-int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+void pci_device_hot_add(Monitor *mon, const QDict *qdict);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 int pci_device_hot_remove(Monitor *mon, const char *pci_addr);
 int do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 2/2] Revert "monitor: Convert do_pci_device_hot_remove() to QObject"

[Markus Armbruster]

We don't want pci_del in QMP.  Use device_del instead.

This reverts commit 6848d827162fea039f2658414a4adb6164a4f9b0.

Conflicts:

	hw/pci-hotplug.c
	sysemu.h

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-hotplug.c |    5 ++---
 qemu-monitor.hx  |    3 +--
 sysemu.h         |    3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 22a7ce4..37ac015 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -277,8 +277,7 @@ int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
     return qdev_unplug(&d->qdev);
 }
 
-int do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
-                             QObject **ret_data)
+void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
 {
-    return pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
+    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index fba4c3f..b6e3467 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -874,8 +874,7 @@ ETEXI
         .args_type  = "pci_addr:s",
         .params     = "[[<domain>:]<bus>:]<slot>",
         .help       = "hot remove PCI device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_pci_device_hot_remove,
+        .mhandler.cmd = do_pci_device_hot_remove,
     },
 #endif
 
diff --git a/sysemu.h b/sysemu.h
index 47975b5..643c0c6 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -204,8 +204,7 @@ DriveInfo *add_init_drive(const char *opts);
 void pci_device_hot_add(Monitor *mon, const QDict *qdict);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 int pci_device_hot_remove(Monitor *mon, const char *pci_addr);
-int do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
-                             QObject **ret_data);
+void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
 
 /* serial ports */
 
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH v2 0/2] Drop pci_add and pci_del from QMP

[Luiz Capitulino]

On Wed, 12 May 2010 10:52:59 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> See PATCH 1/1 for rationale.
> 
> v2: Cover pci_del, better commit message, rebased (no conflicts)
> 
> Markus Armbruster (2):
>   Revert "PCI: Convert pci_device_hot_add() to QObject"
>   Revert "monitor: Convert do_pci_device_hot_remove() to QObject"

 Looks good to me.

> 
>  hw/pci-hotplug.c |   51 +++++++--------------------------------------------
>  qemu-monitor.hx  |    6 ++----
>  sysemu.h         |    6 ++----
>  3 files changed, 11 insertions(+), 52 deletions(-)
>