Re: [PATCH] Xen: Improve parsing of PCI addresses in config converter
On 8/10/20 8:14 PM, Jim Fehlig wrote: There was a report on libvirt-users [1] about the domxml to/from native converter in the Xen driver not handling PCI addresses without a domain specification. This patch improves parsing of PCI addresses in the converter and allows PCI addresses with only bb:ss.f. xl.cfg(5) also allows either the :bb:ss.f or bb:ss.f format. A test has been added to check the conversion from xl.cfg to domXML. [1] https://www.redhat.com/archives/libvirt-users/2020-August/msg00040.html Signed-off-by: Jim Fehlig --- Reviewed-by: Daniel Henrique Barboza
Re: [PATCH] Xen: Improve parsing of PCI addresses in config converter
thx! --- -- Greetz Am 11.08.2020 01:14, schrieb Jim Fehlig: There was a report on libvirt-users [1] about the domxml to/from native converter in the Xen driver not handling PCI addresses without a domain specification. This patch improves parsing of PCI addresses in the converter and allows PCI addresses with only bb:ss.f. xl.cfg(5) also allows either the :bb:ss.f or bb:ss.f format. A test has been added to check the conversion from xl.cfg to domXML. [1] https://www.redhat.com/archives/libvirt-users/2020-August/msg00040.html Signed-off-by: Jim Fehlig --- src/libxl/xen_common.c | 81 +- tests/xmconfigdata/test-pci-dev-syntax.cfg | 26 +++ tests/xmconfigdata/test-pci-dev-syntax.xml | 71 +++ tests/xmconfigtest.c | 1 + 4 files changed, 129 insertions(+), 50 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 75fe7e0644..73ce412fe7 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -372,63 +372,44 @@ static virDomainHostdevDefPtr xenParsePCI(char *entry) { virDomainHostdevDefPtr hostdev = NULL; -char domain[5]; -char bus[3]; -char slot[3]; -char func[2]; -char *key, *nextkey; -int domainID; -int busID; -int slotID; -int funcID; +VIR_AUTOSTRINGLIST tokens = NULL; +size_t ntokens = 0; +size_t nexttoken = 0; +char *slotstr; +char *funcstr; +int domain = 0x0; +int bus; +int slot; +int func; -domain[0] = bus[0] = slot[0] = func[0] = '\0'; - -/* pci=[':00:1b.0',':00:13.0'] */ -if (!(key = entry)) -return NULL; -if (!(nextkey = strchr(key, ':'))) -return NULL; -if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Domain %s too big for destination"), key); +/* pci=['00:1b.0',':00:13.0'] */ +if (!(tokens = virStringSplitCount(entry, ":", 3, ))) return NULL; -} -key = nextkey + 1; -if (!(nextkey = strchr(key, ':'))) -return NULL; -if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Bus %s too big for destination"), key); -return NULL; +/* domain */ +if (ntokens == 3) { +if (virStrToLong_i(tokens[nexttoken], NULL, 16, ) < 0) +return NULL; +nexttoken++; } -key = nextkey + 1; -if (!(nextkey = strchr(key, '.'))) +/* bus */ +if (virStrToLong_i(tokens[nexttoken], NULL, 16, ) < 0) return NULL; -if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Slot %s too big for destination"), key); -return NULL; -} +nexttoken++; -key = nextkey + 1; -if (strlen(key) != 1) -return NULL; -if (virStrncpy(func, key, 1, sizeof(func)) < 0) { +/* slot and function */ +slotstr = tokens[nexttoken]; +if (!(funcstr = strchr(slotstr, '.'))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Function %s too big for destination"), key); + _("Malformed PCI address %s"), slotstr); return NULL; } - -if (virStrToLong_i(domain, NULL, 16, ) < 0) -return NULL; -if (virStrToLong_i(bus, NULL, 16, ) < 0) -return NULL; -if (virStrToLong_i(slot, NULL, 16, ) < 0) +*funcstr = '\0'; +funcstr++; +if (virStrToLong_i(slotstr, NULL, 16, ) < 0) return NULL; -if (virStrToLong_i(func, NULL, 16, ) < 0) +if (virStrToLong_i(funcstr, NULL, 16, ) < 0) return NULL; if (!(hostdev = virDomainHostdevDefNew())) @@ -436,10 +417,10 @@ xenParsePCI(char *entry) hostdev->managed = false; hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; -hostdev->source.subsys.u.pci.addr.domain = domainID; -hostdev->source.subsys.u.pci.addr.bus = busID; -hostdev->source.subsys.u.pci.addr.slot = slotID; -hostdev->source.subsys.u.pci.addr.function = funcID; +hostdev->source.subsys.u.pci.addr.domain = domain; +hostdev->source.subsys.u.pci.addr.bus = bus; +hostdev->source.subsys.u.pci.addr.slot = slot; +hostdev->source.subsys.u.pci.addr.function = func; return hostdev; } diff --git a/tests/xmconfigdata/test-pci-dev-syntax.cfg b/tests/xmconfigdata/test-pci-dev-syntax.cfg new file mode 100644 index 00..e0f00d9bd7 --- /dev/null +++ b/tests/xmconfigdata/test-pci-dev-syntax.cfg @@ -0,0 +1,26 @@ +name = "test" +uuid = "cc2315e7-d26a-307a-438c-6d188ec4c09c" +maxmem = 382 +memory = 350 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "destroy" +on_crash = "destroy" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused
[PATCH] Xen: Improve parsing of PCI addresses in config converter
There was a report on libvirt-users [1] about the domxml to/from native converter in the Xen driver not handling PCI addresses without a domain specification. This patch improves parsing of PCI addresses in the converter and allows PCI addresses with only bb:ss.f. xl.cfg(5) also allows either the :bb:ss.f or bb:ss.f format. A test has been added to check the conversion from xl.cfg to domXML. [1] https://www.redhat.com/archives/libvirt-users/2020-August/msg00040.html Signed-off-by: Jim Fehlig --- src/libxl/xen_common.c | 81 +- tests/xmconfigdata/test-pci-dev-syntax.cfg | 26 +++ tests/xmconfigdata/test-pci-dev-syntax.xml | 71 +++ tests/xmconfigtest.c | 1 + 4 files changed, 129 insertions(+), 50 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 75fe7e0644..73ce412fe7 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -372,63 +372,44 @@ static virDomainHostdevDefPtr xenParsePCI(char *entry) { virDomainHostdevDefPtr hostdev = NULL; -char domain[5]; -char bus[3]; -char slot[3]; -char func[2]; -char *key, *nextkey; -int domainID; -int busID; -int slotID; -int funcID; +VIR_AUTOSTRINGLIST tokens = NULL; +size_t ntokens = 0; +size_t nexttoken = 0; +char *slotstr; +char *funcstr; +int domain = 0x0; +int bus; +int slot; +int func; -domain[0] = bus[0] = slot[0] = func[0] = '\0'; - -/* pci=[':00:1b.0',':00:13.0'] */ -if (!(key = entry)) -return NULL; -if (!(nextkey = strchr(key, ':'))) -return NULL; -if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Domain %s too big for destination"), key); +/* pci=['00:1b.0',':00:13.0'] */ +if (!(tokens = virStringSplitCount(entry, ":", 3, ))) return NULL; -} -key = nextkey + 1; -if (!(nextkey = strchr(key, ':'))) -return NULL; -if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Bus %s too big for destination"), key); -return NULL; +/* domain */ +if (ntokens == 3) { +if (virStrToLong_i(tokens[nexttoken], NULL, 16, ) < 0) +return NULL; +nexttoken++; } -key = nextkey + 1; -if (!(nextkey = strchr(key, '.'))) +/* bus */ +if (virStrToLong_i(tokens[nexttoken], NULL, 16, ) < 0) return NULL; -if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Slot %s too big for destination"), key); -return NULL; -} +nexttoken++; -key = nextkey + 1; -if (strlen(key) != 1) -return NULL; -if (virStrncpy(func, key, 1, sizeof(func)) < 0) { +/* slot and function */ +slotstr = tokens[nexttoken]; +if (!(funcstr = strchr(slotstr, '.'))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Function %s too big for destination"), key); + _("Malformed PCI address %s"), slotstr); return NULL; } - -if (virStrToLong_i(domain, NULL, 16, ) < 0) -return NULL; -if (virStrToLong_i(bus, NULL, 16, ) < 0) -return NULL; -if (virStrToLong_i(slot, NULL, 16, ) < 0) +*funcstr = '\0'; +funcstr++; +if (virStrToLong_i(slotstr, NULL, 16, ) < 0) return NULL; -if (virStrToLong_i(func, NULL, 16, ) < 0) +if (virStrToLong_i(funcstr, NULL, 16, ) < 0) return NULL; if (!(hostdev = virDomainHostdevDefNew())) @@ -436,10 +417,10 @@ xenParsePCI(char *entry) hostdev->managed = false; hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; -hostdev->source.subsys.u.pci.addr.domain = domainID; -hostdev->source.subsys.u.pci.addr.bus = busID; -hostdev->source.subsys.u.pci.addr.slot = slotID; -hostdev->source.subsys.u.pci.addr.function = funcID; +hostdev->source.subsys.u.pci.addr.domain = domain; +hostdev->source.subsys.u.pci.addr.bus = bus; +hostdev->source.subsys.u.pci.addr.slot = slot; +hostdev->source.subsys.u.pci.addr.function = func; return hostdev; } diff --git a/tests/xmconfigdata/test-pci-dev-syntax.cfg b/tests/xmconfigdata/test-pci-dev-syntax.cfg new file mode 100644 index 00..e0f00d9bd7 --- /dev/null +++ b/tests/xmconfigdata/test-pci-dev-syntax.cfg @@ -0,0 +1,26 @@ +name = "test" +uuid = "cc2315e7-d26a-307a-438c-6d188ec4c09c" +maxmem = 382 +memory = 350 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "destroy" +on_crash = "destroy" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vif = [