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 <phrdina(a)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 {