]> git.ipfire.org Git - thirdparty/libvirt.git/log
thirdparty/libvirt.git
4 years agokbase: sev: Provide more details on virtio-net configuration
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>
4 years agodocs: formatdomain: fix superscripts
Ján Tomko [Tue, 11 Aug 2020 22:18:23 +0000 (00:18 +0200)] 
docs: formatdomain: fix superscripts

There needs to be a space before the :sup: directive.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
4 years agobhyve: fix NULL pointer check position
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>
4 years agovircgroupv2devices: fix counting entries in BPF map
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.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1833321

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
4 years agomeson: define IFCONFIG_PATH for virnetdevip
Roman Bogorodskiy [Sat, 8 Aug 2020 09:16:16 +0000 (13:16 +0400)] 
meson: define IFCONFIG_PATH for virnetdevip

When checking for ifconfig(8), set not only IFCONFIG value,
but also IFCONFIG_PATH as it's used in util/virnetdevip.c.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
4 years agomeson: fix BSD bridge management routines check
Roman Bogorodskiy [Sat, 8 Aug 2020 09:16:16 +0000 (13:16 +0400)] 
meson: fix BSD bridge management routines check

Add missing prerequisite headers for checking BRDGSFD, BRDGADD,
BRDGDEL in net/if_bridgevar.h.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
4 years agomeson: fix link_addr(3) check
Roman Bogorodskiy [Sat, 8 Aug 2020 09:16:16 +0000 (13:16 +0400)] 
meson: fix link_addr(3) check

Add missing prerequisite headers for checking link_addr(3)
in net/if_dl.h.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
4 years agoapparmor: let qemu load old shared objects after upgrades
Christian Ehrhardt [Mon, 3 Aug 2020 12:03:19 +0000 (14:03 +0200)] 
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.

[1]: https://github.com/qemu/qemu/commit/bd83c861
[2]: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361

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>
4 years agoapparmor: qemu access to @{PROC}/*/auxv for hw_cap
Stefan Bader [Mon, 3 Aug 2020 11:44:27 +0000 (13:44 +0200)] 
apparmor: qemu access to @{PROC}/*/auxv for hw_cap

On some architectures (ppc, s390x, sparc, arm) qemu will read auxv
to detect hardware capabilities via qemu_getauxval.

Allow that access read-only for the entry owned by the current
qemu process.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
4 years agoapparmor: read only access to overcommit_memory
Jamie Strandboge [Mon, 3 Aug 2020 11:41:33 +0000 (13:41 +0200)] 
apparmor: read only access to overcommit_memory

Allow qemu to read @{PROC}/sys/vm/overcommit_memory.
This is read on guest start-up and (as read-only) not a
critical secret that has to stay hidden.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Jamie Strandboge <jamie@ubuntu.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
4 years agoapparmor: allow libvirtd to call pygrub
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.

