Re: [PATCH] Xen: Improve parsing of PCI addresses in config converter

2020-08-20 Thread Daniel Henrique Barboza




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

2020-08-17 Thread Christoph

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

2020-08-10 Thread 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 = 1
+vif = [