Commit Graph

30164 Commits

Author SHA1 Message Date
Kristina Hanicova
b5d25b8183 domain_validate: Move virDomainCheckVirtioOptionsAreAbsent() a few lines forward
Move this function in order to use it in the next patch before
its previous declaration.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 15:03:01 +01:00
Kristina Hanicova
e6c6efc208 domain_conf: Refactor virDomainDeviceInfoParseXML to use XPath
Currently, virDomainDeviceInfoParseXML() uses node->children
evaluation which is too verbose. Use XPath evaluation which is
nicer.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 14:30:25 +01:00
Kristina Hanicova
df3359fb18 domain_conf: Propagate xmlXPathContextPtr into virDomainDeviceInfoParseXML()
Function virDomainDeviceInfoParseXML() will need it soon, because it
will be doing XPath evaluation.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 14:30:16 +01:00
Jonathon Jongsma
caf23cdc9b nodedev: Don't crash when exiting before init is done
If libvirtd is terminated before the node driver finishes
initialization, it can crash with a backtrace similar to the following:

    Stack trace of thread 1922933:
    #0  0x00007f8515178774 g_hash_table_find (libglib-2.0.so.0)
    #1  0x00007f851593ea98 virHashSearch (libvirt.so.0)
    #2  0x00007f8515a1dd83 virNodeDeviceObjListSearch (libvirt.so.0)
    #3  0x00007f84cceb40a1 udevAddOneDevice (libvirt_driver_nodedev.so)
    #4  0x00007f84cceb5fae nodeStateInitializeEnumerate (libvirt_driver_nodedev.so)
    #5  0x00007f85159840cb virThreadHelper (libvirt.so.0)
    #6  0x00007f8511c7d14a start_thread (libpthread.so.0)
    #7  0x00007f851442bdb3 __clone (libc.so.6)

    Stack trace of thread 1922863:
    #0  0x00007f851442651d syscall (libc.so.6)
    #1  0x00007f85159842d4 virThreadSelfID (libvirt.so.0)
    #2  0x00007f851594e240 virLogFormatString (libvirt.so.0)
    #3  0x00007f851596635d vir_object_finalize (libvirt.so.0)
    #4  0x00007f8514efe8e9 g_object_unref (libgobject-2.0.so.0)
    #5  0x00007f85159667f8 virObjectUnref (libvirt.so.0)
    #6  0x00007f851517755f g_hash_table_remove_all_nodes.part.0 (libglib-2.0.so.0)
    #7  0x00007f8515177e62 g_hash_table_unref (libglib-2.0.so.0)
    #8  0x00007f851596637e vir_object_finalize (libvirt.so.0)
    #9  0x00007f8514efe8e9 g_object_unref (libgobject-2.0.so.0)
    #10 0x00007f85159667f8 virObjectUnref (libvirt.so.0)
    #11 0x00007f84cceb2b42 nodeStateCleanup (libvirt_driver_nodedev.so)
    #12 0x00007f8515b37950 virStateCleanup (libvirt.so.0)
    #13 0x00005648085348e8 main (libvirtd)
    #14 0x00007f8514352493 __libc_start_main (libc.so.6)
    #15 0x00005648085350fe _start (libvirtd)

This is because the initial population of the device list is done in a
separate initialization thread. If we attempt to exit libvirtd before
this init thread has completed, we'll try to free the device list while
accessing it from the other thread. In order to guarantee that this
init thread is not accessing the device list when we're cleaning up the
nodedev driver, make it joinable and wait for it to finish before
proceding with the cleanup. This is similar to how we handle the udev
event handler thread.

The separate initialization thread was added in commit
9f0ae0b1.

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

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 13:09:44 +01:00
Kristina Hanicova
aaa98e3cfa virxml: Fix possible memory leak in virXMLNodeContentString()
Previously, if xml node passed to the virXMLNodeContentString()
was not of type XML_ELEMENT_NODE, @ret could have caused a memory
leak because xmlNodeGetContent() works for other types of nodes
as well.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 12:45:05 +01:00
Hao Wang
fee42ea120 migration/dirtyrate: Extend dirtyrate statistics for domGetStats
Extend dirtyrate statistics for domGetStats to display the information
of a domain's memory dirty rate produced by domainStartDirtyRateCalc.

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 08:50:25 +01:00
Hao Wang
4ae60b1caf migration/dirtyrate: Implement qemuMonitorQueryDirtyRate
Implement qemuMonitorQueryDirtyRate which query domain's memory
dirty rate calling qmp "query-dirty-rate".

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 08:50:25 +01:00
Hao Wang
fbe99823e2 migration/dirtyrate: Implement qemuDomainStartDirtyRateCalc
Implement qemuDomainStartDirtyRateCalc which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 08:50:25 +01:00
Hao Wang
df5c5c3e60 migration/dirtyrate: Introduce virDomainStartDirtyRateCalc API
Introduce virDomainStartDirtyRateCalc API for start calculation of
a domain's memory dirty rate with a specified time.

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-18 08:50:25 +01:00
Daniel P. Berrangé
0a92f70c8f docs: stop mentioning insecure / broken SASL mechanisms
We don't need to go to the trouble of telling users about existance of
insecure SASL mechanisms only to then say that they shouldn't be used.
We should only tell people about the GSSAPI mechanism for TCP sockets.

For the SCRAM mechanism we should be telling people about the SHA256
variant only, and also warning that the password database stores the
passwords in clear text.

Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-17 09:31:57 +00:00
Daniel P. Berrangé
fa56310e18 util: tell users that memory locking ulimit is too low for BPF
If running libvirtd via systemd, it gets a 64 MB memlock limit, but if
running from the shell it will only get 64 KB on a Fedora 33 system.
The latter low limit causes any attempt to use BPF to fail and it is
not obvious why.

This improves the error message thus:

  # virsh -c lxc:/// start sh
error: Failed to start domain 'sh'
error: internal error: guest failed to start: Failure in libvirt_lxc startup: failed to initialize device BPF map; locked memory limit for libvirtd probably needs to be raised: Operation not permitted

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-17 09:16:44 +00:00
Daniel P. Berrangé
695bdb3841 src: ensure GSource background unref happens in correct event loop
The g_idle_add function adds a callback to the primary GMainContext.

To workaround the GSource unref bugs, we need to add our callbacks
to the GMainContext that is associated with the GSource being
unref'd. Thus code using the per-VM virEventThread must use its
private GMainContext.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-17 09:16:05 +00:00
Michal Privoznik
d8576d70e5 lib: Debug print all arguments of virNetworkUpdate()
Somehow, command argument was not printed into debug logs. It is
imperative that all arguments are logged.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-16 09:17:08 +01:00
Michal Privoznik
6e9c4811be qemu_process: Use accessor for def->mem.total_memory
When connecting to the monitor, a timeout is calculated that is
bigger the more memory guest has (because QEMU has to allocate
and possibly zero out the memory and what not, empirically
deducted). However, when computing the timeout the @total_memory
mmember is accessed directly even though
virDomainDefGetMemoryTotal() should have been used.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-16 09:16:13 +01:00
Pavel Hrdina
10bf55c99f qemu_driver: fix setting vcpu_quota if not all vCPUs are online
When switching to g_autoptr this was incorrectly changed from
'continue;' into 'return -1;' resulting into an error when user tries
to set vcpu_quota of running VM:

    error: An error occurred, but the cause is unknown

Fixes: e4a8bbfaf2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-03-15 15:00:53 +01:00
Michal Privoznik
9d3cd0c1d4 lib: Put some variable declarations on individual lines
In short, virXXXPtr type is going away. With big bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-15 09:38:18 +01:00
Michal Privoznik
a1d058a1db virconftypes: Fix name of virCapsGuestArchPtr
The name is supposed to be virCapsGuestArchPtr not ..ptr.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-15 09:37:33 +01:00
Michal Privoznik
4fc66ab262 gendispatch: Don't use virXXXPtr for internal types
The use of virXXXPtr is going away soon, therefore use 'virXXX *'
instead.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-15 09:36:21 +01:00
Michal Privoznik
ab9afee6e7 virsysinfo: Define and use auto cleanup func for virSysinfoDef properly
What we are using really is heap allocated structure rather than
stack allocated. And for that it's better to use g_autoptr() +
G_DEFINE_AUTOPTR_CLEANUP_FUNC() combo, as Glib documentation for
g_auto() reads:

  This is meant to be used with stack-allocated structures and
  non-pointer types. For the (more commonly used) pointer
  version, see g_autoptr().

This will be even more visible, when virSysinfoDefPtr type is
gone. Stay tuned.

Fixes: cee3a900a0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-15 09:34:18 +01:00
Michal Privoznik
52a937d8a2 qemu_shim: Don't hang if failed to start domain
The qemu shim spawns a separate thread in which the event loop is
ran. The virEventRunDefaultImpl() call is wrapped in a while()
loop, just like it should. There are few lines of code around
which try to ensure that domain is destroyed (when quitting) and
that the last round of event loop is ran after the
virDomainDestroy() call. Only after that the loop is quit from
and the thread quits.

However, if domain creation fails, there is no @dom to call
destroy over, the @quit flag is never set and while() never
exits. Set the flag regardless of @dom pointer.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920337
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-03-12 17:05:52 +01:00
Peter Krempa
ba2593bddc qemu: command: Generate commandline of iothread objects JSON
The commandline generator for 'iothread' objects has a private
implementation of the properties. Convert it to JSON so that it can be
later validated.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 16:15:05 +01:00
Peter Krempa
9d5a668d4a qemu: command: Generate commandline of 'sev0' sev-guest object via JSON
While the 'sev0' sev-guest object will never be hotplugged, but we want
to generate it through JSON so that we'll be able to validate all
parameters of '-object' against the QAPI schema once 'object-add' is
qapified in qemu.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 16:15:05 +01:00
Peter Krempa
f4e00d6d61 qemu: command: Generate commandline of 'masterKey0' secret via JSON
While the 'masterKey0' secret object will never be hotplugged we want to
generate it through JSON so that we'll be able to validate all
parameters of '-object' against the QAPI schema once 'object-add' is
qapified in qemu.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 16:15:05 +01:00
Peter Krempa
ccc6dd8f11 virtlo(g|ck)d: Fix exec-restart
Commit 94e45d1042 broke exec-restart of virtlogd and virtlockd as the
code waiting for the daemon shutdown closed the daemons before
exec-restarting.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912243
Fixes: 94e45d1042
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-12 16:08:28 +01:00
Peter Krempa
c363f03e6d virnetdaemon: Introduce virNetDaemonQuitExecRestart
Recent changes which meant to fix daemon shutdown broke the exec-restart
capability of virtlogd and virtlockd, since the code actually closed all
the sockets and shut down all the internals.

Add virNetDaemonQuitExecRestart, which requests a shutdown of the
process, but keeps all the services open and registered since they are
preserved across the restart.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-12 16:08:28 +01:00
Michal Privoznik
76f3b2988b qemu_shim: Always pre-create root dir
This problem is reproducible only with secret driver. When
starting a domain via virt-qemu-run and both secret and
(nonexistent) root directory specified this is what happens:

1) virt-qemu-run opens "secret:///embed?root=$rootdir"
   connection, which results in the secret driver initialization
   (done in secretStateInitialize()). During this process, the
   driver creates its own configDir (derived from $rootdir)
   including those parents which don't exists yet. This is all
   done with the mode S_IRWXU and thus results in the $rootdir
   being created with very restrictive mode (specifically, +x is
   missing for group and others).

2) now, virt-qemu-run opens "qemu:///embed?root=$rootdir" and
   calls virDomainCreateXML(). This results in the master-key.aes
   being written somewhere under the $rootdir and telling qemu
   where to find it.

But because the secret driver created $rootdir with too
restrictive mode, qemu can't access the file (even though it
knows the full path) and fails to start.

It looks like the best solution is to pre-create the root
directory before opening any connection (letting any driver
initialize itself) and set its mode to something less
restrictive.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1859873
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-03-12 15:40:13 +01:00
Michal Privoznik
b1fef73225 virConnectOpen: Require root dir to be absolute path
In theory, users might want to use a relative path as a root
directory for embed drivers. But in practice, nothing in driver
initialization (specifically QEMU driver since it's the only one
that supports embedding now), is prepared for that. Document and
enforce absolute paths.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1883725
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-03-12 15:40:13 +01:00
Peter Krempa
eab7ae6bfe virLockSpaceNewPostExecRestart: Fix out-of-bounds array access
'res->owners' is allocated to 'res->nOwners' elements, but unfortunately
'res->nOwners' doesn't contain the proper value until after the
allocation so 0 elements are allocated. The following loop which assumes
that the array has the right number of elements then accesses the
pointer out of bounds. The bug was also faithfully converted from
VIR_ALLOC_N to g_new0.

Fixes: 4a3d6ed5ee
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-12 11:02:35 +01:00
Peter Krempa
6b8e961399 virLockSpacePreExecRestart: Avoid use-after-free
Recent refactor marked 'object' which is returned from the function as
autofree but forgot to use g_steal_pointer in the return statement to
prevent freeing it.

Fixes: 9a1651f64d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-12 11:02:35 +01:00
Peter Krempa
55d175c073 qemuBackupJobTerminate: Fix job termination for inactive VMs
Commit cb29e4e801 didn't take into account that the VM can be inactive
when it's destroyed. This means that the job would remain active also
when the VM became inactive.

To fix this properly:

1) Remove the bogus VM liveness check and early return
    (reverts the aforementioned commit)

2) Conditionalize the stats assignment only when the stats object is
   present
    (properly fix the crash when VM dies when reconnecting)

3) end the asyncjob only when it was already set
   (prevent corruption of priv->jobs_queued)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937598
Fixes: cb29e4e801
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 10:59:05 +01:00
Peter Krempa
aa372e5a01 backup: Store 'apiFlags' in private section of virDomainBackupDef
'qemuBackupJobTerminate' needs the API flags to see whether
VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL. Unfortunately when called via
qemuProcessReconnect()->qemuProcessStop() early (e.g. if the qemu
process died while we were reconnecting) the job is cleared temporarily
so that other APIs can be called. This would mean that we couldn't clean
up the files in some cases.

Save the 'apiFlags' inside the backup object and set it from the
'qemuDomainJobObj' 'apiFlags' member when reconnecting to a VM.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 10:59:05 +01:00
Peter Krempa
07c6e493b2 virSystemdCreateMachine: Use proper format string for uint64_t when constructing gvariant
g_variant_new_parsed uses '%t' for a uint64_t rather than printf-like
%llu. Additionally ensure that the passed value is a uint64_t since the
argument used is a 'unsigned int'.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937287
Fixes: bf5f2ed09c
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-03-12 10:59:05 +01:00
Peter Krempa
d380dd0efd util: virstring: Remove virStrncpy
The function is now unused and motivated users to write crazy parsers
which were hard to understand, had pointless error paths just to avoid
few memory allocations.