[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=931768
[2]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1326003

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
4 years agoapparmor: allow default pki path
Sam Hartman [Mon, 3 Aug 2020 10:08:41 +0000 (12:08 +0200)] 
apparmor: allow default pki path

/etc/pki/qemu is a pki path recommended by qemu tls docs [1]
and one that can cause issues with spice connections when missing.

Add the path to the allowed list of pki paths to fix the issue.

Note: this is active in Debian/Ubuntu [1] for quite a while already.

[1]: https://www.qemu.org/docs/master/system/tls.html
[2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930100

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
4 years agoqemu: consider available CPUs in iothread info output
Pavel Hrdina [Fri, 7 Aug 2020 14:04:41 +0000 (16:04 +0200)] 
qemu: consider available CPUs in iothread info output

Following the rationale from commit
<2020c6af8a8e4bb04acb629d089142be984484c8> we should do the same thing
for iothread info as well.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
4 years agotest: fix emulator pin info in test driver
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>
4 years agoconf: fix detection of available host CPUs for vcpupin
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>
4 years agoqemu: remove use of gettid() syscall
Daniel P. Berrangé [Fri, 7 Aug 2020 12:51:16 +0000 (13:51 +0100)] 
qemu: remove use of gettid() syscall

This is not expose in most historical versions of glibc, nor
non-glibc impls. We must use our wrapper API instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
4 years agoqemu: fix race in signal interrupt during QEMU startup
Daniel P. Berrangé [Wed, 29 Jul 2020 18:00:25 +0000 (19:00 +0100)] 
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>
4 years agoutil: avoid race in releasing the GSource in event thread
Daniel P. Berrangé [Wed, 29 Jul 2020 16:51:53 +0000 (17:51 +0100)] 
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>
4 years agoutil: avoid crash due to race in glib event loop code
Daniel P. Berrangé [Tue, 28 Jul 2020 15:52:47 +0000 (16:52 +0100)] 
util: avoid crash due to race in glib event loop code

There is a fairly long standing race condition bug in glib which can hit
if you call g_source_destroy or g_source_unref from a non-main thread:

  https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358

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>
4 years agoutil: keep track of full GSource object not source ID number
Daniel P. Berrangé [Tue, 28 Jul 2020 14:54:13 +0000 (15:54 +0100)] 
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>
5 years agomeson: fix readline detection if there is no pkg-config file
Pavel Hrdina [Wed, 5 Aug 2020 10:39:24 +0000 (12:39 +0200)] 
meson: fix readline detection if there is no pkg-config file

Commit <74416b1d4849ef77ef31de5344dd75f03094434b> added check for
rl_completion_quote_character to make sure we have correct readline
library. Commit <a9443bc9a9ef451b46306e66ed3b706756fc1414> added
inaccurate comment that it's a function.

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>
5 years agodocs/manpages/meson: fix sanlock detection
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>
5 years agoscripts: Fix meson-install-symlink.py overwriting existing links
Erik Skultety [Tue, 4 Aug 2020 15:27:21 +0000 (17:27 +0200)] 
scripts: Fix meson-install-symlink.py overwriting existing links

By default, symlink re-creation fails if the link already exists, more
specifically in case of meson-install-symlink.py:

Traceback (most recent call last):
  File "/<path_to_libvirt_repo>/scripts/meson-install-symlink.py",
    line 15, in <module>
        os.symlink(target, link)
FileExistsError: File exists: '../default.xml' -> 'default.xml'

Unfortunately, Python can't mimic "ln -sf", so we have to fix this
differently - remove the existing link first and then try re-creating
it.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
5 years agoutil: Fix logic in virFileSetCOW
Jiri Denemark [Wed, 5 Aug 2020 08:01:45 +0000 (10:01 +0200)] 
util: Fix logic in virFileSetCOW

When COW is not explicitly requested to be disabled or enabled, the
function is supposed to do nothing on non-BTRFS file systems.

Fixes commit 7230bc95aa78379c9ee20cf59394c5fc4305b75b.

https://bugzilla.redhat.com/show_bug.cgi?id=1866157

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agotreat all NULL returns from virXMLNodeContentString() as an error
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>
5 years agoutil: log an error if virXMLNodeContentString will return NULL
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>
5 years agoutil: replace all calls to xmlNodeGetContent with virXMLNodeContentString
Laine Stump [Tue, 14 Jul 2020 04:21:28 +0000 (00:21 -0400)] 
util: replace all calls to xmlNodeGetContent with virXMLNodeContentString

No functional change

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoconf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent
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>
5 years agodocs: formatdomain: fyx a tipo
Ján Tomko [Tue, 4 Aug 2020 17:36:42 +0000 (19:36 +0200)] 
docs: formatdomain: fyx a tipo

s/hystorical/historical/

Signed-off-by: Ján Tomko <jtomko@redhat.com>
5 years agosrc: add G_GNUC_NO_INLINE annotations for mocked symbols
Daniel P. Berrangé [Tue, 4 Aug 2020 14:17:07 +0000 (15:17 +0100)] 
src: add G_GNUC_NO_INLINE annotations for mocked symbols

We should prevent inlining of symbols from the driver .so files that are
mocked, as well as those in the main libvirt.so

This isn't fixing any currently known problem, just trying to prevent
future issues.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agotests: qemublocktest: fix crashing with SIGBUS
Roman Bogorodskiy [Tue, 4 Aug 2020 16:07:00 +0000 (20:07 +0400)] 
tests: qemublocktest: fix crashing with SIGBUS

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>
5 years agodocs: fix libvirt go-import metadata in pages and links in 404 page
Daniel P. Berrangé [Tue, 4 Aug 2020 13:33:15 +0000 (14:33 +0100)] 
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>
5 years agobuild-aux: remove some syntax-check cruft
Daniel P. Berrangé [Tue, 4 Aug 2020 13:17:55 +0000 (14:17 +0100)] 
build-aux: remove some syntax-check cruft

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>
5 years agoconf: Add support for initiator IQN setting for iSCSI hostdevs
Peter Krempa [Fri, 10 Jul 2020 12:00:12 +0000 (14:00 +0200)] 
conf: Add support for initiator IQN setting for iSCSI hostdevs

We already allow controlling the initiator IQN for iSCSI based disks.
Add the same for host devices.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agodocs: formatdomain: Convert to rst
Peter Krempa [Fri, 10 Jul 2020 15:09:35 +0000 (17:09 +0200)] 
docs: formatdomain: Convert to rst

For now just plain conversion to rst. Anchors which existed until now
are preserved, but the table of contents now uses the docutils-generated
ones.

Additionally <code> which was nested in a link (<a>) was removed as rst
doesn't support nesting of inline markup.

The only anchor which wasn't restored is
'elementsDiskBackingStoreIndex' and its only reference was removed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agonetwork: Use single quotes in default network configuration
Andrea Bolognani [Tue, 4 Aug 2020 12:18:12 +0000 (14:18 +0200)] 
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>
5 years agomdev: Fix daemon crash when reattaching mdevs on assignment conflict
Binfeng Wu [Tue, 21 Jul 2020 09:21:10 +0000 (17:21 +0800)] 
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>
5 years agoci: Temporarily enable verbose mode for cirrus-run
Andrea Bolognani [Tue, 4 Aug 2020 10:00:41 +0000 (12:00 +0200)] 
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

  https://github.com/sio/cirrus-run/commit/5299874bbb71a84bec7476d9d0ca4ba49bb7ed97

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>
5 years agoci: Show build log for Cirrus CI jobs
Andrea Bolognani [Tue, 4 Aug 2020 10:00:20 +0000 (12:00 +0200)] 
ci: Show build log for Cirrus CI jobs

This feature has been implemented as of

  https://github.com/sio/cirrus-run/commit/b9b2278f1d6c45b22d5f84fca0621e77622a6da4

and will be included in the upcoming 0.3.0 release.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agosrc/logging: no need to include log_protocol.h in log_manager.h
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>
5 years agoremote: remove duplicated header
Pavel Hrdina [Tue, 4 Aug 2020 07:35:03 +0000 (09:35 +0200)] 
remote: remove duplicated header

We already include viraccessapicheck.h few lines above.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agomeson_options: change default value to auto for driver_esx
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>
5 years agomeson: Fix libvirtd|virtproxyd socket prefixes
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>
5 years agomeson: Fix "numaclt" typo
Erik Skultety [Mon, 3 Aug 2020 15:17:02 +0000 (17:17 +0200)] 
meson: Fix "numaclt" typo

Signed-off-by: Erik Skultety <eskultet@redhat.com>
5 years agoqemuDomainNamespaceTeardownInput: Deduplicate code
Michal Privoznik [Tue, 21 Jul 2020 14:27:52 +0000 (16:27 +0200)] 
qemuDomainNamespaceTeardownInput: Deduplicate code

We can use qemuDomainSetupInput() to obtain the path that we
need to unlink() from within domain's namespace.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuDomainNamespaceTeardownRNG: Deduplicate code
Michal Privoznik [Tue, 21 Jul 2020 14:26:48 +0000 (16:26 +0200)] 
qemuDomainNamespaceTeardownRNG: Deduplicate code

We can use qemuDomainSetupRNG() to obtain the path that we
need to unlink() from within domain's namespace.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuDomainNamespaceTeardownChardev: Deduplicate code
Michal Privoznik [Tue, 21 Jul 2020 14:25:44 +0000 (16:25 +0200)] 
qemuDomainNamespaceTeardownChardev: Deduplicate code

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>
5 years agoqemuDomainNamespaceTeardownMemory: Deduplicate code
Michal Privoznik [Tue, 21 Jul 2020 13:09:30 +0000 (15:09 +0200)] 
qemuDomainNamespaceTeardownMemory: Deduplicate code

We can use qemuDomainSetupMemory() to obtain the path that we
need to unlink() from within domain's namespace.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuDomainNamespaceTeardownHostdev: Unlink paths in one go
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>
5 years agoqemuDomainNamespaceUnlinkPaths: Turn @paths into string list
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>
5 years agoqemu_namespace: Rename qemuDomainNamespaceUnlinkPath() to qemuNamespaceUnlinkPath()
Michal Privoznik [Sat, 25 Jul 2020 07:33:50 +0000 (09:33 +0200)] 
qemu_namespace: Rename qemuDomainNamespaceUnlinkPath() to qemuNamespaceUnlinkPath()

To match how Mknod counterpart was renamed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuDomainDetachDeviceUnlink: Unlink paths in one go
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>
5 years agoqemu_domain_namespace: Drop unused functions
Michal Privoznik [Tue, 21 Jul 2020 17:14:40 +0000 (19:14 +0200)] 
qemu_domain_namespace: Drop unused functions

After previous cleanup, creating /dev nodes from pre-exec hook is
no longer needed and thus can be removed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuDomainBuildNamespace: Populate SEV from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate loader from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate RNGs from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate inputs from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate graphics from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate TPM from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate chardevs from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate memory from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate hostdevs from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate disks from daemon's namespace
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>
5 years agoqemuDomainBuildNamespace: Populate basic /dev from daemon's namespace
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>
5 years agoqemu_domain_namespace: Repurpose qemuDomainBuildNamespace()
Michal Privoznik [Tue, 21 Jul 2020 16:12:26 +0000 (18:12 +0200)] 
qemu_domain_namespace: Repurpose qemuDomainBuildNamespace()

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>
5 years agoqemuDomainSetupDisk: Accept @src
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>
5 years agoqemuNamespaceMknodPaths: Turn @paths into string list
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>
5 years agoqemuDomainNamespaceMknodPaths: Create more files in one go
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>
5 years agoqemuDomainAttachDeviceMknodHelper: Create more files in a single go
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>
5 years agoqemuDomainAttachDeviceMknodRecursive: Isolate bind mounted devices condition
Michal Privoznik [Tue, 21 Jul 2020 07:44:36 +0000 (09:44 +0200)] 
qemuDomainAttachDeviceMknodRecursive: Isolate bind mounted devices condition

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>
5 years agoqemu_domain_namespace.c: Rename qemuDomainAttachDeviceMknodData
Michal Privoznik [Mon, 20 Jul 2020 18:01:54 +0000 (20:01 +0200)] 
qemu_domain_namespace.c: Rename qemuDomainAttachDeviceMknodData

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>
5 years agoqemuDomainAttachDeviceMknodHelper: Don't leak data->target
Michal Privoznik [Mon, 20 Jul 2020 16:54:34 +0000 (18:54 +0200)] 
qemuDomainAttachDeviceMknodHelper: Don't leak data->target

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>
5 years agoqemuDomainNamespaceSetupHostdev: Create paths in one go
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>
5 years agoqemu_domain_namespace: Check for namespace enablement earlier
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>
5 years agoqemu_domain_namespace: Drop unused @cfg argument
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>
5 years agoqemu_domain_namespace: Rename qemuDomainCreateNamespace()
Michal Privoznik [Mon, 20 Jul 2020 14:54:11 +0000 (16:54 +0200)] 
qemu_domain_namespace: Rename qemuDomainCreateNamespace()

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>
5 years agoqemu: Separate out namespace handling code
Michal Privoznik [Mon, 20 Jul 2020 14:18:57 +0000 (16:18 +0200)] 
qemu: Separate out namespace handling code

The qemu_domain.c file is big as is and we should split it into
separate semantic blocks. Start with code that handles domain
namespaces.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemu: capabilities: add missing comma
Ján Tomko [Mon, 3 Aug 2020 14:20:28 +0000 (16:20 +0200)] 
qemu: capabilities: add missing comma

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: bab9257a649386a908069ba890a23f2a5ff5b131
5 years agodocs: css: Hide redundant 'Contents:' line in table of contents of RST docs
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>
5 years agodocs: css: Add separation for table of contents generated from RST
Peter Krempa [Mon, 3 Aug 2020 06:23:11 +0000 (08:23 +0200)] 
docs: css: Add separation for table of contents generated from RST

The table of contents of documents generated from RST is quite squeezed
together. Add 2em-s worth of vertical separation on each side.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agodocs: css: Make definition list 'code' entries bold when converted from rst
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>
5 years agodocs: build: Don't include stylesheet in intermediate html files generated from RST
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>
5 years agokbase: debuglogs: Add some more example filters settings
Peter Krempa [Mon, 3 Aug 2020 10:33:09 +0000 (12:33 +0200)] 
kbase: debuglogs: Add some more example filters settings

Show various usage of filters including some useful examples.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
5 years agokbase: debuglogs: Include more data to attach to bugreports
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>
5 years agodocs: build: Use proper path to file in 'contribute' link
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>
5 years agodocs: meson: Simplify generation of html files
Peter Krempa [Wed, 22 Jul 2020 18:20:43 +0000 (20:20 +0200)] 
docs: meson: Simplify generation of html files

Just convert 'docs_html_in_files' into 'docs_html_in_gen'. The target
definitions for those were almost the same.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoRemove unused variables
Ján Tomko [Mon, 3 Aug 2020 11:36:39 +0000 (13:36 +0200)] 
Remove unused variables

These variables are only used for assignment and have
no other effect.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
5 years agoutil: cgroup: remove unused opts in virCgroupV2BindMount
Ján Tomko [Mon, 3 Aug 2020 11:06:54 +0000 (13:06 +0200)] 
util: cgroup: remove unused opts in virCgroupV2BindMount

In virCgroupV2BindMount there is an unused variable containing
what seem to be tmpfs mount options.

Delete it. Unlike with cgroups v1, we do not create a tmpfs
here.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
5 years agoutil: delete virStringListFree
Ján Tomko [Sun, 2 Aug 2020 22:00:55 +0000 (00:00 +0200)] 
util: delete virStringListFree

Now that everything uses g_strfreev, this function is no longer
needed.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: recommend g_strfreev instead of virStringListFree
Ján Tomko [Sun, 2 Aug 2020 22:01:11 +0000 (00:01 +0200)] 
util: recommend g_strfreev instead of virStringListFree

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoUse g_strfeev instead of virStringFreeList
Ján Tomko [Sun, 2 Aug 2020 17:36:03 +0000 (19:36 +0200)] 
Use g_strfeev instead of virStringFreeList

Both accept a NULL value gracefully and virStringFreeList
does not zero the pointer afterwards, so a straight replace
is safe.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: remove virStringListCopy
Ján Tomko [Sun, 2 Aug 2020 11:37:20 +0000 (13:37 +0200)] 
util: remove virStringListCopy

The g_strdupv function from GLib provides
the same functionality.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoconf: use g_strdupv instead of virStringListCopy
Ján Tomko [Sun, 2 Aug 2020 11:33:55 +0000 (13:33 +0200)] 
conf: use g_strdupv instead of virStringListCopy

Also remove the temporary variable - even virStringListCopy
aborts on OOM now.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agovirt-login-shell: correctly calculate string length
Ján Tomko [Sun, 2 Aug 2020 12:58:38 +0000 (14:58 +0200)] 
virt-login-shell: correctly calculate string length

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>
5 years agotools: remove unnecessary includes
Ján Tomko [Sun, 23 Feb 2020 21:45:34 +0000 (22:45 +0100)] 
tools: remove unnecessary includes

After the split of virsh to multiple files, and the subsequent
split to vsh/virt-admin, there are quite a few leftovers.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: virlog: unexport virLogVMessage
Ján Tomko [Thu, 13 Feb 2020 23:12:24 +0000 (00:12 +0100)] 
util: virlog: unexport virLogVMessage

Last usage out of virlog.c was removed by
commit 91268c715cf0293f0751de0450e4d0c06bea18d8
    node_device_udev: remove deprecated logging function

Also drop the virbuffer.h include - it seems it was never used
for anything else than the transitive stdarg.h include.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: log: move virLogMessage
Ján Tomko [Sun, 16 Feb 2020 20:22:18 +0000 (21:22 +0100)] 
util: log: move virLogMessage

This function calls virLogVMessage. Move it below the definition
of virLogVMessage so it can call it even without a prototype.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: viruri: move libxml include
Ján Tomko [Sun, 16 Feb 2020 22:02:12 +0000 (23:02 +0100)] 
util: viruri: move libxml include

The XML function is needed in the C file,
not in the header.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agohyperv: include virxml.h
Ján Tomko [Sun, 23 Feb 2020 22:01:44 +0000 (23:01 +0100)] 
hyperv: include virxml.h

This file is using XML functions without including
the header.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
5 years agoutil: virstring.h: remove stdarg.h include
Ján Tomko [Thu, 13 Feb 2020 22:59:22 +0000 (23:59 +0100)] 
util: virstring.h: remove stdarg.h include

It was needed for virAsprintf, which is now dropped.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 33ed622106c61d257fd4a5750d4df511bcdd1b2b
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>