Andrea Bolognani [Thu, 27 Jan 2022 16:17:36 +0000 (17:17 +0100)]
qemu: Rename path-building functions
Using the word "create" can give users the impression that disk
operations will be performed, when in reality all these functions
do is string formatting.
Follow the naming convention established by virBuildPath(),
virFileBuildPath() and virPidFileBuildPath().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Mon, 31 Jan 2022 11:55:47 +0000 (12:55 +0100)]
qemu: Validate domain definition even on migration
When we are about to spawn QEMU, we validate the domain
definition against qemuCaps. Except when domain is/was already
running before (i.e. on incoming migration, snapshots, resume
from a file). However, especially on incoming migration it may
happen that the destination QEMU is different to the source
QEMU, e.g. the destination QEMU may have some devices disabled.
And we have a function that validates devices/features requested
in domain XML against the desired QEMU capabilities (aka
qemuCaps) - it's virDomainDefValidate() which calls
qemuValidateDomainDef() and qemuValidateDomainDeviceDef()
subsequently.
But the problem here is that the validation function is
explicitly skipped over in specific scenarios (like incoming
migration, restore from a snapshot or previously saved file).
This in turn means that we may spawn QEMU and request
device/features it doesn't support. When that happens QEMU fails
to load migration stream:
qemu-kvm: ... 'virtio-mem-pci' is not a valid device model name
(NB, while the example shows one particular device, the problem
is paramount)
This problem is easier to run into since we are slowly moving
validation from qemu_command.c into said validation functions.
The solution is simple: do the validation in all cases. And while
it may happen that users would be unable to migrate/restore a
guest due to a bug in our validator, spawning QEMU without
validation is worse (especially when you consider that users can
supply their own XMLs for migrate/restore operations - these were
never validated).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2048435 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:17 +0000 (17:28 +0100)]
qemu: gpu: Get pid without binary validation
The binary validation in virPidFileReadPathIfAlive may fail with EACCES
if the calling process does not have CAP_SYS_PTRACE capability.
Therefore instead do only the check that the pidfile is locked by the
correct process.
Fixes the same issue as with swtpm.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:16 +0000 (17:28 +0100)]
qemu: tpm: Get swtpm pid without binary validation
Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
in containers) and any attempt to stat(2) or readlink(2) the file will
result in 'permission denied' error if the calling process does not have
CAP_SYS_PTRACE capability. According to proc(5) manpage:
Permission to dereference or read (readlink(2)) this symbolic link is
governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
ptrace(2).
The binary validation in virPidFileReadPathIfAlive may fail with EACCES.
Therefore instead do only the check that the pidfile is locked by the
correct process. To ensure this is always the case the daemonization and
pidfile handling of the swtpm command is now controlled by libvirt.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:15 +0000 (17:28 +0100)]
virpidfile: Add virPidFileReadPathIfLocked func
The function will attempt to read a pid from @path, and store it in
@pid. The @pid will only be set, however, if @path is locked by
virFileLock() at byte 0 and the pid in @path is running.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
conf: Fix documentation for virDomainDefGetShortName()
The @unique argument didn't exist even when the function was
introduced in a042275a396e, and the @vm argument was not renamed
when the function was changed to take a virDomainDef* instead of
a virDomainObj* in 7ed6934f3b92.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
SmartNIC DPUs may not expose some privileged eswitch operations
to the hypervisor hosts. For example, this happens with Bluefield
devices running in the ECPF (default) mode for security reasons. While
VF MAC address programming is possible via an RTM_SETLINK operation,
trying to set a VLAN ID in the same operation will fail with EPERM.
The equivalent ip link commands below provide an illustration:
1. This works:
sudo ip link set enp130s0f0 vf 2 mac de:ad:be:ef:ca:fe
2. Setting (or clearing) a VLAN fails with EPERM:
sudo ip link set enp130s0f0 vf 2 vlan 0
RTNETLINK answers: Operation not permitted
3. This is what Libvirt attempts to do today (when trying to clear a
VF VLAN at the same time as programming a VF MAC).
sudo ip link set enp130s0f0 vf 2 vlan 0 mac de:ad:be:ef:ca:fe
RTNETLINK answers: Operation not permitted
If setting an explicit VLAN ID results in an EPERM, clearing a VLAN
(setting a VLAN ID to 0) can be handled gracefully by ignoring the
EPERM error with the rationale being that if we cannot set this state
in the first place, we cannot clear it either.
In order to keep explicit clearing of VLAN ID working as it used to
be passing a NULL pointer for VLAN ID is used.
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There should be a way to show no intent in programming a VLAN at all
(including clearing it). This allows handling error conditions
differently when VLAN clearing is explicit (vlan id == 0) vs implicit
(vlanid == NULL - try to clear it if possible).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set VF MAC and VLAN ID in two different operations
This has a benefit of being able to handle error codes for those
operations separately which is useful when drivers allow setting a MAC
address but do not allow setting a VLAN (which is the case with some
SmartNIC DPUs).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Most people will want to use isa-debugcon to obtain debug output
for SeaBIOS / EDK II, so let's include a ready-made example for
that scenario in our documentation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The current implementation of the workaround for yajl's broken
pkg-config file accidentally overwrites the value of includedir
that is later used by the installation process. Rename the
local variable to avoid this issue.
Fixes: c97075e1e46e9305d62620d8b05046aae0139438 Closes: https://gitlab.com/libvirt/libvirt/-/issues/271 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
which is used as a way to receive debug messages from the
firmware on x86 platforms.
Note that the default port is hypervisor specific, with QEMU
currently using 0xe9 since that's the original Bochs debug port.
For use with SeaBIOS/OVMF, the iobase port needs to be explicitly
set to 0x402.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()
When virNodeDeviceObjListRemove() is called, the passed
virNodeDeviceObj is removed from internal list of node devices
and then unrefed and unlocked. While the former is warranted (the
object was refed at the beginning of the function) the unlock is
not. In fact, it's wrong from conceptual POV. We still want
threads working on the object tu mutually exclude each other.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Andrea Bolognani [Mon, 17 Jan 2022 15:07:38 +0000 (16:07 +0100)]
qemu: Drop qemuAudioDriver enumeration
This mostly overlaps with virDomainAudioType, but in a couple of
cases the string representations are different.
Right now we're doing that in a somewhat sketchy way, in that we
store values of one enumeration and then convert them to strings
using TypeToString() implementation for the other enumeration;
when converting from string, we open-code the handling of the
special values mentioned above.
Drop the second enumeration and introduce two helpers to deal
with conversion. Most calling sites don't need to be changed, and
one can even be simplified significantly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We recently started listing these in the spec file and, since we
were not creating them during the installation phase, that broke
RPM builds.
Fixes: 4b43da0bff9b78dcf1189388d4c89e524238b41d Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Thu, 27 Jan 2022 10:13:53 +0000 (11:13 +0100)]
qemu_command: Generate memory only after controllers
Currently, memory device (def->mems) part of cmd line is
generated before any controller. In majority of cases it doesn't
matter because neither of memory devices live on a bus that's
created by an exposed controller (e.g. there's no DIMM
controller, at least not exposed). Except for virtio-mem and
virtio-pmem, which do have a PCI address. And if it so happens
that the device goes onto non-default bus (pci.0) starting such
guest fails, because the controller that creates the desired bus
wasn't processed yet. QEMU processes arguments in order.
For instance, if virtio-mem has address with bus='0x01' QEMU
refuses to start with the following message:
Bus 'pci.1' not found
Similarly for virtio-pmem. I've successfully tested migration and
changing the order does not affect migration stream.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2047271 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Mon, 24 Jan 2022 18:05:44 +0000 (19:05 +0100)]
rpm: List more directories
This has two advantages: it makes it possible for the admin to
ask rpm what package they belong to, and results in them ending
up with stricter permissions than they would have if we let
libvirt create them at runtime.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Mon, 24 Jan 2022 15:54:01 +0000 (16:54 +0100)]
rpm: Move /etc/libvirt from -daemon to -libs
Files like libvirt.conf influence the behavior of the library
itself. The daemon depends on the library, so the directory is
guaranteed to be present both on the client side and on the
server side.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 28 Jan 2022 17:06:32 +0000 (18:06 +0100)]
src: Use g_steal_pointer() more
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:
@ rule1 @
expression a, b;
@@
<...
- b = a;
... when != b
- a = NULL;
+ b = g_steal_pointer(&a);
...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Fri, 28 Jan 2022 19:37:42 +0000 (20:37 +0100)]
virpcivpdtest: Fix potential double-free()
Inside the testPCIVPDResourceCustomCompareIndex() function we
have two variables @a and @b, both marked as g_autoptr(). Then,
towards the end of the function b->value is freed and set to
a->value. This is to make sure
virPCIVPDResourceCustomCompareIndex() works correctly even if
->value member is the same for both arguments.
Nevertheless, if the function returns anything else than 0 then
the control executes subsequent return statement and since
b->value points to the very same string as a->value a double free
will occur. Avoid this by setting b->value to NULL explicitly,
just like we are already doing for the successful path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Mon, 31 Jan 2022 12:52:25 +0000 (13:52 +0100)]
test_driver: Don't leak @group_name
In testDomainSetBlockIoTune() the info.group_name is strdup()-ed
and just after the whole @info structure is passed to
virDomainDiskSetBlockIOTune() the @group_name member is set to
NULL. This creates a memleak, because
virDomainDiskSetBlockIOTune() creates its own copy of the string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Tim Wiederhake [Tue, 31 Aug 2021 13:09:51 +0000 (15:09 +0200)]
lxcDomainDetachDeviceHostdevUSBLive: Use VIR_WITH_OBJECT_LOCK_GUARD
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Tue, 24 Aug 2021 15:20:43 +0000 (17:20 +0200)]
bhyveAutostartDomain: Use virObjectLockGuard
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Wed, 25 Aug 2021 08:25:32 +0000 (10:25 +0200)]
virChrdevFree: Use VIR_WITH_MUTEX_LOCK
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Wed, 25 Aug 2021 08:25:32 +0000 (10:25 +0200)]
virChrdevFDStreamCloseCb: Use virLockGuardLock
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Mon, 23 Aug 2021 12:47:25 +0000 (14:47 +0200)]
virobject: Introduce VIR_WITH_OBJECT_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Mon, 23 Aug 2021 14:03:54 +0000 (16:03 +0200)]
virobject: Introduce virObjectLockGuard
Typical usage:
void foobar(virObjectLockable *obj)
{
VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
/* `obj` is locked, and released automatically on scope exit */
...
}
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Wed, 25 Aug 2021 08:25:32 +0000 (10:25 +0200)]
virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Wed, 25 Aug 2021 08:25:32 +0000 (10:25 +0200)]
virthread: Introduce virLockGuard
Locks a virMutex on creation and unlocks it in its destructor.
The VIR_LOCK_GUARD macro is used instead of "g_auto(virLockGuard)" to
work around a clang issue (see https://bugs.llvm.org/show_bug.cgi?id=3888
and https://bugs.llvm.org/show_bug.cgi?id=43482).
Typical usage:
void function(virMutex *m)
{
VIR_LOCK_GUARD lock = virLockGuardLock(m);
/* `m` is locked, and released automatically on scope exit */
...
while (expression) {
VIR_LOCK_GUARD lock2 = virLockGuardLock(...);
/* similar */
}
}
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tim Wiederhake [Wed, 25 Aug 2021 08:25:32 +0000 (10:25 +0200)]
internal: Add CONCAT macro
Using the two-step idiom to force resolution of other macros, e.g.:
#define bar BAR
CONCAT_(foo, bar) // foobar
CONCAT(foo, bar) // fooBAR
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 1 Feb 2022 12:13:05 +0000 (13:13 +0100)]
systemd: Use correct man page name in modular daemon service files
The service files were copied out of the service file for libvirtd and
the name of the corresponding manpage was not fixed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2045959 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The idea behind virNWFilterBindingObjNew() is to create and
return an object of virNWFilterBindingObjClass class. The class
is virObjectLockable (and the corresponding
_virNWFilterBindingObj structure has virObjectLockable parent).
But for some reason plain virObjectNew() is called. This is wrong
because the mutex in the parent is left uninitialized.
Next, the returned object is not locked. This is wrong because in
some cases the returned object is added onto a list of bindings
and then passed to virNWFilterBindingObjEndAPI() which unlocks it
right away. This is potentially dangerous because we might just
have unlocked the object for another thread.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we abort on OOM now, switch testSELinuxMungePath to void,
return NULL explicitly on XML parsing failure and remove
the (now pointless) cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 28 Jan 2022 11:15:01 +0000 (12:15 +0100)]
domain_cgroup: Don't put semicolon at the end of VIR_GET_LIMIT_PARAMETER macro
In domain_cgroup.c there's VIR_GET_LIMIT_PARAMETER macro which
has a semicolon at the end of its declaration. Well, remove it so
that the places where macro is used have to put the semicolon
explicitly. This helps with automatic reformatting (at least in
vim).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vineeth Pillai [Fri, 28 Jan 2022 11:51:33 +0000 (12:51 +0100)]
ch: Move and rename chDomObjFromDomain()
The chDomObjFromDomain() function which currently lives as a
static one in ch_driver.c is going to be needed in other parts
of the driver. Move it into ch_domain.c, rename to
virCHDomainObjFromDomain() and expose in corresponding header
file for the rest of the driver to use.
Signed-off-by: Vineeth Pillai <viremana@linux.microsoft.com> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
qemu, hypervisor: refactor some cgroup mgmt methods
Refactor some cgroup management methods from qemu into hypervisor.
These methods will be shared with ch driver for cgroup management.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
projects / libvirt.git / log