[Qemu-devel] [PATCH 1/2] monitor: Drop pci_addr prefix from hotplug commands

[Qemu-devel] [PATCH 1/2] monitor: Drop pci_addr prefix from hotplug commands

[Jan Kiszka]

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

The "pci_addr=" prefix currently required by pci_add/remove and
drive_add has no practical use. Drop it, but still silently accept it
for backward compatibility.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 hw/pci-hotplug.c |   22 +++++++++-------------
 hw/pci.c         |   16 ++++++++++------
 hw/pci.h         |    3 ++-
 qemu-monitor.hx  |    6 +++---
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 952e27a..f7d38a7 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -56,8 +56,7 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
     int success = 0;
     PCIDevice *dev;
 
-    if (pci_read_devaddr(pci_addr, &dom, &pci_bus, &slot)) {
-        monitor_printf(mon, "Invalid pci address\n");
+    if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
         return;
     }
 
@@ -148,21 +147,19 @@ void pci_device_hot_add(Monitor *mon, const char *pci_addr, const char *type,
                         const char *opts)
 {
     PCIDevice *dev = NULL;
-    const char *devaddr = NULL;
-    char buf[32];
 
-    if (!get_param_value(buf, sizeof(buf), "pci_addr", pci_addr)) {
-        monitor_printf(mon, "Invalid pci address\n");
-        return;
+    /* strip legacy tag */
+    if (!strncmp(pci_addr, "pci_addr=", 9)) {
+        pci_addr += 9;
     }
 
-    if (strcmp(buf, "auto"))
-        devaddr = buf;
+    if (!strcmp(pci_addr, "auto"))
+        pci_addr = NULL;
 
     if (strcmp(type, "nic") == 0)
-        dev = qemu_pci_hot_add_nic(mon, devaddr, opts);
+        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
     else if (strcmp(type, "storage") == 0)
-        dev = qemu_pci_hot_add_storage(mon, devaddr, opts);
+        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
     else
         monitor_printf(mon, "invalid type: %s\n", type);
 
@@ -183,8 +180,7 @@ void pci_device_hot_remove(Monitor *mon, const char *pci_addr)
     int dom, bus;
     unsigned slot;
 
-    if (pci_read_devaddr(pci_addr, &dom, &bus, &slot)) {
-        monitor_printf(mon, "Invalid pci address\n");
+    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
         return;
     }
 
diff --git a/hw/pci.c b/hw/pci.c
index f2d7daf..57f4060 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -227,14 +227,18 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *s
     return 0;
 }
 
-int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
+int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
+                     unsigned *slotp)
 {
-    char devaddr[32];
-
-    if (!get_param_value(devaddr, sizeof(devaddr), "pci_addr", addr))
+    /* strip legacy tag */
+    if (!strncmp(addr, "pci_addr=", 9)) {
+        addr += 9;
+    }
+    if (pci_parse_devaddr(addr, domp, busp, slotp)) {
+        monitor_printf(mon, "Invalid pci address\n");
         return -1;
-
-    return pci_parse_devaddr(devaddr, domp, busp, slotp);
+    }
+    return 0;
 }
 
 static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
diff --git a/hw/pci.h b/hw/pci.h
index c8625de..3be660c 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -192,7 +192,8 @@ void pci_for_each_device(int bus_num, void (*fn)(PCIDevice *d));
 PCIBus *pci_find_bus(int bus_num);
 PCIDevice *pci_find_device(int bus_num, int slot, int function);
 
-int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
+int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
+                     unsigned *slotp);
 
 void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index d7c7078..2ea9840 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -493,7 +493,7 @@ Set maximum tolerated downtime (in seconds) for migration.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "drive_add", "ss", drive_hot_add, "pci_addr=[[<domain>:]<bus>:]<slot>\n"
+    { "drive_add", "ss", drive_hot_add, "[[<domain>:]<bus>:]<slot>\n"
                                          "[file=file][,if=type][,bus=n]\n"
                                         "[,unit=m][,media=d][index=i]\n"
                                         "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
@@ -506,7 +506,7 @@ Add drive to PCI storage controller.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "pci_add", "sss", pci_device_hot_add, "pci_addr=auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", "hot-add PCI device" },
+    { "pci_add", "sss", pci_device_hot_add, "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", "hot-add PCI device" },
 #endif
 STEXI
 @item pci_add
@@ -514,7 +514,7 @@ Hot-add PCI device.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "pci_del", "s", pci_device_hot_remove, "pci_addr=[[<domain>:]<bus>:]<slot>", "hot remove PCI device" },
+    { "pci_del", "s", pci_device_hot_remove, "[[<domain>:]<bus>:]<slot>", "hot remove PCI device" },
 #endif
 STEXI
 @item pci_del


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [PATCH 1/2] monitor: Drop pci_addr prefix from hotplug commands

[Markus Armbruster]

Jan Kiszka <jan.kiszka@web.de> writes:

> The "pci_addr=" prefix currently required by pci_add/remove and
> drive_add has no practical use. Drop it, but still silently accept it
> for backward compatibility.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

It's backward compatible, just not bug-compatible: junk like
a=b,pci_addr=auto,c=d no longer works.  Fine with me.

I wish we could use the addr= from the options argument and be done
with it.  Not backward compatible, so no go.

Acked-by: Markus Armbruster <armbru@redhat.com>