July 2021 - Devel - libvirt List Archives

Re: [PATCH 0/2] Remove deprecated qemu-img backing file without format
by Eric Blake 08 Jul '21

08 Jul '21

On Mon, May 03, 2021 at 04:35:58PM -0500, Eric Blake wrote: > We've gone enough release cycles without noticeable pushback on our > intentions, so time to make it harder to create images that can form a > security hole due to a need for format probing rather than an explicit > format. > > Eric Blake (2): > qcow2: Prohibit backing file changes in 'qemu-img amend' > qemu-img: Require -F with -b backing image Ping. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

2 1

Re: [PATCH 4/4] x86/tsx: Add cmdline tsx=fake to not clear CPUID bits RTM and HLE
by Eduardo Habkost 08 Jul '21

08 Jul '21

CCing libvir-list, Jiri Denemark, Michal Privoznik, so they are aware that the definition of "supported CPU features" will probably become a bit more complex in the future. On Tue, Jul 6, 2021 at 5:58 PM Paolo Bonzini <pbonzini(a)redhat.com> wrote: > > On 06/07/21 23:33, Eduardo Habkost wrote: > > On Tue, Jul 6, 2021 at 5:05 PM Paolo Bonzini <pbonzini(a)redhat.com> wrote: > >> It's a bit tricky, because HLE and RTM won't really behave well. An old > >> guest that sees RTM=1 might end up retrying and aborting transactions > >> too much. So I'm not sure that a QEMU "-cpu host" guest should have HLE > >> and RTM enabled. > > > > Is the purpose of GET_SUPPORTED_CPUID to return what is supported by > > KVM, or to return what "-cpu host" should enable by default? They are > > conflicting requirements in this case. > > In theory there is GET_EMULATED_CPUID for the former, so it should be > the latter. In practice neither QEMU nor Libvirt use it; maybe now we > have a good reason to add it, but note that userspace could also check > host RTM_ALWAYS_ABORT. > > > Returning HLE=1,RTM=1 in GET_SUPPORTED_CPUID makes existing userspace > > take bad decisions until it's updated. > > > > Returning HLE=0,RTM=0 in GET_SUPPORTED_CPUID prevents existing > > userspace from resuming existing VMs (despite being technically > > possible). > > > > The first option has an easy workaround that doesn't require a > > software update (disabling HLE/RTM in the VM configuration). The > > second option doesn't have a workaround. I'm inclined towards the > > first option. > > The default has already been tsx=off for a while though, so checking > either GET_EMULATED_CPUID or host RTM_ALWAYS_ABORT in userspace might > also be feasible for those that are still on tsx=on. This sounds like a perfect use case for GET_EMULATED_CPUID. My only concern is breaking existing userspace. But if this was already broken for a few kernel releases due to tsx=off being the default, maybe GET_EMULATED_CPUID will be a reasonable approach. -- Eduardo

3 5

[PATCH] virresctrl: Fix updating the mask for a cache resource
by Vinayak Kale 07 Jul '21

07 Jul '21

In 'virResctrlAllocUpdateMask', mask is updated only if 'previous mask' is NULL. By default, the bitmask for a cache resource for a VM is initialized with 'default-resctrl-group' bitmask. So the 'previous mask' would not be NULL and mask won't get updated if cachetune is configured for a VM. This causes libvirt to use same bitmask as 'default-resctrl-group' bitmask for a cache resource for a VM. This patch fixes the issue. Fixes: d8a354954aba9cd45ab0317915a0a2be27c04767 Signed-off-by: Vinayak Kale <vkale(a)nvidia.com> --- src/util/virresctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 35e92022db..a7d36f492c 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1104,8 +1104,10 @@ virResctrlAllocUpdateMask(virResctrlAlloc *alloc, VIR_EXPAND_N(a_type->masks, a_type->nmasks, cache - a_type->nmasks + 1); - if (!a_type->masks[cache]) - a_type->masks[cache] = virBitmapNewCopy(mask); + if (a_type->masks[cache]) + virBitmapFree(a_type->masks[cache]); + + a_type->masks[cache] = virBitmapNewCopy(mask); return 0; } -- 2.17.1

3 3

vfio-user in libvirt
by John Levon 07 Jul '21

07 Jul '21

