Commit Graph

38825 Commits

Author SHA1 Message Date
Christian Ehrhardt
a132ba9035
apparmor: fix code style error in reduced if statement
sc_spacing-check  FAIL reporting a case of "Curly brackets around
single-line body:" in a recent commit.

Fixes: d9c21f4b "apparmor: allow adding permanent per guest rules"

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
2020-08-13 15:39:26 +02:00
Christian Ehrhardt
d61d8206f3
apparmor: allow unmounting .dev entries
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.

[1]: https://www.redhat.com/archives/libvir-list/2020-August/msg00236.html

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2020-08-13 12:52:45 +02:00
Christian Ehrhardt
d9c21f4bfc
apparmor: allow adding permanent per guest rules
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.

[1]: https://gitlab.com/apparmor/apparmor/-/wikis/Libvirt#advanced-usage

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>
2020-08-13 12:52:41 +02:00
Pavel Hrdina
a6886aafac 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.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-08-12 14:43:01 +02:00
Erik Skultety
61e56729ff 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>
2020-08-12 09:10:36 +02:00
Ján Tomko
551d1aa1fd docs: formatdomain: fix superscripts
There needs to be a space before the :sup: directive.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2020-08-12 00:18:23 +02:00
Ján Tomko
05c1b9e8e8 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>
2020-08-11 21:49:54 +02:00
Pavel Hrdina
7e574d1a07 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>
2020-08-11 15:11:15 +02:00
Roman Bogorodskiy
4ae4e991e0 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>
2020-08-10 18:40:32 +04:00
Roman Bogorodskiy
154ecac509 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>
2020-08-10 18:40:32 +04:00
Roman Bogorodskiy
f7ccff027e 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>
2020-08-10 18:40:19 +04:00
Christian Ehrhardt
3ef2af8ed3
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>
2020-08-10 07:32:07 +02:00
Stefan Bader
7c5ef98c00
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>
2020-08-10 07:32:06 +02:00
Jamie Strandboge
e16967fd6e
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>
2020-08-10 07:32:06 +02:00
Stefan Bader
8b6ee1afdb
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>
2020-08-10 07:32:06 +02:00
Sam Hartman
155d4fe3fa
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>
2020-08-10 07:32:05 +02:00
Pavel Hrdina
b94cde18ff 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>
2020-08-08 11:07:17 +02:00
Pavel Hrdina
6a00352f67 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>
2020-08-08 11:07:15 +02:00
Pavel Hrdina
bd53831e67 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>
2020-08-08 11:07:02 +02:00
Daniel P. Berrangé
4b696beee3 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>
2020-08-07 13:51:16 +01:00
Daniel P. Berrangé
11188d5a19 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>
2020-08-07 12:44:57 +01:00
Daniel P. Berrangé
2998ba2012 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>
2020-08-07 12:44:05 +01:00
Daniel P. Berrangé
0db4743645 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>
2020-08-07 12:43:59 +01:00
Daniel P. Berrangé
da0a182708 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>
2020-08-07 12:43:56 +01:00
Pavel Hrdina
84bb5fd1ab 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>
2020-08-05 13:32:59 +02:00
Pavel Hrdina
f1a298c20d 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>
2020-08-05 13:32:59 +02:00
Erik Skultety
3a29b2fb79 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>
2020-08-05 13:11:16 +02:00
Jiri Denemark
2edd63a0db 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 7230bc95aa.

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>
2020-08-05 11:04:17 +02:00
Laine Stump
d293a556d7 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>
2020-08-05 00:06:27 -04:00
Laine Stump
cb373a0068 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>
2020-08-05 00:04:48 -04:00
Laine Stump
c42e161000 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>
2020-08-05 00:04:26 -04:00
Laine Stump
b595f44525 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>
2020-08-05 00:00:18 -04:00
Ján Tomko
ed8d1385f7 docs: formatdomain: fyx a tipo
s/hystorical/historical/

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2020-08-04 19:36:42 +02:00
Daniel P. Berrangé
ba6d9264c6 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>
2020-08-04 17:58:08 +01:00
Roman Bogorodskiy
e960bedee9 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>
2020-08-04 20:19:48 +04:00
Daniel P. Berrangé
08376431b4 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>
2020-08-04 17:06:33 +01:00
Daniel P. Berrangé
abab5c47f8 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>
2020-08-04 16:44:56 +01:00
Peter Krempa
90df0f8288 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>
2020-08-04 15:49:13 +02:00
Peter Krempa
fdc0d13b2c 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>
2020-08-04 15:49:12 +02:00
Andrea Bolognani
2b9e277106 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>
2020-08-04 15:12:18 +02:00
Binfeng Wu
8361d335ab 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>
2020-08-04 14:03:54 +02:00
Andrea Bolognani
9b5622d31f 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

  5299874bbb

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>
2020-08-04 13:29:34 +02:00
Andrea Bolognani
d8f13661e0 ci: Show build log for Cirrus CI jobs
This feature has been implemented as of

  b9b2278f1d

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>
2020-08-04 13:29:34 +02:00
Pavel Hrdina
76e79e0e77 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>
2020-08-04 13:28:22 +02:00
Pavel Hrdina
fdb92c9dac 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>
2020-08-04 13:28:19 +02:00
Pavel Hrdina
657263fbc6 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>
2020-08-04 11:25:51 +02:00
Erik Skultety
a4a20cc34f 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: dd4f2c73ad

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-08-04 10:41:26 +02:00
Erik Skultety
b175de4e18 meson: Fix "numaclt" typo
Signed-off-by: Erik Skultety <eskultet@redhat.com>
2020-08-04 09:16:01 +02:00
Michal Privoznik
f4f3e6de4a 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>
2020-08-03 20:01:01 +02:00
Michal Privoznik
b9338334d5 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>
2020-08-03 20:00:34 +02:00