[libvirt] [PATCH 1/4] virsh-domain: use correct base for virStrToLong_ui
Laine Stump
laine at laine.org
Thu Oct 22 15:24:02 UTC 2015
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 at 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 {
More information about the libvir-list
mailing list