Re: [libvirt] [PATCH 1/4] virsh-domain: use correct base for virStrToLong_ui

[Laine Stump]

On 10/21/2015 08:22 AM, Pavel Hrdina wrote:
While parsing device addresses we should use correct base and don't
count on auto-detect.  For example, PCI address uses hex numbers, but
each number starting with 0 will be auto-detected as octal number and
that's wrong.  Another wrong use-case is for PCI address if for example
bus is 10, than it's incorrectly parsed as decimal number.
PCI and CCW addresses have all values as hex numbers, IDE and SCSI
addresses are in decimal numbers.

I've seen examples for PCI that use decimal a number for the slot (which 
is the one item that's likely to have a value > 7 unless you have a 
system with a whole bunch of PCI controllers)[*], and those that use hex 
numbers always prefix the number with "0x". libvirt itself always 
prefixes the domain, bus, and slot with 0x, so an auto-detected base 
will always get those right.

So I I think the existing code is correct, and don't see an upside to 
making this restriction/invisible change in semantics, and it could 
potentially lead to incorrect results if someone is thinking that 
they're entering decimal numbers.

[*] There was one particular document that even went to the trouble of 
explaining how to convert the hex value of slot into a decimal number to 
use in the libvirt config!


Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
---
  tools/virsh-domain.c | 30 +++++++++++++++---------------
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4191548..e8503ec 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -455,19 +455,19 @@ static int str2PCIAddress(const char *str, struct 
PCIAddress *pciAddr)
  
      domain = (char *)str;
  
-    if (virStrToLong_ui(domain, &bus, 0, &pciAddr->domain) != 0)
+    if (virStrToLong_ui(domain, &bus, 16, &pciAddr->domain) != 0)
          return -1;
  
      bus++;
-    if (virStrToLong_ui(bus, &slot, 0, &pciAddr->bus) != 0)
+    if (virStrToLong_ui(bus, &slot, 16, &pciAddr->bus) != 0)
          return -1;
  
      slot++;
-    if (virStrToLong_ui(slot, &function, 0, &pciAddr->slot) != 0)
+    if (virStrToLong_ui(slot, &function, 16, &pciAddr->slot) != 0)
          return -1;
  
      function++;
-    if (virStrToLong_ui(function, NULL, 0, &pciAddr->function) != 0)
+    if (virStrToLong_ui(function, NULL, 16, &pciAddr->function) != 0)
          return -1;
  
      return 0;
@@ -484,15 +484,15 @@ static int str2SCSIAddress(const char *str, struct 
SCSIAddress *scsiAddr)
  
      controller = (char *)str;
  
-    if (virStrToLong_uip(controller, &bus, 0, &scsiAddr->controller) != 0)
+    if (virStrToLong_uip(controller, &bus, 10, &scsiAddr->controller) != 0)
          return -1;
  
      bus++;
-    if (virStrToLong_uip(bus, &unit, 0, &scsiAddr->bus) != 0)
+    if (virStrToLong_uip(bus, &unit, 10, &scsiAddr->bus) != 0)
          return -1;
  
      unit++;
-    if (virStrToLong_ullp(unit, NULL, 0, &scsiAddr->unit) != 0)
+    if (virStrToLong_ullp(unit, NULL, 10, &scsiAddr->unit) != 0)
          return -1;
  
      return 0;
@@ -509,15 +509,15 @@ static int str2IDEAddress(const char *str, struct 
IDEAddress *ideAddr)
  
      controller = (char *)str;
  
-    if (virStrToLong_ui(controller, &bus, 0, &ideAddr->controller) != 0)
+    if (virStrToLong_ui(controller, &bus, 10, &ideAddr->controller) != 0)
          return -1;
  
      bus++;
-    if (virStrToLong_ui(bus, &unit, 0, &ideAddr->bus) != 0)
+    if (virStrToLong_ui(bus, &unit, 10, &ideAddr->bus) != 0)
          return -1;
  
      unit++;
-    if (virStrToLong_ui(unit, NULL, 0, &ideAddr->unit) != 0)
+    if (virStrToLong_ui(unit, NULL, 10, &ideAddr->unit) != 0)
          return -1;
  
      return 0;
@@ -534,15 +534,15 @@ static int str2CCWAddress(const char *str, struct 
CCWAddress *ccwAddr)
  
      cssid = (char *)str;
  
-    if (virStrToLong_ui(cssid, &ssid, 0, &ccwAddr->cssid) != 0)
+    if (virStrToLong_ui(cssid, &ssid, 16, &ccwAddr->cssid) != 0)
          return -1;
  
      ssid++;
-    if (virStrToLong_ui(ssid, &devno, 0, &ccwAddr->ssid) != 0)
+    if (virStrToLong_ui(ssid, &devno, 16, &ccwAddr->ssid) != 0)
          return -1;
  
      devno++;
-    if (virStrToLong_ui(devno, NULL, 0, &ccwAddr->devno) != 0)
+    if (virStrToLong_ui(devno, NULL, 16, &ccwAddr->devno) != 0)
          return -1;
  
      return 0;
@@ -739,8 +739,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
          } else if (STRPREFIX((const char *)target, "hd")) {
              if (diskAddr.type == DISK_ADDR_TYPE_IDE) {
                  virBufferAsprintf(&buf,
-                                  "<address type='drive' controller='%d'"
-                                  " bus='%d' unit='%d' />\n",
+                                  "<address type='drive' controller='%u'"
+                                  " bus='%u' unit='%u' />\n",
                                    diskAddr.addr.ide.controller, 
diskAddr.addr.ide.bus,
                                    diskAddr.addr.ide.unit);
              } else {

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list