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>
