[PATCH] Xen: Improve parsing of PCI addresses in config converter
Christoph
mangel at gmx.de
Tue Aug 11 10:00:18 UTC 2020
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 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 at suse.com>
> ---
> 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");
More information about the libvir-list
mailing list