Remove the function as we're fine with g_strndup and virStrcpy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 10:57:42 +01:00
Peter Krempa
a7cb4dbca5 xenParseVif: Refactor parser
Use g_strsplit to split the string and avoid use of stack'd strings.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-12 10:57:42 +01:00
Michal Privoznik
c2840e90ea virutil: Do not use g_get_host_name() to obtain hostname
The problem is that g_get_host_name() caches the hostname in a
thread local variable. Therefore, it doesn't reflect any
subsequent hostname changes. While this might be acceptable for
logs where the hostname is printed exactly once when the libvirtd
starts up, it is not optimal for virGetHostnameImpl() which is
what our public virConnectGetHostname() API calls. If the
hostname at the moment of the first API invocation happens to
start with "localhost" or contains a dot, then no further
hostname changes will ever be reflected.

This reverts 26d9748ff1, partially.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-12 08:45:04 +01:00
Peter Krempa
2d91b1405d virNetLibsshAuthenticatePrivkeyCb: Use virStrcpy instead of virStrncpy
We already assume that 'retr_passphrase.result' is a string, thus we can
use virStrcpy instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
1b50f2d102 virNetLibsshAuthenticatePrivkeyCb: Use g_autofree for 'actual_prompt'
So that the 'error' label can be removed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
08372887ec xenParseXLUSB: Rewrite to avoid virStrncpy
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
207a9db8eb xenParseXLUSBController: Avoid use of virStrncpy
Use g_strndup with a freed buffer instead of the more complex approach
using virStrncpy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
bd1728e969 xenParseXLChannel: Use g_strndup instead of virStrncpy
Make the temporary string an autofree-ing pointer and copy the contents.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
dae4dddb12 openvzReadNetworkConf: Rework parser
Rewrite so that the parser doesn't use virStrncpy by employing
g_strsplit.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Peter Krempa
8b0e845d67 xenParseSxprSound: Refactor parsing of model list
Copy the input string so that we don't have to use a static buffer and
virStrncpy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-11 18:00:35 +01:00
Kristina Hanicova
f11f32326f XML validate that 'ramfb' has no address
With this, XML fails if config video type 'ramfb' contains
address, since address is not supported for 'ramfb' video
devices. Previously it didn't raise error.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1891416
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-10 20:07:09 +01:00
Yi Li
7dfa87aed3 virQEMUCapsInitQMPArch: Refactor cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Yi Li <yili@winhong.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-10 19:57:49 +01:00
Daniel P. Berrangé
16634feb1d qemu: wire up support for backend specific audio settings
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:53 +00:00
Daniel P. Berrangé
3e97d81a81 conf: add support for audio backend specific settings
This pulls in the remaining QEMU audio backend specific settings to the
XML schema.

    <audio id="1" type="alsa">
      <input dev="/dev/dsp0"/>
      <output dev="/dev/dsp1"/>
    </audio>

    <audio id="1" type="coreaudio">
      <input bufferCount="50"/>
      <output bufferCount="42"/>
    </audio>

    <audio id="1" type="file" path="audio.wav"/>

    <audio id="1" type="jack">
      <input serverName="fish" clientName="food" connectPorts="yum"/>
      <output serverName="fish" clientName="food" connectPorts="yum"/>
    </audio>

    <audio id="1" type="oss" tryMMap="yes" exclusive="yes" dspPolicy="3">
      <input dev="/dev/dsp0" bufferCount="50" tryPoll="yes"/>
      <output dev="/dev/dsp1" bufferCount="30" tryPoll="no"/>
    </audio>

    <audio id="1" type="pulseaudio" serverName="acme.example.org">
      <input name="fish" streamName="food" latency="100"/>
      <output name="fish" streamName="food" latency="200"/>
    </audio>

    <audio type='sdl' id='1' driver='pulseaudio'>
      <input bufferCount='40'/>
      <output bufferCount='40'/>
    </audio>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:49 +00:00
Daniel P. Berrangé
c99e72d18d qemu: wire up support for common audio backend settings
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:45 +00:00
Daniel P. Berrangé
46b77b3e6a conf: introduce support for common audio settings
This introduces support for the QEMU audio settings that are common to
all audio backends. These are expressed in the QAPI schema as settings
common to all backends, but in reality some backends ignore some of
them. For example, some backends are output only. The parser isn't
attempting to apply restrictions that QEMU itself doesn't apply.

    <audio id='1' type='pulseaudio'>
      <input mixingEngine='yes' fixedSettings='yes' voices='1' bufferLength='100'>
        <settings frequency='44100' channels='2' format='s16'/>
      </input>
      <output mixingEngine='yes' fixedSettings='yes' voices='2' bufferLength='100'>
        <settings frequency='22050' channels='4' format='f32'/>
      </output>
    </audio>

The <settings> child is only valid if fixedSettings='yes'

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:39 +00:00
Daniel P. Berrangé
d96fb5cb31 qemu: add support for generating -audiodev arguments
The -audiodev argument is replacing the QEMU_AUDIO_DRV env variable (and
its relations).

Sadly we still have to use the SDL_AUDIODRIVER env variable because that
wasn't mapped into QAPI schema.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:36 +00:00
Daniel P. Berrangé
e07994aade qemu: probe for -vnc audiodev property
The -audiodev arg is a new way to configure audio devices in QEMU to
replace the QEMU_AUDIO_DRV env variable. This arg is not visible in
the "query-command-line-options" output since it is entirely QAPI
driven, not QemuOpts. It also isn't in "query-qmp-schema" though
since there's no QMP command that uses the Audiodev type yet.

So probe for the existance of this feature by looking for the
-vnc "audiodev" property. This won't let us determine which
precise audio backends QEMU has been built with, but for now
that's no worse than with env variables today.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:33 +00:00
Daniel P. Berrangé
e88367095f qemu: populate <audio> element with default config
Currently the QEMU driver secretly sets the QEMU_AUDIO_DRV env variable

 - VNC - set to "none", unless passthrough of host env variable is set
 - SPICE - always set to "spice"
 - SDL - always passthrough host env
 - No graphics - set to "none", unless passthrough of host env variable is set

The setting of the QEMU_AUDIO_DRV env variable is done in the code which
configures graphics.

If no <audio> element is present, we now auto-populate <audio> elements
to reflect this historical default config. This avoids need to set audio
env when processing graphics.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:30 +00:00
Daniel P. Berrangé
6be99c99c5 qemu: support use of <audio> elements
Currently the QEMU driver secretly sets the QEMU_AUDIO_DRV env variable
depending on how <graphics> are configured.

This introduces support for configuring audio backends from the <audio>
elements in the XML config.

The existing default behaviour is now only used if no <audio> element is
present.

All except the 'jack' audio driver are supported via QEMU's old env
variable config.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:26 +00:00
Daniel P. Berrangé
efdab67f44 conf: rename and improve virDomainDefFindAudioForSound
The virDomainDefFindAudioForSound only takes a virDomainSoundDefPtr as
its arg, but we want to use the same functionality for VNC graphics.
In addition if audio ID is zero, then we want to return the first
available audio backend.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:23 +00:00
Daniel P. Berrangé
562a495b92 conf: add validation of audio backend IDs
Validate that if a non-zero audio ID is given for <sound> or <graphics>
elements, it must map to an <audio> backend that exists.

Validate that audio IDs given in <audio> are unique.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:20 +00:00
Daniel P. Berrangé
cf1c5c6344 conf: add support for audio backend for the VNC server
When there are multiple <audio> backends specified, it is possible to
assign a specific one to the VNC server using

  <graphics type='vnc'...>
    <audio id='1'/>
  </graphics>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:18 +00:00
Daniel P. Berrangé
915b637257 conf: add coverage for all QEMU audio backend types
The current <audio> element only allows an "OSS" audio backend, as this
is all that BHyve needed. This is now extended to cover most QEMU audio
backends. These backends all have a variety of attributes they support,
but this initial impl does the bare minimum, relying on built-in
defaults for everything. The only QEMU backend omitted is "dsound" since
the libvirt QEMU driver is not built on Windows platforms.

The SDL audio driver names are based on the SDL 2.0 drivers. It is not
intended to support SDL 1.2 drivers.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:15 +00:00
Daniel P. Berrangé
8149518ee1 conf: refactor OSS audio backend specific options
To prepare for the introduction for more backend specific audio options,
move the OSS options into a dedicated struct and introduce separate
helper methods for parse/format/free.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:12 +00:00
Daniel P. Berrangé
d0ae9b429a conf: add missing iteration over audio backends
Fixes 9375bc7373
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:07 +00:00
Daniel P. Berrangé
657999b04f conf: add helper to test for sound device codec support
The check for ICH6 || ICH9 is repeated in many places in the code. The
new virDomainSoundModelSupportsCodecs() method provides a helper to
standardize this check.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:04 +00:00
Daniel P. Berrangé
fe304b02f0 conf: don't force existance of audio child elements
The attributes on the elements are optional, so we should not force the
elements themselves to be present, especially since we omit them when
formating the XML thus breaking round-tripping.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:58:00 +00:00
Daniel P. Berrangé
d6b79a64e6 conf: stronger error reporting when parsing audio related params
Check for varuous mandatory elements and improve error message
clarity

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-09 22:57:48 +00:00
Aleksei Zakharov
4719ec15e9 qemu: add per-vcpu delay stats
This patch adds delay time (steal time inside guest) to libvirt
domain per-vcpu stats. Delay time is an important performance metric.
It is a consequence of the overloaded CPU. Knowledge of the delay
time of a virtual machine helps to understand if it is affected and
estimate the impact.

As a result, it is possible to react exactly when needed and
rebalance the load between hosts. This is used by cloud providers
to provide quality of service, especially when the CPU is
oversubscribed.

It's more convenient to work with this metric in a context of a
libvirt domain. Any monitoring software may use this information.

Signed-off-by: Aleksei Zakharov <zaharov@selectel.ru>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-09 13:35:05 +01:00
Andrea Bolognani
9b2f6c1030 util: Fix error reporting in virnetlink
The preprocessor macro we use to check whether we're on Linux
has not been spelled properly, and so we will always report the
error message intended for other platforms.

Fixes: 879bcee08c
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-09 13:33:09 +01:00
Peter Krempa
568d7358ab qemuMigrationSrcRun: Don't jump to 'exit_monitor' from outside of the monitor
Failure of 'qemuMigrationSetDBusVMState' would jump to 'exit_monitor'
but the function isn't called inside of the monitor context.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-09 13:27:43 +01:00
Michal Privoznik
01e206c3e3 virnetdevbandwidth: Don't generate burst outside of boundaries
When generating TC rules for domain's outbound traffic, Libvirt
will use the 'average' as the default for 'burst' - it's been
this way since the feature introduction in v0.9.4-rc1~22. The
reason is that 'average' considers 'burst' for policing. However,
when parsing its command line TC uses an unsigned int (with
overflow detection) to store the 'burst' size. This means, that
the upper limit for the value is UINT_MAX, well UINT_MAX / 1024
because we are putting the value in KiB onto the command line.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912210
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-09 09:56:48 +01:00
Andrea Bolognani
6564cb01e1 tests: Mock virProcessGetMaxMemLock()
Up until now we've implicitly relied on the fact that failures
reported from this function were simply ignored, but that's
about to change and so we need a proper mock.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
b431f3c7fd conf: Rename original_memlock -> originalMemlock
That's more consistent with our usual naming convention.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
cfeb497f3f util: Don't special-case setting a limit to zero
This behavior reflects the needs of the QEMU driver and has no
place in a generic module such as virProcess.

Thanks to the changes made with the previous commit, it is now
safe to remove these checks and make all virProcessSetMax*()
functions finally behave the same way.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
c2180c2fd6 qemu: Set limits only when explicitly asked to do so
The current code is written under the assumption that, for all
limits except the core size, asking for the limit to be set to
zero is a no-op, and so the operation is performed
unconditionally.

While this is the behavior we want for the QEMU driver, the
virCommand and virProcess facilities are generic, and should not
implement this kind of policy: asking for a limit to be set to
zero should result in that limit being set to zero every single
time.

Add some checks in the QEMU driver, effectively moving the
policy where it belongs.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
e098340cc4 util: Have virCommand remember whether limits are set
Currently this only happens for the core size, but we want the
behavior to be consistent for other limits as well.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
bd33680f02 qemu: Set all limits at the same time
qemuProcessLaunch() is the correct place to set process limits,
and in fact is where we were dealing with almost all of them,
but the memory locking limit was handled in
qemuBuildCommandLine() instead for some reason.

The code is rewritten so that the desired limit is calculated
and applied in separated steps, which will help with further
changes, but this doesn't alter the behavior.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
9bf5c00f9b qemu: Make some minor tweaks
Doing this now will make the next changes nicer.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
6a6d6bb520 util: Introduce virProcess{Get,Set}Limit()
These functions abstract part of the existing logic, which is
the same in all virProcessSetMax*() functions, and changes it
so that which underlying syscall is used depends on their
availability rather than on the context in which they are
called: since prlimit() and {g,s}etrlimit() have slightly
different requirements, using the same one every single time
should make for a more consistent experience.

As part of the change, we also remove the special case for
passing zero to virProcessSetMax*() functions: we have removed
all callers that depended on that functionality in the previous
commit, so this is now safe to do and makes the semantics
simpler.

This commit is better viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Andrea Bolognani
3d44a809c2 util: Always pass a pid to virProcessSetMax*()
Currently, the functions accept either an explicit pid or zero,
in which case the current process should be modified: the latter
might sound like a convenient little feature, but in reality
obtaining the pid of the current process is a single additional
function call away, so it hardly makes a difference.

Removing the few cases in which we're passing zero will allow us
to simplify and improve the functions later.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 22:41:40 +01:00
Daniel P. Berrangé
fe82fdfb52 qemu: wire up support for VNC power control options
This allows the VNC client user to perform a shutdown, reboot and reset
of the VM from the host side.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-08 21:04:06 +00:00
Daniel P. Berrangé
bad5280a53 qemu: probe for -vnc power-control option support
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-08 21:04:06 +00:00
Daniel P. Berrangé
f84f04350b conf: add support for VNC power control setting
The <graphics type="vnc" .... powerControl="yes"/> option instructs the
VNC server to enable an extension that lets the client perform a
graceful shutdown, reboot and hard reset.

