August 2020 - Devel - libvirt List Archives

[PATCH] Xen: Improve parsing of PCI addresses in config converter
by Jim Fehlig 21 Aug '20

21 Aug '20

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> --- 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"); -- 2.28.0

3 2

qemu modularization of qemu-5.1 vs libvirt domcapabilities cache?
by Mark Mielke 21 Aug '20

21 Aug '20

Hi all: In testing qemu-5.1rc2 on my Fedora 32 home system, I found that the Fedora rawhide package has broken out both the QXL display device and the USB redirect device into separate RPM modules: qemu-device-display-qxl.x86_64 2:5.1.0-0.1.rc2.fc32 @@commandline qemu-device-usb-redirect.x86_64 2:5.1.0-0.1.rc2.fc32 @@commandline The upgrade from 5.0.0 to 5.1.0 does not treat these sub-packages as mandatory packages, therefore a straight upgrade of packages causes these domcapabilities to disappear. If the user tries to use libvirt after this, then existing domains using QXL display device or USB redirect device will fail to start. If the user then investigates and realizes that they now need to install the above packages separately, they find that qemu-kvm recognizes the modules right away, but libvirt does not. This looks to be due to the libvirt domcapabilities cache? The domcapabilities cache will be automatically updated only under certain conditions such as the qemu binary ctime changing - but that isn't triggering any action here? With the devices broken out into modules, such as the Fedora rawhide RPM .spec file is proposing, this allows the devices to be individually installed or uninstalled at any time, and causes libvirt domcapabilities cache to be out-of-date. I was able to sometimes see it work by downgrading to qemu-5.0, upgrading to qemu-5.1rc2, and installing the device packages prior to calling "virsh domcapabilities" (or otherwise using them). I was also able to do the same by removing the libvirt cache files and restarted libvirtd service. Both of these are pretty non-obvious actions for a user. I'm wondering how to codify this when I use it for real on a production system. The upgrade path here seems unreliable, especially given that libvirt domcapabilities cache may even persist across reboots? This means I need to be very careful about the procedure to upgrade, and also I need to make sure to never install or remove any of the device packages without checking the procedure. Ouch. :-( Thoughts? -- Mark Mielke <mark.mielke(a)gmail.com>

7 23

[PATCH] lxc: Add TPM passthrough option for LXC driver
by Julio Faracco 20 Aug '20

20 Aug '20

There is no support to use TPM for passthrough for LXC libvirt driver this commit adds the option to use host TPM inside containers. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> --- src/lxc/lxc_cgroup.c | 27 +++++++++++++++++++ src/lxc/lxc_controller.c | 56 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index d13f2adde5..955d2b4fc1 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -374,6 +374,33 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, return -1; } + for (i = 0; i < def->ntpms; i++) { + virDomainTPMDefPtr tpm = def->tpms[i]; + const char *dev = NULL; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + dev = "/dev/tpm0"; + break; + } + + if (!dev) + continue; + + if (!virFileExists(dev)) { + VIR_DEBUG("Ignoring non-existent device %s", dev); + continue; + } + + if (virCgroupAllowDevicePath(cgroup, dev, + VIR_CGROUP_DEVICE_READ, + false) < 0) + return -1; + } + VIR_DEBUG("Device ACL setup complete"); return 0; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index ae6b737b60..70ca773bbf 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1644,6 +1644,59 @@ virLXCControllerSetupHostdevSubsysUSB(virDomainDefPtr vmDef, } +static int +virLXCControllerSetupTPM(virLXCControllerPtr ctrl) +{ + virDomainDefPtr def = ctrl->def; + size_t i; + + for (i = 0; i < def->ntpms; i++) { + virDomainTPMDefPtr tpm = def->tpms[i]; + g_autofree char *path = NULL; + const char *tpm_dev = NULL; + struct stat sb; + dev_t dev; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + case VIR_DOMAIN_TPM_TYPE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported timer type (name) '%s'"), + virDomainTPMBackendTypeToString(tpm->type)); + return -1; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + tpm_dev = "/dev/tpm0"; + path = g_strdup_printf("/%s/%s.dev/%s", LXC_STATE_DIR, + def->name, "/rtc"); + break; + } + + if (!tpm_dev) + continue; + + if (stat(tpm_dev, &sb) < 0) { + virReportSystemError(errno, _("Unable to access %s"), + tpm_dev); + return -1; + } + + dev = makedev(major(sb.st_rdev), minor(sb.st_rdev)); + if (mknod(path, S_IFCHR, dev) < 0 || + chmod(path, sb.st_mode)) { + virReportSystemError(errno, + _("Failed to make device %s"), + path); + return -1; + } + + if (lxcContainerChown(def, path) < 0) + return -1; + } + + return 0; +} + + static int virLXCControllerSetupHostdevCapsStorage(virDomainDefPtr vmDef, virDomainHostdevDefPtr def, @@ -2358,6 +2411,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupAllHostdevs(ctrl) < 0) goto cleanup; + if (virLXCControllerSetupTPM(ctrl) < 0) + goto cleanup; + if (virLXCControllerSetupFuse(ctrl) < 0) goto cleanup; -- 2.25.1

2 1

[PATCH 0/2] Fix a few deadlocks with musl libc
by Natanael Copa 19 Aug '20

19 Aug '20

Fix a couple of deadlocks due to use of async-unsafe calls (malloc/free) after fork() and before exec(). They don't fix all theoretical problems but at least they make libvirt usable again with musl 1.2 on Alpine Linux. Natanael Copa (2): util: avoid free() when reset log after fork util: command: improve generic mass close of fds src/util/vircommand.c | 80 ++++++++++++++++++++++++++++++++----------- src/util/virlog.c | 44 ++++++++++++++++++------ src/util/virlog.h | 1 + 3 files changed, 95 insertions(+), 30 deletions(-) -- 2.28.0

2 5

[PATCH] virdevmapper: Ignore all errors when opening /dev/mapper/control
by Michal Privoznik 19 Aug '20

19 Aug '20

So far, only ENOENT is ignored (to deal with kernels without devmapper). However, as reported on the list, under certain scenarios a different error can occur. For instance, when libvirt is running inside a container which doesn't have permissions to talk to the devmapper. If this is the case, then open() returns -1 and sets errno=EPERM. Assuming that multipath devices are fairly narrow use case and using them in a restricted container is even more narrow the best fix seems to be to ignore all open errors BUT produce a warning on failure. To avoid flooding logs with warnings on kernels without devmapper the level is reduced to a plain debug message. Reported-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virdevmapper.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c index 3cc399f382..7c89c2a952 100644 --- a/src/util/virdevmapper.c +++ b/src/util/virdevmapper.c @@ -35,9 +35,12 @@ # include "viralloc.h" # include "virstring.h" # include "virfile.h" +# include "virlog.h" # define VIR_FROM_THIS VIR_FROM_STORAGE +VIR_LOG_INIT("util.virdevmapper"); + # define PROC_DEVICES "/proc/devices" # define DM_NAME "device-mapper" # define DEV_DM_DIR "/dev/" DM_DIR @@ -130,11 +133,15 @@ virDMOpen(void) memset(&dm, 0, sizeof(dm)); if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) { - if (errno == ENOENT) - return -2; - - virReportSystemError(errno, _("Unable to open %s"), CONTROL_PATH); - return -1; + /* We can't talk to devmapper. Produce a warning and let + * the caller decide what to do next. */ + if (errno == ENOENT) { + VIR_DEBUG("device mapper not available"); + } else { + VIR_WARN("unable to open %s: %s", + CONTROL_PATH, g_strerror(errno)); + } + return -2; } if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { @@ -309,9 +316,9 @@ virDevMapperGetTargets(const char *path, if ((controlFD = virDMOpen()) < 0) { if (controlFD == -2) { - /* The CONTROL_PATH doesn't exist. Probably the - * module isn't loaded, yet. Don't error out, just - * exit. */ + /* The CONTROL_PATH doesn't exist or is unusable. + * Probably the module isn't loaded, yet. Don't error + * out, just exit. */ return 0; } -- 2.26.2

2 1

[libvirt PATCH 0/2] meson: AppArmor fixes
by Andrea Bolognani 19 Aug '20

19 Aug '20

Found while updating the Debian package for libvirt to a snapshot taken from master. Possibly more to come. Andrea Bolognani (2): meson: Set WITH_APPARMOR_PROFILES meson: Don't hardcode /etc in APPARMOR_DIR meson.build | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.26.2

2 3

[PATCH v2 0/2] Deal with kernels without DM support
by Michal Privoznik 19 Aug '20

19 Aug '20

v2 of: https://www.redhat.com/archives/libvir-list/2020-August/msg00489.html diff to v1: - After discussion to v1 I've decided to not cache DM major number and thus the patches looks a bit different. Michal Prívozník (2): virdevmapper: Don't cache device-mapper major virdevmapper: Handle kernel without device-mapper support src/util/virdevmapper.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) -- 2.26.2

3 8

[libvirt PATCH 0/2] meson: Introduce rpath option
by Andrea Bolognani 19 Aug '20

19 Aug '20

See 1/2 for details. Andrea Bolognani (2): meson: Introduce rpath option spec: Disable RPATH libvirt.spec.in | 1 + meson.build | 7 + meson_options.txt | 1 + src/meson.build | 485 +++++++++++++++++++++++++++++++--------------- tools/meson.build | 332 ++++++++++++++++++++----------- 5 files changed, 567 insertions(+), 259 deletions(-) -- 2.26.2

2 4

libvirt 6.6.0 tarball breaks on Homebrew/MacOS
by Scott Shambarger 19 Aug '20

19 Aug '20

The latest release at https://libvirt.org/sources/libvirt-6.6.0.tar.xz includes a configure script that breaks Homebrew (and other builds on MacOS/Darwin). The breaking change is related to a new version of the file m4/libtool.m4; line 2648 was changed to: shrext_cmds='`test .$module = .yes && echo .bundle || echo .dylib`' However, src/driver.c loads modules with ".so" extensions, at line 56: if (!(modfile = virFileFindResourceFull(name, "libvirt_driver_", ".so", The released tarball for 6.5.0 had the same shrext_cmds as upstream libtool: shrext_cmds='`test .$module = .yes && echo .so || echo .dylib`' Not sure where to submit the issue (as the m4 files aren't in libvirt source control)... it appears to only affect the distribution tarball. Perhaps the tarball could be re-released with autoconf from the official upstream libtool? (2.4.6) Thanks, Scott PS. MacPorts is unaffected as it runs autogen.sh... but that shouldn't be necessary with a distribution tarball :)

3 2

[libvirt PATCH 0/2] meson: Small style fixes
by Andrea Bolognani 19 Aug '20

19 Aug '20

All pushed as trivial. Andrea Bolognani (2): meson: Fix indentation meson: Don't use spaces after parentheses meson.build | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) -- 2.26.2

1 2