Hi, has anyone considered support for vfio-user devices in libvirt? I was thinking of something like: <controller type='vfio-user' index='0'> <socket type="path">/path/to/vfio-user.sock</socket> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </controller> The main reason why this is useful is to get a PCI slot assigned for PCI-based vfio-user. Thoughts? thanks john

1 0

[PATCH 0/6] libxl: Misc cleanups and improvements
by Jim Fehlig 07 Jul '21

07 Jul '21

My main objective with this series was to decompose libxlDomainStart a bit, as it had become quite unwieldy. Along the way I made a few other small improvements to the code. Jim Fehlig (6): libxl: Drop unused 'cfg' parameter from libxlDomainSaveImageOpen libxl: Move managed save logic to libxlDomainStartNew libxl: Add a helper function to unprepare network devices libxl: Introduce libxlDomainStartPrepare libxl: Introduce libxlDomainStartPerform libxl: Add helper function for running the hook script src/libxl/libxl_domain.c | 384 ++++++++++++++++++------------------ src/libxl/libxl_domain.h | 11 +- src/libxl/libxl_driver.c | 30 +-- src/libxl/libxl_migration.c | 15 +- 4 files changed, 218 insertions(+), 222 deletions(-) -- 2.31.1

2 8

[PATCH 0/2] qemu: fix migration with non-shared storage (incremental)
by Peter Krempa 07 Jul '21

07 Jul '21

See patch 1/2. Peter Krempa (2): qemu: migration: Use correct flag constant for enabling storage migration NEWS: Mention implications of the bug in migration code NEWS.rst | 7 +++++++ src/qemu/qemu_migration.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) -- 2.31.1

2 4

[PATCH] qemu: Don't use memory-backend-memfd for NVDIMMs
by Michal Privoznik 07 Jul '21

07 Jul '21