This is enabled by default since it cannot be assumed that the VNC
client user has administrator rights over the guest OS. In the case
where the VNC user is a guest administrator though, it is reasonable
to allow direct power control host side too.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-08 21:04:06 +00:00
Andrea Bolognani
4114fb2712 util: Simplify stubs
Calling a stub should always result in ENOSYS being raised,
regardless of what arguments are passed to it.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 18:55:23 +01:00
Andrea Bolognani
0f5e0b44d7 util: Document limit-related functions
We're going to change their behavior, so it's good to have the
current one documented to serve as baseline.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-08 18:55:21 +01:00
Michal Privoznik
4f30c1bb8c virDevMapperGetTargetsImpl: Use correct length when copying into dm.name
For reasons unknown, when rewriting this code and dropping
libdevmapper I've mistakenly used incorrect length of dm.name. In
linux/dm-ioctl.h the dm_ioctl struct is defined as follows:

  #define DM_NAME_LEN 128

  struct dm_ioctl {
    ...
    char name[DM_NAME_LEN];     /* device name */
    ...
  };

However, when copying string into this member, DM_TABLE_DEPS was
used, which is defined as follows:

  #define DM_TABLE_DEPS    _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)

After decryption, this results in the following size: 3241737483.

Fixes: 2249455654
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-08 12:16:13 +01:00
Roman Bogorodskiy
d5b2644815 meson: tools: depend on keycode generated sources
Tools depend on keycode generated sources, so declare that as an
explicit dependency, otherwise it might fail with:

../tools/virsh-completer-domain.c:35:10: fatal error: 'virkeynametable_linux.h' file not found
         ^~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: b0f4cf25a6
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 20:05:54 +04:00
Peter Krempa
0a3d0c610a virFirewallApply: Fix possible NULL dereference on error
Commit bbc25f0d03 juggled around some
error reporting. Unfortunately virFirewallApply tries to report the
errno stored in the firewall object and we'd try to do that when the
firewall object is NULL too. Report EINVAL if 'firewall' is NULL.

Found by Coverity.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
1553e72567 virBufferAdd: Ensure that the buffer is initialized also when len == 0
There's an optimization in virBufferAdd which returns early when the
length of the added string is 0 (given that auto-indent is disabled).

The optimization causes inconsistent behaviour between these two cases:

 virBufferAdd(buf, "", 0);  // this doesn't initialize the buffer

and

 virBufferAdd(buf, "", -1); //this initializes the buffer

Since using an empty string is used to prime the buffer to an empty
string it can be confusing. Remove the optimization.

This fixes such a wrong initialization done in x86FeatureNames.

Note that our code in many places expects that if no virBuffer APIs are
used on a buffer object, then NULL should be retured, so we can't always
prime the buffer to an empty string.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
3a11950af2 conf: Rewrite network virtio driver formatting using virXMLFormatElement
Simplify the logic picking which element form to format by using
virBuffers for the partial properties and virXMLFormatElement for
combining them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
4851a99ee0 virHostCPUGetStatsLinux: Avoid 'strcpy'
Use an allocated buffer for 'cpu_header' so that g_strdup(_printf) can
be used to fill it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
14a4f7cbf2 remote_daemon_dispatch: Replace g_new + strcpy with g_strdup
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
ec809ba4ed virIndexToDiskName: Use g_string_prepend(_c) to improve readability
Use a dynamic string helper so that we don't have to calculate the
string lengths and then iterate from the rear.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
3b5eab6e25 virIndexToDiskName: Make 'idx' unsigned and remove check
We can remove the check that 'idx' is negative by forcing callers to
pass unsigned numbers, which they do already or have a check that 'idx'
is positive.

This in turn allows us to remove most return value NULL checks.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
ef4c325f25 virCommandSetSendBuffer: Provide saner semantics
The function is used to automatically feed a buffer into a pipe which
can be used by the command to read contents of the buffer.

Rather than passing in a pipe, let's create the pipe inside
virCommandSetSendBuffer and directly associate the reader end with the
command. This way the ownership of both ends of the pipe will end up
with the virCommand right away reducing the need of cleanup in callers.

The returned value then can be used just to format the appropriate
arguments without worrying about cleanup or failure.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
aa5c57b407 virCommandFDSet: Remove return value
The function can't fail nowadays. Remove the return value and adjust the
only caller which ensures that @cmd is non-NULL and @fd is positive.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
4bdb29e7a8 virCommandAddEnvBuffer: Remove unused function
Last usage was removed by 5745dc123a

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:33:34 +01:00
Peter Krempa
047db95770 util: vircommand: Add wrappers for virCommand error checking
Extract the check and reporting of error from the individual virCommand
APIs into a separate helper. This will aid future refactors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:32:16 +01:00
Peter Krempa
ae87dc3d09 virPipeImpl: Don't overwrite error
If WITH_PIPE2 is not defined we attempt to set the pipe to nonblocking
operation after they are created. We errorneously rewrote the existing
error message on failure to do so or even reported an error if quiet
mode was requested.

Fixes: ab36f72947
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
2339e73f71 util: virerror: Remove VIR_ERROR_MAX_LENGTH macro
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
3c546f7eb4 qemuProcessReportLogError: Don't mark "%s: %s" as translatable
The function is constructing an error message from a prefix and the
contents of the qemu log file. Marking just two string modifiers as
translatable is pointless and will certainly confuse translators.

Remove the marking and add a comment which bypasses the
sc_libvirt_unmarked_diagnostics check.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
c8ff56c7ad qemuProcessReportLogError: Remove unnecessary math for max error message
Now that error message formatting doesn't use fixed size buffers we can
drop the math for calculating the maximum chunk of log to report in the
error message and use a round number. This also makes it obvious that
the chosen number is arbitrary.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
0333b11f03 util: virprocess: Use local maximum error message size
Use of VIR_ERROR_MAX_LENGTH is actually misleading to the readers
because it implies that the strings in virError are 1024 bytes at most.

That isn't true at least for the 'message' field as it's constructed
from concatenating the detail string which (was) max 1024 bytes with
the string variant of the error code without limiting to 1024.

Use a local copy for declaring the struct for error transport with a
comment so that's obvious that it's a local decision to use 1k buffers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
67a710c3c3 util: virerror: Avoid a copy of the error messages
Some error message reporting functions already have allocated buffers
which were used to format the error message, so copying the strings is
redundant.

Extract the internals from 'virRaiseErrorFull' to
'virRaiseErrorInternal' which takes allocated strings as arguments and
steals them, so that callers can reuse the buffers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
3487554736 util: virerror: Don't use stack'd buffers in error report helpers
This was (probably) a relict from times when we cared about OOM
conditions and the possibility to report the error. Nowadays it doesn't
make sense as virRaiseErrorFull will do an allocated copy of the strings
and also concatenate the error message prefix with the detail which
doesn't guarantee that the result will be less than 1024 chars.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
c800ed6399 test_driver: Rewrite testBuildFilename
Use glib functions to do the relative name lookup instead of manual
assembly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
bf5374d736 xenParseXMDisk: Replace g_new + virStrncpy by g_strndup
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
9595c61625 virFileLoopDeviceAssociate: Use virStrcpy instead of virStrncpy
Passing 'strlen(src)' for length makes it equivalent to virStrcpy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
e8f5711274 virDevMapperGetTargetsImpl: Use virStrcpy instead of virStrncpy
virStrncpy was called with -1 for length of the copied source which is
equivalent to virStrcpy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
33122ed16f sanlock: Use virStrcpy instead of virStrncpy
We want a (possibly truncated) copy of the full source string so
virStrcpy is a better fit.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
3442d8da3b virProcessRunInForkHelper: Use virStrcpyStatic for static buffers
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Peter Krempa
f6280b0397 util: virstring: Always copy string in virStrcpy
15 out of 72 invocations of virStrcpy(Static) ignore the return value as
it's either impossible to fail or in certain cases a truncated copy is
still good enough. Unfortunately virStrcpy doesn't copy anything in
such case as the checks are done first.

Fix this by using g_strlcpy for the implementation and removing
G_GNUC_WARN_UNUSED_RESULT from the function so that callers can decide
when it's okay.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-05 15:01:29 +01:00
Martin Kletzander
2a490ce5a0 glib: Use safe glib event workaround in other event loops
Similarly to the crash workaround:

  commit 0db4743645
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Tue Jul 28 16:52:47 2020 +0100

    util: avoid crash due to race in glib event loop code

we need to do this in the other event loop as crash in that one was also
reported:

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-05 10:17:26 +01:00
Martin Kletzander
8964564550 util: Move glib event loop workaround to glibcompat
This way it can be used from other places as well.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-03-05 10:17:26 +01:00
Michal Privoznik
f81d504b71 util: Drop virFileMakePath() and virFileMakePathWithMode()
These functions are now unused.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 20:52:23 +01:00
Michal Privoznik
7f482a67e4 lib: Replace virFileMakePath() with g_mkdir_with_parents()
Generated using the following spatch:

  @@
  expression path;
  @@
  - virFileMakePath(path)
  + g_mkdir_with_parents(path, 0777)

However, 14 occurrences were not replaced, e.g. in
virHostdevManagerNew(). I don't really understand why.
Fixed by hand afterwards.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 20:52:23 +01:00
Michal Privoznik
b1e3728dec lib: Replace virFileMakePathWithMode() with g_mkdir_with_parents()
These functions are identical. Made using this spatch:

  @@
  expression path, mode;
  @@
  - virFileMakePathWithMode(path, mode)
  + g_mkdir_with_parents(path, mode)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 20:52:23 +01:00
Kristina Hanicova
18919bedc8 netdev_bandwidth_conf: Remove redundant variables/labels
In functions: virNetDevBandwidthParseRate(),
virNetDevBandwidthParse()

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 17:30:08 +01:00
Kristina Hanicova
2870a164b9 Use g_autoptr instead of virNetDevBandwidthFree where possible
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 17:30:08 +01:00
Kristina Hanicova
e4dd2a2c2e Use g_steal_pointer where possible
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 17:30:08 +01:00
Kristina Hanicova
bcb63a3bdc netdev_bandwidth_conf: Refractor virNetDevBandwidthParse()
Refractoring includes:
* removal of VIR_FREE
* inversion of the condition

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 17:30:04 +01:00
Kristina Hanicova
a40448c42f netdev_bandwidth_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-04 17:29:07 +01:00
Kristina Hanicova
e7bd2f6a73 node_device_conf: Remove redundant variables/labels
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:52:38 +01:00
Kristina Hanicova
b518f7b16f node_device_conf: use g_autoptr in virNodeDevCapPCIDevIommuGroupParseXML()
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:52:32 +01:00
Kristina Hanicova
fe7f725bf3 node_device_conf: Refactoring functions in order to remove VIR_FREE
By:
* declaration of an autofreed variable in for loop
* use of a new variable
* removal of VIR_FREE

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:52:28 +01:00
Kristina Hanicova
8a0cb5f73a node_device_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:50:59 +01:00
Tim Wiederhake
bcdb634e1b domain_conf: Fix check for hyperv stimer
VIR_DOMAIN_HYPERV_STIMER happens to have the same numerical value as
VIR_DOMAIN_FEATURE_HYPERV, resulting in the if-block to always being
executed when a "<hyperv>" tag is found, whether or not it actually
contained a "<stimer>" tag. This had no ill effects, as virXPathNodeSet()
would simply return 0 if that tag does not exist.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:44:23 +01:00
Tim Wiederhake
b606fffe1b domain_conf: Add missing break in switch
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:44:22 +01:00
Tim Wiederhake
131895eb1e domain_conf: Reduce scope of node in virDomainFeaturesDefParse
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:44:19 +01:00
Tim Wiederhake
0f9f808227 domain_conf: Reduce scope of gic_version in virDomainFeaturesDefParse
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:44:17 +01:00
Tim Wiederhake
94013ee04e domain_conf: Reduce scope of tmp in virDomainFeaturesDefParse
Variables using `g_autofree` should not be manually VIR_FREE'd and reused.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:44:14 +01:00
Pavel Hrdina
a924927c39 vircgroup: drop unused function virCgroupSetupCpuShares
Previous commit removed all usage of this function so we can remove it.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:13:31 +01:00
Pavel Hrdina
e95489d813 cgroup: use virCgroupSetCpuShares instead of virCgroupSetupCpuShares
Now that we enforce the cpu.shares range kernel will no longer silently
change the value that libvirt configures so there is no need to read
the value back to get the actual configuration.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:13:28 +01:00
Pavel Hrdina
1d9d9961ad vircgroup: enforce range limit for cpu.shares
Before the conversion to using systemd DBus API to set the cpu.shares
there was some magic conversion done by kernel which was documented in
virsh manpage as well. Now systemd errors out if the value is out of
range.

Since we enforce the range for other cpu cgroup attributes 'quota' and
'period' it makes sense to do the same for 'shares' as well.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:13:05 +01:00
Jiri Denemark
b5abf9a192 cpu_map: Fix spelling of svme-addr-chk feature
Commit a208176ca1 introduced this feature
with an incorrect "svme-addr-check" spelling.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
2021-03-04 09:33:39 +01:00
Jim Fehlig
ee3dc2c2c8 libxl: Fix node device detach when driver unspecified
Commit 887dd0d331 caused a small regression in NodeDeviceDetach in the libxl
driver when the 'driver' parameter is not specified. E.g.

# virsh nodedev-detach pci_0000_0a_10_0
error: Failed to detach device pci_0000_0a_10_0
error: An error occurred, but the cause is unknown

If the driver name is not specified, NULL is passed to
virDomainDriverNodeDeviceDetachFlags, in which case virPCIDeviceSetStubDriver
is never called to set the stub to pciback. Fix it by setting the driver to
"xen" if it is not specified when invoking NodeDeviceDetach.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-03-02 07:52:14 -07:00
Jiri Denemark
d3de79dbfc cpu_map: Install x86_EPYC-Milan.xml
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-03-02 12:03:26 +01:00
Jiri Denemark
f321a4822e cpu_map: Add EPYC-Milan x86 CPU model
Introduced in QEMU 6.0.0 by 623972ceae091b31331ae4a1dc94fe5cbb891937

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-02 10:29:03 +01:00
Peter Krempa
fd8bfd522b util: virerror: Remove virReportOOMError
Trying to report an OOM error is pointless since our infrastructure to
report error needs to allocate memory to report the error.

In addition our code mistakenly reported OOM errors even in cases where
a function could fail for another reason, which would make issues harder
to debug.

Remove the virReportOOMError and backend so that programmers are forced
to think about what can happen. In case when there's another failure
possible a specific error should be reported and otherwise a direct
abort() is better since the logger would abort on g_new anyways.

