Re: [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 dddd: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 <jfehlig(a)suse.com&gt; --- 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=['0000:00:1b.0','0000: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','0000:00:13.0'] */ + if (!(tokens = virStringSplitCount(entry, ":", 3, &ntokens))) 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, &domain) < 0) + return NULL; + nexttoken++; } - key = nextkey + 1; - if (!(nextkey = strchr(key, '.'))) + /* bus */ + if (virStrToLong_i(tokens[nexttoken], NULL, 16, &bus) < 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, &domainID) < 0) - return NULL; - if (virStrToLong_i(bus, NULL, 16, &busID) < 0) - return NULL; - if (virStrToLong_i(slot, NULL, 16, &slotID) < 0) + *funcstr = '\0'; + funcstr++; + if (virStrToLong_i(slotstr, NULL, 16, &slot) < 0) return NULL; - if (virStrToLong_i(func, NULL, 16, &funcID) < 0) + if (virStrToLong_i(funcstr, NULL, 16, &func) < 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 0000000000..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 = [ "mac=00:16:3e:0a:7b:39,bridge=xenbr0,script=vif-bridge,model=e1000" ] +pci = [ "0001:0c:1b.2", "01:13.0" ] +parallel = "none" +serial = "pty" +builder = "hvm" +kernel = "/usr/lib/xen/boot/hvmloader" +boot = "c" +disk = [ "phy:/dev/sda8,hda,w", ",hdc:cdrom,r" ] diff --git a/tests/xmconfigdata/test-pci-dev-syntax.xml b/tests/xmconfigdata/test-pci-dev-syntax.xml new file mode 100644 index 0000000000..5d5d29c61c --- /dev/null +++ b/tests/xmconfigdata/test-pci-dev-syntax.xml @@ -0,0 +1,71 @@ +<domain type='xen'> + <name>test</name> + <uuid>cc2315e7-d26a-307a-438c-6d188ec4c09c</uuid> + <memory unit='KiB'>391168</memory> + <currentMemory unit='KiB'>358400</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>destroy</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/sda8'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <driver name='phy' type='raw'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='xenbus' index='0'/> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:0a:7b:39'/> + <source bridge='xenbr0'/> + <script path='vif-bridge'/> + <model type='e1000'/> + </interface> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='xen'/> + <source> + <address domain='0x0001' bus='0x0c' slot='0x1b' function='0x2'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='xen'/> + <source> + <address domain='0x0000' bus='0x01' slot='0x13' function='0x0'/> + </source> + </hostdev> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index e4a2b39384..43949fe3b5 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -216,6 +216,7 @@ mymain(void) DO_TEST("escape-paths"); DO_TEST("no-source-cdrom"); DO_TEST("pci-devs"); + DO_TEST_FORMAT("pci-dev-syntax"); DO_TEST("disk-drv-blktap-raw"); DO_TEST("disk-drv-blktap2-raw");