From 464629223ecf1365ae5b21035e789bfc81db7568 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Thu, 19 Nov 2020 12:07:32 +0100
Subject: [PATCH] virsh: Extract address formatting from
 cmdAttachDiskFormatAddress

Introduce virshAddressFormat with code from cmdAttachDiskFormatAddress
to format the address.

Note that this patch fixes some whitespace inconsistencies in the
formatted addresses.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 tests/virsh-output.out |  42 +++++++-------
 tools/virsh-domain.c   | 122 ++++++++++++++++++++++++++---------------
 2 files changed, 99 insertions(+), 65 deletions(-)

diff --git a/tests/virsh-output.out b/tests/virsh-output.out
index a93492e8f8..6af7600a9c 100644
--- a/tests/virsh-output.out
+++ b/tests/virsh-output.out
@@ -182,7 +182,7 @@ error: No support for whatever in command 'attach-disk'
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='hda'/>
-  <address type='drive' controller='1' bus='2' unit='3' />
+  <address type='drive' controller='1' bus='2' unit='3'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4
@@ -190,7 +190,7 @@ error: No support for whatever in command 'attach-disk'
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='hda'/>
-  <address type='drive' controller='1' bus='2' unit='4' />
+  <address type='drive' controller='1' bus='2' unit='4'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5
@@ -198,7 +198,7 @@ error: No support for whatever in command 'attach-disk'
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='hda'/>
-  <address type='drive' controller='1' bus='2' unit='5' />
+  <address type='drive' controller='1' bus='2' unit='5'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3
@@ -214,7 +214,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='usb' bus='12' port='34' />
+  <address type='usb' bus='12' port='34'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address usb:12.3
@@ -222,7 +222,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='usb' bus='12' port='3' />
+  <address type='usb' bus='12' port='3'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address usb:12:34
@@ -230,7 +230,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='usb' bus='12' port='34' />
+  <address type='usb' bus='12' port='34'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address usb:12.34
@@ -246,7 +246,7 @@ error: expecting an ide:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='3' />
+  <address type='drive' controller='1' bus='2' unit='3'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4
@@ -254,7 +254,7 @@ error: expecting an ide:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='4' />
+  <address type='drive' controller='1' bus='2' unit='4'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5
@@ -262,7 +262,7 @@ error: expecting an ide:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='5' />
+  <address type='drive' controller='1' bus='2' unit='5'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3
@@ -278,7 +278,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='3' />
+  <address type='drive' controller='1' bus='2' unit='3'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4
@@ -286,7 +286,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='4' />
+  <address type='drive' controller='1' bus='2' unit='4'/>
 </disk>
 
 + attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5
@@ -294,7 +294,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='sda'/>
-  <address type='drive' controller='1' bus='2' unit='5' />
+  <address type='drive' controller='1' bus='2' unit='5'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3
@@ -310,7 +310,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78
@@ -318,7 +318,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa
@@ -326,7 +326,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0xaa'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa
@@ -342,7 +342,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78' multifunction='on'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction
@@ -350,7 +350,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78' multifunction='on'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction
@@ -358,7 +358,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa' multifunction='on'/>
+  <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0xaa' multifunction='on'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56
@@ -366,7 +366,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' />
+  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56
@@ -374,7 +374,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' />
+  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/>
 </disk>
 
 + attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56
@@ -382,7 +382,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.
 <disk type='file' device='disk'>
   <source file='/nonexistent/file'/>
   <target dev='vda'/>
-  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' />
+  <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/>
 </disk>
 
 + attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84b8a6fb52..e1a0b9087b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -305,7 +305,7 @@ static const vshCmdOptDef opts_attach_disk[] = {
     {.name = NULL}
 };
 
-enum {
+enum diskAddrType {
     DISK_ADDR_TYPE_INVALID,
     DISK_ADDR_TYPE_PCI,
     DISK_ADDR_TYPE_SCSI,
@@ -320,6 +320,7 @@ struct PCIAddress {
     unsigned int bus;
     unsigned int slot;
     unsigned int function;
+    bool multifunction;
 };
 
 struct SCSIAddress {
@@ -521,7 +522,7 @@ static int str2SATAAddress(const char *str, struct SATAAddress *sataAddr)
  * sata disk address: sata:00.00.0 (controller:bus:unit)
  */
 
-static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr)
+static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr, bool multifunction)
 {
     char *type, *addr;
 
@@ -537,6 +538,7 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr)
 
     if (STREQLEN(type, "pci", addr - type)) {
         diskAddr->type = DISK_ADDR_TYPE_PCI;
+        diskAddr->addr.pci.multifunction = multifunction;
         return str2PCIAddress(addr + 1, &diskAddr->addr.pci);
     } else if (STREQLEN(type, "scsi", addr - type)) {
         diskAddr->type = DISK_ADDR_TYPE_SCSI;
@@ -559,6 +561,71 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr)
 }
 
 