This patch also removes the syntas-check which forces use of
virReportOOMError instead of using VIR_ERR_NO_MEMORY with other
functions. This allows possible future use when we'd end up in a
situation where trying to recover from an OOM would make sense, such as
when attempting to allocate a massive buffer.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
d877eb1ddc virVMXConvertToUTF8: Report non-OOM error on failure of xmlBufferCreateStatic
The function has also non-OOM failure case when the passed string has 0
length, so reporting OOM error is not correct.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
5591ba2889 storage: Don't report OOM error on failure of glfs_new
OOM isn't the only failure glfs_new can encounter. Report an error which
might give more insight. libgfapi seems to be setting errno but
reporting a system error migt be misleading.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
0af84a81fc util: json: Report non-OOM error on yajl failure
The yajl library returns a wide range of error codes so reporting OOM on
any failure is wrong. In case the error was really based by memory issue
the error reporting will probably cause an abort anyways. Change the
error message so that we know that it happened in JSON at least.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
e09c852782 virVBoxSnapshotConfSaveVboxFile: abort() on failure to allocate xmlDoc and comment
'xmlNewDoc' and 'xmlNewDocComment' return NULL only on allocation
failure. Attempting to raise an error is pointless.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
c0e65398d9 libxl: abort() on failure of libxl_cpu_bitmap_alloc()
Attempting to report error in case when we ran out of memory is
pointless.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
3fdf0013e5 vbox: abort() on allocation failure in UTF8<->UTF16 conversion
Trying to report an error on OOM is pointless since error handling
allocates memory.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:20 +01:00
Peter Krempa
695d469238 hyperv: abort() failure of wsmc_fault_new()
The function just allocates a helper object. Reporting errors would be
pointless when we encounter OOM situation.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
dda78f0b62 util: iohelper: Don't handle OOM from posix_memalign
Similarly to other allocation calls abort() on failure.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
cc622f5548 virURIFormat: abort() on failure
If the argument of 'xmlSaveUri' is non-NULL the function returns NULL on
OOM failure only. Thus we can directly abort rather than try to do the
impossible recovery.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
06fc9f8e32 util: virprocess: abort() on CPU_ALLOC failure
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
5f5b676086 virXMLParseHelper: abort() on allocation failure
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
45edcd9f89 virXMLXPathContextNew: abort() on allocation failure
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
c3a5c67a84 Don't report OOM error on xmlCopyNode failure
Out of memory isn't the only reason the function can fail. Add a message
stating that copying of a XML node failed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
ec6e2a2c97 util: xml: Add wrapper for 'xmlNewNode'
Add a wrapper that will handle the out of memory condition by abort()
and also prevents callers from having to typecast the argument.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
2b0f2a0a07 util: xml: Add virXMLBufferCreate wrapper
'xmlBufferCreate' returns NULL only on allocation failure. Add a wrapper
which will call 'abort()' in such case in a centralised spot. It doesn't
make much sense to continue execution from here.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
7a0b625ea2 util: virnetlink: Add wrapper for 'nlmsg_alloc_simple'
The function is used in many places and fails only on allocation
failures. Since trying to recover from allocation failure of a small
buffer by reporting error doesn't make sense add a wrapper for
'nlmsg_alloc_simple' which will 'abort()' on failure and replace all
allocations of netlink message with the new helper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
bbc25f0d03 virfirewall: Remove impossible OOM error reporting
There's nothing that would set the 'err' field of virFirewallPtr to
ENOMEM so we can remove the checks.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
2a620b1200 virfirewall: virFirewallAddRuleFullV: Remove OOM check from VIR_APPEND_ELEMENT
VIR_APPEND_ELEMENT_COPY will abort the program on OOM so there's no need
to check.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
1a1a926804 virfirewall: Remove OOM checks from virFirewallStartTransaction
Neither virFirewallGroupNew nor VIR_EXPAND_N can fail.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
b8aa25f560 virfirewall: Don't check OOM in ADD_ARG macro
VIR_RESIZE_N can't fail nowadays, adjust the macro.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
22dbafe092 virCloseCallbacksGetForConn: Remove OOM handling
VIR_EXPAND_N will abort so we can simplify the hash iterator.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
9339163894 util: vircommand: Remove OOM handling
The OOM error handling is dead code nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
c7dc99e84f virDomainDefSetMetadata: Rework memory handling
Switch to use g_autoptr for 'doc' and 'new' local variables.
Additionally report proper error when 'xmlAddChild' fails because OOM is
not the only error it can report.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
e2ac76f707 lxc_process: Remove OOM handling from logging setup
'virLogGetFilters' doesn't return failure and 'virLogGetOutputs' reports
it's own errors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
ccac1c2623 virBuildPath: Remove return value
The function can't fail nowadays, remove the return value and adjust
callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
542b2b2914 virCPUx86DataParse: Don't check error from x86FeatureNames
x86FeatureNames uses virBuffer and thus can't fail nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
e3a792a39b virCommandAddArgBuffer: Simplify clearing of @buf
Get the buffer contents into a temporary variable with automatic
clearing so that the error branches don't have to reset the buffer.
Additionally handle the NULL string case before assignment.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
13a9075cea virCommandAddEnv: Make stealing of argument more obvious
The function is supposed to always consume the passed environment
variable string. Use a temp variable with autofree and g_steal_pointer
to prevent having to free it manually.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
b356a3ce7c virDomainDefSetMetadata: Avoid temporary variable for string copy
Since error checking was removed when switching to g_strdup, it doesn't
make much sense to have 'tmp' around.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
3560f75d2d util: xml: Introduce autoptr cleanup support for 'xmlNode'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Peter Krempa
c419ad8258 Remove useless comments for VIR_FROM_THIS definition
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Michal Privoznik
144cb28e6b virtpm: Fix @path handling in virTPMEmulatorInit()
This function finds "swtmp", "swtpm_setup" and "swtpm_ioctl"
binaries in $PATH and stores resolved paths in global variables
so that they can be obtainer later. Anyway, the resolved path is
marked as g_autofree and to avoid its freeing later on in the
function the variable is set to NULL manually. Well, we have
g_steal_pointer() for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-02 09:49:23 +01:00
Michal Privoznik
25ebb45a81 qemu_tpm: Generate log file path among with storage path
When starting a guest with TPM of type='emulator' an external
process is started with it (swtpm) to emulate TPM. This external
process is passed path to a log file via --logfile. The path to
the log file is generated in qemuTPMEmulatorPrepareHost() which
works, until the daemon is restarted. The problem is that the
path is not stored in private data or anywhere inside live XML
and thus later, when qemuExtTPMStop() is called (when shutting
off the guest) the stored logpath is NULL and thus its seclabel
is not cleaned up (see virSecuritySELinuxRestoreTPMLabels()).

Fortunately, qemuExtDevicesStop() (which calls qemuExtTPMStop()
eventually) does call qemuExtDevicesInitPaths() where the log
path can be generated again.

Basically, tpm->data.emulator.storagepath is generated in
qemuExtTPMInitPaths() and its seclabels are restored properly,
and this commit move logfile onto the same level.

This means, that the log path doesn't have to be generated in
qemuExtDevicesStart() because it was already done in
qemuExtDevicesPrepareHost().

This change also renders @vmname argument of
qemuTPMEmulatorPrepareHost() unused and thus is removed.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1769196
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-02 09:45:49 +01:00
Michal Privoznik
f9cd29a2e4 qemu_tpm: Move logfile path generation into a separate function
Strictly not needed, but the rest of paths is generated in
separate functions. Helps with code readability.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-03-02 09:44:03 +01:00
Kristina Hanicova
78879650e4 Remove redundant variables/labels
In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and
virNetDevIPRouteParseXML()

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-01 18:50:22 -05:00
Kristina Hanicova
5fa51adcf2 Use g_autoptr instead of virNetDevIPRouteFree if possible
In files: src/conf/domain_conf: in virDomainNetIPInfoParseXML(),
src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/vz/vz_sdk: in prlsdkGetRoutes(), src/conf/networkcommon_conf:
in virNetDevIPRouteCreate()

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-01 18:50:22 -05:00
Kristina Hanicova
8c23da73b6 networkcommon_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-01 18:50:22 -05:00
Tim Wiederhake
5ac6ab2fde cpumap: Add support for svme-addr-check CPU feature
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-01 20:09:46 +01:00
Tim Wiederhake
5c17a7ba41 cpumap: Add support for ibrs CPU feature
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-01 20:09:46 +01:00
Tim Wiederhake
a208176ca1 cpu_map/sync_qemu_i386.py: Add mapping for svme-addr-check
This was added to qemu in commit 5447089c2b3b084b51670af36fc86ee3979e04be.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-01 20:09:46 +01:00
Tim Wiederhake
9f156b6e51 cpu_map/sync_qemu_i386.py: Add mapping for ibrs
This was added to qemu in commit 623972ceae091b31331ae4a1dc94fe5cbb891937.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-01 20:09:46 +01:00
Tim Wiederhake
e52150d2cc cpu_map/sync_qemu_i386.py: Add mapping for amd-ssbd
This was added to qemu in commit 623972ceae091b31331ae4a1dc94fe5cbb891937.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-03-01 20:09:46 +01:00
Cole Robinson
e074a89f4d hyperv: Fix 32bit compilation
Example:
../src/hyperv/hyperv_driver.c:3007:54: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
 3007 |             virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not attach serial port %lu"), i);

Signed-off-by: Cole Robinson <crobinso@redhat.com>
2021-03-01 13:46:20 -05:00
Daniel Henrique Barboza
8c9a600457 virhostdev.c: remove missing PCI devs from hostdev manager
virHostdevReAttachPCIDevices() is called when we want to re-attach
a list of hostdevs back to the host, either on the shutdown path or
via a 'virsh detach-device' call.  This function always count on the
existence of the device in the host to work, but this can lead to
problems. For example, a SR-IOV device can be removed via an admin
"echo 0 > /sys/bus/pci/devices/<addr>/sriov_numvfs", making the kernel
fire up and eventfd_signal() to the process, asking for the process to
release the device. The result might vary depending on the device driver
and OS/arch, but two possible outcomes are:

1) the hypervisor driver will detach the device from the VM, issuing a
delete event to Libvirt. This can be observed in QEMU;

2) the 'echo 0 > ...' will hang waiting for the device to be unplugged.
This means that the VM process failed/refused to release the hostdev back
to the host, and the hostdev will be detached during VM shutdown.

Today we don't behave well for both cases. We'll fail to remove the PCI device
reference from mgr->activePCIHostdevs and mgr->inactivePCIHostdevs because
we rely on the existence of the PCI device conf file in the sysfs. Attempting
to re-utilize the same device (assuming it is now present back in the host)
can result in an error like this:

$ ./run tools/virsh start vm1-sriov --console
error: Failed to start domain vm1-sriov
error: Requested operation is not valid: PCI device 0000:01:00.2 is in use by driver QEMU, domain vm1-sriov

For (1), a VM destroy/start cycle is needed to re-use the VF in the guest.
For (2), the effect is more nefarious, requiring a Libvirtd daemon restart
to use the VF again in any guest.

We can make it a bit better by checking, during virHostdevReAttachPCIDevices(),
if there is any missing PCI device that will be left behind in activePCIHostdevs
and inactivePCIHostdevs lists. Remove any missing device found from both lists,
unconditionally, matching the current state of the host. This change affects
the code path in (1) (processDeviceDeletedEvent into qemuDomainRemoveDevice, all
the way back to qemuHostdevReAttachPCIDevices) and also in (b) (qemuProcessStop
into qemuHostdevReAttachDomainDevices).

NB: Although this patch enables the possibility of 'outside Libvirt' SR-IOV
hotunplug of PCI devices, if the hypervisor and the PCI driver copes with it,
our goal is to mitigate what it is still considered a user oopsie. For all
supported purposes, the admin must remove the SR-IOV VFs from all running domains
before removing the VFs from the host.

Resolves:  https://gitlab.com/libvirt/libvirt/-/issues/72
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
ac81176614 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListDel()
This change will allow us to remove PCI devices from a list
without the need of a PCI Device object, which will be need
in the next patch.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
de80a10738 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListSteal()
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
f1370f9ca6 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFind()
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
d7d1479fc0 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFindIndex()
We're going to need a way to remove a PCI Device from a list without having
a valid virPCIDevicePtr, because the device is missing from the host. This
means that virPCIDevicesListDel() must operate with a PCI Device address
instead.

Turns out that virPCIDevicesListDel() and its related functions only use
the virPCIDeviceAddressPtr of the virPCIDevicePtr, so this change is
simple to do and will not cause hassle in all other callers. Let's
start adapting virPCIDeviceListFindIndex() and crawl our way up to
virPCIDevicesListDel().

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
67357e5094 qemu_cgroup.c: skip absent PCI devices in qemuTeardownHostdevCgroup()
There is no need to bother with cgroup tearing down for absent
PCI devices, given that their entries in the sysfs are already
gone.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
a8017c6882 virhostdev.c: add virHostdevIsPCIDevice() helper
Add a helper to quickly determine if a hostdev is a PCI device,
instead of doing a tedious 'if' check with hostdev mode and
subsys type.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
fae5e343fb dac, selinux: skip setting/restoring label for absent PCI devices
If the underlying PCI device of a hostdev does not exist in the
host (e.g. a SR-IOV VF that was removed while the domain was
running), skip security label handling for it.

This will avoid errors that happens during qemuProcessStop() time,
where a VF that was being used by the domain is not present anymore.
The restore label functions of both DAC and SELinux drivers will
trigger errors in virPCIDeviceNew().

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
cb4b59eda8 virhostdev.c: virHostdevGetPCIHostDevice() now reports missing device
Gitlab issue #72 [1] reports that removing SR-IOVs VFs before
removing the devices from the running domains can have strange
consequences. QEMU might be able to hotunplug the device inside the
guest, but Libvirt will not be aware of that, and then the guest is
now inconsistent with the domain definition.

There's also the possibility of the VFs removal not succeeding
while the domain is running but then, as soon as the domain
is shutdown, all the VFs are removed. Libvirt can't handle
the removal of the PCI devices while trying to reattach the
hostdevs, and the Libvirt daemon can be left in an inconsistent
state (see [2]).

This patch starts to address the issue related in Gitlab #72, most
notably the issue described in [2]. When shutting down a domain
with SR-IOV hostdevs that got missing, virHostdevReAttachPCIDevices()
is failing the whole process and failing to reattach all the
PCI devices, including the ones that aren't related to the VFs that
went missing. Let's make it more resilient with host changes by
changing virHostdevGetPCIHostDevice() to return an exclusive error
code '-2' for this case. virHostdevGetPCIHostDeviceList() can then
tell when virHostdevGetPCIHostDevice() failed to find the PCI
device of a hostdev and continue to make the list of PCI devices.

