There is no good reason for qemuDomainAttachDeviceLive() to live
in (ever growing) qemu_driver.c while we have qemu_hotplug.c
which already contains the rest of hotplug code. Move the
function to its new home.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The qemuDomainUpdateDeviceLive() accepts virDomainPtr as one of
its arguments, but use it only to get QEMU driver out of it.
Well, the only caller already does that and thus can pass it
instead of virDomainPtr.
This also makes it look like the rest of device hot(un-)plug
functions: qemuDomainAttachDeviceLive() and
qemuDomainUpdateDeviceLive().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
A new enum type "Default" has been added for Input bus.
The logic that handled default input bus types in
virDomainInputParseXML() has been moved to a new function
virDomainInputDefPostParse() in domain_postparse.c
Link to Issue: https://gitlab.com/libvirt/libvirt/-/issues/8
Signed-off-by: K Shiva <shiva_kr@riseup.net>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The disk private data contain information about the tray and
removability of the disk. Until recently we didn't support hotplug of
removable disks thus it wasn't a problem but now when you can hotplug a
CDROM you would not be able to open its tray.
Fix it by updating the hotplugged disk the same way we do at startup.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2160435
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the logic to update one single disk (without emitting any
events) so that it can be reused when updating the state after a disk
hotplug.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code compares the 'tray_open' boolean from 'struct
qemuDomainDiskInfo' directly against 'disk->tray_status' which is
declared as virDomainDiskTray (enum). Now the logic works correctly
because the _OPEN enum has value '1'.
Separate the event emission code from the update code and remember the
old tray state in a separate variable rather than having the sneaky
logic we have today.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
With cgroupv2 this has better effect on the resource allocation. An
excerpt from Documentation/admin-guide/cgroup-v2.rst explains is this
way:
Migrating a process across cgroups is a relatively expensive operation
and stateful resources such as memory are not moved together with the
process. This is an explicit design decision as there often exist
inherent trade-offs between migration and various hot paths in terms
of synchronization cost.
[...]
Setting a non-empty value to "cpuset.mems" causes memory of
tasks within the cgroup to be migrated to the designated nodes if
they are currently using memory outside of the designated nodes.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Most of them are platform devices and only i6300esb can be plugged
multiple times into different PCI slots.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This makes it also work during attach. Also add a test for attaching a
watchdog with incompatible action.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2187278
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The loop initially skipped the first one because it was mainly checking
the incompatible actions, but was then modified to also check the
duplicity of iTCO watchdogs.
While at it change the type of the iteration variable to the usual size_t.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2187133
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We can launch qemu with it, but it will not work since it's not even
probed by the kernel at the mapped address with different machine types
since they are expected to be connected to ISA and not even its newer
LPC counterpart found on q35. And it does not exist on non-x86
architectures.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting QEMU, or when hotplugging a PCI device QEMU might
lock some memory. How much? Well, that's an undecidable problem.
But despite that, we try to guess. And it more or less works,
until there's a counter example. This time, it's a guest with
both <hostdev/> and an NVMe <disk/>. I've started a simple guest
with 4GiB of memory:
# virsh dominfo fedora
Max memory: 4194304 KiB
Used memory: 4194304 KiB
And here are the amounts of memory that QEMU tried to lock,
obtained via:
grep VmLck /proc/$(pgrep qemu-kvm)/status
1) with just one <hostdev/>
VmLck: 4194308 kB
2) with just one NVMe <disk/>
VmLck: 4328544 kB
3) with one <hostdev/> and one NVMe <disk/>
VmLck: 8522852 kB
Now, what's surprising is case 2) where the locked memory exceeds
the VM memory. It almost resembles VDPA. Therefore, treat is as
such.
Unfortunately, I don't have a box with two or more spare NVMe-s
so I can't tell for sure. But setting limit too tight means QEMU
refuses to start.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2014030
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It's quite difficult, if not impossible, to create a working RISC-V VMs
using the current default machine type of 'spike_v1.10'. Change the
default to the more appropriate and virtualization friendly 'virt'
machine type.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
It's quite difficult, if not impossible, to create a usable ARM VMs
using the current default machine type of 'integratorcp'. Change the
default to the more appropriate and virtualization friendly 'virt'
machine type.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
I've tried, then I've tried even harder, but still wasn't able to
make sense of our console backcompat code in all its fine
details. Since I value my sanity, let's just forbid hotunplug of
<console/>, especially since detaching of corresponding <serial/>
works.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When cleaning up after removed device, qemuDomainChrRemove() is
called. But this may fail, in which case we successfully ignore
the failure and virDomainChrDefFree() the device anyway. While it
decreases our memory consumption, it's a bit too far, especially
if the next step is 'virsh dumpxml'. Then our memory consumption
decreases all the way down to zero as we crash.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For a running guest, a <serial/> device can be hotunplugged. This
will then remove also aliased <console/>. Trying to hotplug a
<console/> device then, libvirtd crashed because it dereferences
def->consoles while there's none.
Fixes: 42d53ac799
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When removing the compat console from domain defintion, removing
it from the vmdef->consoles array is good, but not sufficient.
The console definition might have been fully allocated (after
daemon restarted and reloaded the status XML). Use
virDomainChrDefFree() to free also the definition.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When hotpluging a <serial/> device, we might need to add a
<console/> device with it (because of some crazy backcompat).
Now, hotplugging is done in several phases. In one of them,
qemuDomainChrPreInsert() allocates space for both devices, and
then qemuDomainChrInsertPreAlloced() actually inserts the device
into domain definition and sets up the <console/> device with it.
Except, the condition that checks whether to create the aliased
<console/> is wrong as it compares nconsoles against 0.
Surprisingly, qemuDomainChrInsertPreAllocCleanup() doesn't suffer
from the same error.
Fixes: daf51be5f1
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
igb is a new network device which will be introduced with QEMU 8.0.0.
It is a successor of e1000e so it has PCIe interface and is understands
virtio-net headers as e1000e does.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During qemu driver shutdown, objects are freed in qemuStateCleanup that
could still be used by active worker threads, resulting in crashes. E.g.
a worker thread could be processing a monitor EOF event after the
security manager is already disposed
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007fd9a9a1e1fe in virSecurityManagerMoveImageMetadata (mgr=0x7fd948012160, pid=-1, src=src@entry=0x7fd98c072c90, dst=dst@entry=0x0)
at ../../src/security/security_manager.c:468
#1 0x00007fd9646ff0f0 in qemuSecurityMoveImageMetadata (driver=driver@entry=0x7fd948043830, vm=vm@entry=0x7fd98c066db0, src=src@entry=0x7fd98c072c90,
dst=dst@entry=0x0) at ../../src/qemu/qemu_security.c:182
#2 0x00007fd96462c7b0 in qemuBlockRemoveImageMetadata (driver=driver@entry=0x7fd948043830, vm=vm@entry=0x7fd98c066db0, diskTarget=0x7fd98c072530 "vda",
src=<optimized out>) at ../../src/qemu/qemu_block.c:2628
#3 0x00007fd9646929d6 in qemuProcessStop (driver=driver@entry=0x7fd948043830, vm=vm@entry=0x7fd98c066db0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=<optimized out>) at ../../src/qemu/qemu_process.c:7585
#4 0x00007fd9646fc842 in processMonitorEOFEvent (vm=0x7fd98c066db0, driver=0x7fd948043830) at ../../src/qemu/qemu_driver.c:4794
#5 qemuProcessEventHandler (data=0x561a93febb60, opaque=0x7fd948043830) at ../../src/qemu/qemu_driver.c:4900
#6 0x00007fd9a9971a31 in virThreadPoolWorker (opaque=opaque@entry=0x561a93fb58e0) at ../../src/util/virthreadpool.c:163
(gdb) p mgr->drv
$2 = (virSecurityDriverPtr) 0x0
Prior to commit 7cf76d4e3a, the worker thread pool was freed before
disposing any driver objects. Let's return to that pattern, but leave
the other changes made by 7cf76d4e3a.
Signed-off-by: Tamara Schmitz <tamara.schmitz@suse.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Historically the snapshot code attempted to forbid internal snapshots
with UEFI both in active and inactive case. Unfortunately due to the
intricacies of UEFI probing this didn't really work for inactive VMs
which made users rely on the feature.
Now with the changes to store detected UEFI environment also in the
inactive definition this broke the feature for those users.
Since the varstore doesn't really change that much in the lifecycle of a
VM it usually is okay to simply leave it as is.
Restore the functionality for inactive snapshots by disabling the check.
In the future when uefi snapshotting will be added the rest of the
condition will also be removed.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/460
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Attaching disk into running VM the offline definition may not be
updated and we will end up with that disk existing only in live
definition. Creating snapshot with this state saves both live and
offline definition into snapshot metadata.
When we are deleting an external snapshot we are updating these
definitions in the snapshot metadata so we should just skip over
non-existing disks instead of reporting error.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2174700
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unify validation of VIR_DOMAIN_FEATURE_HTM, VIR_DOMAIN_FEATURE_NESTED_HV,
VIR_DOMAIN_FEATURE_CCF_ASSIST and remove temporary string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The features:
QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE
QEMU_CAPS_MACHINE_PSERIES_CAP_HTM
QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV
QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST
QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC
QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC
QEMU_CAPS_MACHINE_PSERIES_CAP_IBS
are supported by all qemu versions that libvirt supports. Drop the
obsolete checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic pointer freeing, remove 'ret' variable and also remove
return value completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove useless call to virCapabilitiesFreeMachines as the pointers were
cleared and the unneeded 'ret' variable. Since we don't need to clear
the 'machines' pointer now, remove that as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's never set to any real value. Remove it along with the caching code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate the architecture specific code to probe the support for HVF
from the actual setting of the capability.
In upcoming patches 'virQEMUCapsProbeHVF' will be mocked in the
testsuite to provide testing for the HVF hypervisor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The logic in 'virQEMUCapsInitQMP' invokes a second probe of qemu in case
when acceleration is used and TCG is supported to specifically probe the
CPU and features of non-accelerated guests.
The same logic must then be used in 'qemucapabilitiestest' when
replaying the data for testing otherwise the test would fail.
Export 'virQEMUCapsHaveAccel' for test usage and use the same logic
in 'testQemuCaps'.
Fix the comment in 'virQEMUCapsInitQMP' to outline what's happening.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The changes to the output files are the exact opposite of
those from commit 22207713cf: this is proof that the fix is
working as intended, and that existing domains will keep using
raw firmware images regardless of whether or not qcow2 images
are available on the system and have higher priority.
New domains will keep picking whatever firmware is considered
the preferred one according to the order of descriptors, as
evidenced by the fact that the recently introduced
firmware-auto-efi-abi-update-aarch64 test case is unaffected.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The virConnectOpen(), well virConnectOpenInternal() reports an
error if embed root is not an absolute path. This is a fair
requirement, but our qemu_shim doesn't check this requirement and
passes the path to mkdir(), only to fail later on, leaving the
empty directory behind:
$ ls -d asd
ls: cannot access 'asd': No such file or directory
$ virt-qemu-run -r asd whatever.xml
virt-qemu-run: cannot open qemu:///embed?root=asd: unsupported configuration: root path must be absolute
$ ls -d asd
asd
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
After cleanup done in v8.2.0-rc1~47 the
qemuDomainObjExitMonitor() and after v8.7.0-rc1~176 the
qemuDomainObjEnterMonitor() lost the @driver argument. But
corresponding ATTRIBUTE_NONNULL() annotation was not removed and
both functions are still annotated as ATTRIBUTE_NONNULL(2) even
though they accept just one argument (@obj).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Otherwise looking up a secret fails when we try to elevate the identity
in qemuDomainSecretInfoSetupFromSecret.
https://bugzilla.redhat.com/show_bug.cgi?id=2000410
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Even when the user is not taking advantage of firmware
autoselection and instead manually providing all the necessary
information, in most cases they're still going to use firmware
builds that are provided by the OS vendor, are installed in
standard paths and come with a corresponding firmware
descriptor.
Similarly, even when the user is not guiding the autoselection
process by specifying the desired status of certain features
and instead is relying on the system-level descriptor priority
being set up correctly, libvirt will still ultimately decide to
use a specific descriptor, which includes information about the
firmware's features.
In both these cases, take the additional information that were
obtained from the firmware descriptor and reflect them back into
the domain XML, where they can be conveniently inspected by the
user and management applications alike.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we no longer reject configurations that include both
this information and explicit firmware details, as long of
course as everything is internally consistent, and that we've
ensured that we produce maximally compatible XML on migration,
we can stop stripping this information at the end of the
firmware selection process.
There are several advantages to keeping this information around:
* if the user wants to change the firmware configuration for
an existing VM, they can simply drop the <loader> and
<nvram> elements, tweak the firmware autoselection parameters
and let libvirt pick a firmware that matches on the new
requirements;
* management applications can inspect the XML and easily
figure out firmware-related information without having to
reverse-engineer them based on some opaque paths.
Overall, this change makes things more transparent and easier to
understand. The improvement is so significant that, in a
follow-up commit, we're going to ensure that this information is
available in even more cases.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Right now there are a few scenarios in which we skip ahead, and
removing these exceptions will make for more consistent and
predictable behavior.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The requires-smm feature being present in a firmware descriptor
causes loader.secure=yes to be automatically chosen for the
domain, so we have to avoid this situation or the user's choice
will be silently subverted.
Note that we can't actually encounter loader.secure=no in this
function at the moment because of earlier checks, but that's
going to change soon.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Right now we have checks in place that ensure that explicit
paths are not provided when firmware autoselection has been
enabled, but that's going to change soon.
To prepare for that, take into account user-provided paths
during firmware autoselection if present, and discard all
firmware descriptors that don't contain matching information.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Otherwise the build on armv7l breaks:
error: format ‘%lu’ expects argument of type
‘long unsigned int’, but argument 4 has type
‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
Fixes: 1992ae40fa
Fixes: e239f7d0a8
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The newly added luks-any rbd encryption format in qemu
allows for opening both LUKS and LUKS2 encryption formats.
This commit enables libvirt uses to use this wildcard format.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This capability represents that qemu supports the "luks-any" encryption
format for RBD images.
Both LUKS and LUKS2 formats can be parsed using this wildcard format.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit enables libvirt users to use layered encryption
of RBD images, using the librbd encryption engine.
This allows opening of an encrypted cloned image
whose parent is encrypted with a possibly different encryption key.
To open such images, multiple encryption secrets are expected
to be defined under the encryption XML tag.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit changes the _qemuDomainStorageSourcePrivate struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit changes the qemuBlockStorageSourceAttachData struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Change secret aliases from %s-%s-secret0 to %s-%s-secret%lu,
which will later be used for storage encryption requiring more
than a single secret.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This capability represents that qemu supports the layered encryption
of RBD images, where a cloned image is encrypted with a possible
different encryption than its parent image.
Signed-off-by: Or Ozeri <oro@il.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When a thread-context object is specified on the cmd line, then
QEMU spawns a thread and sets its affinity to the list of NUMA
nodes specified in .node-affinity attribute. And this works just
fine, until the main QEMU thread itself is not restricted.
Because of v5.3.0-rc1~18 we restrict the main emulator thread
even before QEMU is executed and thus then it tries to set
affinity of a thread-context thread, it inevitably fails with:
Setting CPU affinity failed: Invalid argument
Now, we could lift the pinning temporarily, let QEMU spawn all
thread-context threads, and enforce pinning again, but that would
require some form of communication with QEMU (maybe -preconfig?).
But that would still be wrong, because it would circumvent
<emulatorpin/>.
Technically speaking, thread-context is an internal
implementation detail of QEMU, and if it weren't for it, the main
emulator thread would be doing the allocation. Therefore, we
should honor the pinning and prune the list of node so that
inaccessible ones are dropped.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2154750
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When building a thread-context object (inside of
qemuBuildThreadContextProps()) we look at given memory-backend-*
object and look for .host-nodes attribute. This works, as long as
we need to just copy the attribute value into another
thread-context attribute. But soon we will need to adjust it.
That's the point where having the value in virBitmap comes handy.
Utilize the previous commit, which made
qemuBuildMemoryBackendProps() set the argument and pass it into
qemuBuildThreadContextProps().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
While it's true that anybody who's interested in getting
.host-nodes attribute value can just use
virJSONValueObjectGetArray() (and that's exactly what
qemuBuildThreadContextProps() is doing, btw), if somebody is
interested in getting the actual virBitmap, they would have to
parse the JSON array.
Instead, introduce an argument to qemuBuildMemoryBackendProps()
which is set to corresponding value used when formatting the
attribute.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
There are two compound conditions in
qemuBuildMemoryBackendProps() and each one checks for nodemask
for NULL first. Join them into one bigger block.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The order of pinning priority (at least for emulator thread) was
set by v1.2.15-rc1~58 (for cgroup code). But later, when
automatic placement was implemented into
qemuDomainGetEmulatorPinInfo(), the priority was not honored.
Now that we have this priority code in a separate function, we
can just call that and avoid this type of error.
Fixes: 776924e376
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The set of if()-s that determines the preference in cpumask used
for setting things like emulatorpin, vcpupin, etc. is going to be
re-used. Separate it out into a function.
You may think that this changes behaviour, but
qemuProcessPrepareDomainNUMAPlacement() ensures that
priv->autoCpuset is set for VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
We have this crazy backwards compatibility when it comes to
serial and console devices. Basically, in same cases the very
first <console/> is just an alias to the very first <serial/>
device. This is to be seen at various places:
1) virDomainDefFormatInternalSetRootName() - when generating
domain XML, the <console/> configuration is basically ignored
and corresponding <serial/> config is formatted,
2) virDomainDefAddConsoleCompat() - which adds a copy of
<serial/> or <console/> into virDomainDef in post parse.
And when talking to QEMU we need a special handling too, because
while <serial/> is generated on the cmd line, the <console/> is
not. And in a lot of place we get it right. Except for generating
device aliases. On domain startup the 'expected' happens and
devices get "serial0" and "console0" aliases, correspondingly.
This ends up in the status XML too. But due to aforementioned
trick when formatting domain XML, "serial0" ends up in both
'virsh dumpxml' and the status XML. But internally, both devices
have different alias. Therefore, detaching the device using
<console/> fails as qemuDomainDetachDeviceChr() tries to detach
"console0".
After the daemon is restarted and status XML is parsed, then
everything works suddenly. This is because in the status XML both
devices have the same alias.
Let's generate correct alias from the beginning.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2156300
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Other APIs that internally use QEMU migration and need to temporarily
suspend a domain already report failure to resume vCPUs by setting
VIR_DOMAIN_PAUSED_API_ERROR state reason and emitting
VIR_DOMAIN_EVENT_SUSPENDED event with
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR.
Let's do the same in qemuMigrationSrcRestoreDomainState for consistent
behavior.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some APIs (migration, save/restore, snapshot, ...) require a domain to
be suspended temporarily. In case resuming the domain fails, the domain
will be unexpectedly left paused when the API finishes. This situation
is reported via VIR_DOMAIN_EVENT_SUSPENDED event with
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR detail. But we do not have a
corresponding reason for VIR_DOMAIN_PAUSED state and the reason would
remain set to the value used when the domain was paused. So the state
reason would suggest the operation is still running.
This patch changes the state reason to a new VIR_DOMAIN_PAUSED_API_ERROR
to make it clear the API that paused the domain already finished, but
failed to resume the domain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For some vhostuser daemons, we validate that the guest memory is shared
with the host.
With earlier versions of QEMU, it was only possible to mark memory
as shared by defining an explicit NUMA topology. Later, QEMU exposed
the name of the default memory backend (defaultRAMid) so we can mark
that memory as shared.
Since libvirt commit:
commit bff2ad5d6b
qemu: Relax validation for mem->access if guest has no NUMA
we already check for the case when user requests shared memory,
but QEMU did not expose defaultRAMid.
Drop the duplicit check from vhostuser device validation, to make
it pass on hotplug even after libvirtd restart.
This avoids the need to store the defaultRAMid, since we don't really
need it for anything after the VM has been already started.
https://bugzilla.redhat.com/show_bug.cgi?id=2078693https://bugzilla.redhat.com/show_bug.cgi?id=2177701
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
set useBinarySpecificLabel = true when calling qemuSecurityCommandRun
for the passt process, so that the new process context will include
the binary-specific label that should be used for passt (passt_t)
rather than svirt_t (as would happen if useBinarySpecificLabel was
false). (The MCS part of the label, which is common to all child
processes related to a particular qemu domain instance, is also set).
Resolves: https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Normally when a child process is started by libvirt, the SELinux label
of that process is set to virtd_t (plus an MCS range). In at least one
case (passt) we need for the SELinux label of a child process label to
match the label that the binary would have transitioned to
automatically if it had been run standalone (in the case of passt,
that label is passt_t).
This patch modifies virSecuritySELinuxSetChildProcessLabel() (and all
the functions above it in the call chain) so that the toplevel
function can set a new argument "useBinarySpecificLabel" to true. If
it is true, then virSecuritySELinuxSetChildProcessLabel() will call
the new function virSecuritySELinuxContextSetFromFile(), which uses
the selinux library function security_compute_create() to determine
what would be the label of the new process if it had been run
standalone (rather than being run by libvirt) - the MCS range from the
normally-used label is added to this newly derived label, and that is
what is used for the new process rather than whatever is in the
domain's security label (which will usually be virtd_t).
In order to easily verify that nothing was broken by these changes to
the call chain, all callers currently set useBinarySpecificPath =
false, so all behavior should be completely unchanged. (The next
patch will set it to true only for the case of running passt.)
https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently it's only possible to set this parameter during domain
creation via QEMU commandline passthrough feature.
With the new delay attribute it's also possible to set this
parameter if you want to attach a new NBD disk
using "virsh attach-device domain device.xml" e.g.:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='nbd' name='foo'>
<host name='example.org' port='6000'/>
<reconnect delay='10'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
Signed-off-by: Christian Nautze <christian.nautze@exoscale.ch>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 54fa1b44af ("conf: Add loadparm boot option for a boot device")
added the ability to specify a loadparm parameter on a <boot/> tag, while
commit 29ba41c2d4 ("qemu: Add loadparm to qemu command line string")
added that value to the QEMU "-machine" command line parameters.
Unfortunately, the latter commit only looked at disks and network
devices for boot information, even though anything with
VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT could potentially have this tag.
In practice, a <hostdev> tag pointing to a passthrough (SCSI or DASD)
disk device can be used in this way, which means the loadparm is
accepted, but not given to QEMU.
Correct this, and add some XML/argv tests.
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Good to have for debugging in case something wrong happens during
incoming migration.
Signed-off-by: Eiichi Tsukata <eiichi.tsukata@nutanix.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For shutoff VMs we don't have the storage source backing chain
populated so it will fail this check and error out. Move it to
part that is done only when VM is running.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This can improve performance for some guests since it reduces copying of
display data between host and guest. Requires udmabuf on the host.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Capability to determine whether this qemu supports the 'blob' option for
virtio-gpu.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than storing the video type as an integer, use the proper enum
type within the struct.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The parameter was added for consistency with virPidFileAcquirePath.
However, all callers of virPidFileAcquire pass false.
Remove the argument.
Partially-reverts: 2250a2b5d2
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The function doesn't set any capability and we don't want to add
arch-dependent always-peresent capabilities in the future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Check the architecture of the guest rather than relying on
QEMU_CAPS_LOADPARM which is set based on architecture.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the guest architecture to decide whether to format
'aes-key-wrap'/'dea-key-wrap' rather than
QEMU_CAPS_AES_KEY_WRAP/QEMU_CAPS_DEA_KEY_WRAP which were set based on
architecture.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU_CAPS_MACH_VIRT_GIC_VERSION is always asserted for VIR_ARCH_AARCH64.
Note that this patch is a direct conversion of the logic originally
residing in the capabilities code. A better coversion would be (based on
whether it is available for just AARCH64 or also ARM) to base it on the
guest architecture.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than asserting a capability based on architecture, format the
fallback parameter based on the presence of the newer capability and an
explicit architecture check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability is based on a platform check rather than what given qemu
supports.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU_CAPS_NO_ACPI is asserted based on architecture, so it can be
replaced by a non-capability check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 24cc9cda82 switched over to use -machine hpet, but one of the
steps it did was to clear the QEMU_CAPS_NO_HPET capability.
The validation check still uses the old capability though which means
that for configs which would explicitly enable HPET we'd report an error.
Since HPET is an x86(_64) platform specific device, convert the
validation check to an architecture check as all supported qemu versions
actually support it.
Modify a test case to request HPET to catch posible future problems.
Fixes: 24cc9cda82
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We always assert the flag for aarch64 qemus and in qemu the 'aarch64'
cpu property doesn't seem to be optional.
Remove checks and remove impossible test case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail at this point. Remove the last outstanding
pointless error check and turn the return type into 'void'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make all callers always pass a valid pointer which in turn allows us to
remove return value check from the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make all callers always pass a valid pointer which in turn allows us to
remove return value check from the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Do the two fixups of CPU as one block and split up the return value
checks to separate conditions. This will make the upcoming refactors
simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The allocation of the object itself can't fail. What can fail is the
creation of the class on a programming error. Rather than punting the
error up the stack abort() directly on the first occurence as the error
can't be fixed during runtime.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Format aliases into temporary strings and append them using
virJSONValueObjectAdd.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'ipv6-prefix' and 'ipv6-prefixlen' fields can be directly added
using virJSONValueObjectAdd rather than by two separate calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virJSONValueObjectAdd and format the string directly via
g_strdup_printf. In the end virJSONValueObjectAppendStringPrintf will be
removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Prefer virJSONValueObjectAdd which we already use internally combined
with local formatting of the string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
VIR_DOMAIN_NET_TYPE_NULL and VIR_DOMAIN_NET_TYPE_VDS are not implemented
for the qemu driver but the formatter code in 'qemuBuildHostNetProps'
didn't report an error for them and didn't even return from the function
when they were encountered.
This caused a crash in 'virJSONValueObjectAppendStringPrintf' which
does not tolerate NULL JSON object to append to when the unsupported
devices were used.
Properly report error when unhandled devices are encountered. This also
includes the case for VIR_DOMAIN_NET_TYPE_HOSTDEV, but that code path
should never be reached.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175582
Fixes: bac6b266fb / 6457619d18
Fixes: 0225483adc
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU deprecated the '-no-acpi' option, thus we should switch to the
modern way to use '-machine'.
Certain ARM machine types don't support ACPI. Given our historically
broken design of using '<acpi/>' without attribute to enable ACPI and
qemu's default of enabling it without '-no-acpi' such configurations
would not work.
Now when qemu reports whether given machine type supports ACPI we can do
a better decision and un-break those configs. Unfortunately not
retroactively.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/297
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The helper returns the 'acpi' flag for a given machine type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The return data from 'query-machines' now contains an 'acpi' field. If
the field is present we can use it to decide how to handle user's
setting of '<acpi/>' domain feature.
Add logic to extract the 'acpi' field and store it in machine type list
along with other properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now always assume support for polling mode of iothreads.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
iothread polling mode and the corresponding properties were added in
qemu-2.9 ( 0d9d86fb4df4882b ). We can always assume that qemu supports
them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
iothreads were introduced in qemu-2.0 and can't be compiled out thus we
can always assume qemu supports them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 068efae5b1 created a copy of this code instead of
simply moving it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Take the information from the descriptor and store it in the
domain definition. Various things, such as the arguments passed
to -blockdev and the path generated for the NVRAM file, will
then be based on it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the user has requested a specific firmware format, then
all firmware builds that are not in that format should be
ignored while looking for matches.
The legacy hardcoded firmware list predates firmware
descriptors and their "format" field, so we can safely
assume that all builds listed in there are in raw format.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This ensures that, as we add support for more formats at the
domain XML level, we don't accidentally cause drivers to
misbehave or users to get confused.
All existing drivers support the raw format, and supporting
additional formats will require explicit opt-in on the
driver's part.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Right now, this results in loader->nvram being NULL, which is
reasonable: loader->nvramTemplate is stored separately, so if
the <nvram> element doesn't contain a path there is really no
useful information inside it.
However, this is about to change, so we will find ourselves
needing to hold on to loader->nvram even when no path is
present. Change the firmware handling code so that such a
scenario is dealt with appropriately.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This helper replaces qemuDomainNVRAMPathFormat() and also
incorporates some common operations that all callers of that
helper needed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, firmware selection is performed as part of the
domain startup process. This mostly works fine, but there's a
significant downside to this approach: since the process is
affected by factors outside of libvirt's control, specifically
the contents of the various JSON firmware descriptors and
their names, it's pretty much impossible to guarantee that the
outcome is always going to be the same. It would only take an
edk2 update, or a change made by the local admin, to render a
domain unbootable or downgrade its boot security.
To avoid this, move firmware selection to the postparse phase.
This way it will only be performed once, when the domain is
first defined; subsequent boots will not need to go through
the process again, as all the paths that were picked during
firmware selection are recorded in the domain XML.
Care is taken to ensure that existing domains are handled
correctly, even if their firmware configuration can't be
successfully resolved. Failure to complete the firmware
selection process is only considered fatal when defining a
new domain; in all other cases the error will be reported
during startup, as is already the case today.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Right now, if the descriptor with the highest priority happens
to describe a firmware in a format other than raw, no domain
that uses autoselection will be able to start.
A better approach is to filter out descriptors that advertise
unsupported formats during autoselection.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
At the moment, if SMM is explicitly disabled in the domain XML
but a firmware descriptor that requires SMM to be enabled has
the highest priority and otherwise matches the requirements,
we pick that firmware only to error out later, when the domain
is started.
A better approach is to take into account the fact that SMM is
disabled while performing autoselection, and ignore all
descriptors that advertise the requires-smm feature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We already clear os.firmware, so it doesn't make sense to keep
the list of features around.
Moreover, our validation routines will reject an XML that
contains a list of firmware features but disables firmware
autoselection, so not clearing these means that the live XML
for a domain that uses feature-based autoselection can't be
fed back into libvirt.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't make sense for non-local sources, since we can't
create or reset the corresponding NVRAM file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This makes the code more compact and less awkward.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For now we just allocate the object, so the only advantage is
that invocations are shorter and look a bit nicer.
Later on, its introduction will pay off by letting us change
things in a single spot instead of all over the library.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move all the boot related parts of qemuDomainDefPostParse()
to a separate helper.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move all the machine type related parts of
qemuDomainDefPostParse() to a separate helper.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting (some) external helpers, callers of
qemuSecurityCommandRun() pass &exitstatus variable, to learn the
exit code of helper process (with qemuTPMEmulatorStart() being
the only exception). Then, if the status wasn't zero they produce
a generic error message, like:
"Starting of helper process failed. exitstatus=%d"
or, in case of qemuPasstStart():
"Could not start 'passt': %s"
This is needless as virCommandRun() (that's called under the
hood), can do both for us, if NULL was passed instead of
@exitstatus. Not only it appends exit status, it also reads
stderr of failed command producing comprehensive error message:
Child process (${args}) unexpected exit status ${exitstatus}: ${stderr}
Therefore, pass NULL everywhere. But in contrast with one of
previous commits which removed @cmdret argument, there could be a
sensible caller which might want to process exit code. So keep
the argument for now and just pass NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every single caller of qemuSecurityCommandRun() calls the
function as:
if (qemuSecurityCommandRun(..., &cmdret) < 0)
goto cleanup;
if (cmdret < 0)
goto cleanup;
(modulo @exitstatus shenanigans)
Well, there's no need for such complication. There isn't a single
caller (and probably will never be (TM)), that would need to
distinguish the reason for the failure. Therefore,
qemuSecurityCommandRun() can be made to pass the retval of
virCommandRun() called under the hood.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The usual pattern when starting a helper daemon is:
if (qemuSecurityCommandRun(..., &exitstatus, &cmdret) < 0)
goto cleanup;
if (cmdret < 0 || exitstatus != 0) {
virReportError();
goto cleanup;
}
The only problem with this pattern is that if virCommandRun()
fails (i.e. cmdret < 0), then proper error was already reported.
But in this pattern we overwrite it (usually with less specific)
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Way back, in v6.2.0-rc1~67 we removed the code that reads slirp's
stderr on failed startup. However, we forgot to remove
corresponding virCommandSetErrorFD() call and variable
declaration. Do that now.
While this may seem like a step in wrong direction (we should be
reading stderr as it may contain reason for failed start), this
is going to be handled in more general way in next commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is used only inside qemu_domain.c, unexport it and move it
above its user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Originally the code was skipping all repeated taints with the same taint
flag but a logic bug introduced in commit 30626ed15b inverted
the condition. This caused that actually the first occurence was NOT
logged but any subsequent was.
This was noticed when going through oVirt logs as they use custom guest
agent commands and the logs are totally spammed with this message.
Fixes: 30626ed15b
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The 'can-offline' member is optional according to agent's schema and in
fact in certain cases it's not returned. Libvirt then spams the logs
if something is polling the bulk guest stats API.
Noticed when going through oVirt logs which appears to call the bulk
stats API repeatedly.
Instead of requiring it we simply reply that the vCPU can't be offlined.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
There are two switch() statements over the same variable inside
of qemuMonitorJSONGetMemoryDeviceInfo(). Join them together into
one switch.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
When processing memory devices (as a reply from QEMU), a bunch of
STREQ()-s is used. Fortunately, the set of strings we process is
the same as virDomainMemoryModel enum. Therefore, we can use
virDomainMemoryModelTypeFromString() and then use integer
comparison (well, switch()). This has an upside: introducing a
new memory model lets us see what places need adjusting
immediately at compile time.
NB, this is in contrast with cmd line generator
(qemuBuildMemoryDeviceProps()), where more specific models are
generated (e.g. "pc-dimm", "virtio-mem-pci", etc.). But QEMU
reports back the parent model, instead of specific child
instance.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
When starting QEMU (or when reconnecting to a running one),
qemuMonitorJSONGetMemoryDeviceInfo() is called to refresh info on
memory devices. In here, query-memory-devices is called which
returns info on all memory devices. The result is then iterated
over and for some memory models runtime information is updated.
The rest is to be ignored. Except, when introducing SGX support,
this was turned into an error leaving us unable to start any
domain with virtio-pmem memory device (as virtio-pmem is to be
ignored).
Fixes: ddb1bc0519
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
When a QEMU netdev is of type "stream", if the socket it uses for
connectivity to the host network gets closed, then QEMU will send a
NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
created is backed by a passt process, and if the socket was closed,
that means the passt process has disappeared.
When we receive this event, we can respond by starting a new passt
process with the same options (including socket path) we originally
used. If we have previously created the stream netdev device with a
"reconnect" option, then QEMU will automatically reconnect to this new
passt process. (If we hadn't used "reconnect", then QEMU will never
try to reconnect to the new passt process, so there's no point in
starting it.)
Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
(ie "host side") of the network device, and so it sends the
"netdev-id" to specify which device was disconnected. But libvirt's
virDomainNetDef (the object used to keep track of network devices) is
the internal representation of both the host-side "netdev", and the
guest side device, and virDomainNetDef doesn't directly keep track of
the netdev-id, only of the device's "alias" (which is the "id"
parameter of the *guest* side of the device). Fortunately, by convention
libvirt always names the host-side of devices as "host" + alias, so in
order to search for the affected NetDef, all we need to do is trim the
1st 4 characters from the netdev-id and look for the NetDef having
that resulting trimmed string as its alias. (Contrast this to
NIC_RX_FILTER_CHANGED, which is an event received for the guest side
of the device, and so directly contains the device alias.)
Resolves: https://bugzilla.redhat.com/2172098
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU's "reconnect" option of "-netdev stream" tells QEMU to
periodically (period is given in seconds as an argument to the option)
attempt to reconnect to the same passt socket to which it had
originally connected to. This is useful in cases where the passt
process terminates, and libvirtd starts a new passt process in its
place (which doesn't happen yet, but will happen automatically after
an upcoming patch in this series).
Since there is no real hueristic for determining the "best" value of
the reconnect interval, rather than clutter up config with a knob that
nobody knows how to properly twiddle, we just set the reconnect timer
to 5 seconds.
"-netdev stream" first appeared in QEMU 7.2.0, but the reconnect
option won't be available until QEMU 8.0.0, so we need to check QEMU
capabilities just in case someone is using QEMU 7.2.0 (and thus can
support passt backend, but not reconnect)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Detect that the 'stream' netdev backend supports reconnecting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuPasstStart() already logs any error that occurs, so having the
caller log a generic error message only serves to obscure the actual
problem.
Fixes: a56f0168d5
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In commit 5af6134e I had added a new capability that is true if QEMU
allows "-netdev stream", but somehow neglected to actually check it in
commit a56f0168d when hooking up passt support to qemu. This isn't
catastrophic, since QEMU itself will still report an error, but that
error isn't as easy to understand as a libvirt-generated error.
Fixes: a56f0168d5
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Just like it can't remove its own PID files, passt can't unlink its
own socket upon exit (unless the initialisation fails), because it
has no access to the filesystem at runtime.
Remove the socket file in qemuPasstKill().
Fixes: a56f0168d5 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Changing any of the attributes of an <interface>'s <backend> would
require removing and re-adding the interface for the new setting to
take effect, so fail any update-device that changes anything in
<backend>
Resolves: https://bugzilla.redhat.com/2169245
Signed-off-by: Laine Stump <laine@redhat.com>
When user creates external snapshot with making only memory snapshot
without any disks deleting that snapshot failed without reporting any
meaningful error.
The issue is that the qemuSnapshotDeleteExternalPrepare function
returns NULL because the returned list is empty. This will not change
so to make it clear if the function fails or not return int instead and
have another parameter where we can pass the list.
With the fixed memory snapshot deletion it will now correctly delete
memory only snapshot as well.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2170826
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting external snapshot we should remove the memory snapshot
file as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
'reconnect' parameter doesn't pass to qemu properly when
hotplug vhost-user device to vm. Fix this by making
'reconnect' to get correct value.
Signed-off-by: Zhenguo Yao <yaozhenguo1@gmail.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
It makes sense to accept pvpanic-pci also without specified PCI
address and assign one if possible.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961326
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This capability detects the availability of the pvpanic-pci
device that is required in order to use pvpanic on Arm (original
pvpanic is an emulated ISA device, for which Arm does not have
support).
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The way we start passt currently is: we use
virCommandSetPidFile() to use our virCommand machinery to acquire
the PID file and leak opened FD into passt. Then, we use
virPidFile*() APIs to read the PID file (which is needed when
placing it into CGroups or killing it). But this does not fly
really because passt daemonizes itself. Thus the process we
started dies soon and thus the PID file is closed and unlocked.
We could work around this by passing '--foreground' argument, but
that weakens passt as it can't create new PID namespace (because
it doesn't fork()).
The solution is to let passt write the PID file, but since it
does not lock the file and closes it as soon as it is written, we
have to switch to those virPidFile APIs which don't expect PID
file to be locked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
There are two places where we kill passt:
1) qemuPasstStop() - called transitively from qemuProcessStop(),
2) qemuPasstStart() - after failed start.
Now, the code from 2) lack error preservation (so if there's
another error during cleanup we might overwrite the original
error). Therefore, move the internals of qemuPasstStop() into a
separate function and call it from both places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When starting passt, it may write something onto its stderr
(convincing it to print even more is addressed later). Pass this
string we read to user.
Since we're not daemonizing passt anymore (see previous commit),
we can let virCommand module do all the heavy lifting and switch
to virCommandSetErrorBuffer() instead of reading error from an
FD.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When passt is started, it daemonizes itself by default. There's
no point in having our virCommand module daemonize it too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The way setting up CGroups for external helpers work, is:
qemuExtDevicesHasDevice() is called first to determine whether
there is a helper process running, the CGroup controller is
created and then qemuExtDevicesSetupCgroup() is called to place
helpers into the CGroup. But when one reads just
qemuExtDevicesSetupCgroup() it's easy to miss this hidden logic.
Therefore, add a warning at the beginning of the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
If qemuPasstGetPid() fails, or the passt's PID is -1 then
qemuPasstSetupCgroup() returns early without any error message
set. Report an appropriate error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
We can have external helper processes running for domain
<interface/> too (e.g. slirp or passt). But this is not reflected
in qemuExtDevicesHasDevice() which simply ignores these.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This reverts commit 0c4e716835.
This patch was pushed by my mistake. Even though it got ACKed on
the list, I've raised couple of issues with it. They will be
fixed in next commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The 'pending' state needs to be handled by the blockjob code only when
the snapshot code requests a block-commit without auto-finalization.
If we always handle it we fail to properly remove the blockjob data for
the 'blockdev-create' job as that also transitions trhough 'pending' but
we'd never update it once it reaches 'concluded' as the code already
thinks that the job has finished and is no longer watching it.
Introduce a 'processPending' property into block job data and set it
only when we know that we need to process 'pending'.
Fixes: 90d9bc9d74
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168769
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We only set up host for VIR_DOMAIN_TPM_TYPE_EMULATOR and thus
similarly, we should do cleanup for the same type. This also
fixes a crasher, in which qemuTPMEmulatorCleanupHost() accesses
tpm->data.emulator.storagepath which is NULL for
VIR_DOMAIN_TPM_TYPE_EXTERNAL.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168762
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
I initially had the passt process being started in an identical
fashion to the slirp-helper - libvirt was daemonizing the new process
and recording its pid in a pidfile. The problem with this is that,
since it is daemonized immediately, any startup error in passt happens
after the daemonization, and thus isn't seen by libvirt - libvirt
believes that the process has started successfully and continues on
its merry way. The result was that sometimes a guest would be started,
but there would be no passt process for qemu to use for network
traffic.
Instead, we should be starting passt in the same manner we start
dnsmasq - we just exec it as normal (along with a request that passt
create the pidfile, which is just another option on the passt
commandline) and wait for the child process to exit; passt then has a
chance to parse its commandline and complete all the setup prior to
daemonizing itself; if it encounters an error and exits with a non-0
code, libvirt will see the code and know about the failure. We can
then grab the output from stderr, log that so the "user" has some idea
of what went wrong, and then fail the guest startup.
Signed-off-by: Laine Stump <laine@redhat.com>
Commit 5ef2582646 added emitting of even when refreshign disk state,
where it wanted to avoid sending the event if disk state didn't change.
This was achieved by using 'continue' in the loop filling the
information. Unfortunately this skips extraction of whether the device
has a tray which is propagated into internal structures, which in turn
broke cdrom media change as the code thought there's no tray for the
device.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166411
Fixes: 5ef2582646
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The virURIFormat() function either returns a string, or aborts
(on OOM). There's no way this function can return NULL (as of
v7.2.0-rc1~277). Therefore, it doesn't make sense to check its
retval against NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In one of recent commits (v9.0.0-rc1~106) I've made our QEMU
namespace code umount the original /dev. One of the reasons was
enhanced security, because previously we just mounted a tmpfs
over the original /dev. Thus a malicious QEMU could just
umount("/dev") and it would get to the original /dev with all
nodes.
Now, on some systems this introduced a regression:
failed to umount devfs on /dev: Device or resource busy
But how this could be? We've moved all file systems mounted under
/dev to a temporary location. Or have we? As it turns out, not
quite. If there are two file systems mounted on the same target,
e.g. like this:
mount -t tmpfs tmpfs /dev/shm/ && mount -t tmpfs tmpfs /dev/shm/
then only the top most (i.e. the last one) is moved. See
qemuDomainUnshareNamespace() for more info.
Now, we could enhance our code to deal with these "doubled" mount
points. Or, since it is the top most file system that is
accessible anyways (and this one is preserved), we can
umount("/dev") in a recursive fashion.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167302
Fixes: 379c0ce4bf
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
When going through debug log of a domain startup process, one can
meet the following line:
debug : qemuProcessLaunch:7668 : Building mount namespace
But this is in fact wrong. Firstly, domain namespaces are just
enabled in domain's privateData. Secondly, the debug message says
nothing about actual state of namespace - whether it was enabled
or not.
Therefore, move the debug printing into
qemuProcessEnableDomainNamespaces() and tweak it so that the
actual value is reflected.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Similar to other error paths in qemuDomainUnshareNamespace(), jump to
the cleanup label on umount error instead of directly returning -1.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting a guest, helper processes are started first. But
they need a bit of special handling. Just consider a regular cold
boot and an incoming migration. For instance, in case of swtpm
with its state on a shared volume, we want to set label on the
state for the cold boot case, but don't want to touch the label
in case of incoming migration (because the source very
specifically did not restore it either).
Until now, these two cases were differentiated by testing
@incoming against NULL. And while that makes sense for other
aspects of domain startup, for external devices we need a bit
more, because a restore from a save file is also 'incoming
migration'.
Now, there is a difference between regular migration and restore
from a save file. In the former case we do not want to set
seclabels in the save state. BUT, in the latter case we do need
to set them, because the code that saves the machine restored
seclabels.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2161557
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When stopping swtpm we can restore the label either on just the
swtpm's domain specific logfile (/var/log/swtpm/libvirt/qemu/...),
or on the logfile and the state too (/var/lib/libvirt/swtpm/...).
The deciding factor is whether the guest is stopped because of
outgoing migration OR the state is on a shared filesystem.
But this is not correct condition, because for instance saving the
guest into a file (virsh save) is also an outgoing migration.
Alternatively, when the swtpm state is stored on a shared
filesystem, but the guest is destroyed (virsh destroy), i.e.
stopped because of different reason than migration, we want to
restore the seclabels.
The correct condition is: skip restoring the state on outgoing
migration AND shared filesystem.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2161557
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When cleaning up host in qemuProcessStop(), our external helper
processes (e.g. swtpm) want to know whether the domain is being
migrated out or not (so that they restore seclabels on a device
state that's on a shared storage).
This fact is reflected in the @outgoingMigration variable which
is set to true if asyncJob is anything but
VIR_ASYNC_JOB_MIGRATION_IN. Well, we have a specific job for
outgoing migration (VIR_ASYNC_JOB_MIGRATION_OUT) and thus we
should check for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free 'cpuinfo' and remove the cleanup label and ret
variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace virJSONValueObjectGet + virJSONValueIsArray by the single API
which returns only an array.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace instances of virJSONValueObjectGet + virJSONValueIsArray by
virJSONValueObjectGetArray.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simplify construction of a single provider by using
virJSONValueObjectAdd and restructuring the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a domain is configured to create a macvtap/macvlan but the
target link already exists, startup fails (as expected) with:
error: error creating macvtap interface test@eth0 (52:54:00:d9:0b:db): File exists
Okay, we could make that error message better, but that's not the
point. Since this error originated while generating cmd line
(the caller is qemuProcessStart(), transitively), the cleanup
after failed start is performed (qemuProcessStop()). Here,
virNetDevMacVLanDeleteWithVPortProfile() is called which removes
the macvtap interface we did not create (as it made us fail in
the first place).
Therefore, we need to track which macvtap/macvlan interface was
created successfully and remove only those.
You'll notice that only qemuProcessStop() has the new check. For
the (failed) hotplug case (qemuDomainAttachNetDevice()) this
function is already in place (the @iface_connected variable), or
not needed (qemuDomainRemoveNetDevice() - we're removing an
interface that was already attached to QEMU).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166235
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every single caller of the
virNetDevMacVLanDeleteWithVPortProfile() function is calling it
wrapped inside of ignore_value() macro. This is because the
function is annotated as G_GNUC_WARN_UNUSED_RESULT. This makes no
sense. Drop the annotation and the macro envelope.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The hotplug code paths need to be able to pass the FDs to the monitor to
ensure that hotplug works.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
To ensure that we can hot-unplug the disk including the associated fdset
we need to store the fdset ID in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Rollback of FD sets passed to qemu is also needed after possible restart
of libvirtd when we need to serialize the data into status XML. For this
purpose we need to access the fdset ID once it was passed to qemu and
potentially re-create a 'qemuFDPass' struct in passed state.
Introduce 'qemuFDPassNewPassed' and 'qemuFDPassIsPassed'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Until now the code didn't expect that we'd want to rollback/detach a FD
passed on the commandline, but whith disk backend FD passing this can
happen.
Properly mark the 'qemuFDPass' object as passed to qemu even when it was
done on the commandline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Copy the pointer to qemuFDPass into struct qemuBlockStorageSourceAttachData
so that it can be used from qemuBuildBlockStorageSourceAttachDataCommandline
rather than looping again in qemuBuildDiskSourceCommandLineFDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Be consistent with other children buffer variable naming scheme.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The iTCO watchdog is part of the q35 machine type since its inception,
we just did not add it implicitly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2137346
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order for the iTCO watchdog to be operational we must disable the
noreboot pin strap in qemu. This is the default starting from 8.0
machine types, but desirable for older ones as well. And we can safely
do that since that is not guest-visible.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is already possible with qemu, and actually already happening with
q35 machines and a specified watchdog since q35 already includes a
watchdog we do not include in the XML. In order to express such
posibility multiple watchdogs need to be supported.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The g_hash_table_unref() function does not accept NULL. Passing
NULL results in a glib warning being triggered. Check whether the
hash table is not NULL and unref it only then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Support virtio-crypto device, also support cryptodev types:
- builtin
- lkcf
Finally, we can launch a VM(QEMU) with one or more crypto devices by
libvirt.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Changes in this commit:
- docs: formatdomaincaps.rst
- conf: crypto related domain caps
- qemu: crypto related
- tests: crypto related test
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce crypto device like:
<crypto model='virtio' type='qemu'>
<backend model='builtin' queues='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
</crypto>
<crypto model='virtio' type='qemu'>
<backend model='lkcf'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
</crypto>
Currently, crypto model supports virtio only, type supports qemu only
(vhost-user in the plan). For the qemu type, backend supports modle
builtin/lkcf, and the queues is optional.
Changes in this commit:
- docs: formatdomain.rst
- schemas: domaincommon.rng
- conf: crypto related domain conf
- qemu: crypto related
- tests: crypto related test
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The field is no longer used so we can remove it and the code filling it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
All callers pass 'false' so we no longer need it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Commit b7798a07f9 (in fall of 2016) changed the way we generate aliases
for 'dimm' memory devices as the alias itself is part of the migration
stream section naming and thus must be treated as ABI.
The code added compatibility layer for VMs with memory hotplug started
with the old scheme to prevent from generating wrong aliases. The
compatibility layer broke though later when 'nvdimm' and 'pmem' devices
were introduced as it wrongly detected them as old configuration.
Now rather than attempting to fix the legacy compat layer to treat other
devices properly we'll be better off simply removing it as it's
extremely unlikely that somebody has a VM started in 2016 running with
today's libvirt and attempts to hotplug more memory.
This fixes a corner case when a user hot-adds a 'dimm' into a VM with a
'dimm' and a 'nvdimm' after restart of libvirtd and then attempts to
migrate the VM.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2158701
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In error case, unref event->vm instead of vm. This makes it
easier for the reader to understand as it is the event struct
that's holding the reference.
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have virDomainGetCPUStats() API which offers querying
statistics on host CPU usage by given guest. And it works in two
modes: getting overall stats (@start_cpu == -1, @ncpus == 1) or
getting per host CPU usage.
For the QEMU driver it is implemented by looking into values
stored in corresponding cpuacct CGroup controller. Well, this
works for system instances, where libvirt has permissions to
create CGroups and place QEMU process into them. But it does not
fly for session connection, where no CGroups are set up.
Fortunately, we can do something similar to v8.8.0-rc1~95 and use
virProcessGetStatInfo() to fill the overall stats. Unfortunately,
I haven't found any source of per host CPU usage, so we just
continue throwing an error in that case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Firstly, the virProcessGetStatInfo() does not fail really. But
even if it did, it sets correct errno only sometimes (and even
that is done in a helper it's calling - virProcessGetStat() and
even there it's the case only in very few error paths).
Therefore, using virReportSystemError() to report errors is very
misleading. Use plain virReportError() instead. Luckily, there
are only two places where the former was used:
chDomainHelperGetVcpus() and qemuDomainHelperGetVcpus() (not a
big surprise since CH driver is heavily inspired by QEMU driver).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In a recent commit of v9.0.0-rc1~192 I've tried to forbid case
where a TAP device already exists, but at the same time it's
managed by Libvirt (<interface type='ethernet'> <target
dev='tap0' managed='yes'/> </interface>). NB, if @managed
attribute is missing then it's assumed to be managed by Libvirt.
Anyway, I've mistakenly put setting of
VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING flag into managed='yes'
branch instead of managed='no' branch in
qemuInterfaceEthernetConnect().
Move the setting of the flag into the correct branch.
Fixes: a2ae3d299c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The systemd service files of the qemu and libxl driver currently have a
'Requires' dependency on virtlockd, which is too strong since virtlockd
is not enabled by default in either driver. Change the dependency to a
'Wants' to avoid a package dependency between the driver subpackages and
the new libvirt-daemon-lock subpackage.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commented-out option was pointed out by jtomko during review, but
I missed taking it out when addressing his comments.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This attribute was added to support setting the --interface option for
passt, but in a post-push/pre-9.0-release review, danpb pointed out
that it would be better to use the existing <source dev='xxx'/>
attribute to set --interface rather than creating a new attribute (in
the wrong place). So we remove backend/upstream, and change the passt
commandline creation to grab the name for --interface from source/dev.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Currently, the ThreadContext object is generated whenever we see
.host-nodes attribute for a memory-backend-* object. The idea was
that when the backend is pinned to a specific set of host NUMA
nodes, then the allocation could be happening on CPUs from those
nodes too. But this may not be always possible.
Users might configure their guests in such way that vCPUs and
corresponding guest NUMA nodes are on different host NUMA nodes
than emulator thread. In this case, ThreadContext won't work,
because ThreadContext objects live in context of the emulator
thread (vCPU threads are moved around by us later, when emulator
thread finished its setup and spawned vCPU threads - see
qemuProcessSetupVcpus()). Therefore, memory allocation is done by
emulator thread which is pinned to a subset of host NUMA nodes,
but tries to create a ThreadContext object with a disjoint subset
of host NUMA nodes, which fails.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2154750
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jumping to the error label and reading the pidfile does not make sense
until we reached qemuSecurityCommandRun which creates the pidfile.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The pidfile is guaranteed to be non-NULL (thanks to glib allocation
functions) and it's dereferenced two lines above anyway.
Reported by coverity:
/src/qemu/qemu_passt.c: 278 in qemuPasstStart()
272 return 0;
273
274 error:
275 ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
276 if (pid != -1)
277 virProcessKillPainfully(pid, true);
>>> CID 404360: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "pidfile" suggests that it may be null, but it
>>> has already been dereferenced on all paths leading to the check.
278 if (pidfile)
279 unlink(pidfile);
280
281 return -1;
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
To ensure same behaviour when remote driver is or is not used we must
not steal the FDs and array holding them passed to qemuDomainFDAssociate
but rather duplicate them. At the same time the remote driver must close
and free them to prevent leak.
Pointed out by Coverity as FD leak on error path:
*** CID 404348: Resource leaks (RESOURCE_LEAK)
/src/remote/remote_daemon_dispatch.c: 7484 in remoteDispatchDomainFdAssociate()
7478 rv = 0;
7479
7480 cleanup:
7481 if (rv < 0)
7482 virNetMessageSaveError(rerr);
7483 virObjectUnref(dom);
>>> CID 404348: Resource leaks (RESOURCE_LEAK)
>>> Variable "fds" going out of scope leaks the storage it points to.
7484 return rv;
Fixes: abd9025c2f
Fixes: f762f87534
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This consists of (1) adding the necessary args to the qemu commandline
netdev option, and (2) starting a passt process prior to starting
qemu, and making sure that it is terminated when it's no longer
needed. Under normal circumstances, passt will terminate itself as
soon as qemu closes its socket, but in case of some error where qemu
is never started, or fails to startup completely, we need to terminate
passt manually.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
passt support requires "-netdev stream", which was added to QEMU in
qemu-7.2.0.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This fits better with the element containing the value (<driver>), and
allows us to use virDomainNetBackend* for things in the <backend>
element.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Assert support for VIR_DOMAIN_DEF_FEATURE_DISK_FD in the qemu driver
now that all code paths are adapted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Probing stats and block copy to a FD passed image is not yet supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We assume that FD passed images already exist so all existance checks
are skipped.
For the case that a FD-passed image is passed without a terminated
backing chain (thus forcing us to detect) we attempt to read the header
from the FD.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Prepare the internal data for passing FDs instead of having qemu open
the file internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When starting up a VM with FD-passed images we need to look up the
corresponding named FD set and associate it with the virStorageSource
based on the name.
The association is brought into virStorageSource as security labelling
code will need to access the FD to perform selinux labelling.
Similarly when startup is complete in certain cases we no longer need to
keep the copy of FDs and thus can close them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The new helper qemuDomainStartupCleanup is used to perform cleanup after
a startup of a VM (successful or not). The initial implementation just
calls qemuDomainSecretDestroy, which can be un-exported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Implement passing and storage of FDs for the qemu driver. The FD tuples
are g_object instances stored in a per-domain hash table and are
automatically removed once the connection is closed.
In the future we can consider supporting also to not tie the lifetime of
the passed FDs bound to the connection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The patch moving the code didn't faithfully represent the typecasting
of the 'bandwidth' variable needed to properly convert from the legacy
'unsigned long' argument which resulted in a build failure on 32 bit
systems:
../src/qemu/qemu_block.c: In function ‘qemuBlockCommit’:
../src/qemu/qemu_block.c:3249:23: error: comparison is always false due to limited range of data type [-Werror=type-limits]
3249 | if (bandwidth > LLONG_MAX >> 20) {
| ^
Fix it by returning the check into qemuDomainBlockCommit as it's needed
only because of the legacy argument type in the old API and use
'unsigned long long' for qemuBlockCommit.
Fixes: f5a77198bf
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Some compilers aren't happy when an automatically freed variable is used
just to free something (thus it's only assigned in the code):
When compiling qemuSnapshotDelete after recent commits they complain:
../src/qemu/qemu_snapshot.c:3153:61: error: variable 'delData' set but not used [-Werror,-Wunused-but-set-variable]
g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL;
^
To work around the issue we can restructure the code which also has the
following semantic implications:
- since qemuSnapshotDeleteExternalPrepare does validation we error out
sooner than attempting to start the VM
- we read the temporary variable at least in one code path
Fixes: 4a4d89a925
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
If the daemon crashes or is restarted while the snapshot delete is in
progress we have to handle it gracefully to not leave any block jobs
active.
For now we will simply abort the snapshot delete operation so user can
start it again. We need to refuse deleting external snapshots if there
is already another active job as we would have to figure out which jobs
we can abort.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When daemon is restarted and libvirt tries to recover domain jobs we
need to know if the snapshot job was a snapshot delete in order to
safely abort running QEMU block jobs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting external snapshots the operation may fail at any point
which could lead to situation that some disks finished the block commit
operation but for some disks it failed and the libvirt job ends.
In order to make sure that the qcow2 images are in consistent state
introduce new element "<snapshotDeleteInProgress/>" that will mark the
disk in snapshot metadata as invalid until the snapshot delete is
completed successfully.
This will prevent deleting snapshot with the invalid disk and in future
reverting to snapshot with the invalid disk.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With external snapshots we need to modify the metadata bit more then
what is required for internal snapshots. Mainly the storage source
location changes with every external snapshot.
This means that if we delete non-leaf snapshot we need to update all
children snapshots and modify the disk sources for all affected disks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting snapshot we are starting block-commit job over all disks
that are part of the snapshot.
This operation may fail as it writes data changes to the backing qcow2
image so we need to wait for all the disks to finish the operation and
wait for correct signal from QEMU. If deleting active snapshot we will
get `ready` signal and for inactive snapshots we need to disable
autofinalize in order to get `pending` signal.
At this point if commit for any disk fails for some reason and we abort
the VM is still in consistent state and user can fix the reason why the
deletion failed.
After that we do `pivot` or `finalize` if it's active snapshot or not to
finish the block job. It still may fail but there is nothing else we can
do about it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In order to save some CPU cycles we will collect all the necessary data
to delete external snapshot before we even start. They will be later
used by code that deletes the snapshots and updates metadata when
needed.
With external snapshots we need data that libvirt gets from running QEMU
process so if the VM is not running we need to start paused QEMU process
for the snapshot deletion and kill at afterwards.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting external snapshots will require to run it as async domain job,
the same way as we do for snapshot creation.
For internal snapshots modify the job mask in order to forbid any other
job to be started.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting internal snapshot when the currently active disk image is
different than where the internal snapshot was taken doesn't work
correctly.
This applies to a running VM only as we are using QMP command and
talking to the QEMU process that is using different disk.
This works correctly when the VM is shut of as in this case we spawn
qemu-img process to delete the snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Prepare the validation function for external snapshot delete support.
There is one exception when deleting `children-only` snapshots. If the
snapshot tree is like this example:
snap1 (external)
|
+- snap2 (internal)
|
+- snap3 (internal)
|
+- snap4 (internal)
and user calls `snapshot-delete snap1 --children-only` the current
snapshot is external but all the children snapshots are internal only
and we are able to delete it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract the code deleting external snapshot metadata to separate
function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Previously the reparent happened before the actual snapshot deletion.
This change moves the code closer to the rest of the code handling
snapshot metadata when deletion happens. This makes the metadate
deletion happen after the data files are deleted.
Following patch will extract it into separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This simplifies the code a bit by reusing existing parts that deletes
a single snapshot.
The drawback of this change is that we will now call the re-parent bits
to keep the metadata in sync for every child even though it will get
deleted as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract code that deletes children of specific snapshot to separate
function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract code that deletes single snapshot to separate function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Move code around to make it clear what is called when deleting single
snapshot or children snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
QEMU emits this signal when the job finished its work and is about to be
finalized. If the job is started with autofinalize disabled the job
waits for user input to finalize the job.
This will be used by snapshot delete code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The created job will be needed by external snapshot delete code so
rework qemuBlockCommit to return that pointer.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
External snapshots will use this to synchronize qemu block jobs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting external snapshots will require configuring autofinalize to
synchronize the block jobs for disks withing single snapshot in order to
be able safely abort of one of the jobs fails.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Upcoming snapshot deletion code will require that multiple commit jobs
are finished in sync. To allow aborting then if one fails we will need
to use manual finalization of the jobs.
This commit implements the monitor code for `job-finalize`.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow to use it while having async domain job active which we
will use when deleting external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow to use it while having async domain job active which we
will use when deleting external snapshots. At the same time we will need
to have the block job started as synchronous.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Move the code for finishing a job in the ready state to qemu_block.c.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Up until commit 629282d884, using mode=restrictive caused
virNumaSetupMemoryPolicy() to be called from qemuProcessHook(),
and that in turn resulted in virNumaNodesetIsAvailable() being
called and the nodeset being validated.
After that change, the only validation for the nodeset is the one
happening in qemuBuildMemoryBackendProps(), which is skipped when
using mode=restrictive.
Make sure virNumaNodesetIsAvailable() is called whenever a
nodeset has been provided by the user, regardless of the mode.
https://bugzilla.redhat.com/show_bug.cgi?id=2156289
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When post-copy migration fails, the domain stays running on the
destination with a VIR_DOMAIN_RUNNING_POSTCOPY_FAILED reason. Both the
state and the reason can later be rewritten in case the domain gets
paused for other reasons (such as an I/O error). Thus we need a separate
place to remember the post-copy migration failed to be able to resume
the migration.
https://bugzilla.redhat.com/show_bug.cgi?id=2111948
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parameter was only used to select which states correspond to an
active or failed post-copy migration. But these states are either
applicable to both operations or the check would just paper over a code
bug in case of an impossible combination of state and operation. By
dropping the check we can make the code simpler and also reuse existing
virDomainObjIsFailedPostcopy function and only check for active
post-copy states.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemu driver uses connection close callbacks in more places requiring
more changes than other drivers, but luckily the changes are very
straightforward. The migration code was written in a way ensuring that
there's just one callback present so this can be preserved directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The function can't fail so there's no point in returning anything.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
1.clear passwd in debug log
2.alignment
3.use the same variable name for function definition and declaration
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In a recent commit I've introduced an umount() call. But the
function where the call lives is compiled on all OSes, not just
Linux. But umount() is Linux specific. Other OSes have unmount
(FreeBSD), or maybe something else. But since namespaces are
Linux specific, we can wrap the call in #ifdef __linux__ and not
care about other OSes.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When calling virConnectGetDomainCapabilities() (exposed as virsh
domcapabilities) users have option to specify whatever sub-set of
{ emulatorbin, arch, machine, virttype } they want. Then we have
a logic (hidden in virQEMUCapsCacheLookupDefault()) that picks
qemuCaps that satisfy values passed by user. And whatever was not
specified is then set to the default value as specified by picked
qemuCaps. For instance: if no machine type was provided but
emulatorbin was, then the machine type is set to the default one
as defined by the emulatorbin.
Or, when just virttype was set then the remaining three values
are set to their respective defaults. Except, we have a crasher
in this case:
# virsh domcapabilities --virttype hvf
error: Disconnected from qemu:///system due to end of file
error: failed to get emulator capabilities
error: End of file while reading data: Input/output error
This is because for 'hvf' virttype (at least my) QEMU does not
have any machine type. Therefore, @machine is set to NULL and the
rest of the code does not expect that.
What we can do about this is to validate all arguments. Well,
except for the emulatorbin which is obtained from passed
qemuCaps. This also fixes the issue when domcapabilities for a
virttype of a different driver are requested, or a different
arch.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When deciding whether to bind mount a path in domain's namespace,
we look at the QEMU mount table (/proc/$pid/mounts) and try to
match prefix of given path with one of mount points. Well, we
do that in a bit clumsy way. For instance, if there's
"/dev/hugepages" already mounted inside the namespace and we are
deciding whether to bind mount "/dev/hugepages1G/..." we decide
to skip over the path and NOT bind mount it. This is because
plain STRPREFIX() is used and yes, the former is prefix of the
latter. What we need to check also is whether the next character
after the prefix is slash.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our code relies on mount events propagating into the namespace we
create for a domain. However, there's one caveat. In v8.8.0-rc1~8
I've tried to make us detect differences in mount tables between
the namespace in which libvirtd runs and the domain namespace.
This is crucial for any mounts that happen after the domain was
started (for instance new hugetlbfs can be mounted on say
/dev/hugepages1G).
Therefore, we take a look into /proc/$(pgrep qemu)/mounts to see
what filesystems are mounted under /dev. Now, since we don't
umount the original /dev, just mount a tmpfs over it, we get all
the events (e.g. aforementioned hugetlbfs mount on
/dev/hugepages1G), but we are not really able to access it
because of the tmpfs that's placed on top. This then confuses our
algorithm for detecting which filesystems are mounted (the
algorithm is implemented in qemuDomainGetPreservedMounts()).
To break the link between host's and guest's /dev we just need to
umount() the original /dev in the namespace. Just before our
artificially created tmpfs is moved into its place.
Fixes: 46b03819ae
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151869#c6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Inside of qemuCaps (for the corresponding accelerator) we have
full host CPU expansion stored, among with supported Hyper-V
Enlightenments. To report them in the domain capabilities, we
just have to pick those starting with "hv-" and see if we know
them.
You may notice that neither of our domaincapsdata test shows any
enlightenment. This is because the test works by parsing
corresponding qemucapabilitiesdata/caps_*.xml file and none of
these store the full host CPU expansion (hostCPU.fullQEMU)
because that is runtime piece of information and not formatted
into virQEMUCaps XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1717611
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we have qemuMonitorGetCPUModelExpansion() aware of
Hyper-V Enlightenments, we can start querying it. Two conditions
need to be met:
1) KVM is in use,
2) Arch is either x86 or arm.
It may look like modifying the first call to
qemuMonitorGetCPUModelExpansion() inside of
virQEMUCapsProbeQMPHostCPU() would be sufficient but it is not.
We really need to ask QEMU for full expansion and the first call
does not guarantee that.
For the test data, I've just copied whatever
'query-cpu-model-expansion' returned earlier, therefore there are
no hv-* props. But that's okay - the full expansion is not stored
in cache (and thus not formatted in
tests/qemucapabilitiesdata/caps_*.replies files either). This is
purely runtime thing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This continues and finishes propagation of the @hv_passthrough
argument started in the previous commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Apart from setting @migratable prop to the
query-cpu-model-expansion command, we will need @hv-passthrough
so that we can query for expansion of Hyper-V Enlightenments
supported on the current host. The idea is to run:
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "full",
"model": {
"name": "host",
"props": {
"hv-passthrough": true
}
}
}
}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In a recent commit, when ditching virXPathULong() the parsing of
<selfvers/> was changed. But it was changed to virXMLPropUInt()
which is not correct because the value we're interested in is not
in an attribute but element itself.
Fixes: a3c7426839
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The @hash variable inside of virQEMUCapsProbeQMPHostCPU() is used
only within a block, but declared at the beginning of the
function. Bring the variable declaration into the said block.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After previous cleanup this function is no longer used and thus
can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When starting swtpm binary, the qemuSecurityStartTPMEmulator() is
called which sets seclabel on the TPM state and then uses
qemuSecurityCommandRun() to execute the swtpm binary with proper
seclabel. Well, the aim is to ditch
qemuSecurityStartTPMEmulator() because it entangles two distinct
operations. Just call functions for them separately.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If swtpm binary fails to start after successful exec() (e.g. it
fails to initialize itself), the seclabels set in
qemuSecurityStartTPMEmulator() are not restored. This is due to
lacking qemuSecurityRestoreTPMLabels() call in the error path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we have qemuSecurityRestoreTPMLabels() we might as well
have qemuSecuritySetTPMLabels(). The aim here is to remove
qemuSecurityStartTPMEmulator() which couples two separate things
into a single function call.
Therefore, introduce qemuSecuritySetTPMLabels() which does only
set seclabels on the TPM state.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The qemuSecurityCleanupTPMEmulator() function calls
virSecurityManagerRestoreTPMLabels() and thus the proper name is
qemuSecurityRestoreTPMLabels(). Rename it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, qemuSecurityCleanupTPMEmulator() returns nothing which
means a caller (well, there's only one - qemuExtTPMStop()) can't
produce a warning when restoring seclabels on TPM state failed.
True, qemuSecurityCleanupTPMEmulator() does report a warning
itself, but only in one specific error path.
Make the function return an integer, just like the rest of
qemuSecurity*Restore() functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemu is about to deprecate the '-no-hpet' option in favor of configuring
the timer via '-machine'.
Use the QEMU_CAPS_MACHINE_HPET capability to switch to the new syntax
and mask out the old QEMU_CAPS_NO_HPET capability at the same time to
prevent using the old syntax.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
The capability represents that qemu accepts the configuration of the
HPET timer via -machine hpet=on/off rather than the
soon-to-be-deprecated '-no-hpet' option.
The capability is detected from 'query-command-line-options' which
recently added the 'hpet' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Introduce a new backend type 'external' for connecting to a swtpm daemon
not managed by libvirtd.
Mostly in one commit, thanks to -Wswitch and the way we generate
capabilities.
https://bugzilla.redhat.com/show_bug.cgi?id=2063723
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Although the qemuMigrationSrcPerformResume actually got called
indirectly via qemuMigrationSrcPerformNative and the recovery process
worked, wrong job phases were used for the "perform" phase, which could
cause issues when libvirt daemon crashed (or was otherwise restarted)
during post-copy recovery.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
It will need to be called from a place above its current definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When qemuDomainObjReleaseAsyncJob is called when the current async job
is already released we emit quite useless warning which was implemented
to warn about releasing a job owned by another thread.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function is called even if QEMU reports migration as
postcopy-paused, i.e., it's not migrating anymore. And while changing
the warning, we can drop the part about unattended migration to make the
warning shorter and consistent with qemuMigrationSrcPostcopyFailed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There are some cases when the internal state of disks can change
without qemu sending events about it (e.g. a disk can close
during reset). In case this happens, we should emit an event
about the modified disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1824722#c20
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting a guest with <interface/> which has the target
device name set (i.e. not generated by us), it may happen that
the TAP device already exists. This then may lead to all sorts of
problems. For instance: for <interface type='network'/> the TAP
device is plugged into the network's bridge, but since the TAP
device is persistent it remains plugged there even after the
guest is shut off. We don't have a code that unplugs TAP devices
from the bridge because TAP devices we create are transient, i.e.
are removed automatically when QEMU closes their FD.
The only exception is <interface type='ethernet'/> with <target
managed='no'/> where we specifically want to let users use
pre-created TAP device and basically not touch it at all.
There's another reason for denying to use a pre-created TAP
devices: if we ever have bug in TAP name generation, we may
re-use a TAP device from another domain.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2144738
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Historically, QEMU took screenshots in PPM. While this might use
to be popular format, as of v7.1.0-rc0~125^2~6 it is possible to
take screenshots in PNG. This is more popular and renders almost
everywhere, which is not the case for PPM (for instance, modern
browsers do not render it).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'screendump' command has new argument 'format'. Let's expose
this on our QMP level so that callers can specify the format, if
they wish so.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For some reason, only @file argument is printed into debug logs.
The rest of arguments was left out. Include all arguments.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In its v7.1.0-rc0~125^2~6 commit, QEMU gained support for taking
screenshots in PNG format. Track this capability.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Internal domain state needs to be refreshed after reset from the guest
side because it may be inconsistent with the internal qemu state.
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>
Internal domain state may change during the reset and qemu does
not always send events about it. In case it happens, internal
state of the domain in libvirt would be inconsistent with the
internal state in qemu which could cause additional problems
(e.g. cdrom tray state can change from open to closed). The
solution is to refresh state after a successful reset to query
qemu about the current internal domain state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1824722
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>
Paths for external devices (well, so far only vTPM) are not
stored in the status XML. Therefore, we need to regenerate them
after we've been restarted and reconnecting to a running domain.
Otherwise these will remain NULL which may later lead to a NULL
dereference.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2150760
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function is going to be called outside of qemu_extdevice.c.
Expose it to the rest of the driver.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The path generation phase belongs conceptually into domain
preparation phase and not host preparation. Move
qemuExtDevicesInitPaths() call from qemuExtDevicesPrepareHost()
into qemuExtDevicesPrepareDomain().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The domain startup process is split into multiple phases. One of
them is preparing the domain (at that point live) XML, private
data, various paths, etc - see qemuProcessPrepareDomain(); the
other prepares the host - see qemuProcessPrepareHost(). It's
obvious that the domain XML preparation function must be called
before the host preparation function (e.g. the host preparation
might try to create a file which path is generated in the domain
preparation phase). Nevertheless, let's document this
expectation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the 'cleanup' label and 'ret' variable as we can now directly
return form all cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use of qemuDomainValidateVcpuInfo in the helpers for hotplug and unplug
of vCPUs can lead to spurious errors reported such as:
internal error: qemu didn't report thread id for vcpu 'XX'"
The reason for this is that qemuDomainValidateVcpuInfo validates the
state of all vCPUs against the expected state of vCPUs. If an unplug
operation completed before libvirt was unable to process it yet the
expected state could not reflect the current state.
To avoid spurious errors the qemuDomainHotplugAddVcpu and
qemuDomainRemoveVcpu functions are modified to do localized validation
only for the vCPUs they actually modify.
We also now ensure that the cgroups are modified before bailing out on
error for any vCPUs which passed validation.
Additionally in order for qemuDomainRemoveVcpuAlias to be able to find
the unplugged vCPU we must ensure that qemuDomainRefreshVcpuInfo does
not clear out the alias in case when the vCPU is no longer reported by
qemu.
Co-authored-by: Partha Satapathy <partha.satapathy@oracle.com>
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Recently, the QEMU driver gained support for migration with TPM
state on a shared volume (e.g. NFS). As a part of that, the
destination side avoids setting seclabels on it to avoid cutting
off the source while it is still using it. Makes sense, except
for a wee bit: the secdriver API does a bit more - it also sets
label on the swtpm log file. And this one definitely needs to be
labeled (it lives under /var/log/swtpm/libvirt/qemu/..., i.e. not
on a shared volume).
Previously, qemuSecurityStartTPMEmulator() took care of that. But
during rework to shared volume migration, the code was changed so
now plain qemuSecurityCommandRun() would be run (i.e. no
relabelling).
But after previous commits, we can now chose whether the TPM
state should be relabelled or just the log file.
Fixes: 2e669ec789
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2130192#c7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is basically just a continuation of the previous commit.
Now that the security driver APIs have a boolean flag that
controls setting/restoring seclabel of either both TPM state and
log files, or just the log file, propagate this boolean into
those APIs that start/stop swtpm emulator. For now, just pass
true. The juicy bits are soon to come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virSecurityDomainSetTPMLabels() and
virSecurityDomainRestoreTPMLabels() APIs set/restore label on two
files/directories:
1) the TPM state (tpm->data.emulator.storagepath), and
2) the TPM log file (tpm->data.emulator.logfile).
Soon there will be a need to set the label on the log file but
not on the state. Therefore, extend these APIs for a boolean flag
that when set does both, but when unset does only 2).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virJSONValueObjectGetArray + virJSONValueArrayToStringList instead
so that the ofvirJSONValueObjectGetStringArray wrapper can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In two instances (qemuMonitorJSONGetStringListProperty,
qemuMonitorJSONGetStringArray) the return value is checked by
qemuMonitorJSONCheckReply and extracted by
virJSONValueObjectGetStringArray.
We can use qemuMonitorJSONGetReply which returns it directly and then
virJSONValueArrayToStringList to convert it without the additional
lookup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Using 'virJSONValueObjectHasKey' when we want to access the value
afterwards is wasteful. Fetch the JSON value right away.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than checking that the object has the correct key and then
fetching it again use fetch the array first and then use
virJSONValueArrayToStringList to directly convert it.
Additionally we can avoid the conversion if there are no members
simplifying the surrounding logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'dependencies' field in the return data may be missing in some
cases. Historically 'virJSONValueObjectGetStringArray' didn't report
error in such case, but later refactor (commit 043b50b948 ) added
an error in order to use it in other places too.
Unfortunately this results in the error log being spammed with an
irrelevant error in case when qemuAgentGetDisks is invoked on a VM
running windows.
Replace the use of virJSONValueObjectGetStringArray by fetching the
array first and calling virJSONValueArrayToStringList only when we have
an array.
Fixes: 043b50b948
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2149752
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use qemuMonitorJSONGetReply and unify the two blocks with the same
condition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use qemuMonitorJSONGetReply in cases where qemuMonitorJSONCheckReply
is followed by virJSONValueObjectGet*(reply, "return").
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace usage of the following pattern with the new helper:
if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
return -1;
data = virJSONValueObjectGetArray(reply, "return");
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace usage of the following pattern with the new helper:
if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
return -1;
data = virJSONValueObjectGetObject(reply, "return");
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than simply checking that the 'return' field is of the expected
type we can directly return it as the caller is very likely going to use
it. Extract the code into the new function and add a wrapper to preserve
old functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't continue with the historical mistake and fix all internal
functions to use a sane type for flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCommandSetSendBuffer() function consumes passed @buffer,
but takes it only as plain pointer. Switch to a double pointer to
make this obvious. This allows us then to drop all
g_steal_pointer() in callers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
QEMU capabilities is the only thing we use from priv so we can just pass
that directly.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When an internal API takes a vm pointer, it's usually just after the
driver argument.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The vm object is used inside qemuMigrationCookieParse based on the flags
passed to qemuMigrationCookieParse and the content of the cookie. The
callers should not just blindly guess and pass NULL if they
(incorrectly) think the vm object is not needed. We should always pass
the vm object unless it does not exist yet.
This fixes a bug when statistics of a completed migration reported
"Unknown" operation instead of "Incoming migration" on the destination
host.
https://bugzilla.redhat.com/show_bug.cgi?id=2137298
Fixes: v8.7.0-79-g0150f7a8c1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Inside of qemuTPMEmulatorBuildCommand() there are two calls to
qemuTPMSetupEncryption() which simply ignore returned error. This
is suboptimal because then we rely on swtpm binary reporting a
generic error (something among invalid command line arguments)
while an error reported by qemuTPMSetupEncryption() is more
specific.
However, since virCommandSetSendBuffer() only sets an error
inside of virCommand structure (the error is then reported in
virCommandRun()), we need to exempt its retval from error
checking. Thus, the signature of qemuTPMSetupEncryption() is
changed a bit so that -1/0 can be returned to indicate error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When there is no vIOMMU, vfio devices don't need to lock the entire guest
memory per-device, but they still need to lock the entire guest memory to
share between all vfio devices. This memory accounting is not shared
with vDPA devices, so it should be added to the memlock limit separately.
Commit 8d5704e2 added support for multiple vfio/vdpa devices but
calculated the limits incorrectly when there were both vdpa and vfio
devices and no vIOMMU. In this case, the memory lock limit was not
increased separately for the vfio devices.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2143838
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When post-copy migration is running in Finish phase we already did
everything needed and we're just waiting for all the memory to transfer
to the destination. The domain is already running on there at this
point. Once all data is transferred (QEMU sends a MIGRATION completed
event) we're done. So in this specific post-copy case the source does
not need to care about the result of the Finish call as long as QEMU
says migration completed. The Finish call to the destination daemon may
fail for reasons that do not affect QEMU, e.g., libvirt daemon was
restarted there or the libvirt connection broke.
Currently we just mark the post-copy migration as failed on the source
and keep the domain paused there. But when libvirt daemon is restarted
at this point, it will detect migration finished successfully and kill
the domain as migrated. It make sense to do this even without having to
restart the daemon.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/338
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We need the restored job even in case the migration already finished
even though we will stop it just a few lines below as the functions we
call in between require an existing migration job.
This fixes a crash on reconnect when post-copy migration finished while
the daemon was not running.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When generating memory for main guest memory memory-backend-*
might be used. This means, we may need to generate thread-context
objects too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When generating memory for memory devices memory-backend-* might
be used. This means, we may need to generate thread-context
objects too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When generating memory for guest NUMA memory-backend-* might be
used. This means, we may need to generate thread-context objects
too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
While technically thread-context objects can be reused, we only
use them (well, will use them) to pin memory allocation threads.
Therefore, once we connect to QEMU monitor, all memory (with
prealloc=yes) was allocated and thus these objects are no longer
needed and can be removed. For on demand allocation the TC object
is left behind.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The aim of thread-context object is to set affinity on threads
that allocate memory for a memory-backend-* object. For instance:
-object '{"qom-type":"thread-context","id":"tc-ram-node0","node-affinity":[3]}' \
-object '{"qom-type":"memory-backend-memfd","id":"ram-node0","hugetlb":true,\
"hugetlbsize":2097152,"share":true,"prealloc":true,"prealloc-threads":8,\
"size":15032385536,"host-nodes":[3],"policy":"preferred",\
"prealloc-context":"tc-ram-node0"}' \
allocates 14GiB worth of memory, backed by 2MiB hugepages from
host NUMA node 3, using 8 threads. If it weren't for
thread-context these threads wouldn't have any affinity and thus
theoretically could be scheduled to run on CPUs of different NUMA
node (which is what I saw occasionally).
Therefore, whenever we are pinning memory (IOW setting host-nodes
attribute), we can generate thread-context object with the same
affinity.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In its commit v7.1.0-1429-g7208429223 QEMU gained new object
thread-context, which allows running specialized tasks with
affinity set to a given subset of host CPUs/NUMA nodes. Even
though only memory allocation task accepts this new object, it's
exactly what we aim to implement in libvirt. Therefore, introduce
a new capability to track whether QEMU is capable of this object.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
