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>
That's already the case in practice, but it's a better
experience for the user if we reject this configuration
outright instead of silently ignoring part of it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Follow better meson build system conventions. This allows to find
keymap-gen or CSV without explicitly setting the paths.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function does not exist on win32.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
meson wraps python scripts already on win32, so we end up with these
failing commands:
[1/359] "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture" "src/util/virkeycodetable_atset1.h" "--" "sh" "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh" "C:/msys64/ucrt64/bin/python3.EXE" "python" "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen" "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1" "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv" "atset1"
FAILED: src/util/virkeycodetable_atset1.h
"C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture" "src/util/virkeycodetable_atset1.h" "--" "sh" "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh" "C:/msys64/ucrt64/bin/python3.EXE" "python" "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen" "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1" "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv" "atset1"
If LC_ALL, LANG and LC_CTYPE need to be set, it would probably be better
to use a meson environment() instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'dirs' argument to locate the program.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A number of changes, but notably python script shebang fixing build
issues in CI:
Daniel P. Berrangé (1):
Revert "Add local argparse for compat with python 2.6"
Dawid Dziurla (1):
Don't hardcode python3 path in shebang
Eli Schwartz (1):
make the meson.build stub a bit more well-rounded by exporting files
Pierre Ossman (1):
Fix macOS "ISO" key
Ross Lagerwall (2):
Use python3 binary rather than unversioned python
Fix Hangeul/Hanja scancodes
William (1):
Add Qemu qcode support for F13 to F24
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rewrite the code to improve maintainability and also re-do construction
of error messages which are assembled from non-translatable parts.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/455
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit dbf1f68410 ("security: do not remember/recall labels for VFIO")
rightly changed the DAC and SELinux labeling parameters to fix a problem
with "VFIO hostdevs" but really only addressed the PCI codepaths.
As a result, we can still encounter this with VFIO MDEVs such as
vfio-ccw and vfio-ap, which can fail on a hotplug:
[test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-eff72715c4f0
[test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-eff72715c4f0
[test@host ~]# cat disk.xml
<hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
<source>
<address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/>
</source>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/>
</hostdev>
[test@host ~]# virsh attach-device guest ~/disk.xml
error: Failed to attach device from /home/test/disk.xml
error: Requested operation is not valid: Setting different SELinux label on /dev/vfio/3 which is already in use
Make the same changes as reported in commit dbf1f68410, for the mdev paths.
Reported-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
Introduce a small kludge in the parser to avoid unnecessarily
blocking incoming migration from a range of recent libvirt
releases.
https://bugzilla.redhat.com/show_bug.cgi?id=2184966
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In an effort to separate the validation steps from the Parse stage,
a few validation checks of virDomainGraphicsListenDef have been moved from
virDomainGraphicsListenDefParseXML() in domain_conf.c to
virDomainGraphicsDefListensValidate() in domain_validate.c
Signed-off-by: K Shiva <shiva_kr@riseup.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
The function always returns 0. Remove the return value and refactor
caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of virAcpiParseIORTNodeHeader() there's an
virReportError() which reports size of a structure using sizeof()
operator. Well, it's not well documented but the returned type of
sizeof() is apparently size_t but the format string uses %lu.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The aim of this new module is to contain code that's parsing ACPI
tables. For now, only parsing of IORT table is implemented (it's
ARM specific table). And since we only need to check whether the
table contains SMMU record, the code is very simplified.
I've followed the specification published here:
https://developer.arm.com/documentation/den0049/latest/
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@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>
Simplify use of the function by determining the number of elements
inside the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Get rid of nested ternaries by adding a few helper variables and more
explicit if conditions to fill them appropriately.
Note that 'virCapabilitiesAllocMachines' doesn't require return value
check any more as it can't fail.
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>
There's more stuff than device info to clear nowadays. Drop the
misleading comment. Shorten the comment saying that device info is freed
elsewhere when 'parentnet' is present.
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 capabilities generated on OSX hosts with 'hvf' accelerator will not
pass schema testing as the 'hvf' type was not allowed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When opening a connection, it may be necessary to provide user
credentials, or some additional info (e.g. whether to trust an
ssh key). We have a special API for that: virConnectOpenAuth()
where and additional callback can be passed. This callback is
then called with _virConnectCredential struct filled partially
and it's callback's responsibility to get desired data (e.g. by
prompting user) and store it into .result member of the struct.
But we document the callback behaviour as:
When authentication requires one or more interactions, this callback
is invoked. For each interaction supplied, data must be gathered
from the user and filled in to the 'result' and 'resultlen' fields.
If an interaction cannot be filled, fill in NULL and 0.
Returns 0 if all interactions were filled, or -1 upon error
But there are some buggy callbacks out there, which set:
.result = NULL;
.resultlen = 0;
and return 0. Report an error when such buggy callback is met.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181235
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
'virDomainHostdevDefClear' must clear the pointers too as it can be
invoked multiple times on the same object e.g. inside
qemuDomainRemoveHostDevice once via virDomainHostdevDefFree which skips
freeing the object if it's used via <interface> and thus has a 'net'
definition corresponding to it, and then subsequently via
virDomainNetDefFree.
Fix it by clearing the pointer along with freeing it.
Fixes: d9e4075d4e
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2182961
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
desc length should be always less than VIR_STORAGE_MAX_HEADER.
If len = VIR_STORAGE_MAX_HEADER, desc may be out of bounds.
Fixes: 296032bfb2 ("util: extract storage file probe code into virtstoragefileprobe.c")
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
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>
While it's true that the default username is:
administrator@${SSO-Domain}
in majority of cases the ${SSO-Domain} is "vsphere.local". But
our code (and what virsh displays then) says it's just
"administrator".
This is wrong also from a different POV: the username must
contain the suffix no matter what and our default suggests
otherwise.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181234
Signed-off-by: Michal Privoznik <mprivozn@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>
The reason why it was in postparse in the first place was so
that we could could automatically enable the secure-boot feature
in some cases, but that no longer happens so we can finally move
it to the proper location.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we're adding information obtained from the firmware
descriptor to the domain XML, this will happen automatically
whenever a firmware that has the enrolled-keys feature ends up
being selected.
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>
The combination of explicit firmware paths, which we now
produce in all cases, and firmware autoselection knobs is
explicitly rejected by libvirt 8.6.0 and newer.
Right now we produce inherently migratable XML in all cases,
since we always strip those bits, but that's going to change
soon. To prepare for that, make sure that we always skip the
problematic elements and attributes when preparing a
migratable XML.
The destination will simply receive a fully specified firmware
configuration, which is indistinguishable from one that was
manually provided by the user and is thus accepted by any old
version of libvirt, regardless of whether or not firmware
autoselection was used on the source host.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
libvirt 8.6.0 introduced these checks and very clearly delineated
two possible firmware selection scenarios: manual firmware
selection, where the user is responsible for providing all
information, and firmware autoselection, where a list of desired
features is provided and everything else is handled by libvirt.
In the interest of maintaining the clear separation between these
two scenarios, setting most attributes when firmware autoselection
is active will result in the configuration being rejected.
This works fine, but is unnecessarily restrictive: in most cases,
the additional information that the user has provided matches
the information that libvirt would have discovered on its own by
looking at firmware descriptors, and asking the user to scrub it
from the XML only result in pointless friction.
Remove these checks entirely.
Unsurprisingly, this results in a few test cases that were
rejected until now to suddenly start working and producing
sensible results.
The firmware-auto-efi-loader-path-nonstandard test case is
notable: while we can now enable the xml2xml part of the test,
the xml2argv part is still failing, although in a slightly
different way. This is expected: since the firmware binary is a
non-standard one, libvirt is unable to figure out the missing
information from a firmware descriptor, and the configuration
is still ultimately an invalid one. However, if we were to find
such a configuration on disk at daemon startup, we would not
ignore it completely and instead would offer the user a chance
to fix it.
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>
The function name is already logged, and these can happen only as a
result of a programmer error.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Both callers in the VirtualBox driver handle the error and only
call this function with a non-NULL argument.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Both callers in the VirtualBox driver error out if the path
can't be fetched via VirtualBox APIs and abort on conversion error
from UTF-16 to UTF-8.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
over-writing a variable in inner while-loop without freeing previous memory
leaks it over time.
To fix this, we can just change scope of bank variable to the inner loop.
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Fixes: 5c84485439
Signed-off-by: Ján Tomko <jtomko@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>
In some translations, the RNG initials were mistranslated
as a random number generator.
Spell it out as RelaxNG to make it clearer.
Include the word 'schema' and quotes around the filename.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This brings the tool's list of features in sync with qemu
commit 9832009d9dd2386664c15cc70f6e6bfe062be8bd.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
Since qemuxml2argvtest is now using virnumamock, there's no need
for qemuxml2argvmock to offer reimplementation of virNuma*()
functions. Also, the comment about CLang and FreeBSD (introduced
in v4.3.0-40-g77ac204d14) is no longer true. Looks like noinline
attribute was the missing culprit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Introduce a mock of virNumaGetNodeOfCPU() because soon we will
need virNumaCPUSetToNodeset() to return predictable results.
Also, fill in missing symlinks in vircaps2xmldata/.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
So far, we have a function that expands given list of NUMA nodes
into list of CPUs. But soon, we are going to need the inverse -
expand list of CPUs into list of NUMA nodes. Introduce
virNumaCPUSetToNodeset() for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Technically, there's nothing libnuma specific about
virNumaNodesetToCPUset(). It just implements a generic algorithm
over virNumaGetNodeCPUs() (which is then libnuma dependant).
Nevertheless, there's no need to have this function living inside
WITH_NUMACTL block. Any error returned from virNumaGetNodeCPUs()
(including the one that !WITH_NUMACTL stub returns) is propagated
properly.
Move the function out of the block into a generic one and drop
the !WITH_NUMACTL stub.
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>
The platform check which determines when to apply the fixups mentions
all officially supported build targets (per docs/platforms.rst) thus
it's not really necessary.
Additionally while not explicitly written as supported the check does
not work properly when building with the MinGW toolchain on Windows as
it does not apply the needed transformations. They are necessary
there the same way as with MinGW on Linux.
https://gitlab.com/libvirt/libvirt/-/issues/453
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In a few places we still use the good old:
sizeof(var) / sizeof(var[0])
sizeof(var) / sizeof(int)
The G_N_ELEMENTS() macro is preferred though. In a few places we
don't link with glib, so provide the macro definition.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@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>
Neither of these are modified anywhere in the function, and the
function will soon be called with an arg that actually is a const.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The binary to be exec'ed by virExec() is stored in
virCommand::args[0], and is resolved to a full absolute path (stored
in a local of virExec() just prior to execve().
Since we will have another use for the full absolute path, lets make
an API to resolve/retrieve the absolute path, and cache it in
virCommand::binaryPath so we only have to do the resolution once.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
libxl added support for specifying custom firmware paths long ago. The
functionality exists in all Xen version supported by libvirt. This patch
adds support for user-specified efi firmware paths in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
passt provides an AppArmor abstraction that covers all the
inner details of its operation, so we can simply import that
and add the libvirt-specific parts on top: namely, passt
needs to be able to create a socket and pid file, while
the libvirt daemon needs to be able to kill passt.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@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>
In virDomainVideoModelDefParseXML(), use the virXMLProp* functions
rather than reimplementing them with virXPath* functions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function allows you to specify a default value to return if the
property is not found rather than always setting *result to 0.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Factor out a separate function to parse out the <model> element for
video devices.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ensure that new virDomainVideoDef objects have their 'type' set to
VIR_DOMAIN_VIDEO_TYPE_DEFAULT and remove places that this value is set
after construction. Since virDomainVideoDefNew() uses g_new0() allocate
the instance, all fields are initialized to 0.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the quiet version of virPidFileAcquirePath and remove the error
reset, since there's nothing to reset anymore.
https://bugzilla.redhat.com/show_bug.cgi?id=2171973
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The only caller that actually wants to wait for the lock.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
For now, add the 'Full' suffix to virPidFileAcquirePath and make
virPidFileAcquirePath a 'wrapper' around it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@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>
Inside of virSCSIHostFindByPCI() there's a loop which iterates of
entries of "/sys/class/scsi_host" directory trying to identify
all symlinks (which then point to a SCSI device, but that's not
important right now). But the way virFileIsLink() is called can
never return a truthful reply - because it's called over
dent->d_name instead of full path. Fix this by moving the
virFileIsLink() call and passing constructed path into it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanups, we're left with a couple of needless
labels, that contain nothing but a return statement. Drop those.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of virSCSIHostFindByPCI() there're some variables that are
used from a while() loop exclusively. Bring their declaration
into the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove some obvious uses of VIR_FREE() in favor of automatic
cleanup. This also means, that some variables affected are
brought into the inner most block, so that automatic cleanup is
effective.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
In tests we need to be able to populate the cache with a deterministic
set of entries. This means we need to drop the contents of the cache
between runs to prevent spillage between test cases.
virFileCacheClear drops all entries from the hash table used for the
cache.
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>
The function doesn't use XPath at all. Don't pass the context to it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The separate API perms XML is no longer used. Remove the support for
generating it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we now build it into the libvirt-api.xml or equivalents we don't
need the extra XML files.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For unmanaged ethernet <interface/>, it is user's responsibility
to set up the interface. And as such it can be just anything.
Therefore, it's (almost) impossible for the
virDomainInterfaceStats() API to tell whether RX/TX values need
to be swapped or copied verbatim into the return structure.
Document this limitation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When fetching stats for a domain's <interface/>, or when setting
up its QoS, we can face two situations:
1) the device "shares" the host view, meaning each packet
sent/received on the interface by a domain is accounted for in
the same category on the host, or
2) the device is at the other side, and a packet send by a
domain, is in fact packet received on the host.
This fact affects whether we need to swap RX/TX values when
fetching stats, or setting up QoS. We have this convenient helper
function (virDomainNetTypeSharesHostView()), which returns to
which category given interface type falls into.
Now, for unmanaged type='ethernet' our options are quite limited,
because it's user's responsibility to set up the host side of the
interface. And it can be just anything. Fortunately, we have
another convenience function (virNetDevMacVLanIsMacvtap()), which
determines whether given interface is a macvtap (which is
notoriously known for falling into the first category).
Let's use it to help virDomainNetTypeSharesHostView() determine
the view more accurately.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175449
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@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>
All of the drivers will reject this value, at least for now.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
The default is raw, which corresponds to the historical
behavior and is also the only accepted value, at least for
now.
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>
We already handle the <nvram> element in a separate helper,
which is cleaner than having all the logic in the top-level
virDomainLoaderDefParseXML() function.
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>
SUSE installs edk2 firmwares for both x86_64 and aarch64 in /usr/share/qemu.
Add support for this path in virt-aa-helper and allow locking files within
the path in the libvirt qemu abstraction.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@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>
Outline what the function does, especially the return value.
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>
The virDomainMemoryModelTypeFromString() is not exported, though
the enum translation functions are declared in
src/conf/domain_conf.h.
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>
It was used briefly and subsequently removed in 3592b81c4c.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'newapi.xsl' stylesheet was referencing non-existing paths to the
XML files holding ACL permission flags for individual APIs. Additionally
the 'document()' XSL function doesn't even allow concatenation of the
path as it was done via '{$builddir}/src..', but requires either direct
argument or use of the 'concat()' function.
This meant that the 'acls' variable was always empty and thus none of
our API documentation was actually generated with the 'acl' section.
Fix it by passing the path to the XML via an argument to the stylesheet
as the files differ based on which document is being generated.
Since the 'admin' API does not have ACL we need to handle it separately
now in the build system.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Both the object name and permission name in ACL use '-' instead of '_'
separator when referring to them in the docs or even when used inside of
polkit. Unfortunately the generators used for generating our docs don't
honour this in certain cases which would result in broken names in the
API docs (once they will be generated).
Rename both object and permission name to use dash and reflect that in
the anchor names in the documentation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In selinux driver there's virSecuritySELinuxSetFileconImpl()
which is responsible for actual setting of SELinux label on given
file and handling possible failures. In fhe failure handling code
we decide whether failure is fatal or not. But there is a bug:
depending on SELinux mode (Permissive vs. Enforcing) the ENOENT
is either ignored or considered fatal. This not correct - ENOENT
must always be fatal for couple of reasons:
- In virSecurityStackTransactionCommit() the seclabels are set
for individual secdrivers (e.g. SELinux first and then DAC),
but if one secdriver succeeds and another one fails, then no
rollback is performed for the successful one leaking remembered
labels.
- QEMU would fail opening the file anyways (if neither of
secdrivers reported error and thus cancelled domain startup)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004850
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In virSecuritySELinuxSetFileconImpl() we have code that handles
setfilecon_raw() failure. The code consists of two blocks: one
for dealing with shared filesystem like NFS (errno is ENOTSUP or
EROFS) and the other block that's dealing with EPERM for
privileged daemon. Well, the order of these two blocks is a bit
confusing because the comment above them mentions the NFS case
but EPERM block follows. Swap these two blocks to make it less
confusing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@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>
Fetching whether a node-device is marked for autostart can be allowed
from read-only connections similarly to other objects.
Fixes: c6607a25b9
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
For all other objects we allow the 'read' permission for anonymous
users. In fact the idea is to allow all permissions users using the
readonly connection would have.
This impacts the following APIs (in terms of RPC procedure names):
$ git grep -A 3 node_device:read | grep REMOTE
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_GET_XML_DESC = 114,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_NUM_OF_CAPS = 116,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_LIST_CAPS = 117,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 433,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 435,
src/remote/remote_protocol.x- REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 436,
Fixes: a93cd08f
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@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>
Shutdown of virtlogd prints:
(process:54742): GLib-CRITICAL **: 11:00:40.873: g_regex_unref: assertion 'regex != NULL' failed
Use g_clear_pointer instead which prevents it in the NULL case.
Fixes: 69eeef5dfb
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The warning about max_client_requests is hit inside virtlogd every time
a VM starts which spams the logs.
Emit the warning only when the client request limit is not 1 and add a
warning into the daemon config to not configure it too low instead.
Fixes: 031878c236
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2145188
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virNetServerClientDispatchRead checked the return value but it's not
necessary any more as it can't return NULL nowadays.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@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>
Integrated PCI devices can be either PCIe (virtio-iommu) or
conventional PCI (pvpanic-pci). Right now libvirt will refuse
to assign an address on pcie.0 for the latter, but that's an
undesirable limitation that we can easily remove.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainDefAddConsoleCompat in post parsing step appends a stub console
of type VIR_DOMAIN_CHR_TYPE_NULL to ch VMs' Domain XML. Cloud-hypervisor's
deviceValidateCallback (chValidateDomainDeviceDef) checks that the type of
stub console is not of type VIR_DOMAIN_CHR_TYPE_PTY and throws an error.
This commit introduces NO_STUB_CONSOLE feature check to Domain features and
uses it to skip adding stub console to ch VMs.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
When running "virsh domcapabilities" on a s390x host, all the CPU
models show up with vendor='unknown' - which sounds kind of weird
since the vendor of these mainframe CPUs is well known: IBM.
All CPUs starting with either "z" or "gen" match a real mainframe
CPU by IBM, so let's return the string "IBM" for those now.
The only remaining ones are now the artifical "qemu" and "max"
models from QEMU itself, so it should be OK to get an "unknown"
vendor for those two.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Boris Fiuczynski<fiuczy@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
In recent commit of v9.0.0-191-gc71c159248 I've introduced
remoteConnectFormatURI() function and in the function @query
variable. Even though, the variable is used, clang-13 fails to
see it. Surprisingly, newer clang is not affected. Fortunately,
swapping the order in which variables are set makes clang happy
again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When handling virConnectOpen(), we parse given URI, specifically
all those parameters we know, like ?mode, ?socket, ?name, etc.
ignoring those we don't recognize yet. Then, we reconstruct the
URI back, but ignoring all parameters we've parsed. In other
words:
qemu:///system?mode=legacy&foo=bar
becomes:
qemu:///system?foo=bar
The reconstructed URI is then passed to the corresponding driver
(QEMU in our example) with intent of it parsing parameters
further (or just ignoring them). But for some transport modes,
where virt-ssh-helper is ran on the remote host (libssh, libssh2,
ssh) we need to pass ?mode and ?socket parameters, so that it can
do the right thing, e.g. for 'mode=legacy' start the monolithic
daemon, or for 'socket=' connect to the given socket.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/433
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The aim of this helper is to manipulate the .ignore value for
given list of parameters. For instance:
virURIParamsSetIgnore(uri, false, {"mode", "socket", NULL});
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There's a piece of code in doRemoteOpen() that is going to be
called twice. Instead of duplicating the code, move it into a
function that will be called twice, later on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Similarly to the previous commit, let's accept "socket" parameter
in the connection URI. This change will allow us to use
virt-ssh-helper instead of 'nc' in all cases (done in one of
future commits).
Please note, when the parameter is used it effectively disables
automatic daemon spawning and an error is reported. But this is
intentional - so that the helper behaves just like regular
virConnectOpen() with different transport than ssh, e.g. unix.
But this 'change' is acceptable - there's no way for users to
make our remote code pass the argument to virt-ssh-helper, yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When split daemons were introduced, we also made connection URI
accept new parameter: mode={auto,legacy,direct} so that a client
can force connecting to either old, monolithic daemon, or to
split daemon (see v5.7.0-rc1~257 for more info).
Now, the change was done to the remote driver, but not to
virt-ssh-helper. True, our remote driver code still does not pass
the 'mode' parameter, but that will be addressed in next commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our own coding style suggest not inventing new names for labels
and stick with 'cleanup' (when the path is used in both,
successful and unsuccessful returns), or 'error' (when the code
below the label is used only upon error). Well, 'failed' label
falls into the latter category. Rename it then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@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>
Our URI handling code (doRemoteOpen() specifically), uses case
insensitive parsing of query part of URI. For instance:
qemu:///system?socket=/some/path
qemu:///system?SoCkEt=/some/path
are the same URI. Even though the latter is probably not used
anywhere, let's switch to STRCASEEQ() instead of STREQ() at two
places: virURIGetParam() and virURICheckUnixSocket().
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>
Actually use the log cleaner introduced by previous commit.
Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Before, logs from deleted machines have been piling up, since there were
no garbage collection mechanism. Now, virtlogd can be configured to
periodically scan the log folder for orphan logs with no recent modifications
and delete it.
A single chain of recent and rotated logs is deleted in a single transaction.
Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We want to specify the folder to clean and how much time can a log
chain live.
Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use virDomainDeviceType as type and update all switch statements which
didn't mention all possible values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the code into a new function named virDomainDeviceDefParseType. The
separation will make it easier to change the type of the 'type' field in
side of virDomainDeviceDef.
Signed-off-by: Peter Krempa <pkrempa@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>
virBitmapNextSetBit/virBitmapLastSetBit/virBitmapNextClearBit can be
used for iteration of a bitmap. Allow NULL bitmap so that iteration of a
bitmap can be simplified in certain cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor the code to use a bitmap with an enum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The virBitmapIsBitSet API is a permissive one which returns false when
the bit is not set or is out of range. We can do the same if the bitmap
is NULL to aid certain situations when this can happen, but we don't
want to add extra checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This also prevents a potential memleak when multiple elements would be
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This also prevents a potential memleak when multiple elements would be
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use the helper designed to find the subelement. A slight semantic
difference after this patch is that the first <zpci> element will be
considered instead of the last, but only one is expected in a valid XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Note that the schema doesn't allow us to represent the two branches of
optional <devnode type='dev'> and zero or more <devnode type='link'>
definitions, so I've merged them under the <zeroOrMore> case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'osxen' RNG type defines options for the <os> element in certain
modes. Allow interleaving of subelements recursively.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow interleave of the top level sub-elements as well as the
subelements in the 'host-certificates' mode. Note that '<interleave>'
doesn't work properly if there's multiple definitions of the same
sub-element in the interleave so for this patch I chose to '<group>' the
'certificate' subelements. Another options would require us to stop
enforcing that there's exactly 3 of them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
The virDomainNetDef struct has privateData (which is currently
used by QEMU driver to store FDs opened during cmd line building
phase and pass them onto cmd line).
Soon, we will need to store additional information that needs to
survive daemon restart. Let's introduce machinery for parsing and
formatting privateData.
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>
In domain_conf.c there's virDomainChrSourceModeTypeFromString()
which is open coded. Let's rewrite it using VIR_ENUM_DECL() +
VIR_ENUM_IMPL() combo.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