virHostdevReAttachPCIDevices() will now be able to proceed reattaching
all other valid PCI devices, at least. The 'ghost hostdevs' will be
handled later on.

[1] https://gitlab.com/libvirt/libvirt/-/issues/72
[2] https://gitlab.com/libvirt/libvirt/-/issues/72#note_459032148

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
3acc65e1b0 virpci: introduce virPCIDeviceExists()
We're going to add logic to handle the case where a previously
existing PCI device does not longer exist in the host.

The logic was copied from virPCIDeviceNew(), which verifies if a
PCI device exists in the host, returning NULL and throwing an
error if it doesn't. The NULL is used for other errors as well
(product/vendor id read errors, dev id overflow), meaning that we
can't re-use virPCIDeviceNew() for the purpose of detecting
if the device exists.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Kristina Hanicova
155151a3d0 Use g_steal_pointer where possible
Via coccinelle (not the handbag!)
spatches used:
@ rule1 @
identifier a, b;
symbol NULL;
@@

- b = a;
  ... when != a
- a = NULL;
+ b = g_steal_pointer(&a);

@@

- *b = a;
  ... when != a
- a = NULL;
+ *b = g_steal_pointer(&a);

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-01 15:54:42 +01:00
Peter Krempa
cb29e4e801 qemuBackupJobTerminate: Don't calculate backup job stats if VM isn't active
If the VM isn't active calculating the job stats doesn't make sense.
Additionally this prevents a crash of libvirtd if qemu terminates while
libvirt wasn't running:

Thread 28 "init-backup-tes" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffb9310640 (LWP 3201116)]
qemuDomainJobInfoUpdateTime (jobInfo=0x0) at ../../../libvirt/src/qemu/qemu_domainjob.c:275
275	    if (!jobInfo->started)
(gdb) bt
 #0  qemuDomainJobInfoUpdateTime (jobInfo=0x0) at ../../../libvirt/src/qemu/qemu_domainjob.c:275
 #1  0x00007fffcba1a12d in qemuBackupJobTerminate (vm=0x7fff9c1bc840, jobstatus=QEMU_DOMAIN_JOB_STATUS_CANCELED) at ../../../libvirt/src/qemu/qemu_backup.c:563
 #2  0x00007fffcbaefcae in qemuProcessStop
    (driver=0x7fff9c144ff0, vm=0x7fff9c1bc840, reason=VIR_DOMAIN_SHUTOFF_DAEMON, asyncJob=QEMU_ASYNC_JOB_NONE, flags=<optimized out>)
    at ../../../libvirt/src/qemu/qemu_process.c:7812
 #3  0x00007fffcbaf2a10 in qemuProcessReconnect (opaque=<optimized out>) at ../../../libvirt/src/qemu/qemu_process.c:8578
 #4  0x00007ffff7c46bb5 in virThreadHelper (data=<optimized out>) at ../../../libvirt/src/util/virthread.c:233
 #5  0x00007ffff6e453f9 in start_thread () at /lib64/libpthread.so.0
 #6  0x00007ffff766fb53 in clone () at /lib64/libc.so.6

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:47 +01:00
Peter Krempa
5627f62ec4 qemuBackupJobTerminate: Move cleanup of temp files earlier
Upcoming patch will remove unnecessary actions if the VM crashed. The
cleanup needs to be performed always, thus needs to be moved earlier.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:47 +01:00
Peter Krempa
6423e30828 virStorageVolDefFormat: Don't format empty <source>
If there are no source extents the volume XML has an empty <source>
element. Remove it if there's nothing in it by using
virXMLFormatElement.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:47 +01:00
Peter Krempa
80d9ef8068 virStorageVolDefFormat: Extract formatting of source extents
Move the extent formatting code into
virStorageVolDefFormatSourceExtents.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:42 +01:00
Peter Krempa
104db1951d storageBackendProbeTarget: Don't fail if backing store can't be parsed
When the backing store of the image can't be parsed
virStorageSourceNewFromBacking returns -1. storageBackendProbeTarget
then also fails which makes the pool refresh fail or even the storage
pool becomes inactive after (re)start of libvirtd.

In situations when we can't access the backing store via network we
just report the backing store string, thus we can do the same thing for
unparsable backing store to prevent the pool from going offline.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:20 +01:00
Peter Krempa
e83fbead66 storageBackendProbeTarget: Check return value of virStorageSourceNewFromBacking
Commit bc3a78f61a errorneously removed the return value check from
virStorageSourceNewFromBacking. In cases when we e.g. can't parse the
backing store string this leads to a crash:

 #0  virStorageSourceGetActualType (def=0x0) at ../../../libvirt/src/conf/storage_source_conf.c:1014
 #1  0x00007ffff7cee4f9 in virStorageSourceIsLocalStorage (src=<optimized out>) at ../../../libvirt/src/conf/storage_source_conf.c:1026
 #2  0x00007ffff455c97c in storageBackendProbeTarget (encryption=0x7fff9c122ce8, target=0x7fff9c122c68) at ../../../libvirt/src/storage/storage_util.c:3443
 #3  virStorageBackendRefreshVolTargetUpdate (vol=0x7fff9c122c30) at ../../../libvirt/src/storage/storage_util.c:3519
 #4  0x00007ffff455cdc0 in virStorageBackendRefreshLocal (pool=0x7fff9c010ea0) at ../../../libvirt/src/storage/storage_util.c:3593
 #5  0x00007ffff454f0a1 in storagePoolRefreshImpl
    (backend=backend@entry=0x7ffff4711180 <virStorageBackendDirectory>, obj=obj@entry=0x7fff9c010ea0, stateFile=stateFile@entry=0x7fff9c111a90 "/var/run/libvirt/storage/tmp.xml") at ../../../libvirt/src/storage/storage_driver.c:103
 #6  0x00007ffff4550ea5 in storagePoolUpdateStateCallback (obj=0x7fff9c010ea0, opaque=<optimized out>) at ../../../libvirt/src/storage/storage_driver.c:165
 #7  0x00007ffff7cefef4 in virStoragePoolObjListForEachCb (payload=<optimized out>, name=<optimized out>, opaque=0x7fffc8a489c0)
    at ../../../libvirt/src/conf/virstorageobj.c:435
 #8  0x00007ffff7c03195 in virHashForEachSafe
    (table=<optimized out>, iter=iter@entry=0x7ffff7cefec0 <virStoragePoolObjListForEachCb>, opaque=opaque@entry=0x7fffc8a489c0)
    at ../../../libvirt/src/util/virhash.c:414
 #9  0x00007ffff7cf0520 in virStoragePoolObjListForEach
    (pools=<optimized out>, iter=iter@entry=0x7ffff4550e10 <storagePoolUpdateStateCallback>, opaque=opaque@entry=0x0)
    at ../../../libvirt/src/conf/virstorageobj.c:468
 #10 0x00007ffff454f43a in storagePoolUpdateAllState () at ../../../libvirt/src/storage/storage_driver.c:184
 #11 storageStateInitialize (privileged=<optimized out>, root=<optimized out>, callback=<optimized out>, opaque=<optimized out>)
    at ../../../libvirt/src/storage/storage_driver.c:315
 #12 0x00007ffff7e10c04 in virStateInitialize
    (opaque=0x555555621820, callback=0x55555557b1d0 <daemonInhibitCallback>, root=0x0, mandatory=<optimized out>, privileged=true)
    at ../../../libvirt/src/libvirt.c:656
 #13 virStateInitialize
    (privileged=<optimized out>, mandatory=mandatory@entry=false, root=root@entry=0x0, callback=callback@entry=0x55555557b1d0 <daemonInhibitCallback>, opaque=opaque@entry=0x555555621820) at ../../../libvirt/src/libvirt.c:638
 #14 0x000055555557b230 in daemonRunStateInit (opaque=0x555555621820) at ../../../libvirt/src/remote/remote_daemon.c:605
 #15 0x00007ffff7c46bb5 in virThreadHelper (data=<optimized out>) at ../../../libvirt/src/util/virthread.c:233
 #16 0x00007ffff6e453f9 in start_thread () at /lib64/libpthread.so.0
 #17 0x00007ffff766fb53 in clone () at /lib64/libc.so

An invalid image can be easily created by:

$ qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u img.qcow2 10M

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-01 15:44:20 +01:00
Michal Privoznik
6458b9d94a qemu_monitor: Document qemuMonitorUnregister()
The most important bit is that the caller is expected to pass
locked monitor.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-02-25 10:22:10 +01:00
Jiri Denemark
c8f3b83c72 qemu_domainjob: Make copy of owner API
Using the job owner API name directly works fine as long as it is a
static string or the owner's thread is still running. However, this is
not always the case. For example, when the owner API name is filled in a
job when we're reconnecting to existing domains after daemon restart,
the dynamically allocated owner name will disappear with the
reconnecting thread. Any follow up usage of the pointer will read random
memory.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-25 09:55:31 +01:00
Laine Stump
98e67d4d8c qemu: allow migration of generic <hostdev> with <teaming>
Commit 010ed0856b and commit db64acfbda introduced the ability to use
the <teaming> element in a generic <hostdev> (previously it could only
be used with <interface type='hostdev'>). However, the patch omitted
one crucial detail - along with parsing the <teaming> element in
<hostdev>, and adding the necessary info to the qemu commandline, we
also need to modify qemuMigrationSrcIsAllowedHostdev() to allow
migration when the generic <hostdev> has a <teaming> element.

https://bugzilla.redhat.com/1927984

Fixes: 010ed0856b
Reported-by: Yalan Zhang <yalzhang@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-24 12:31:51 -05:00
Peng Liang
1ac703a7d0 qemu: Add missing lock in qemuProcessHandleMonitorEOF
qemuMonitorUnregister will be called in multiple threads (e.g. threads
in rpc worker pool and the vm event thread).  In some cases, it isn't
protected by the monitor lock, which may lead to call g_source_unref
more than one time and a use-after-free problem eventually.

Add the missing lock in qemuProcessHandleMonitorEOF (which is the only
position missing lock of monitor I found).

Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-24 15:00:51 +01:00
Jim Fehlig
fa58f571ee libxl: Add lock process indicator to libxlDomainObjPrivate object
The libvirt libxl driver has no access to FDs associated with VM disks.
The disks are opened by libxl.so and any related FDs are not exposed to
applications. The prevents using virtlockd's auto-release feature to
release locks when the FD is closed. Acquiring and releasing locks is
explicitly handled by the libxl driver.

The current logic is structured such that locks are acquired in
libxlDomainStart and released in libxlDomainCleanup. This works well
except for migration, where the locks must be released on the source
host before the domain can be started on the destination host, but the
domain cannot be cleaned up until the migration confirmation stage.
When libxlDomainCleanup if finally called in the confirm stage, locks
are again released resulting in confusing errors from virtlockd and
libvirtd

virtlockd[8095]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: Unable to release lease on testvm

The error is also encountered in some error cases, e.g. when
libxlDomainStart fails before acquiring locks and libxlDomainCleanup
is still used for cleanup.

In lieu of a mechanism to check if a lock has been acquired, this patch
takes an easy approach to fixing the unnecessary lock releases by adding
an indicator to the libxlDomainPrivate object that can be set when the
lock is acquired and cleared when the lock is released. libxlDomainCleanup
can then skip releasing the lock in cases where it was previously released
or never acquired in the first place.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-23 17:55:19 -07:00
Jim Fehlig
87a9d3a6b0 libxl: Fix domain shutdown
Commit fa30ee04a2 caused a regression in normal domain shutown.
Initiating a shutdown from within the domain or via 'virsh shutdown'
does cause the guest OS running in the domain to shutdown, but libvirt
never reaps the domain so it is always shown in a running state until
calling 'virsh destroy'.

The shutdown thread is also an internal user of the driver shutdown
machinery and eventually calls libxlDomainDestroyInternal where
the ignoreDeathEvent inhibitor is set, but running in a thread
introduces the possibility of racing with the death event from
libxl. This can be prevented by setting ignoreDeathEvent before
running the shutdown thread.

An additional improvement is to handle the destroy event synchronously
instead of spawning a thread. The time consuming aspects of destroying
a domain have been completed when the destroy event is delivered.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-22 10:47:11 -07:00
Pavel Hrdina
22cae2ea4b domain_validate: use defines for cpu period and quota limits
Commints <bc760f4d7c4f964fadcb2a73e126b0053e7a9b06> and
<98a09ca48ed4fc011abf2aa290e02ce1b8f1bb5f> fixed the code to use
defines instead of magic numbers but missed this place.

Following commit <ed1ba69f5a8132f8c1e73d2a1f142d70de0b564a> changed
the cpu quota limit to reflect what kernel actually allows so using
the defines fixes XML validations as well.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-22 14:03:04 +01:00
Peter Krempa
8298a5bd69 virJSONParserInsertValue: Take double pointer for @value
The function calls virJSONValueObjectAppend/virJSONValueArrayAppend, so
by taking a double pointer we can drop the pointer clearing from
callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
6b12e220b0 virJSONValueNewNumber: Take ownership of passed string
Avoid pointless copies of temporary strings when constructing number
JSON objects.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
395ecd7a8c virJSONParserHandle*: Refactor memory cleanup and drop NULL checks
virJSONValueNew* won't return error nowadays so NULL checks are not
necessary. The memory can be cleared via g_autoptr.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
ad3f3bd048 qemuAgentSetVCPUsCommand: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
0b236a9260 qemuMonitorJSONTransactionAdd: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
6431b20c3e virJSONValueArrayAppend: Clear pointer when taking ownership of passed value
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
d4e369a4c3 virMACMapHashDumper: Refactor array addition
Use automatic memory freeing and don't check return value of
virJSONValueNewString as it can't fail.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
83a50fe2a5 qemuAgentMakeStringsArray: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
5fc3892891 virJSONValueObjectAppend: Clear pointer when taking ownership of passed value
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
e4b26c48cb virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
94ab321ffc virJSONValueNewArrayFromBitmap: Refactor cleanup
Use g_autoptr for the JSON value objects and remove the cleanup label
and inline freeing of objects.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
7d3a33b26b virJSONValue(Array|Object)Append*: Simplify handling of appended object
Use g_autofree for the pointer of the added object and remove the NULL
checks for values returned by virJSONValueNew* (except
virJSONValueNewNumberDouble) since they can't fail nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
3e411cbc5f virJSONValueCopy: Don't use virJSONValue(Object|Array)Append
We know the exact number of keys or array members for the copied objects
so we can pre-allocate the arrays rather than inserting into them in a
loop incurring realloc copy penalty.