+static void
+virshAddressFormat(virBufferPtr buf,
+                   struct DiskAddress *addr)
+{
+    switch ((enum diskAddrType) addr->type) {
+    case DISK_ADDR_TYPE_PCI:
+        virBufferAsprintf(buf,
+                          "<address type='pci' domain='0x%04x' bus='0x%02x' slot='0x%02x' function='0x%0x'",
+                          addr->addr.pci.domain,
+                          addr->addr.pci.bus,
+                          addr->addr.pci.slot,
+                          addr->addr.pci.function);
+
+        if (addr->addr.pci.multifunction)
+            virBufferAddLit(buf, " multifunction='on'");
+
+        virBufferAddLit(buf, "/>\n");
+        break;
+
+    case DISK_ADDR_TYPE_SCSI:
+        virBufferAsprintf(buf,
+                          "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n",
+                          addr->addr.scsi.controller,
+                          addr->addr.scsi.bus,
+                          addr->addr.scsi.unit);
+        break;
+
+    case DISK_ADDR_TYPE_IDE:
+        virBufferAsprintf(buf,
+                          "<address type='drive' controller='%u' bus='%u' unit='%u'/>\n",
+                          addr->addr.ide.controller,
+                          addr->addr.ide.bus,
+                          addr->addr.ide.unit);
+        break;
+
+    case DISK_ADDR_TYPE_CCW:
+        virBufferAsprintf(buf,
+                          "<address type='ccw' cssid='0x%02x' ssid='0x%01x' devno='0x%04x'/>\n",
+                          addr->addr.ccw.cssid,
+                          addr->addr.ccw.ssid,
+                          addr->addr.ccw.devno);
+        break;
+
+    case DISK_ADDR_TYPE_USB:
+        virBufferAsprintf(buf,
+                          "<address type='usb' bus='%u' port='%u'/>\n",
+                          addr->addr.usb.bus,
+                          addr->addr.usb.port);
+        break;
+
+    case DISK_ADDR_TYPE_SATA:
+        virBufferAsprintf(buf,
+                          "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n",
+                          addr->addr.sata.controller,
+                          addr->addr.sata.bus,
+                          addr->addr.sata.unit);
+        break;
+
+    case DISK_ADDR_TYPE_INVALID:
+    default:
+        return;
+    }
+}
+
+
 static int
 cmdAttachDiskFormatAddress(vshControl *ctl,
                            virBufferPtr buf,
@@ -568,67 +635,34 @@ cmdAttachDiskFormatAddress(vshControl *ctl,
 {
     struct DiskAddress diskAddr;
 
-    if (str2DiskAddress(straddr, &diskAddr) != 0) {
+    if (str2DiskAddress(straddr, &diskAddr, multifunction) != 0) {
         vshError(ctl, _("Invalid address."));
         return -1;
     }
 
     if (STRPREFIX((const char *)target, "vd")) {
-        if (diskAddr.type == DISK_ADDR_TYPE_PCI) {
-            virBufferAsprintf(buf,
-                              "<address type='pci' domain='0x%04x'"
-                              " bus ='0x%02x' slot='0x%02x' function='0x%0x'",
-                              diskAddr.addr.pci.domain, diskAddr.addr.pci.bus,
-                              diskAddr.addr.pci.slot, diskAddr.addr.pci.function);
-            if (multifunction)
-                virBufferAddLit(buf, " multifunction='on'");
-            virBufferAddLit(buf, "/>\n");
-        } else if (diskAddr.type == DISK_ADDR_TYPE_CCW) {
-            virBufferAsprintf(buf,
-                              "<address type='ccw' cssid='0x%02x'"
-                              " ssid='0x%01x' devno='0x%04x' />\n",
-                              diskAddr.addr.ccw.cssid, diskAddr.addr.ccw.ssid,
-                              diskAddr.addr.ccw.devno);
-        } else {
+        if (diskAddr.type != DISK_ADDR_TYPE_PCI &&
+            diskAddr.type != DISK_ADDR_TYPE_CCW) {
             vshError(ctl, "%s",
                      _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address."));
             return -1;
         }
     } else if (STRPREFIX((const char *)target, "sd")) {
-        if (diskAddr.type == DISK_ADDR_TYPE_SCSI) {
-            virBufferAsprintf(buf,
-                              "<address type='drive' controller='%u'"
-                              " bus='%u' unit='%llu' />\n",
-                              diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus,
-                              diskAddr.addr.scsi.unit);
-        } else if (diskAddr.type == DISK_ADDR_TYPE_USB) {
-            virBufferAsprintf(buf,
-                              "<address type='usb' bus='%u' port='%u' />\n",
-                              diskAddr.addr.usb.bus, diskAddr.addr.usb.port);
-        } else if (diskAddr.type == DISK_ADDR_TYPE_SATA) {
-            virBufferAsprintf(buf,
-                              "<address type='drive' controller='%u'"
-                              " bus='%u' unit='%llu' />\n",
-                              diskAddr.addr.sata.controller, diskAddr.addr.sata.bus,
-                              diskAddr.addr.sata.unit);
-        } else {
+        if (diskAddr.type != DISK_ADDR_TYPE_SCSI &&
+            diskAddr.type != DISK_ADDR_TYPE_USB &&
+            diskAddr.type != DISK_ADDR_TYPE_SATA) {
             vshError(ctl, "%s",
                      _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address."));
             return -1;
         }
     } else if (STRPREFIX((const char *)target, "hd")) {
-        if (diskAddr.type == DISK_ADDR_TYPE_IDE) {
-            virBufferAsprintf(buf,
-                              "<address type='drive' controller='%u'"
-                              " bus='%u' unit='%u' />\n",
-                              diskAddr.addr.ide.controller, diskAddr.addr.ide.bus,
-                              diskAddr.addr.ide.unit);
-        } else {
+        if (diskAddr.type != DISK_ADDR_TYPE_IDE) {
             vshError(ctl, "%s", _("expecting an ide:00.00.00 address."));
             return -1;
         }
     }
 
+    virshAddressFormat(buf, &diskAddr);
     return 0;
 }
 
@@ -1006,7 +1040,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
         break;
     case VIR_DOMAIN_NET_TYPE_HOSTDEV:
     {
-        struct PCIAddress pciAddr = {0, 0, 0, 0};
+        struct PCIAddress pciAddr = {0, 0, 0, 0, false};
 
         if (str2PCIAddress(source, &pciAddr) < 0) {
             vshError(ctl, _("cannot parse pci address '%s' for network "