If guest is configured to use memfd then the function that build memory-backend-* part of command line will put memory-backend-memfd, always. Even for NVDIMMs. This is not correct, because NVDIMMs need a backing path (usually to a real host NVDIMM device). Therefore, regardless of memfd being requested, we have to stick with memory-backend-file. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_command.c | 3 ++- .../memfd-memory-numa.x86_64-latest.args | 6 ++++-- tests/qemuxml2argvdata/memfd-memory-numa.xml | 11 +++++++++++ tests/qemuxml2xmltest.c | 3 ++- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea513693f7..0473e7deaa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3052,7 +3052,8 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps, props = virJSONValueNewObject(); - if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) { + if (!mem->nvdimmPath && + def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) { backendType = "memory-backend-memfd"; if (useHugepage) { diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args index 5e54908666..3b33db3c55 100644 --- a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args +++ b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args @@ -10,13 +10,15 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-instance-00000092/.config \ -name guest=instance-00000092,debug-threads=on \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-instance-00000092/master-key.aes"}' \ --machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off \ +-machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off,nvdimm=on \ -cpu qemu64 \ --m 14336 \ +-m size=14680064k,slots=16,maxmem=1099511627776k \ -overcommit mem-lock=off \ -smp 8,sockets=1,dies=1,cores=8,threads=1 \ -object '{"qom-type":"memory-backend-memfd","id":"ram-node0","hugetlb":true,"hugetlbsize":2097152,"share":true,"prealloc":true,"size":15032385536,"host-nodes":[3],"policy":"preferred"}' \ -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \ +-object '{"qom-type":"memory-backend-file","id":"memnvdimm0","mem-path":"/tmp/nvdimm","share":true,"prealloc":true,"size":536870912,"host-nodes":[3],"policy":"preferred"}' \ +-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \ -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ -display none \ -no-user-config \ diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.xml b/tests/qemuxml2argvdata/memfd-memory-numa.xml index 3f448790a6..d9e1a9f564 100644 --- a/tests/qemuxml2argvdata/memfd-memory-numa.xml +++ b/tests/qemuxml2argvdata/memfd-memory-numa.xml @@ -1,6 +1,7 @@ <domain type='kvm'> <name>instance-00000092</name> <uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> <memory unit='KiB'>14680064</memory> <currentMemory unit='KiB'>14680064</currentMemory> <memoryBacking> @@ -41,5 +42,15 @@ <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </memballoon> + <memory model='nvdimm'> + <source> + <path>/tmp/nvdimm</path> + </source> + <target> + <size unit='KiB'>523264</size> + <node>0</node> + </target> + <address type='dimm' slot='0'/> + </memory> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 40e027aaa4..c5005d41a0 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1305,7 +1305,8 @@ mymain(void) DO_TEST("memfd-memory-numa", QEMU_CAPS_OBJECT_MEMORY_MEMFD, QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, - QEMU_CAPS_OBJECT_MEMORY_FILE); + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("memfd-memory-default-hugepage", QEMU_CAPS_OBJECT_MEMORY_MEMFD, QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, -- 2.31.1

2 1

[PATCH] virtlockd: Don't report error if lockspace exists
by Jim Fehlig 07 Jul '21

07 Jul '21

When the qemu or libxl driver is configured to use lockd and file_lockspace_dir is set, virtlockd emits an error when libvirtd is retarted May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not valid: Lockspace for path /data/libvirtd/lockspace already exists There is really no need to fail when the lockspace already exists, paricularly since the user is expected to create the lockspace specified in file_lockspace_dir. Failure to do so will prevent starting any domains virsh start test error: Failed to start domain 'test' error: Unable to open/create resource /data/libvirtd/lockspace/de22c4bf931e7c48b49e8ca64b477d44e78a51543e534df488b05ccd08ec5caa: No such file or directory Also, virLockManagerLockDaemonSetupLockspace already has logic to ignore the error. Since callers are not interested in the error, change virtlockd to not report or return an error when the specified lockspace already exists. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- In response to my earlier query about this problem https://listman.redhat.com/archives/libvir-list/2021-May/msg00872.html src/locking/lock_daemon_dispatch.c | 5 ++--- src/locking/lock_driver_lockd.c | 11 ++--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index e8b9832453..13e688f2e2 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -405,9 +405,8 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServer *server G_GNUC_UNUSED, } if (virLockDaemonFindLockSpace(lockDaemon, args->path) != NULL) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("Lockspace for path %s already exists"), - args->path); + VIR_DEBUG("Lockspace for path %s already exists", args->path); + rv = 0; goto cleanup; } diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 3a7386af30..87afdbfb78 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -281,15 +281,8 @@ static int virLockManagerLockDaemonSetupLockspace(const char *path) VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE, 0, NULL, NULL, NULL, (xdrproc_t)xdr_virLockSpaceProtocolCreateLockSpaceArgs, (char*)&args, - (xdrproc_t)xdr_void, NULL) < 0) { - if (virGetLastErrorCode() == VIR_ERR_OPERATION_INVALID) { - /* The lockspace already exists */ - virResetLastError(); - rv = 0; - } else { - goto cleanup; - } - } + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; rv = 0; -- 2.31.1

2 1

[libvirt PATCH] meson: check for `XDR` and `driver_remote` early
by Pavel Hrdina 07 Jul '21

07 Jul '21

We have several other options that depend on result of `driver_remote` option check so we need to do it early to have the result available. Fixes: https://gitlab.com/libvirt/libvirt/-/issues/185 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- meson.build | 65 ++++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/meson.build b/meson.build index c95c7f7444..ca4291e37a 100644 --- a/meson.build +++ b/meson.build @@ -848,6 +848,37 @@ foreach name : optional_programs endforeach +# early checks where lot of other packages depend on the result + +# On MinGW portablexdr provides XDR functions, on linux they are +# provided by libtirpc and on FreeBSD/macOS there is no need to +# use extra library as it's provided by libc directly. +if host_machine.system() == 'windows' + xdr_dep = cc.find_library('portablexdr', required: false) +elif host_machine.system() == 'linux' + xdr_dep = dependency('libtirpc', required: false) +elif host_machine.system() in [ 'freebsd', 'darwin' ] + xdr_dep = cc.find_library('c', required: false) +else + xdr_dep = dependency('', required: false) +endif + +if not get_option('driver_remote').disabled() + use_remote = true + + if not xdr_dep.found() + use_remote = false + if get_option('driver_remote').enabled() + error('XDR is required for remote driver') + endif + endif + + if use_remote + conf.set('WITH_REMOTE', 1) + endif +endif + + # generic build dependencies # FIXME rewrite to use dependency() @@ -1005,7 +1036,7 @@ else endif libssh_version = '0.7' -if not get_option('driver_remote').disabled() +if conf.has('WITH_REMOTE') libssh_dep = dependency('libssh', version: '>=' + libssh_version, required: get_option('libssh')) if libssh_dep.found() conf.set('WITH_LIBSSH', 1) @@ -1028,7 +1059,7 @@ else endif libssh2_version = '1.3' -if not get_option('driver_remote').disabled() +if conf.has('WITH_REMOTE') libssh2_dep = dependency('libssh2', version: '>=' + libssh2_version, required: get_option('libssh2')) if libssh2_dep.found() conf.set('WITH_SSH2', 1) @@ -1172,7 +1203,7 @@ if not get_option('sanlock').disabled() endif sasl_version = '2.1.26' -if get_option('driver_remote').enabled() +if conf.has('WITH_REMOTE') sasl_dep = dependency('libsasl2', version: '>=' + sasl_version, required: get_option('sasl')) if sasl_dep.found() conf.set('WITH_SASL', 1) @@ -1280,19 +1311,6 @@ if wireshark_dep.found() endif endif -# On MinGW portablexdr provides XDR functions, on linux they are -# provided by libtirpc and on FreeBSD/macOS there is no need to -# use extra library as it's provided by libc directly. -if host_machine.system() == 'windows' - xdr_dep = cc.find_library('portablexdr', required: false) -elif host_machine.system() == 'linux' - xdr_dep = dependency('libtirpc', required: false) -elif host_machine.system() in [ 'freebsd', 'darwin' ] - xdr_dep = cc.find_library('c', required: false) -else - xdr_dep = dependency('', required: false) -endif - yajl_version = '2.0.3' yajl_dep = dependency('yajl', version: '>=' + yajl_version, required: get_option('yajl')) if yajl_dep.found() @@ -1406,21 +1424,6 @@ endif # build driver options -if not get_option('driver_remote').disabled() - use_remote = true - - if not xdr_dep.found() - use_remote = false - if get_option('driver_remote').enabled() - error('XDR is required for remote driver') - endif - endif - - if use_remote - conf.set('WITH_REMOTE', 1) - endif -endif - remote_default_mode = get_option('remote_default_mode') if remote_default_mode == 'direct' conf.set('REMOTE_DRIVER_AUTOSTART_DIRECT', '1') -- 2.31.1

2 1

[PATCH] virDomainMachineNameAppendValid: Handle special characters better
by Michal Privoznik 07 Jul '21

07 Jul '21

When constructing guest name for machined we have to be very cautious as machined expects a name that's basically a valid URI. Therefore, if there's a dot it has to be followed by a letter or a number. And if there's a sequence of two or more dashes they should be joined into a single dash. These rules are implemented in virDomainMachineNameAppendValid(). There's the @skip variable which is supposed to track whether it is safe to append a dot or a dash into name. However, the variable is set to false (meaning it is safe to append a dot or a dash) even if the current character we are processing is not in the set of allowed characters (and thus skipped over). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1948433 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/hypervisor/domain_driver.c | 8 ++++---- tests/virsystemdtest.c | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index f7d49bc38a..29e11c0447 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -63,18 +63,18 @@ virDomainMachineNameAppendValid(virBuffer *buf, break; if (*name == '.' || *name == '-') { - if (!skip) + if (!skip) { virBufferAddChar(buf, *name); - skip = true; + skip = true; + } continue; } - skip = false; - if (!strchr(HOSTNAME_CHARS, *name)) continue; virBufferAddChar(buf, *name); + skip = false; } /* trailing dashes or dots are not allowed */ diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index f1c9a4ee5f..a09b428a8a 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -736,6 +736,9 @@ mymain(void) TEST_MACHINE("demo.-.test.", NULL, 11, "qemu-11-demo.test"); TEST_MACHINE("demo", "/tmp/root1", 1, "qemu-embed-0991f456-1-demo"); TEST_MACHINE("demo", "/tmp/root2", 1, "qemu-embed-95d47ff5-1-demo"); + TEST_MACHINE("|.-m", NULL, 1, "qemu-1-m"); + TEST_MACHINE("Auto-esx7.0-rhel7.9-special-characters~!@#$%^&*_=+,?><:;|.\"[]()`\\-m", + NULL, 1, "qemu-1-Auto-esx7.0-rhel7.9-special-characters.m"); # define TESTS_PM_SUPPORT_HELPER(name, function) \ do { \ -- 2.31.1

2 1