Also virJSONValueCopy now can't fail since all of the functions
allocating the different cases use just g_new/g_strdup internally so we
can remove the NULL checks from the recursive calls.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
b116e715a8 virJSONValueObjectInsert: Clear @value on successful insertion
The function takes ownership of @value on success so the proper
semantics will be to clear out the @value pointer. Convert @value to a
double pointer to do this.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
a3f237cb00 qemuAgentMakeCommand: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
9a1651f64d virLockSpacePreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
49efa299b5 virNetServerPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
6e35dc7bbe virNetServerPreExecRestart: Drop error reporting from virJSONValueObjectAppend* calls
The functions report errors already and the error can nowadays only
happen on programmer errors (if the passed virJSONValue isn't an
object), which won't happen. Remove the reporting.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
edc09e83da virNetServerClientPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
0738ac55ec virNetServerServicePreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
86ac11380a virNetDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
eef2bcc7d7 virLogHandlerPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
c357518776 virLogDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
bf666cac6b virLockDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
013e7564ff qemu: capabilities: Enable QEMU_CAPS_INCREMENTAL_BACKUP
For incremental backup we need QEMU_CAPS_BLOCKDEV,
QEMU_CAPS_BLOCKDEV_REOPEN, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
9f61d14a9e qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints
Preserve block dirty bitmaps after migration with
QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC).

This patch implements functions which offer the bitmaps to the
destination, check for eligibility on destination and then configure
source for the migration.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
1475baed36 qemu: migration: Clean up temporary bitmaps when cancelling a migration
In case when the block migration job required temporary bitmaps for
merging the appropriate checkpoints we need to clean them up when
cancelling the job. On success we don't need to do that though as the
bitmaps are just temporary thus are not written to disk.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
516906d681 qemu: domain: Store list of temporary bitmaps for migration in status XML
Add status XML infrastructure for storing a list of block dirty bitmaps
which are temporarily used when migrating a VM with
VIR_MIGRATE_NON_SHARED_DISK for cleanup after a libvirtd restart during
migration.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
49957f5f69 qemu: migration_cookie: Add helpers for transforming the cookie into migration params
'qemuMigrationCookieBlockDirtyBitmapsMatchDisks' maps the bitmaps from
the migration cookie to actual disk objects definition pointers.

'qemuMigrationCookieBlockDirtyBitmapsToParams' converts the bitmap
definitions from the migration cookie into parameters for the
'block-bitmap-mapping' migration parameter.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
f5cd60d478 qemu: migration_cookie: Add XML handling for setting up bitmap migration
In cases where we are copying the storage we need to ensure that also
bitmaps are copied properly. This patch adds migration cookie XML
infrastructure which will allow the migration sides reach consensus on
which bitmaps to migrate.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
b0104664c6 qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature
Add the migration capability flag and the propagation of the
corresponding mapping configuration. The mapping will be produced from
the bitmaps on disk depending on both sides of the migration and the
necessity to perform merges.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
b44e3ca306 qemu: blockjob: Use qemuMonitorBitmapRemove for single bitmap removal
There's no need in the cleanup steps to invoke a transaction to delete a
single bitmap.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
7a8d7d75df qemu: monitor: Introduce qemuMonitorBitmapRemove
The non-transaction wrapper is useful for code paths which want to
delete individual bitmaps or for cleanup after a failed job where we
want to attempt to delete every bitmap individually to prevent a failure
from cleaning up the rest.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:21 +01:00
Peter Krempa
8da9c7456d qemu: migration: Create qcow2 v3 images for VIR_MIGRATE_NON_SHARED_DISK
Use the new format when pre-creating the image for the user. Users
wishing to use the legacy format can always provide their own images or
use shared storage.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:21:15 +01:00
Peter Krempa
253bce93d3 qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes
Such images don't support stuff like dirty bitmaps. Note that the
synthetic test for detecting bitmaps is used as an example to prevent
adding additional test cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:20:32 +01:00
Peter Krempa
9e855e7589 qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING
The capability represents qemu's ability to setup mappings for migrating
block dirty bitmaps and is based on presence of the 'transform' property
of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP
command.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-20 13:20:32 +01:00
Stefan Berger
f30aa2ec74 qemu: Fix libvirt hang due to early TPM device stop
This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop()
was moved to a location before the QEMU process is stopped. It may be
alright to tear down some devices before QEMU is stopped, but it doesn't work
for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop
it before libvirt may try to clean it up. So this patch moves the
virFileDeleteTree() calls after the call to qemuExtDevicesStop() so that the
pid file of virtiofsd is not deleted before that call.

Afftected libvirt versions are 6.10 and 7.0.

Fixes: 5cde9dee8c
Cc: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-19 17:31:37 +01:00
Tim Wiederhake
c0ac0144a3 qemu_validate: Allow kvm hint-dedicated on non-passthrough VMs
A VM defined similar to:
  ...
  <features><kvm><hint-dedicated state='on'/></kvm></features>
  <cpu mode="host-model"/>
  ...
is currently invalid, as hint-dedicated is only allowed if cpu mode
is host-passthrough or maximum. This restriction is unnecessary, see
https://bugzilla.redhat.com/show_bug.cgi?id=1857671

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-02-19 11:11:12 +01:00
Ján Tomko
634516262a hyperv: check return value of virUUIDGenerate
Fixes: fa66bd8cad
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-02-18 17:14:47 +01:00
Ján Tomko
35ce344bfe qemu: monitor: clear cpu props properly in CPUInfoClear
Stay true to the name of the function and clear the pointer
after freeing it.

This also silences a bogus Coverity report about a double
free in qemuMonitorGetCPUInfo where qemuMonitorCPUInfoClear
is called right after allocating a new qemuMonitorCPUInfo
to fill out the non-zero defaults.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-02-18 17:14:47 +01:00
Ján Tomko
a8ed27e70a qemu: saveimage: only steal domXML on success
The comment and the caller assume virQEMUSaveDataNew only steals
domXML on success, but it is copied even on failure.

Also remove the misleading g_steal_pointer call on a local variable.

Reported by coverity.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-02-18 17:14:47 +01:00
Ján Tomko
f4943d3c18 security: dac: remove leftover virPCIDeviceFree
The switch to g_auto left this one call behind.

Reported by Coverity.

Fixes: 4ab0d1844a
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-02-18 17:14:47 +01:00
Daniel Henrique Barboza
d9977cc0f1 qemu_driver.c: Coverity fix in qemuNodeDeviceDetachFlags()
Commit 76f4788932 made qemuNodeDeviceDetachFlags() unusable due to an
'if then else if' chain that will always results in a 'return -1',
regardless of 'driverName' input.

Found by Coverity.

Fixes: 76f4788932
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-18 10:06:30 -03:00
Ján Tomko
e2ee16dcd3 esx: use g_autofree for datastoreRelatedPath
Reported by Coverity.

Fixes: 213662813c
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-02-18 13:54:02 +01:00
Yi Li
21d83d37ff qemuBlockDiskDetectNodes: just return when alias is null
Just return when alias is null and Remove the 'ret' variable.

Signed-off-by: Yi Li <yili@winhong.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-02-18 08:35:08 +01:00
Daniel Henrique Barboza
33ddfaf4e6 scripts/check-aclrules.py: check ACL for domain_driver.c ACL callers
This script works under two specific conditions. For each opened file,
search for all functions that has ACL calls and store them, and see
if there is a vir*DriverPtr struct declared in it. For each implementation
found, check if there is an ACL verification inside it, and error out if
none was found. The script also supports the concept of stub, where another
function takes the responsibility for the ACL call instead of the
original API.

Unfortunately this is not enough to cover the new scenario we have now,
with domain_driver.c containing helper functions that execute the ACL
calls. The script does not store state between files because, until now,
it wasn't needed to - APIs and stubs and vir*DriverPtr declarations were
always in the same file. Also, the script will not check for ACL in functions
that does not belong to a vir*DriverPtr interface. What we have now in
domain_driver.c breaks both assumptions: the functions are in a different
file, and there is no vir*DriverPtr being implemented in the file that
uses these functions.

This patch changes check-aclrules.py to accomodate this scenario. The helpers
that have ACL checks are stored beforehand in aclFuncHelpers, allowing other
files to use them to recognize a stub situation. In case the current file
being analyzed is domain_driver.c itself, we'll do a manual check using
aclFuncHelpers to verify that these functions indeed have ACL checks.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:56:53 -03:00
Daniel Henrique Barboza
22a026fbcd domain_driver.c: use g_auto* in virDomainDriverNodeDeviceDetachFlags()
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:56:39 -03:00
Daniel Henrique Barboza
887dd0d331 qemu, libxl, hypervisor: use virDomainDriverNodeDeviceDetachFlags() helper
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.

Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.

The helper is named with the 'Flags' suffix, even when the helper itself
isn't receiving the flags from the callers, to be compliant with the
ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called
inside it and was called from the original functions. Renaming the helper
would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the
related structs inside remote_protocol.x, to be compliant with the ACL
rules.

This is not being checked at this moment, but we'll fix check-aclrules.py to
verify all the helpers that calls ACL functions in domain_driver.c shortly.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:56:27 -03:00
Daniel Henrique Barboza
76f4788932 qemu_driver.c: validate 'driverName' earlier in qemuNodeDeviceDetachFlags()
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:53:00 -03:00
Daniel Henrique Barboza
04871c511e libxl_driver.c: validate 'driverName' earlier in libxlNodeDeviceDetachFlags()
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:53:00 -03:00
Daniel Henrique Barboza
fc71d91428 domain_driver.c: use g_auto* in virDomainDriverNodeDeviceReAttach()
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:53:00 -03:00
Daniel Henrique Barboza
23cdab6a3d qemu, libxl, hypervisor: use virDomainDriverNodeDeviceReAttach() helper
libxlNodeDeviceReAttach() and qemuNodeDeviceReAttach() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.

Put the common code into virDomainDriverNodeDeviceReAttach() to reduce
code duplication.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:52:50 -03:00
Daniel Henrique Barboza
89450b5e73 domain_driver.c: use g_auto* in virDomainDriverNodeDeviceReset()
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:47:47 -03:00
Daniel Henrique Barboza
714b22d745 datatypes.h: register AUTOPTR_CLEANUP_FUNC for virNodeDevicePtr
Next patch will use g_autoptr() with virNodeDevicePtr for cleanups.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:47:47 -03:00
Daniel Henrique Barboza
42923b1356 qemu, libxl, hypervisor: use virDomainDriverNodeDeviceReset() helper
libxlNodeDeviceReset() and qemuNodeDeviceReset() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.

Put the common code into virDomainDriverNodeDeviceReset() to reduce
code duplication.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-17 15:47:40 -03:00
BiaoXiang Ye
613e994af0 rpc: avoid crash when system time jump back
Setting the system time backward would lead to a
 multiplication overflow in function virKeepAliveStart.
 The function virKeepAliveTimerInternal got the same bug too.

 Backtrace below:
 #0  0x0000ffffae898470 in raise () from /usr/lib64/libc.so.6
 #1  0x0000ffffae89981c in abort () from /usr/lib64/libc.so.6
 #2  0x0000ffffaf9a36a8 in __mulvsi3 () from /usr/lib64/libvirt.so.0
 #3  0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
     count=count entry=0) at ../../src/rpc/virkeepalive.c:283
 #4  0x0000ffffaf908560 in virNetServerClientStartKeepAlive (client=0xaaaaf954cbe0)
     at ../../src/rpc/virnetserverclient.c:1628
 #5  0x0000aaaac57eb6dc in remoteDispatchConnectSupportsFeature (server=0xaaaaf95309d0,
     msg=0xaaaaf9549d90, ret=0xffff8c007fc0, args=0xffff8c002e70, rerr=0xffff9ea054a0,
     client=0xaaaaf954cbe0) at ../../src/remote/remote_daemon_dispatch.c:5063
 #6  remoteDispatchConnectSupportsFeatureHelper (server=0xaaaaf95309d0, client=0xaaaaf954cbe0,
     msg=0xaaaaf9549d90, rerr=0xffff9ea054a0, args=0xffff8c002e70, ret=0xffff8c007fc0)
     at ./remote/remote_daemon_dispatch_stubs.h:3503
 #7  0x0000ffffaf9053a4 in virNetServerProgramDispatchCall(msg=0xaaaaf9549d90, client=0xaaaaf954cbe0,
     server=0x0, prog=0xaaaaf953a170) at ../../src/rpc/virnetserverprogram.c:451
 #8  virNetServerProgramDispatch (prog=0xaaaaf953a170, server=0x0, server entry=0xaaaaf95309d0,
     client=0xaaaaf954cbe0, msg=0xaaaaf9549d90) at ../../src/rpc/virnetserverprogram.c:306
 #9  0x0000ffffaf90a6bc in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>,
     client=<optimized out>, srv=0xaaaaf95309d0) at ../../src/rpc/virnetserver.c:137
 #10 virNetServerHandleJob (jobOpaque=0xaaaaf950df80, opaque=0xaaaaf95309d0)
     at ../../src/rpc/virnetserver.c:154
 #11 0x0000ffffaf812e14 in virThreadPoolWorker (opaque=<optimized out>)
     at ../../src/util/virthreadpool.c:163
 #12 0x0000ffffaf81237c in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:246
 #13 0x0000ffffaea327ac in ?? () from /usr/lib64/libpthread.so.0
 #14 0x0000ffffae93747c in ?? () from /usr/lib64/libc.so.6
 (gdb) frame 3
 #3  0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
     count=count entry=0) at ../../src/rpc/virkeepalive.c:283
 283            timeout = ka->interval - delay;
 (gdb) list
 278    now = time(NULL);
 279    delay = now - ka->lastPacketReceived; <='delay' got a negative value
 280    if (delay > ka->interval)
 281        timeout = 0;
 282    else
 283        timeout = ka->interval - delay;
 284    ka->intervalStart = now - (ka->interval - timeout);
 285    ka->timer = virEventAddTimeout(timeout * 1000, virKeepAliveTimer, <= multiplication overflow
 286                                   ka, virObjectFreeCallback);
 287    if (ka->timer < 0)
 (gdb) p now
 $2 = 18288001
 (gdb) p ka->lastPacketReceived
 $3 = 1609430405

