Boris Fiuczynski [Thu, 13 Aug 2020 14:03:44 +0000 (16:03 +0200)]
qemu: avoid maybe-uninitialized warning by GCC 10
GCC 10 complains about "well_formed_uri" may be used uninitialzed.
Even though it is a false positive, we can easily avoid it.
Avoiding
../src/qemu/qemu_migration.c: In function ‘qemuMigrationDstPrepareDirect’:
../src/qemu/qemu_migration.c:2920:16: error: ‘well_formed_uri’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
2920 | if (well_formed_uri) {
| ^
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
With qemu 5.0 and libvirt 6.6 there are new apparmor denials:
apparmor="DENIED" operation="umount" profile="libvirtd"
name="/run/libvirt/qemu/1-kvmguest-groovy-norm.dev/" comm="rpc-worker"
These are related to new issues around devmapper handling [1] and the
error path triggered by these issues now causes this new denial.
There are already related rules for mounting and it seems right to
allow also the related umount.
The design of apparmor in libvirt always had a way to define custom
per-guest rules as described in docs/drvqemu.html and [1].
A fix meant to clean the profiles after guest shutdown was a bit
overzealous and accidentially removed this important admin feature as
well.
Therefore reduce the --delete option of virt-aa-helper to only delete
the .files that would be re-generated in any case.
Users/Admins are always free to clean the profiles themselve if they
prefer a clean directory - they will be regenerated as needed. But
libvirt should never remove the base profile meant to allow per-guest
overrides and thereby break a documented feature.
Fixes: eba2225b "apparmor: delete profile on VM shutdown" Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Tue, 11 Aug 2020 13:56:54 +0000 (15:56 +0200)]
qemu: fix crash in qemuDomainSetBlkioParameters without cgroups
If we don't have cgroups available and user tries to update blkio
parameters for running VM it will crash.
It should have been protected by the virCgroupHasController() check but
it was never called if the API was executed without any flags.
We call virDomainObjGetDefs() which sets `def` and `persistentDef` based
on the flags and these two variables should be used to figure out if we
need to update LIVE, CONFIG or both states.
Erik Skultety [Fri, 7 Aug 2020 11:13:39 +0000 (13:13 +0200)]
kbase: sev: Provide more details on virtio-net configuration
With virtio-net we also need to disable the iPXE option ROM otherwise
a SEV-enabled guest would not boot. While at it, fix the full machine
XML examples accordingly.
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ján Tomko [Sun, 2 Aug 2020 20:50:37 +0000 (22:50 +0200)]
bhyve: fix NULL pointer check position
src/bhyve/bhyve_parse_command.c:437:9: warning: Either the condition
'!config' is redundant or there is possible null pointer dereference:
config. [nullPointerRedundantCheck]
src/bhyve/bhyve_parse_command.c:280:23: warning: Either the condition
'!separator' is redundant or there is pointer arithmetic
with NULL pointer. [nullPointerArithmeticRedundantCheck]
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Pavel Hrdina [Tue, 11 Aug 2020 09:07:06 +0000 (11:07 +0200)]
vircgroupv2devices: fix counting entries in BPF map
BPF syscall BPF_MAP_GET_NEXT_KEY returns -1 if something fails but it
will also return -1 if trying to get next key using the last key in the
map with errno set to ENOENT.
If there are VMs running and libvirtd is restarted and user tries to
call some cgroup devices operation on a VM we need to get the count of
entries in BPF map and it fails which will result in error when trying
to attach/detech devices.
apparmor: let qemu load old shared objects after upgrades
Since [1] qemu can after upgrade fall back to pre-upgrade modules
to still be able to dynamically load qemu-module based features.
The paths for these modules are pre-defined by the code and should
be allowed to be mapped and loaded from which will allow packagers
avoiding the inability of late feature load [2] after package upgrades.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@canonical.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange redhat com>
Stefan Bader [Mon, 3 Aug 2020 10:21:23 +0000 (12:21 +0200)]
apparmor: allow libvirtd to call pygrub
When using xen through libxl in Debian/Ubuntu it needs to be able to
call pygrub.
This is placed in a versioned path like /usr/lib/xen-4.11/bin.
In theory the rule could be more strict by rendering the libexec_dir
setting pkg-config can derive from libbxen-dev. But that would make
particular libvirt/xen packages version-depend on each other. It seems
more reasonable to avoid these versioned dependencies and use a wildcard
rule instead as it is already in place for libxl-save-helper.
Note: This change was in Debian [1] and Ubuntu [2] for quite some time
already.
Pavel Hrdina [Fri, 7 Aug 2020 13:57:28 +0000 (15:57 +0200)]
test: fix emulator pin info in test driver
Commit <6328da04285d9f65cb323d399f731c20caf63f5a> introduced
testDomainGetEmulatorPinInfo() into test driver but used
virHostCPUGetCount() function to get the number of host CPUs.
This would be correct for other drivers but in test driver we must not
depend on the host, we have to use hard-coded host representation that
we have in test driver.
Follows the logic of testDomainGetVcpuPinInfo().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Fri, 7 Aug 2020 13:48:27 +0000 (15:48 +0200)]
conf: fix detection of available host CPUs for vcpupin
Commit <2020c6af8a8e4bb04acb629d089142be984484c8> fixed an issue with
QEMU driver by reporting offline CPUs as well. However, doing so it
introduced a regression into libxl and test drivers by completely
ignoring the passed `hostcpus` variable.
Move the virHostCPUGetAvailableCPUsBitmap() out of the helper into QEMU
driver so it will not affect other drivers which gets the number of host
CPUs differently.
This was uncovered by running libvirt-dbus test suite which counts on
the fact that test driver has hard-coded host definition and must not
depend on the host at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: fix race in signal interrupt during QEMU startup
If a Ctrl-C arrives while we are in the middle of executing the
virDomainCreateXML call, we will have no "virDomainPtr" object
available, but QEMU may none the less be running.
This means we'll never try to stop the QEMU process before we
honour the Ctrl-C and exit.
To deal with this race we need to postpone quit of the event
loop if it is requested while in the middle of domain startup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: avoid race in releasing the GSource in event thread
There is a race between vir_event_thread_finalize and
virEventThreadWorker in releasing the last reference on
the GMainContext. If virEventThreadDataFree() runs after
vir_event_thread_finalize releases its reference, then
it will release the last reference on the GMainContext.
As a result g_autoptr cleanup on the GSource will access
free'd memory.
The race can be seen in non-deterministic crashes of the
virt-run-qemu program during its shutdown, but could
also likely affect the main libvirtd QEMU driver:
Thread 2 (Thread 0x7f508ffff700 (LWP 222813)):
#0 0x00007f509c8e26b0 in malloc_consolidate (av=av@entry=0x7f5088000020) at malloc.c:4488
#1 0x00007f509c8e4b08 in _int_malloc (av=av@entry=0x7f5088000020, bytes=bytes@entry=2048) at malloc.c:3711
#2 0x00007f509c8e6412 in __GI___libc_malloc (bytes=2048) at malloc.c:3073
#3 0x00007f509d6e925e in g_realloc (mem=0x0, n_bytes=2048) at gmem.c:164
#4 0x00007f509d705a57 in g_string_maybe_expand (string=string@entry=0x7f5088001f20, len=len@entry=1024) at gstring.c:102
#5 0x00007f509d705ab6 in g_string_sized_new (dfl_size=dfl_size@entry=1024) at gstring.c:127
#6 0x00007f509d708c5e in g_test_log_dump (len=<synthetic pointer>, msg=<synthetic pointer>) at gtestutils.c:3330
#7 0x00007f509d708c5e in g_test_log
(lbit=G_TEST_LOG_ERROR, string1=0x7f508800fcb0 "GLib:ERROR:ghash.c:377:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > 0)", string2=<optimized out>, n_args=0, largs=0x0) at gtestutils.c:975
#8 0x00007f509d70af2a in g_assertion_message
(domain=<optimized out>, file=0x7f509d7324a2 "ghash.c", line=<optimized out>, func=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", message=<optimized out>)
at gtestutils.c:2504
#9 0x00007f509d70af8e in g_assertion_message_expr
(domain=domain@entry=0x7f509d72d76e "GLib", file=file@entry=0x7f509d7324a2 "ghash.c", line=line@entry=377, func=func@entry=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", expr=expr@entry=0x7f509d732488 "hash_table->ref_count > 0") at gtestutils.c:2555
#10 0x00007f509d6d197e in g_hash_table_lookup_node (hash_table=0x55b70ace1760, key=<optimized out>, hash_return=<synthetic pointer>) at ghash.c:377
#11 0x00007f509d6d197e in g_hash_table_lookup_node (hash_return=<synthetic pointer>, key=<optimized out>, hash_table=0x55b70ace1760) at ghash.c:361
#12 0x00007f509d6d197e in g_hash_table_remove_internal (hash_table=0x55b70ace1760, key=<optimized out>, notify=1) at ghash.c:1371
#13 0x00007f509d6e0664 in g_source_unref_internal (source=0x7f5088000b60, context=0x55b70ad87e00, have_lock=0) at gmain.c:2103
#14 0x00007f509d6e1f64 in g_source_unref (source=<optimized out>) at gmain.c:2176
#15 0x00007f50a08ff84c in glib_autoptr_cleanup_GSource (_ptr=<synthetic pointer>) at /usr/include/glib-2.0/glib/glib-autocleanups.h:58
#16 0x00007f50a08ff84c in virEventThreadWorker (opaque=0x55b70ad87f80) at ../../src/util/vireventthread.c:114
#17 0x00007f509d70bd4a in g_thread_proxy (data=0x55b70acf3850) at gthread.c:784
#18 0x00007f509d04714a in start_thread (arg=<optimized out>) at pthread_create.c:479
#19 0x00007f509c95cf23 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Thread 1 (Thread 0x7f50a1380c00 (LWP 222802)):
#0 0x00007f509c8977ff in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f509c881c35 in __GI_abort () at abort.c:79
#2 0x00007f509d72a823 in g_mutex_clear (mutex=0x55b70ad87e00) at gthread-posix.c:1307
#3 0x00007f509d72a823 in g_mutex_clear (mutex=mutex@entry=0x55b70ad87e00) at gthread-posix.c:1302
#4 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:582
#5 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:541
#6 0x00007f50a08ffabb in vir_event_thread_finalize (object=0x55b70ad83180 [virEventThread]) at ../../src/util/vireventthread.c:50
#7 0x00007f509d9c48a9 in g_object_unref (_object=<optimized out>) at gobject.c:3340
#8 0x00007f509d9c48a9 in g_object_unref (_object=0x55b70ad83180) at gobject.c:3232
#9 0x00007f509583d311 in qemuProcessQMPFree (proc=proc@entry=0x55b70ad87b90) at ../../src/qemu/qemu_process.c:8355
#10 0x00007f5095790f58 in virQEMUCapsInitQMPSingle
(qemuCaps=qemuCaps@entry=0x55b70ad88010, libDir=libDir@entry=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=runUid@entry=107, runGid=runGid@entry=107, onlyTCG=onlyTCG@entry=false) at ../../src/qemu/qemu_capabilities.c:5409
#11 0x00007f509579108f in virQEMUCapsInitQMP (runGid=107, runUid=107, libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", qemuCaps=0x55b70ad88010)
at ../../src/qemu/qemu_capabilities.c:5420
#12 0x00007f509579108f in virQEMUCapsNewForBinaryInternal
(hostArch=VIR_ARCH_X86_64, binary=binary@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=107, runGid=107, hostCPUSignature=0x55b70ad01320 "GenuineIntel, Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz, family: 6, model: 85, stepping: 7", microcodeVersion=83898113, kernelVersion=0x55b70ad00d60 "4.18.0-211.el8.x86_64 #1 SMP Thu Jun 4 08:08:16 UTC 2020") at ../../src/qemu/qemu_capabilities.c:5472
#13 0x00007f5095791373 in virQEMUCapsNewData (binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", privData=0x55b70ad5b8f0) at ../../src/qemu/qemu_capabilities.c:5505
#14 0x00007f50a09a32b1 in virFileCacheNewData (name=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", cache=<optimized out>) at ../../src/util/virfilecache.c:208
#15 0x00007f50a09a32b1 in virFileCacheValidate (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", data=data@entry=0x7ffca39ffd90)
at ../../src/util/virfilecache.c:277
#16 0x00007f50a09a37ea in virFileCacheLookup (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/util/virfilecache.c:310
#17 0x00007f5095791627 in virQEMUCapsCacheLookup (cache=0x55b70ad5c030, binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/qemu/qemu_capabilities.c:5647
#18 0x00007f50957c34c3 in qemuDomainPostParseDataAlloc (def=<optimized out>, parseFlags=<optimized out>, opaque=<optimized out>, parseOpaque=0x7ffca39ffe18)
at ../../src/qemu/qemu_domain.c:5470
#19 0x00007f50a0a34051 in virDomainDefPostParse
(def=def@entry=0x55b70ad7d200, parseFlags=parseFlags@entry=258, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0)
at ../../src/conf/domain_conf.c:5970
#20 0x00007f50a0a464bb in virDomainDefParseNode
(xml=xml@entry=0x55b70aced140, root=root@entry=0x55b70ad5f020, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22520
#21 0x00007f50a0a4669b in virDomainDefParse
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., filename=filename@entry=0x0, xmlopt=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258) at ../../src/conf/domain_conf.c:22474
#22 0x00007f50a0a467ae in virDomainDefParseString
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., xmlopt=<optimized out>, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22488
#23 0x00007f50958ce112 in qemuDomainCreateXML
(conn=0x55b70acf9090, xml=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/qemu/qemu_driver.c:1744
#24 0x00007f50a0c268ac in virDomainCreateXML
(conn=0x55b70acf9090, xmlDesc=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/libvirt-domain.c:176
#25 0x000055b709547e7b in main (argc=<optimized out>, argv=<optimized out>) at ../../src/qemu/qemu_shim.c:289
The solution is to explicitly unref the GSource at a safe time instead
of letting g_autoptr unref it when leaving scope.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Unfortunately it is really common for libvirt to call g_source_destroy
from a non-main thread. This glib bug is the cause of non-determinstic
crashes in eventtest, and probably in libvirtd too.
To work around the problem we need to ensure that we never release
the last reference on a GSource from a non-main thread. The previous
patch replaced our use of g_source_destroy with a pair of
g_source_remove and g_source_unref. We can now delay the g_source_unref
call by using a idle callback to invoke it from the main thread which
avoids the race condition.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: keep track of full GSource object not source ID number
The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We need to check for generic symbol instead of checking for function.
In addition the readline/readline.h file requires stdio.h to by included
beforehand which was done in autotools but I dropped it in meson.
And lastly the final condition to print error or disable readline was
broken as well by replacing the readline_dep every time if readline was
not explicitly enabled.
Reported-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Pavel Hrdina [Wed, 5 Aug 2020 08:04:12 +0000 (10:04 +0200)]
docs/manpages/meson: fix sanlock detection
In meson.build we define sanlock_dep only if it is available but in
addition we add 'WITH_SANLOCK' into conf. Use the presence of
'WITH_SANLOCK' in conf to figure out if we need to install
virt-sanlock-cleanup man page.
Reported-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Laine Stump [Tue, 14 Jul 2020 17:43:00 +0000 (13:43 -0400)]
treat all NULL returns from virXMLNodeContentString() as an error
and stop erroneously equating NULL with "". The latter means that the
element has empty content, while the former means there was an error
during parsing (either internal with the parser, or the content of the
XML was bad).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 14 Jul 2020 04:21:28 +0000 (00:21 -0400)]
util: log an error if virXMLNodeContentString will return NULL
Many of our calls to xmlNodeGetContent() (which are now all via
virXMLNodeContentString() are failing to check for a NULL return. We
need to remedy that, but in order to make the remedy simpler, let's
log an error in virXMLNodeContentString(), so that the callers don't
all individually need to (since it would be the same error message for
all of them anyway).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 19 Jun 2020 01:52:38 +0000 (21:52 -0400)]
conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent
virDomainBlkioDeviceParseXML() calls xmlNodeGetContent() multiple
times in a loop, but can easily be refactored to call it once for all
element nodes, and then use the result of that one call in each of the
(mutually exclusive) blocks that previously each had their own call to
xmlNodeGetContent.
This is being done in order to reduce the number of changes needed in
an upcoming patch that will eliminate the lack of checking for NULL on
return from xmlNodeGetContent().
As part of the simplification, the while() loop has been changed into
a for() so that we can use "continue" without bypassing the
"node = node->next".
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit bcbb026993 converted qemublocktest to use
g_autoptr for virQEMUCaps. To prevent it from crashing,
don't explicitly call virObjectUnref() on this object.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
docs: fix libvirt go-import metadata in pages and links in 404 page
The meson conversion lost the <meta> tags providing the go-import,
because the "$pagename" variable lost the .html suffix. Rather
than fix that, just change to using "$pagesrc" instead, as it is a
better fit.
The 404 page also needs to use absolute links to work correctly for
pages in sub-folders.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
sc_proper_name_utf8_requires_ICONV looks for Makefile.am files, so is
not going to work correctly with meson, nor did we ever use the GNULIB
"proper_name_utf8" function.
The 'today' variable is not referenced anywhere.
The 'writable-files' target is not used anywhere
sc_prohibit_reversed_compare_failure only checks 'init.sh' which does
not exist in libvirt.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
network: Use single quotes in default network configuration
Whenever libvirt is upgraded on a Debian system, the user will be
prompted along the lines of
Configuration file '/etc/libvirt/qemu/networks/default.xml'
==> Modified (by you or by a script) since installation.
==> Package distributor has shipped an updated version.
What would you like to do about it ? Your options are:
Y or I : install the package maintainer's version
N or O : keep your currently-installed version
D : show the differences between the versions
Z : start a shell to examine the situation
The default action is to keep your current version.
*** default.xml (Y/I/N/O/D/Z) [default=N] ? d
--- /etc/libvirt/qemu/networks/default.xml 2020-08-04 12:57:25.450911143 +0200
+++ /etc/libvirt/qemu/networks/default.xml.dpkg-new 2020-08-03 22:47:15.000000000 +0200
@@ -1,19 +1,11 @@
-<!--
-WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
-OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
- virsh net-edit default
-or other application using the libvirt API.
--->
-
<network>
<name>default</name>
- <uuid>612a2cab-72fb-416d-92bc-4d9e597bfb63</uuid>
- <forward mode='nat'/>
- <bridge name='virbr0' stp='on' delay='0'/>
- <mac address='52:54:00:1f:03:79'/>
- <ip address='192.168.122.1' netmask='255.255.255.0'>
+ <uuid>d020b839-4379-492c-aa74-eab7365076e6</uuid>
+ <bridge name="virbr0"/>
+ <forward/>
+ <ip address="192.168.122.1" netmask="255.255.255.0">
<dhcp>
- <range start='192.168.122.2' end='192.168.122.254'/>
+ <range start="192.168.122.2" end="192.168.122.254"/>
</dhcp>
</ip>
</network>
The UUID situation should probably be handled the same way it is
in the spec file by stripping it, and in general we could behave
much better towards users, but one part of the diff that
immediately stands out is that some lines are highlighted not
because they are semantically different, but simply because they
use different types of quotes around attributes.
Since the canonical version of all libvirt XML documents (as
returned by the various vir*GetXMLDesc() APIs) as well as the
on-disk representations use single quotes, let's use the same
for configuration files we install as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
mdev: Fix daemon crash when reattaching mdevs on assignment conflict
If there's a list of mdevs to be assigned to a domain, but one of them
(NOT the first) is already assigned to a different domain we're going
to crash in the qemuProcessStop phase in
virMediatedDeviceListFindIndex, because some of the pointers in
mgr->activeMediatedHostdevs are dangling. This is due to
virMediatedDeviceListMarkDevices using cleanup instead of rollback when
we find out that a device is already taken.
Reproducer steps:
1. start vm1 with mdev1
2. start vm2 with mdev2, mdev1 (the order is important!)
Backtrace:
#0 0x0000ffffb8c36250 in strcmp
#1 0x0000ffffb9b80754 in virMediatedDeviceListFindIndex
#2 0x0000ffffb9b80870 in virMediatedDeviceListFind
#3 0x0000ffffb9c9e168 in virHostdevReAttachMediatedDevices
#4 0x0000ffff9949f724 in qemuHostdevReAttachMediatedDevices
#5 0x0000ffff9949f7f8 in qemuHostdevReAttachDomainDevices
#6 0x0000ffff994bcd70 in qemuProcessStop
#7 0x0000ffff994bf4e0 in qemuProcessStart
Signed-off-by: Binfeng Wu <wubinfeng@huawei.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
ci: Temporarily enable verbose mode for cirrus-run
We've hit issues with GitLab CI jobs reporting a failure despite
the corresponding Cirrus CI job finishing successfully: this is
apparently caused by the underlying VM being rescheduled.
A workaround for this issue has been implemented as of
which will be included in the upcoming 0.3.0 release; however, in
order to validate that this workaround is effective it would be
useful to have more data.
Based on the conversation in
https://github.com/sio/cirrus-run/issues/4
enabling verbose mode allows to collect this data while not having
any impact on performance, so let's enable it temporarily and then
disable it again once cirrus-run 0.3.0 is out.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Tue, 4 Aug 2020 09:04:04 +0000 (11:04 +0200)]
src/logging: no need to include log_protocol.h in log_manager.h
The header log_manager.h doesn't use anything from log_protocol.h and
the only other place than logging using log_protocol.h is qemu_command.c
where we can include log_protocol.h directly to have enum value
VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE available.
Fixes race-condition compilation error with meson:
In file included from ../tests/qemuhotplugmock.c:21:
In file included from ../src/qemu/qemu_hotplug.h:25:
In file included from ../src/qemu/qemu_domain.h:42:
../src/logging/log_manager.h:25:10: fatal error: 'logging/log_protocol.h' file not found
^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Reported-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Mon, 3 Aug 2020 18:59:42 +0000 (20:59 +0200)]
meson_options: change default value to auto for driver_esx
Reported-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Erik Skultety [Mon, 3 Aug 2020 15:20:59 +0000 (17:20 +0200)]
meson: Fix libvirtd|virtproxyd socket prefixes
During the switch to meson, one of the patches mistakenly changed the
runtime socket prefix for {libvirtd, virtproxyd} to "libvirtd-" from
the original "libvirt-". Not to be mistaken with the systemd unit name
which actually follows the daemon name, IOW the systemd unit name
remains as e.g. "libvirtd.socket", but the actual unix socket created
on the filesystem that the daemon binds to must be named "libvirt-sock"
and not "libvirtd-sock".
Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can use qemuDomainSetupChardev() to obtain the path that we
need to unlink() from within domain's namespace. Note, while
previously we unlinked only VIR_DOMAIN_CHR_TYPE_DEV chardevs,
with this change we unlink some other types too - exactly those
types we created when plugging the device in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 13:08:13 +0000 (15:08 +0200)]
qemuDomainNamespaceTeardownHostdev: Unlink paths in one go
In my attempt to deduplicate the code, we can use
qemuDomainSetupHostdev() to obtain the list of paths to unlink
and then pass it to qemuDomainNamespaceUnlinkPaths() to unlink
them in a single fork.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 13:05:52 +0000 (15:05 +0200)]
qemuDomainNamespaceUnlinkPaths: Turn @paths into string list
So far, the only caller qemuDomainNamespaceUnlinkPath() will
always pass a single path to unlink, but similarly to
qemuDomainNamespaceMknodPaths() - there are a few callers that
would like to pass two or more files to unlink at once (held in a
string list). Make the @paths argument a string list then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 13:04:38 +0000 (15:04 +0200)]
qemuDomainDetachDeviceUnlink: Unlink paths in one go
Simirarly to qemuDomainAttachDeviceMknodHelper() which was
modified just a couple of commits ago, modify the unlink helper
which is called on device detach so that it can unlink multiple
files in one go instead of forking off for every single one of
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:51:13 +0000 (14:51 +0200)]
qemuDomainBuildNamespace: Populate SEV from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain SEV into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:49:42 +0000 (14:49 +0200)]
qemuDomainBuildNamespace: Populate loader from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain loader into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:37:22 +0000 (14:37 +0200)]
qemuDomainBuildNamespace: Populate RNGs from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain RNGs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:34:22 +0000 (14:34 +0200)]
qemuDomainBuildNamespace: Populate inputs from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain inputs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:30:44 +0000 (14:30 +0200)]
qemuDomainBuildNamespace: Populate graphics from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain graphics (render node)
into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:29:09 +0000 (14:29 +0200)]
qemuDomainBuildNamespace: Populate TPM from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain TPM into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:26:49 +0000 (14:26 +0200)]
qemuDomainBuildNamespace: Populate chardevs from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain chardevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 11:30:31 +0000 (13:30 +0200)]
qemuDomainBuildNamespace: Populate memory from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain memory (nvdimms) into
daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:18:59 +0000 (14:18 +0200)]
qemuDomainBuildNamespace: Populate hostdevs from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain hostdevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 12:11:41 +0000 (14:11 +0200)]
qemuDomainBuildNamespace: Populate disks from daemon's namespace
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain disks into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 15:13:11 +0000 (17:13 +0200)]
qemuDomainBuildNamespace: Populate basic /dev from daemon's namespace
As mentioned in previous commit, populating domain's namespace
from pre-exec() hook is dangerous. This commit moves population
of the namespace with basic /dev nodes (e.g. /dev/null, /dev/kvm,
etc.) into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Okay, here is the deal. Currently, the way we build namespace is
very fragile. It is done from pre-exec hook when starting a
domain, after we mass closed all FDs and before we drop
privileges and exec() QEMU. This fact poses some limitations onto
the namespace build code, e.g. it has to make sure not to keep
any FD opened (not even through a library call), because it would
be leaked to QEMU. Also, it has to call only async signal safe
functions. These requirements are hard to meet - in fact as of my
commit v6.2.0-rc1~235 we are leaking a FD into QEMU by calling
libdevmapper functions.
To solve this issue and avoid similar problems in the future, we
should change our paradigm. We already have functions which can
populate domain's namespace with nodes from the daemon context.
If we use them to populate the namespace and keep only the bare
minimum in the pre-exec hook, we've mitigated the risk.
Therefore, the old qemuDomainBuildNamespace() is renamed to
qemuDomainUnshareNamespace() and new qemuDomainBuildNamespace()
function is introduced. So far, the new function is basically a
NOP and domain's namespace is still populated from the pre-exec
hook - next patches will fix it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 11:32:32 +0000 (13:32 +0200)]
qemuDomainSetupDisk: Accept @src
The aim to make it look as close to
qemuDomainNamespaceSetupDisk() as possible. The latter will call
the former and this change makes that diff easier to read.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 11:08:36 +0000 (13:08 +0200)]
qemuNamespaceMknodPaths: Turn @paths into string list
Every caller does the same - counts the number of items in a
string list they have, only to pass the number to
qemuDomainNamespaceMknodPaths(). This is needless - the function
can accept the string list and count the items itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 08:58:41 +0000 (10:58 +0200)]
qemuDomainNamespaceMknodPaths: Create more files in one go
While the previous commit prepared the helper function run in a
forked off helper (with corresponding struct), this commit
modifies the caller, which now create all files requested in a
single process and does not fork off for every single path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 20 Jul 2020 18:00:19 +0000 (20:00 +0200)]
qemuDomainAttachDeviceMknodHelper: Create more files in a single go
So far, when attaching a device needs two or more /dev nodes
created into a domain, we fork off and run the helper for every
node separately. For majority of devices this is okay, because
they need no or one node created anyway. But the idea is to use
this attach code to build the namespace when starting a domain,
in which case there will be way more nodes than one.
To achieve this, the recursive approach for handling symlinks has
to be turned into an iterative one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
When attaching a device into a domain, the corresponding /dev
node might need to be created in the domain's namespace. For some
types of files we call mknod(), for symlinks we call symlink(),
but for others - which exist in the host namespace - we need to
so called 'bind mount' them (which is a way of passing a
file/directory between mount namespaces). There is this condition
in qemuDomainAttachDeviceMknodRecursive() which decides whether a
bind mount will be used, move it into a separate function so that
it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This structure is going to be used from not only device attach
code, but also when building the namespace. Moreover, the code
lives in a separate file so the chances of clashing with another
name are minimal.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's not really a problem since this is a helper process that
dies as soon as the helper function returns, but the cleanup code
will be replaced with a function soon and this change prepares
the code for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 10:14:02 +0000 (12:14 +0200)]
qemuDomainNamespaceSetupHostdev: Create paths in one go
While qemuDomainNamespaceMknodPaths() doesn't actually create
files in the namespace in one go (it forks for each path), it a
few commits time it will.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 20 Jul 2020 16:01:16 +0000 (18:01 +0200)]
qemu_domain_namespace: Check for namespace enablement earlier
Functions that create a device node after domain startup (used
from hotplug) will get a list of paths they want to create and
eventually call qemuDomainNamespaceMknodPaths() which then checks
whether domain mount namespace is enabled in the first place.
Alternatively, on device hotunplug, we might want to delete a
path inside domain namespace in which case
qemuDomainNamespaceUnlinkPaths() checks whether the namespace is
enabled. While this is not dangerous, it certainly burns a couple
of CPU cycles needlessly.
Check whether mount namespace is enabled upfront.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 21 Jul 2020 10:57:49 +0000 (12:57 +0200)]
qemu_domain_namespace: Drop unused @cfg argument
There is a lot of functions called from
qemuDomainBuildNamespace() that accept @cfg
(virQEMUDriverConfigPtr) as an argument and don't use it.
Historically, it was done so that all qemuDomainSetupAll*()
functions look the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The name of this function is not very helpful, because it doesn't
create anything, it just flips a bit in a bitmask when domain is
starting up. Move the function internals into qemu_process.c and
forget the function ever existed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 3 Aug 2020 06:24:30 +0000 (08:24 +0200)]
docs: css: Hide redundant 'Contents:' line in table of contents of RST docs
'docutils' add line saying "Contents:" on top of the table of contents.
We don't have that in other documents nor it's really necessary. Hide it
in the stylesheet as we can select it easily.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 14 Jul 2020 10:18:45 +0000 (12:18 +0200)]
docs: css: Make definition list 'code' entries bold when converted from rst
Docutils don't generate <code> for inline literals (``blah``) in rst
but rather put them in the '.literal' class. Add a selector for making
them bold when used in definition list headers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 3 Aug 2020 05:32:29 +0000 (07:32 +0200)]
docs: build: Don't include stylesheet in intermediate html files generated from RST
'docutils' add a stylesheet to the output html file for direct
consumption. Since we use the html files just as an intermediate step
which is post-processed to add our own stylesheet and drop the docutils
one in the process we can ask 'rst2html' to not add any for an
intermediate file with less garbage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 3 Aug 2020 10:09:21 +0000 (12:09 +0200)]
kbase: debuglogs: Include more data to attach to bugreports
Promote the 'What to attach?' section to a first level heading and
request also the XML config of a VM, coredump backtrace if something
crashed and ask to not tear down the environment for the possibility to
ask for additional data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
Peter Krempa [Mon, 3 Aug 2020 14:38:31 +0000 (16:38 +0200)]
docs: build: Use proper path to file in 'contribute' link
NEWS.rst is based in the root of the repository and 'hvsupport.html'
doesn't have a backing file which can be edited since it's fully
generated. Our 'contribute -> edit this page' link on the bottom of the
page is wrong in those cases.
Fix it by adding the contribute section only when there's a source and
base the 'source' of a html file in the root of the repository.
Along with that we need to modify the scripts/meson-html-gen.py script
to accept optional 'pagesrc' and the XSL template to skip the
'contribute' section when we don't have a source.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virLoginShellGetShellArgv was not dereferencing the pointer
to the string list containing the shell parameters from the
config file, thus setting some random number as shargvlen.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 740e4d705284ba0598258291c2656f50075486f0 Reviewed-by: Martin Kletzander <mkletzan@redhat.com>