Signed-off-by: BiaoXiang Ye <yebiaoxiang@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-02-17 18:53:44 +01:00
Jim Fehlig
6f8a8671cc libxl: Use g_autofree for char* where easily possible
All of these strings are allocated once, freed once, and are never
returned out of the function where they are declared.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-17 10:25:53 -07:00
Bruno Haible
66611bc0df util: Fix file descriptor passing on 64-bit FreeBSD and NetBSD.
* src/util/virsocket.c (virSocketRecvFD): Set msg.msg_controllen as documented
in the man pages.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-17 10:15:33 +00:00
Michal Privoznik
1f8e6a6172 esx: Bump minimal version of curl
According to meson.build the minimal version of curl needed is
7.18.0 which was released in January 2008. If the minimal version
is bumped to 7.19.1 (released in November 2008) we can drop some
workarounds because this newer version provides APIs we need.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-17 10:21:26 +01:00
Laine Stump
fa3d8a609a esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
These are all cases when 1) the pointer is passed by reference from
the caller (ie.e. **) and expects it to be NULL on return if there is
an error, or 2) the variable holding the pointer is being checked or
re-used in the same function, but not right away.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:05 -05:00
Laine Stump
b10402ece1 esx: eliminate unnecessary cleanup: labels and result variables
switching to g_autofree left many cleanup: sections empty.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:05 -05:00
Laine Stump
d6e357c47c esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope
Or when it will be immediately have a new value assigned to it.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:05 -05:00
Laine Stump
0a69fa61cd esx: reorder code to avoid need to VIR_FREE mimeType
mimeType is initialized to NULL, and then only set in one place, just
before a check (not involving mimeType) that then VIR_FREEs mimeType
if it fails. If we just reorder the code to do the check prior to
setting mimeType, then there won't be any need to VIR_FREE(mimeType)
on failure (because it will already be empty/NULL).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:05 -05:00
Laine Stump
33d3ebff71 esx: use g_steal_pointer+g_autofree on return value
If we put the potential return string into the g_autofreed tmpResult,
and the move it to the returned "result" only as a final step ater, we
can avoid the need to explicitly VIR_FREE (or g_free) on failure.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:05 -05:00
Laine Stump
443c79dd7f esx: switch VIR_FREE->g_free in esx*Free*()
Although the three functions esxFreePrivate(), esxFreeStreamPrivate(),
and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in
theory the caller of the function might rely on "object" (the free
function's arg) being set to NULL, in practice these functions are
only called from a couple places each, and in all cases the pointer
that is passed is a local variable, and goes out of scope almost
immediately after calling the Free function, so it is safe to change
VIR_FREE() into g_free().

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:04 -05:00
Laine Stump
d79a6e2c8c esx: fix memory leak by switching to g_autofree
volumeName was defined at the top of the function, then a new string
was assigned to it each time through a loop, but after the first
iteration of the loop, the previous string wasn't freed before
allocating a new string the next time. By reducing the scope of
volumeName to be just the loop, and making it g_autofree, we eliminate
the leak.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:04 -05:00
Laine Stump
22a370d8b1 esx: use g_autofree when made possible by reducing scope
These strings were being VIR_FREEd multiple times because they were
defined at the top of a function, but then set each time through a
loop. But they are only used inside that loop, so they can be
converted to use g_autofree if their definition is also placed inside
that loop.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:04 -05:00
Laine Stump
213662813c esx: use g_autofree for char* where it is trivially possible
All of these strings are allocated once, freed once, and are never
returned out of the function where they are created, used, and are
freed.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-16 13:50:04 -05:00
Daniel P. Berrangé
2931839966 qemu: remove support for generating yes|no boolean options
All callers are now using the on|off syntax, so yes|no is a unreachable
code path.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Daniel P. Berrangé
0d981fcd97 qemu: use on|off instead of yes|no for -drive boolean properties
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.

The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:

  https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html

Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Daniel P. Berrangé
8851d87556 qemu: use on|off instead of yes|no for -object boolean properties
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.

The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:

  https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html

Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Daniel P. Berrangé
cff6236105 qemu: use on|off for -vnc boolean option values
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.

The long format has been supported with -vnc since the change to use
QemuOpts in 2.2.0, so we check based on the new capability flag.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:11 +00:00
Daniel P. Berrangé
a4f57fa37d qemu: probe for -vnc supporting use of QemuOpts syntax
This was introduced in QEMU 2.2.0, and is visible by -vnc appearing in
the "query-command-line-options" data.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:02:59 +00:00
Michal Privoznik
2642cc0f66 qemu: Don't lie about @ndevAlias when translating FSInfo
When virDomainGetFSInfo() is called over a QEMU/KVM domain it
results into calling of 'guest-get-fsinfo' guest agent command to
which it replies with info on guest (mounted) filesystems. When
filling return structure we also try to do basic lookup and
translate guest agent provided disk address into disk target (as
seen in domain XML). This can of course fail - guest can have
variety of disks not recorded in domain XML (iSCSI, scsi_debug,
NFS to name a few). If that's the case, a debug message is logged
and no disk target is added into the return structure.

However, due to the way our code is written the caller is led to
believe that the target was added into the structure. This may
lead to a situation where the array of disk targets (strings)
contains NULL. But our RPC structure says the array contains only
non-NULL strings. This results in somewhat 'cryptic' (at least to
users) error message:

  error: Unable to get filesystem information
  error: Unable to encode message payload

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919783
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-16 14:06:31 +01:00
Michal Privoznik
9ad1e1d897 qemu: Bring if() outside from loop in virDomainFSInfoFormat()
After previous commit, the freeing of @info_ret inside of
virDomainFSInfoFormat() looks like this:

  for () {
    if (info_ret)
      virDomainFSInfoFree(info_ret[i]);
  }

It is needless to compare @info_ret against NULL in each
iteration. We can switch the order and do the comparison first
followed by the loop.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-16 14:06:31 +01:00
Michal Privoznik
59c80e9fd0 qemu: Move qemuAgentFSInfo array free into qemuDomainGetFSInfo()
When qemuDomainGetFSInfo() is called it calls
qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
guest agent command, parses returned JSON and returns an array of
qemuAgentFSInfo structures (well, pointers to those structs).
Then it grabs a domain job and tries to do some matching of guest
returned info against domain definition. This matching is done in
virDomainFSInfoFormat() which also frees the array of
qemuAgentFSInfo structures allocated earlier.

But this is not just. If acquiring the domain job fails (or
domain activeness check executed right after that fails) then
virDomainFSInfoFormat() is not called, leaking the array of
structs.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-16 14:06:31 +01:00
Michal Privoznik
5922b2b104 qemu: Drop needless check in virDomainFSInfoFormat()
As the very first thing, this function checks whether the number
of items inside @agentinfo array is not negative. This is
redundant as the only caller - qemuDomainGetFSInfo() already
checked for that and would not even call this function if that
was the case.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-02-16 14:06:31 +01:00
Daniel P. Berrangé
67f8ccb4e2 qemu: use long on|off syntax for -spice boolean option values
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.

The long format has been supported with -spice since at least 1.5.3,
so we don't need to check for it.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 12:38:30 +00:00
Daniel P. Berrangé
43c9c0859f qemu: use long on|off syntax for -chardev boolean option values
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.

The long format has been supported with -chardev since at least 1.5.3,
so we don't need to check for it.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 12:38:20 +00:00
Peter Krempa
ce8670ef46 qemuSnapshotFSFreeze: Don't return -2
The -2 value is misleading because if 'qemuAgentFSFreeze' fails it
doesn't necessarily mean that the command was sent to the agent.

Since callers don't care about the -2 value specifically, remove it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-02-16 12:25:30 +01:00
Peter Krempa
ee91c82533 qemuSnapshotCreateActiveExternal: Don't thaw filesystems when freeze fails
If we didn't freeze any filesystems we should not even attempt thawing
them. Additionally 'guest-fsfreeze-freeze' fails if the filesystems are
already frozen, where thawing them may break users data integrity if
they used VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE accidentally after an
explicit virDomainFSFreeze and the next snapshot without that flag would
be taken with already thawed filesystems.

This effectively reverts 7c736bab06 .
Libvirt nowadays checks whether the guest agent is connected and pings
it before issuing an command so it's very unlikely that we'd end up in a
situation where qemuSnapshotCreateActiveExternal froze filesystems and
didn't thaw them.

Additionally we now discourage the use of
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE since users have better control if
they freeze the FS themselves.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-02-16 12:25:30 +01:00
Peter Krempa
ec86b8fa29 api: Discourage use of VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
The flag creates additional points of failure which are hard to recover
from, such as when thawing of the filesystems fails after an otherwise
successful snapshot.

Encourage use of explicit virDomainFSFreeze/virDomainFSThaw.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-02-16 12:25:30 +01:00
Peter Krempa
b805ff66d4 qemuMigrationSrcPerformPeer2Peer3: Don't leak 'dom_xml' on cleanup
Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code
paths jumping to cleanup prior to the deallocation which is done right
after it's not needed any more since it's a big string.

Noticed when running under valgrind:

==2204780== 8,192 bytes in 1 blocks are definitely lost in loss record 2,539 of 2,551
==2204780==    at 0x483BCE8: realloc (vg_replace_malloc.c:834)
==2204780==    by 0x4D890DF: g_realloc (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780==    by 0x4DA3AF0: g_string_append_vprintf (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780==    by 0x4917293: virBufferAsprintf (virbuffer.c:307)
==2204780==    by 0x49B0B75: virDomainChrDefFormat (domain_conf.c:26109)
==2204780==    by 0x49E25EF: virDomainDefFormatInternalSetRootName (domain_conf.c:28956)
==2204780==    by 0x15F81D24: qemuDomainDefFormatBufInternal (qemu_domain.c:6204)
==2204780==    by 0x15F8270D: qemuDomainDefFormatXMLInternal (qemu_domain.c:6229)
==2204780==    by 0x15F8270D: qemuDomainDefFormatLive (qemu_domain.c:6279)
==2204780==    by 0x15FD8100: qemuMigrationSrcBeginPhase (qemu_migration.c:2395)
==2204780==    by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer3 (qemu_migration.c:4640)
==2204780==    by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer (qemu_migration.c:5093)
==2204780==    by 0x15FE0F0D: qemuMigrationSrcPerformJob (qemu_migration.c:5168)
==2204780==    by 0x15FE280E: qemuMigrationSrcPerform (qemu_migration.c:5372)
==2204780==    by 0x15F9BA3D: qemuDomainMigratePerform3Params (qemu_driver.c:11841)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-16 12:25:30 +01:00
Peter Krempa
fb1fb62db6 virDomainMigrateVersion3Full: Don't set 'cancelled' to the same value
It's already initialized to '1'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2021-02-16 12:25:30 +01:00
Michal Privoznik
a1229335f6 qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()
In one of my previous commits I've made an attempt to restore the
noqueue qdisc on a TAP corresponding to domain's <interface/> if
QoS is cleared out. The commit consisted of two almost identical
hunks. In both the pointer is dereferenced. But in one of them,
the pointer to new bandwidth can't be NULL while in the other it
can leading to a crash.

Fixes: d53b092353
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-16 09:05:33 +01:00
Pavel Hrdina
6a1f5e8a4f vircgroup: correctly free nested virCgroupPtr
Fixes: 184245f53b

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-15 19:21:35 +01:00
Laine Stump
d2b0ee0aff vmware: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
04e90f72a7 util: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
4abf2d5d74 qemu: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
d835c0affe remote: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
c80efb9b60 openvz: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
f42be9ae48 locking: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
11c48fe6eb conf: convert VIR_FREE to g_free in other functions that free their arg
Previous patches have converted VIR_FREE to g_free in functions with
names ending in Free() and Dispose(), but there are a few similar
functions with names that don't fit that pattern, but server the same
purpose (and thus can survive the same conversion). in particular
*Free*(), and *Unref().

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Laine Stump
e5339e38ca esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE
Invocations of the macro ESX_VI__TEMPLATE__FREE() will free the main
object (referenced as "item") that's pointing to all the things being
VIR_FREEd in the body, so it is safe for all the pointers in item to
just be g_freed rather that VIR_FREEd.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Michal Privoznik
859f7e2072 qemu_shim: URI escape root directory
The root directory can be provided by user (or a temporary one is
generated) and is always formatted into connection URI for both
secret driver and QEMU driver, like this:

  qemu:///embed?root=$root

But if it so happens that there is an URI unfriendly character in
root directory or path to it (say a space) then invalid URI is
formatted which results in unexpected results. We can trust
g_dir_make_tmp() to generate valid URI but we can't trust user.
Escape user provided root directory. Always.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 17:59:42 +01:00
Daniel P. Berrangé
970a59d746 qemu: implement virDomainGetMessages API
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 09:19:13 +00:00
Daniel P. Berrangé
07308b9789 remote: add RPC support for the virDomainGetMessages API
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 09:19:13 +00:00
Daniel P. Berrangé
c80911f2de src: define virDomainGetMessages API
This API allows fetching a list of informational messages recorded
against the domain. This provides a way to give information about
tainting of the guest due to undesirable actions/configs, as well
as provide details of deprecated features.

The output of this API is explicitly targetted at humans, not
machines, so it is inappropriate to attempt to pattern match on
the strings and take action off them, not least because the messages
are marked for translation.

Should there be a demand for machine targetted information, this
would have to be addressed via a new API, and is not planned at
this point in time.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 09:19:12 +00:00
Daniel P. Berrangé
17f001c451 qemu: record deprecation messages against the domain
These messages are only valid while the domain is running.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 09:19:12 +00:00
Daniel P. Berrangé
842900dc1e conf: record deprecation messages against the domain
These messages will be stored in the live status XML.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-12 09:19:12 +00:00
Laine Stump
010ed0856b qemu: plug <teaming> config from <hostdev> into qemu commandline
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:21:59 -05:00
Laine Stump
db64acfbda conf: parse/format <teaming> element in plain <hostdev>
The <teaming> element in <interface> allows pairing two interfaces
together as a simple "failover bond" network device in a guest. One of
the devices is the "transient" interface - it will be preferred for
all network traffic when it is present, but may be removed when
necessary, in particular during migration, when traffic will instead
go through the other interface of the pair - the "persistent"
interface. As it happens, in the QEMU implementation of this teaming
pair (called "virtio failover" in QEMU) the transient interface is
always a host network device assigned to the guest using VFIO (aka
"hostdev"); the persistent interface is always an emulated virtio NIC.

When support was initially added for <teaming>, it was written to
require that the transient/hostdev device be defined using <interface
type='hostdev'>; this was done because the virtio failover
implementation in QEMU and the virtio guest driver demands that the
two interfaces in the pair have matching MAC addresses, and the only
way libvirt can guarantee the MAC address of a hostdev network device
is to use <interface type='hostdev'>, whose main purpose is to
configure the device's MAC address before handing the device to
QEMU. (note that <interface type='hostdev'> in turn requires that the
network device be an SRIOV VF (Virtual Function), as that is the only
type of network device whose MAC address we can set in a way that will
survive the device's driver init in the guest).

It has recently come up that some users are unable to use <teaming>
because they are running in a container environment where libvirt
doesn't have the necessary privileges or resources to set the VF's MAC
address (because setting the VF MAC is done via the same device's PF
(Physical Function), and the PF is not exposed to libvirt's container).

At the same time, these users *are* able to set the VF's MAC address
themselves in advance of staring up libvirt in the container. So they
could theoretically use the <teaming> feature if libvirt just skipped
the "setting the MAC address" part.

Fortunately, that is *exactly* the difference between <interface
type='hostdev'> (which must be a "hostdev VF") and <hostdev> (a "plain
hostdev" - it could be *any* PCI device; libvirt doesn't know what type
of PCI device it is, and doesn't care).

But what is still needed is for libvirt to provide a small bit of
information on the QEMU commandline argument for the hostdev, telling
QEMU that this device will be part of a team ("failover pair"), and
the id of the other device in the pair.

To make both of those goals simultaneously possible, this patch adds
support for the <teaming> element to plain <hostdev> - libvirt doesn't
try to set any MAC addresses, and QEMU gets the extra commandline
argument it needs)

(actually, this patch adds only the parsing/formatting of the
<teaming> element in <hostdev>. The next patch will actually wire that
into the qemu driver.)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:15:34 -05:00
Laine Stump
dea2710911 conf: separate Parse/Format functions for virDomainNetTeamingInfo
In preparation for using the same element in two places, split the
parsing/formating for that subelement out of the virDomainNetDef
functions into their own functions.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 16:31:52 -05:00
Laine Stump
13be68094d conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo
To make it easier to split out the parsing/formatting of the <teaming>
element into separate functions (so we can more easily add the
<teaming> element to <hostdev>, change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 16:31:51 -05:00
Laine Stump
5d74e2f168 conf: make teaming info an official type
This struct was previously defined only within virDomainNetDef where
it was used, but I need to also use it in virDomainHostdevDef, so move
the internal struct out to its own "official" struct and give it the
standard typedef duo and *Free() function.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 16:31:51 -05:00
Laine Stump
114e3b4232 qemu: match alias when looking for proper <interface> to detach.
Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).

Resolves: https://bugzilla.redhat.com/1926190
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2021-02-11 16:07:09 -05:00
Jim Fehlig
afb823fc50 qemu: Validate TPM TIS device
TPM devices with model='tpm-tis' are only valid with x86 and aarch64
virt machines. Add a check to qemuValidateDomainDeviceDefTPM() to
ensure VIR_DOMAIN_TPM_MODEL_TIS is only used with these architectures.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-02-11 10:08:52 -07:00
Jim Fehlig
7cf60006ce qemu: Fix swtpm device with aarch64
Starting a VM with swtpm device fails with qemu-system-aarch64.
E.g. with TPM device config

     <tpm model='tpm-tis'>
       <backend type='emulator' version='2.0'/>
      </tpm>

QEMU reports the following error

error: internal error: process exited while connecting to monitor:
2021-02-07T05:15:35.378927Z qemu-system-aarch64: -device
tpm-tis,tpmdev=tpm-tpm0,id=tpm0: 'tpm-tis' is not a valid device model name

Indeed the TPM device name is 'tpm-tis-device' [1][2] for aarch64,
versus the shorter 'tpm-tis' for x86. The devices are the same from
a functional POV, i.e. they both emulate a TPM device conforming to
the TIS specification. Account for the unfortunate name difference
when building the TPM device option in qemuBuildTPMDevStr(). Also
include a test case for 'tpm-tis-device'.

[1] https://qemu.readthedocs.io/en/latest/specs/tpm.html
[2] c294ac327c

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2021-02-11 10:08:52 -07:00
Peter Krempa
cafde24a9a util: virstring: Remove virStringListJoin
The glib alternative is now used everywhere.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
480fecaa21 Replace virStringListJoin by g_strjoinv
Our implementation was inspired by glib anyways. The difference is only
the order of arguments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
dc7ac81d37 virStringSplitCount: Reimplement using g_strsplit and g_strv_length
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
50cc5d7035 util: virstring: Remove virStringSplit
Callers were replaced by g_strsplit.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
10157731f4 Replace virStringSplit with g_strsplit
Our implementation was heavily inspired by the glib version so it's a
drop-in replacement.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
51f8baee8d util: virstring: Remove virStringListLength
glib provides g_strv_length.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
dcd547aec1 Replace virStringListLength by g_strv_length
The glib implementation doesn't tolerate NULL but in most cases we check
before anyways. The rest of the callers adds a NULL check.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
1114cf5e7e virPolkitCheckAuth: Avoid virStringListLength in loop condition
Don't re-calculate the string list length on every iteration. Convert
the loop to NULL-terminated iteration.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
2f9b2c0cdd Replace virStringListLength where actual lenght is not needed
Some callers don't need to know the actual lenght of the list but only
care whether the required element is present or the list is non-empty.
Don't calculate the list length in those cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
81030c44e0 virStorageBackendSheepdogAddVolume: Clean up memory handling
'cells' can be pushed into the loop removing the need for manual
cleanup, the check whether 'line' is NULL inside of the loop is always
false since the loop checks it right before and 'line' variable is
unnecessary.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
4661ea3578 util: virstring: Remove virStringListHasString
All callers were converted to the glib alternative. Providing our own
just to have NULL tolerance doesn't make sense.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
56cedfcf38 Replace virStringListHasString by g_strv_contains
The glib variant doesn't accept NULL list, but there's just one caller
where it wasn't checked explicitly, thus there's no need for our own
wrapper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
d9f7e87673 qemuProcessUpdateDevices: Refactor cleanup and memory handling
Use automatic memory freeing and remove the 'cleanup' label. Also make
it a bit more obvious that nothing happens if the 'old' list wasn't
present.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
ca1899fc45 vz: Replace virStringSplitCount(, , , NULL) with virStringSplit
The caller doesn't care about the number of tokens so use the function
which doesn't return it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
e6c9c44e9a virStringListGetFirstWithPrefix: Remove unused helper
This is a uncommon and trivial operation, so having an utility function
for it is pointless.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
7cc3418915 virCgroupGetValueForBlkDev: Rewrite lookup of returned string
Lookup the string with prefix locally so that we can remove the helper
which isn't universal at all.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
2b3e390674 util: virstring: Remove virStringListAdd and virStringListRemove
virStringListAdd hides the fact that a O(n) count of elements is
performed every time it's called which makes it inefficient.

Stop supporting such semantics and remove the helpers. Users have a
choice of using GSList or an array with a counter variable rather than
repeated lookups.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
4a33825314 xenParseXLNamespaceData: Pre-calculate the length of array
Precalculate the lenght to avoid use of 'virStringListAdd' in a loop.
The code is also simplified by using APIs which don't return errors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
00dfd9c97d util: macmap: Convert to use GSList for storing macs instead of string lists
Since adding and removing is the main use case for the macmap module,
convert the code to a more efficient data structure.

The refactor also optimizes the loading from file where previously we'd
do a hash lookup + list lenght calculation for every entry.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
4ebc278657 qemu: Convert 'priv->dbusVMStateIds' to a GSList
The conversion removes the use of virStringListAdd/virStringListRemove
which try to add dynamic properties to a string list which is really
inefficient.

Storing the dbus VMState ids in a GSList is pretty straightforward and
the slightly increased complexity of the code will be paid back by
removing the string list helpers later.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
b297714793 virResctrlMonitorGetStats: Don't use 'virStringListAdd'
The iner loop copies the 'resources' array multiple times using
'virStringListAdd' which has O(n^2) complexity.

Pre-calculate the length so we can allocate the array upfront and just
copy the strings in the loop.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
34088ea47f virResctrlInfoGetMonitorPrefix: Don't use 'virStringListAdd' to construct list
Pre-allocate a buffer for the upper limit and shrink it afterwards to
avoid use of 'virStringListAdd' in a loop.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
fb51b85ea2 x86ModelParseFeatures: Don't construct list using 'virStringListAdd'
Pre-allocate the list to the upper bound and fill it gradually. Since
the data is kept long-term and the list won't be populated much shrink
it to the actual size after parsing.

While using 'virStringListAdd' here wouldn't be as expensive as this
function is used just once, the removal will allow to remove
'virStringListAdd' altogether to discourage the antipattern it promotes.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
dad3827d6a virCPUDefCheckFeatures: Don't use 'virStringListAdd' to construct list
We already know the upper bound of items we might need so we can
allocate the array upfront and avoid the quadratic complexity of
'virStringListAdd'.

In this instance the returned data is kept only temporarily so a
potential unused space due to filtered-out entries doesn't impose a
long-term burden on memory.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
f060d62c75 qemuInteropFetchConfigs: Don't use 'virStringListAdd' to construct list
'virHashGetItems' already returns the number of entries which will be
considered for addition to the list so we can allocate it to the upper
bound upfront rather than growing it in a loop. This avoids the
quadratic complexity of 'virStringListAdd'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
97fd333fde virHookCall: Don't use 'virStringListAdd' to construct list in loop
'virStringListAdd' calculates the string list length on every invocation
so constructing a string list using it results in O(n^2) complexity.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
9afed29b45 qemu: namespace: Don't use 'virStringListAdd' inside loops
'virStringListAdd' calculates the string list length on every invocation
so constructing a string list using it results in O(n^2) complexity.

Use a GSList which has cheap insertion and iteration and doesn't need
failure handling.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
0ec601bc48 util: Add helpers for auto-freeing GSList filled with strings
glib's 'g_autoslist()' doesn't support lists of 'char *' strings. Add a
type alias 'virGSListString' so that we can register an 'autoptr'
function for it for simple usage of GSList with strings.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
b020663381 qemuNamespaceUnlinkPaths: Fix inconsistent cleanup handling
Some code paths return -1 directly while others jump to 'cleanup' which
cleans the list of mounts. Since qemuDomainGetPreservedMounts now
returns a NULL-terminated list, convert devMountsPath to g_auto(GStrv)
and remove the cleanup altoghether.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
e310900e50 qemuNamespaceUnlinkPaths: Fix wrong use of iterator variable
'i' is used in both outer and inner loop. Since 'devMountsPath' is now a
NULL-terminated list, we can use a GStrv to iterate it;

Additionally rewrite the conditional of adding to the 'unlinkPaths'
array so that it's more clear what's happening.

Fixes: 5c86fbb72d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
5ab8342e64 qemuDomainGetPreservedMounts: Refactor to return NULL-terminated string lists
Refactor the handling of internals so that NULL-terminated lists are
always returned.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
32def543d1 util: macmap: Remove unused cleanup labels and 'ret' variables
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
a2e64fc6af util: virmacmap: Use g_autofree for virJSONValue
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:31 +01:00
Michal Privoznik
677c90cc1d qemu: Do not Use canonical path for system memory
In commit 88957116c9 I've adapted
libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
switched to memory-backend-* even for system memory. My claim was
that that's what QEMU does under the hood anyway. And indeed it
was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
look at function create_default_memdev().

However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
merged into QEMU. While it was fixing a bug, it also changed the
create_default_memdev() function in which it started turning off
use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This
wasn't documented until QEMU commit
8db0b20415c129cf5e577a593a4a0372d90b7cc9. The path affects
migration - the same path has to be used on the source and on the
destination. Therefore, if there is old guest started with '-m X'
it has "pc.ram" block which doesn't use canonical path and thus
when migrating to newer QEMU which uses memory-backend-* we have
to turn off the canonical path explicitly. Otherwise,
"/objects/pc.ram" path would be expected by QEMU which doesn't
match the source.

Ideally, we would need to set it only for some machine types
(4.0 and older) because newer machine types already do what we
are doing. However, we treat machine types as opaque strings and
therefore we don't want to parse nor inspect their versions. But
then again, newer machine types already do what we are doing in
this commit, so when old machine types are deprecated and removed
we can remove our hack and forget it ever happened.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-11 08:43:14 +01:00
Michal Privoznik
204dfbe15d qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
This capability tracks whether memory-backend-file has
"x-use-canonical-path-for-ramblock-id" attribute. Introduced into
QEMU by commit fa0cb34d2210cc749b9a70db99bb41c56ad20831. As of
QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9 the property
is considered stable by qemu despite the 'x-' prefix to preserve
compatibility with released qemu versions.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-11 08:42:06 +01:00
Jim Fehlig
b9a063cd8e build: Remove unused 'conflicts' key from virt_daemon_unit
The 'conflict' key in a virt_daemon_unit dictionary is not used when
generating systemd service and socket files. The comment associated
with the key claims the default is 'true', and a few build files
needlessly set it to 'true' when defining their virt_daemon_unit.
Remove the 'conflict' key and its use in the affect build files.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 09:30:41 -07:00
Pavel Hrdina
184245f53b vircgroup: introduce nested cgroup to properly work with systemd
When running on host with systemd we register VMs with machined.
In this case systemd creates the root VM cgroup for us. This has some
implications where one of them is that systemd owns all files inside
the root VM cgroup and we should not touch them.

We already use DBus calls for some of the APIs but for the remaining
ones we will continue accessing the files directly. Systemd doesn't
support threaded cgroups so we need to do this.

The reason why we don't use DBus for most of the APIs is that we already
have a code that works with files and we would have to check if systemd
supports each API.

This change introduces new topology on systemd hosts:

$ROOT
  |
  +- machine.slice
     |
     +- machine-qemu\x2d1\x2dvm1.scope
        |
        +- libvirt
           |
           +- emulator
           +- vcpu0
           +- vcpu0

compared to the previous topology:

$ROOT
  |
  +- machine.slice
     |
     +- machine-qemu\x2d1\x2dvm1.scope
        |
        +- emulator
        +- vcpu0
        +- vcpu0

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
badc2bcc73 vircgroup: introduce virCgroupV1Exists and virCgroupV2Exists
This will check if the cgroup actually exists on the system.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
382fa15cde vircgroupv2: move task into cgroup before enabling controllers
When we create a new child cgroup and the parent cgroup has any process
attached to it enabling controllers for the child cgroup fails with
error. We need to move the process into the child cgroup first before
enabling any controllers.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
5f56dd7c83 vircgroupv1: refactor virCgroupV1DetectPlacement
Remove one level of indentation by splitting the condition.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
9c1693eff4 vircgroup: use DBus call to systemd for some APIs
When running on host with systemd we register VMs with machined.
In this case systemd creates the root VM cgroup for us. This has some
implications where one of them is that systemd owns all files inside
the root VM cgroup and we should not touch them.

If we change any value in file that systemd knows about it will be
changed to what systemd thinks it should be when executing
`systemctl daemon-reload`.

These are the APIs that we need to call using systemd because they set
limits that are proportional to sibling cgroups.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
d3fb774b1e virsystemd: introduce virSystemdGetMachineUnitByPID
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:11 +01:00