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>
While it's true that the virDomainNetVhostuserMode enum is used
solely in virDomainNetDefParseXML(), its placement just above the
function is rather unfortunate. Let's put it at the beginning of
the file with the rest of the enum declarations/implementations.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The hotplug code paths need to be able to pass the FDs to the monitor to
ensure that hotplug works.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
To ensure that we can hot-unplug the disk including the associated fdset
we need to store the fdset ID in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Rollback of FD sets passed to qemu is also needed after possible restart
of libvirtd when we need to serialize the data into status XML. For this
purpose we need to access the fdset ID once it was passed to qemu and
potentially re-create a 'qemuFDPass' struct in passed state.
Introduce 'qemuFDPassNewPassed' and 'qemuFDPassIsPassed'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Until now the code didn't expect that we'd want to rollback/detach a FD
passed on the commandline, but whith disk backend FD passing this can
happen.
Properly mark the 'qemuFDPass' object as passed to qemu even when it was
done on the commandline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Copy the pointer to qemuFDPass into struct qemuBlockStorageSourceAttachData
so that it can be used from qemuBuildBlockStorageSourceAttachDataCommandline
rather than looping again in qemuBuildDiskSourceCommandLineFDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Be consistent with other children buffer variable naming scheme.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Automatically unref the 'conn' object and remove the 'cleanup' section
and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use automatic pointer for 'conn' and remove the 'cleanup' label and
'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Automatically free 'conn' and remove the 'cleanup' section and 'ret'
variable. 'datatypes.h' contains the declaration of the autoptr cleanup
function for virConnect.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Prepare the buffer for encryption only after initializing the cipher, so
that there's just one failure point. This allows to remove the 'error'
label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The <source/> child element of <mac/> is formatted the old way.
Switch to virXMLFormatElement().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For VIR_DOMAIN_NET_TYPE_{SERVER,CLIENT,MCAST,UDP} we need to put
(optionally) 'address' attribute and 'port' attributes of
<source/> element. But the way we currently do that is
particularly verbose. It can be shortened using
virBufferEscapeString().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The <source/> child element of <interface/> is formatted the old
way. Switch to virXMLFormatElement().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The <guest/> child element of <interface/> is formatted the old
way. Switch to virXMLFormatElement(). Since this element is used
in LXC driver, this part of the function is tested by
lxcxml2xmltest (specifically lxc-ethernet* test cases).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The <tune/> child element of <interface/> is formatted the old
way. Switch to virXMLFormatElement().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The @attrBuf variable in virDomainNetDefFormat() is named too
broadly. It holds attribute buffer to the <target/> element.
Rename it to @targetAttrBuf then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Theoretically, when remoteDomainMigrateFinish3* is called without a
pointer for storing migration cookie or its length (i.e., either
cookieout == NULL or cookieoutlen == NULL), we would leak the freshly
created virDomain object referenced by rv.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In a recent commit of v9.0.0-104-g0211e430a8 I've turned all args
vars in src/remote/remote_driver.c to be initialized wit {0}.
What I've missed was the generated code.
Do what we've done in v9.0.0-13-g1c656836e3 and init also args,
not just ret.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The iTCO watchdog is part of the q35 machine type since its inception,
we just did not add it implicitly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2137346
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order for the iTCO watchdog to be operational we must disable the
noreboot pin strap in qemu. This is the default starting from 8.0
machine types, but desirable for older ones as well. And we can safely
do that since that is not guest-visible.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is already possible with qemu, and actually already happening with
q35 machines and a specified watchdog since q35 already includes a
watchdog we do not include in the XML. In order to express such
posibility multiple watchdogs need to be supported.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function cannot fail once it starts populating
ret->params.params_val[i].field.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case the API returned success and a NULL pointer in uri_out, we would
leak the preallocated buffer used for storing the uri_out pointer.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The daemon side of this API has been broken ever since the API was
introduced in 2012. Instead of sending the error from
virDomainGetSecurityLabelList via RPC so that the client can see it, the
dispatcher would just send a successful reply with return value set to
-1 (and an empty array of labels). The client side would propagate this
return value so the client can see the API failed, but the original
error would be lost.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Recently, in v9.0.0-7-gb2034bb04c we've dropped initialization of
@args variable. The reasoning was that eventually, all members of
the variable will be set. Well, this is not correct. For
instance, in remoteConnectGetAllDomainStats() the
args.doms.doms_val pointer is set iff @ndoms != 0. However,
regardless of that, the pointer is then passed to VIR_FREE().
Worse, the whole args is passed to
xdr_remote_connect_get_all_domain_stats_args() which then calls
xdr_array, which tests the (uninitialized) pointer against NULL.
This effectively reverts b2034bb04c.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The g_hash_table_unref() function does not accept NULL. Passing
NULL results in a glib warning being triggered. Check whether the
hash table is not NULL and unref it only then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Support virtio-crypto device, also support cryptodev types:
- builtin
- lkcf
Finally, we can launch a VM(QEMU) with one or more crypto devices by
libvirt.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Changes in this commit:
- docs: formatdomaincaps.rst
- conf: crypto related domain caps
- qemu: crypto related
- tests: crypto related test
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce crypto device like:
<crypto model='virtio' type='qemu'>
<backend model='builtin' queues='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
</crypto>
<crypto model='virtio' type='qemu'>
<backend model='lkcf'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
</crypto>
Currently, crypto model supports virtio only, type supports qemu only
(vhost-user in the plan). For the qemu type, backend supports modle
builtin/lkcf, and the queues is optional.
Changes in this commit:
- docs: formatdomain.rst
- schemas: domaincommon.rng
- conf: crypto related domain conf
- qemu: crypto related
- tests: crypto related test
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are tests in qemuxml2argvtest that will fail if we enable RISC-V
testing, with an error like the following:
"cpuGetSubDriver:64 : this function is not supported by the connection
driver: 'riscv64' architecture is not supp orted by CPU driver"
This happens because we don't have a RISC-V driver yet.
Add a barebone RISC-V driver to allow tests to be executed. The only 2
callbacks implemented here are 'compare' and 'validateFeatures', both
acting as a no-op. More callbacks and features will be added in the
future.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Commit f007940cb2 tried to change the error message so that it is unified
later in 35afa1d2d6, but various rewrites missed this particular error message
which does not make sense. Fix it so that it is the same as the other two
messages checking the same thing in this file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The field is no longer used so we can remove it and the code filling it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
All callers pass 'false' so we no longer need it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Commit b7798a07f9 (in fall of 2016) changed the way we generate aliases
for 'dimm' memory devices as the alias itself is part of the migration
stream section naming and thus must be treated as ABI.
The code added compatibility layer for VMs with memory hotplug started
with the old scheme to prevent from generating wrong aliases. The
compatibility layer broke though later when 'nvdimm' and 'pmem' devices
were introduced as it wrongly detected them as old configuration.
Now rather than attempting to fix the legacy compat layer to treat other
devices properly we'll be better off simply removing it as it's
extremely unlikely that somebody has a VM started in 2016 running with
today's libvirt and attempts to hotplug more memory.
This fixes a corner case when a user hot-adds a 'dimm' into a VM with a
'dimm' and a 'nvdimm' after restart of libvirtd and then attempts to
migrate the VM.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2158701
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The function can't return NULL to the callers so it doesn't make sense
to check it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'sess->authPath' is modified before locking the 'sess' object.
Additionally on failure of 'virAuthGetConfigFilePathURI' 'sess' would be
unlocked even when it was not yet locked.
Fixes: 6917467c2b
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't return NULL to the callers so it doesn't make sense
to check it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'sess->authPath' is modified before locking the 'sess' object.
Additionally on failure of 'virAuthGetConfigFilePathURI' 'sess' would be
unlocked even when it was not yet locked.
Fixes: 273745b431
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail so it's pointless to check its return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As shown in the commit that introduced vboxReportError(), we are
appending the retval of a failed VirtualBox API onto our error
messages. Well, this is no longer needed because
vboxReportError() already appends the VirtualBox error in plain
text.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Our coding style suggests error messages to be on a single line
for easier git grep. Since I'm touching them anyways, let's make
them follow our own suggestion.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Now that we have vboxReportError() which reports VirtualBox
errors too, we can switch the code to use the former. And since
the vboxReportError() is designed to behave exactly like
virReportError() we can do that almost everywhere, regardless of
the source of the error.
There are a few exceptions though, for instance, when
initializing VirtualBox SDK (we don't have all the objects needed
for querying exceptions yet), or when invalid combination of
arguments was passed to an API of ours, or when a function from
other module (e.g. src/conf/) failed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When a VirtualBox API fails it produced an exception. Until now,
we did not have correct APIs wired up to get the exception and
its error message. Thus, we were left with plain:
virReportError("virtualbox API failed, rc=%08x", rc);
This is not very user friendly because those rc values are hard
to parse (e.g. some values are defined as a sum of a base value
and some other value) and also it expects users to know where to
look.
But now that we have all machinery needed for querying
exceptions, vboxReportError() can be introduced. The aim is to
query VirtualBox exceptions and append them after the error
message we intent to report. If the exception can't be queried
successfully, this behaves exactly like virReportError().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The ClearException() method clears the latest exception inside of
VirtualBox. This needed because obtaining an exception via
GetException() does not clear it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The GetException() method can be used to obtain the latest
exception that occurred in VirtualBox. Calling the method does
not reset the exception though. For that we'll need to call
another method (introduced in following commit).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The IVirtualBoxErrorInfo interface allows us to query error
messages from VirtualBox. Since VirtualBox has stacked errors we
need the GetNext() method too.
The odd one, that sticks out is GetIID() as it is not part of the
interface as defined by VirtualBox header files. BUT, we need to
get the interface UUID (which MAY change across each release) so
that it can be passed to VBOX_QUERY_INTERFACE() introduced
earlier.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
So far we haven't needed to use a different interface for objects
we are working with. We were happy with calling their respective
vtbl callbacks. Well, this will change soon as we will query an
exception (type of nsIException) but will need to promote it to
IVirtualBoxErrorInfo class. This promoting is done by
QueryInterface() callback which accepts 3 arguments: the original
object, ID of the new interface and address where to store the
promoted object.
As this is very basic operation, available to every object, it is
part of the ISupports interface among with other goodies like
AddRef() and Release().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
As advertised in previous commit that added the SDK header file,
there were some changes to the API:
1) IVirtualBox::OpenMachine() and IVirtualBox::CreateMachine()
now have @password argument to deal with password protected
settings files. Well, we don't have that wired now (and we
don't create such files). If we ever want to support user
settings files that are password protected (e.g. via
virSecret) we can wire this argument. For now, just pass NULL.
2) IMachine::GetAudioAdapter() is gone. But it can be replaced
with IMachine::GetAudioSettings() + IMachine::GetAdapter()
combo.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/419
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Notable changes in the API:
- Both IVirtualBox::OpenMachine() and
IVirtualBox::CreateMachine() have new @password argument for
password protected settings files.
- The IMachine::GetAudioAdapter() function is gone and to be
replaced with IMachine::GetAudioSettings() +
IMachine::GetAdapter() combo.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
To avoid including a header file more than once, either:
#pragma once
can be used, or the older trick (that vbox still uses):
#ifndef MACRO
# define MACRO
Well, vbox still uses the latter and in its 7.0 release the macro
was renamed from ___VirtualBox_CXPCOM_h to ___VirtualBox_CAPI_h.
Now, ideally, we wouldn't touch those header files for older
versions, but we need to use the same macro across all header
files (because vbox_tmpl.c includes corresponding vbox_CAPI_XXX.h
and then includes vbox_XPCOMCGlue.h which in turn includes
vbox_CAPI_v6_1.h to get the basic typedefs).
Instead of changing the newer 7.0 header file (and having to
change all subsequent versions), let's change the old ones and as
we drop support for them, we can forget this ever happened.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @networkName argument of UIDHCPServer::Start() callback is
unused. Drop it and also its propagation from parent functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
According to VirtualBox download page [1], the 6.0.0 release is
no longer supported (the support ended 2020/07). Drop it from
Libvirt too.
1: https://www.virtualbox.org/wiki/Download_Old_Builds
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
According to VirtualBox download page [1], the 5.2.0 release is
no longer supported (the support ended 2020/07). Drop it from
Libvirt too.
1: https://www.virtualbox.org/wiki/Download_Old_Builds
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The UIUSBCommon::GetEnabled() function is not needed really, as
it sets a boolean to true and always succeeds. We can live
without the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The UIUSBCommon::Enable() function is no longer needed as it is a
NOP. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @data and @name arguments of
UIHost::CreateHostOnlyNetworkInterface() callback are unused.
Drop them and also their propagation from parent functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @iid argument of UISession::OpenExisting() callback is
unused. Drop it and also its propagation from parent functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @iid argument of UISession::Open() callback is unused. Drop
it and also its propagation from parent functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @iid argument of UIMachine::LaunchVMProcess() callback is
unused. Drop it and also its propagation from parent functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The @mediaChangeOnly argument of vboxDomainAttachDeviceImpl()
function is unused. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There are few cases where a function argument is marked as
unused, but it's used later in the function. The majority of such
occurrences are in vbox_tmpl.c as a residue of older vbox
versions, but a pair was found in vbox_common.c too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In d9ee51e, virNetDevIPCheckIPv6Forwarding was updated to walk the
contents of /proc/net/ipv6_route so that it could check to see if the
RTF_ADDRCONF was set on any IPv6 routes to ultimately determine if
enabling forwarding would result in an error due to accept_ra=1 being
set on the interface.
The implementation added in that commit limited the number of routes
that could be read from /proc/net/ipv6_route to 100_000, each with 150
characters. This is problematic for machines that have a full IPv6
routing table, as the IPv6 routing table has now grown to over 160_000
(it was closer to 100_000 at the time of that commit).
This patch increases the maximum route size from 100_000 to 1_000_000.
While a million routes is somewhat arbitrary, it's meant to be a value
that can be supported for the forseeable future. APNIC, one of the five
regional internet registries, recently published a forecast of IPv6
table growth which anticipates a worst-case growth to 1_000_000 in
January of 2029.
Signed-off-by: Brooks Swinnerton <bswinnerton@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In error case, unref event->vm instead of vm. This makes it
easier for the reader to understand as it is the event struct
that's holding the reference.
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virAuthGetPasswordPath can return the same password over and over if
it's configured in the config. We rather want to try that only the first
time and then ask the user instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Rework the code to use the new helper instead of open coding the auth
callback interaction.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The helper uses the user-provided auth callbacks to ask the user. The
helper encapsulates the steps we do to query the user in few places into
a common helper which can be then used further.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
We only ever allow one username so there's no point passing it to each
authentication registration function. Additionally the only caller
(virNetClientNewLibSSH2) always passes a username so all the checks were
pointless.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
None of the callers actually set it. Remove the field and corresponding
logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
With g_strdup not failing we can remove all of the 'error' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The only caller doesn't pass the password. Remove the argument.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Fix and clean up the error paths in virAuthConfigNew*.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The field was never populated so we can remove it and all the associated
logic.
Both for password authentication and fetching the password for the
public key we still can use the authentication callbacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The only caller doesn't actually populate it. Remove it to simplify
internals.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
When starting a VirtualBox domain, we try to guess which frontend
to use. While the whole algorithm looks a bit outdated, it may
happen that we tell VirtualBox to use "gui" frontend, but not
which DISPLAY= to use.
I haven't found any documentation on the algorithm we use, but if
I make us fallback onto DISPLAY=:0 when no other configuration is
found then I'm able to start my guests just fine.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The _virtualboxCreateMachine() function allocates
@createFlagsUtf16 but never frees it.
==12481== 236 bytes in 2 blocks are definitely lost in loss record 2,060 of 2,216
==12481== at 0x48407E5: malloc (vg_replace_malloc.c:393)
==12481== by 0xB6C6D1B: RTStrToUtf16Tag (utf-8.cpp:1033)
==12481== by 0xB4DB500: _virtualboxCreateMachine (vbox_tmpl.c:634)
==12481== by 0xB4E68A3: vboxDomainDefineXMLFlags (vbox_common.c:1976)
==12481== by 0x4C7DF83: virDomainDefineXMLFlags (libvirt-domain.c:6666)
==12481== by 0x13C2DA: remoteDispatchDomainDefineXMLFlags (remote_daemon_dispatch_stubs.h:5271)
==12481== by 0x13C265: remoteDispatchDomainDefineXMLFlagsHelper (remote_daemon_dispatch_stubs.h:5252)
==12481== by 0x4AD9DF7: virNetServerProgramDispatchCall (virnetserverprogram.c:428)
==12481== by 0x4AD9931: virNetServerProgramDispatch (virnetserverprogram.c:302)
==12481== by 0x4AE28AC: virNetServerProcessMsg (virnetserver.c:135)
==12481== by 0x4AE2972: virNetServerHandleJob (virnetserver.c:155)
==12481== by 0x49BC275: virThreadPoolWorker (virthreadpool.c:164)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We have virDomainGetCPUStats() API which offers querying
statistics on host CPU usage by given guest. And it works in two
modes: getting overall stats (@start_cpu == -1, @ncpus == 1) or
getting per host CPU usage.
For the QEMU driver it is implemented by looking into values
stored in corresponding cpuacct CGroup controller. Well, this
works for system instances, where libvirt has permissions to
create CGroups and place QEMU process into them. But it does not
fly for session connection, where no CGroups are set up.
Fortunately, we can do something similar to v8.8.0-rc1~95 and use
virProcessGetStatInfo() to fill the overall stats. Unfortunately,
I haven't found any source of per host CPU usage, so we just
continue throwing an error in that case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Firstly, the virProcessGetStatInfo() does not fail really. But
even if it did, it sets correct errno only sometimes (and even
that is done in a helper it's calling - virProcessGetStat() and
even there it's the case only in very few error paths).
Therefore, using virReportSystemError() to report errors is very
misleading. Use plain virReportError() instead. Luckily, there
are only two places where the former was used:
chDomainHelperGetVcpus() and qemuDomainHelperGetVcpus() (not a
big surprise since CH driver is heavily inspired by QEMU driver).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Two of the messages referred to 'backend type' when dealing
with the source type and one mentioned the 'client' attribute
from an earlier iteration of the patches, even though the attribute
was later changed to 'connect'.
https://bugzilla.redhat.com/show_bug.cgi?id=2063723
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In a recent commit of v9.0.0-rc1~192 I've tried to forbid case
where a TAP device already exists, but at the same time it's
managed by Libvirt (<interface type='ethernet'> <target
dev='tap0' managed='yes'/> </interface>). NB, if @managed
attribute is missing then it's assumed to be managed by Libvirt.
Anyway, I've mistakenly put setting of
VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING flag into managed='yes'
branch instead of managed='no' branch in
qemuInterfaceEthernetConnect().
Move the setting of the flag into the correct branch.
Fixes: a2ae3d299c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING flag is documented as:
/* The device is allowed to exist before creation */
VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING = 1 << 4,
and yet, the documentation to virNetDevTapCreate() documents its
behavior when the flag is passed as:
* VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING
* - The device creation fails if @ifname already exists
Fortunately, the function is implemented so that it follows the
expected behavior (i.e. the former flag documentation). Fix the
function documentation then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use g_autoptr() for virNWFilterDef and virNWFilterRuleDef and remove
unnecessary label.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The systemd service files of the qemu and libxl driver currently have a
'Requires' dependency on virtlockd, which is too strong since virtlockd
is not enabled by default in either driver. Change the dependency to a
'Wants' to avoid a package dependency between the driver subpackages and
the new libvirt-daemon-lock subpackage.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 379c0ce4bf introduced a call to umount(/dev) performed
inside the namespace that we run QEMU in.
As a result of this, on machines using AppArmor, VM startup now
fails with
internal error: Process exited prior to exec: libvirt:
QEMU Driver error: failed to umount devfs on /dev: Permission denied
The corresponding denial is
AVC apparmor="DENIED" operation="umount" profile="libvirtd"
name="/dev/" pid=70036 comm="rpc-libvirtd"
Extend the AppArmor configuration for virtqemud and libvirtd so
that this operation is allowed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The CURLOPT_PUT constant causes a deprecation warning when compiling on
Alpine Edge. The docs indicate it is deprecated since 7.2.1
https://curl.se/libcurl/c/CURLOPT_PUT.html
Since 7.87 the deprecation is now exposed at build time via a compiler
warning.
We already use CURLOPT_UPLOAD in the ESX driver, so this brings the CH
driver into line.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This fixes a bug in
commit fda53ab3a5
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Thu Dec 22 10:29:32 2022 -0500
remote: use VIR_LOCK_GUARD in client code
Reviewed-by: Erik Skultety <eskultet@redhat>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD enables the 'done' goto label to be
eliminated.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD enables the 'done' goto label to be
eliminated.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD helps to simplify the control flow
logic.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Every enum/struct/union implicitly includes a typedef in the
emitted C code. Furthermore, the syntax used to declare the
redundant typedef is not compliant with the XDR spec.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The RFC spec for XDR does not allow enums to omit their
values, they must be explicitly given. Don't rely on this
rpcgen language extension.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Every member of the args variable will be initialized
explicitly. A few methods had a redundant call to memset
the args which can be removed.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This check is done when VM is defined but doesn't take into account what
cgroups version is currently used on the host system so it doesn't work
correctly.
To make proper check at this point we would have to figure out cgroups
version while defining a VM but that will still not guarantee that the
VM will start correctly in the future as the host may be rebooted with
different cgroups version.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The cgroup v2 cpu.weight limits are different than cgroup v1 cpu.shares
limits.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This commented-out option was pointed out by jtomko during review, but
I missed taking it out when addressing his comments.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This attribute was added to support setting the --interface option for
passt, but in a post-push/pre-9.0-release review, danpb pointed out
that it would be better to use the existing <source dev='xxx'/>
attribute to set --interface rather than creating a new attribute (in
the wrong place). So we remove backend/upstream, and change the passt
commandline creation to grab the name for --interface from source/dev.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Currently, the ThreadContext object is generated whenever we see
.host-nodes attribute for a memory-backend-* object. The idea was
that when the backend is pinned to a specific set of host NUMA
nodes, then the allocation could be happening on CPUs from those
nodes too. But this may not be always possible.
Users might configure their guests in such way that vCPUs and
corresponding guest NUMA nodes are on different host NUMA nodes
than emulator thread. In this case, ThreadContext won't work,
because ThreadContext objects live in context of the emulator
thread (vCPU threads are moved around by us later, when emulator
thread finished its setup and spawned vCPU threads - see
qemuProcessSetupVcpus()). Therefore, memory allocation is done by
emulator thread which is pinned to a subset of host NUMA nodes,
but tries to create a ThreadContext object with a disjoint subset
of host NUMA nodes, which fails.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2154750
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For SGX type of memory, QEMU needs to open and talk to
/dev/sgx_vepc and /dev/sgx_provision files. But we do not set nor
restore SELinux labels on these files when starting a guest.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Because of kernel doesn't allow passing negative values to
cpu.max as quota, it's needing to convert negative values to "max" token.
Signed-off-by: Anton Fadeev <anton.fadeev@red-soft.ru>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jumping to the error label and reading the pidfile does not make sense
until we reached qemuSecurityCommandRun which creates the pidfile.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The pidfile is guaranteed to be non-NULL (thanks to glib allocation
functions) and it's dereferenced two lines above anyway.
Reported by coverity:
/src/qemu/qemu_passt.c: 278 in qemuPasstStart()
272 return 0;
273
274 error:
275 ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
276 if (pid != -1)
277 virProcessKillPainfully(pid, true);
>>> CID 404360: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "pidfile" suggests that it may be null, but it
>>> has already been dereferenced on all paths leading to the check.
278 if (pidfile)
279 unlink(pidfile);
280
281 return -1;
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In our current code the function is not called with NULL argument, but
we should follow our common practice and make it safe anyway.
Reported by coverity:
/src/conf/domain_conf.c: 2635 in virDomainNetPortForwardFree()
2629 {
2630 size_t i;
2631
2632 if (pf)
2633 g_free(pf->dev);
2634
>>> CID 404359: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "pf".
2635 for (i = 0; i < pf->nRanges; i++)
2636 g_free(pf->ranges[i]);
2637
2638 g_free(pf->ranges);
2639 g_free(pf);
2640 }
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
To ensure same behaviour when remote driver is or is not used we must
not steal the FDs and array holding them passed to qemuDomainFDAssociate
but rather duplicate them. At the same time the remote driver must close
and free them to prevent leak.
Pointed out by Coverity as FD leak on error path:
*** CID 404348: Resource leaks (RESOURCE_LEAK)
/src/remote/remote_daemon_dispatch.c: 7484 in remoteDispatchDomainFdAssociate()
7478 rv = 0;
7479
7480 cleanup:
7481 if (rv < 0)
7482 virNetMessageSaveError(rerr);
7483 virObjectUnref(dom);
>>> CID 404348: Resource leaks (RESOURCE_LEAK)
>>> Variable "fds" going out of scope leaks the storage it points to.
7484 return rv;
Fixes: abd9025c2f
Fixes: f762f87534
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The remote_*_args methods will generally borrow pointers
passed in the caller, so should not be freed.
On failure of the virTypedParamsSerialize method, however,
xdr_free was being called. This is presumably because it
was thought that the params may have been partially
serialized and need cleaning up. This is incorrect, as
virTypedParamsSerialize takes care to cleanup partially
serialized data. This xdr_free call would lead to free'ing
the borrowed cookie pointers, which would be a double free.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A few admin client methods had the xdr_free call the wrong
side of the cleanup label, so typed parameters would not
be freed on error.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This consists of (1) adding the necessary args to the qemu commandline
netdev option, and (2) starting a passt process prior to starting
qemu, and making sure that it is terminated when it's no longer
needed. Under normal circumstances, passt will terminate itself as
soon as qemu closes its socket, but in case of some error where qemu
is never started, or fails to startup completely, we need to terminate
passt manually.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
passt support requires "-netdev stream", which was added to QEMU in
qemu-7.2.0.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This implements XML config to represent a subset of the features
supported by 'passt' (https://passt.top), which is an alternative
backend for emulated network devices that requires no elevated
privileges (similar to slirp, but "better").
Along with setting the backend to use passt (via <backend
type='passt'/> when the interface type='user'), we also support
passt's --log-file and --interface options (via the <backend>
subelement logFile and upstream attributes) and its --tcp-ports and
--udp-ports options (which selectively forward incoming connections to
the host on to the guest) via the new <portForward> subelement of
<interface>. Here is an example of the config for a network interface
that uses passt to connect:
<interface type='user'>
<mac address='52:54:00:a8:33:fc'/>
<ip address='192.168.221.122' family='ipv4'/>
<model type='virtio'/>
<backend type='passt' logFile='/tmp/xyzzy.log' upstream='eth0'/>
<portForward address='10.0.0.1' proto='tcp' dev='eth0'>
<range start='2022' to='22'/>
<range start='5000' end='5099' to='1000'/>
<range start='5010' end='5029' exclude='yes'/>
</portForward>
<portForward proto='udp'>
<range start='10101'/>
</portForward>
</interface>
In this case:
* the guest will be offered address 192.168.221.122 for its interface
via DHCP
* the passt process will write all log messages to /tmp/xyzzy.log
* routes to the outside for the guest will be derived from the
addresses and routes associated with the host interface "eth0".
* incoming tcp port 2022 to the host will be forwarded to port 22
on the guest.
* incoming tcp ports 5000-5099 (with the exception of ports 5010-5029)
to the host will be forwarded to port 1000-1099 on the guest.
* incoming udp packets on port 10101 will be forwarded (unchanged) to
the guest.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Initial support for network devices using passt (https://passt.top)
for the backend connection will require:
* new attributes of the <backend> subelement:
* "type" that can have the value "passt" (to differentiate from
slirp, because both slirp and passt will use <interface
type='user'>)
* "logFile" (a path to a file that passt should use for its logging)
* "upstream" (a netdev name, e.g. "eth0").
* a new subelement <portForward> (described in more detail later)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This will allow us to call parser/formatter functions with a pointer
to just the backend part.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This fits better with the element containing the value (<driver>), and
allows us to use virDomainNetBackend* for things in the <backend>
element.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Assert support for VIR_DOMAIN_DEF_FEATURE_DISK_FD in the qemu driver
now that all code paths are adapted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Probing stats and block copy to a FD passed image is not yet supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We assume that FD passed images already exist so all existance checks
are skipped.
For the case that a FD-passed image is passed without a terminated
backing chain (thus forcing us to detect) we attempt to read the header
from the FD.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Unfortunately unlike with DAC we can't simply ignore labelling for the
FD and it also influences the on-disk state.
Thus we need to relabel the FD and we also store the existing label in
cases when the user will request best-effort label replacement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
DAC security label is irrelevant once you have the FD. Disable all
labelling for such images.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Prepare the internal data for passing FDs instead of having qemu open
the file internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When starting up a VM with FD-passed images we need to look up the
corresponding named FD set and associate it with the virStorageSource
based on the name.
The association is brought into virStorageSource as security labelling
code will need to access the FD to perform selinux labelling.
Similarly when startup is complete in certain cases we no longer need to
keep the copy of FDs and thus can close them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The helper will be used in various places that need to check that a disk
source struct is using FD passing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The new helper qemuDomainStartupCleanup is used to perform cleanup after
a startup of a VM (successful or not). The initial implementation just
calls qemuDomainSecretDestroy, which can be un-exported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The 'fdgroup' will allow users to specify a passed FD (via the
'virDomainFDAssociate()' API) to be used instead of opening a path.
This is useful in cases when e.g. the file is not accessible from inside
a container.
Since this uses the same disk type as when we open files via names this
patch also introduces a hypervisor feature which the hypervisor asserts
that code paths are ready for this possibility.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Introduce a new argument type for testQemuInfoSetArgs named ARG_FD_GROUP
which allows users to instantiate tests with populated FD passing hash
table.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Implement passing and storage of FDs for the qemu driver. The FD tuples
are g_object instances stored in a per-domain hash table and are
automatically removed once the connection is closed.
In the future we can consider supporting also to not tie the lifetime of
the passed FDs bound to the connection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
For FD-passing of disk sources we'll need to keep the FDs around.
Introduce a data type helper based on a g_object so that we get
reference counting.
One instance will (due to security labelling) will need to be part of
the virStorageSource struct thus it's declared in the storage_source_conf
module.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The API can be used to associate one or more (e.g. a RO and RW fd for a
disk backend image) FDs to a VM. They can be then used per definition.
The primary use case for now is for complex deployment where
libvirtd/virtqemud may be run inside a container and getting the image
into the container is complicated.
In the future it will also allow passing e.g. vhost FDs and other
resources to a VM without the need to have a filesystem representation
for it.
Passing raw FDs has few intricacies and thus libvirt will by default not
restore security labels.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The patch moving the code didn't faithfully represent the typecasting
of the 'bandwidth' variable needed to properly convert from the legacy
'unsigned long' argument which resulted in a build failure on 32 bit
systems:
../src/qemu/qemu_block.c: In function ‘qemuBlockCommit’:
../src/qemu/qemu_block.c:3249:23: error: comparison is always false due to limited range of data type [-Werror=type-limits]
3249 | if (bandwidth > LLONG_MAX >> 20) {
| ^
Fix it by returning the check into qemuDomainBlockCommit as it's needed
only because of the legacy argument type in the old API and use
'unsigned long long' for qemuBlockCommit.
Fixes: f5a77198bf
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Some compilers aren't happy when an automatically freed variable is used
just to free something (thus it's only assigned in the code):
When compiling qemuSnapshotDelete after recent commits they complain:
../src/qemu/qemu_snapshot.c:3153:61: error: variable 'delData' set but not used [-Werror,-Wunused-but-set-variable]
g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL;
^
To work around the issue we can restructure the code which also has the
following semantic implications:
- since qemuSnapshotDeleteExternalPrepare does validation we error out
sooner than attempting to start the VM
- we read the temporary variable at least in one code path
Fixes: 4a4d89a925
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use a temporary variable to avoid memory alignment issues on ARM:
../src/nwfilter/nwfilter_dhcpsnoop.c: In function ‘virNWFilterSnoopLeaseFileLoad’:
../src/nwfilter/nwfilter_dhcpsnoop.c:1745:20: error: cast increases required alignment of target type [-Werror=cast-align]
1745 | (unsigned long long *) &ipl.timeout,
|
Fixes: 0d278aa089
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Now that deletion of external snapshot is implemented document the
current virDomainSnapshotDelete supported state.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If the daemon crashes or is restarted while the snapshot delete is in
progress we have to handle it gracefully to not leave any block jobs
active.
For now we will simply abort the snapshot delete operation so user can
start it again. We need to refuse deleting external snapshots if there
is already another active job as we would have to figure out which jobs
we can abort.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When daemon is restarted and libvirt tries to recover domain jobs we
need to know if the snapshot job was a snapshot delete in order to
safely abort running QEMU block jobs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting external snapshots the operation may fail at any point
which could lead to situation that some disks finished the block commit
operation but for some disks it failed and the libvirt job ends.
In order to make sure that the qcow2 images are in consistent state
introduce new element "<snapshotDeleteInProgress/>" that will mark the
disk in snapshot metadata as invalid until the snapshot delete is
completed successfully.
This will prevent deleting snapshot with the invalid disk and in future
reverting to snapshot with the invalid disk.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With external snapshots we need to modify the metadata bit more then
what is required for internal snapshots. Mainly the storage source
location changes with every external snapshot.
This means that if we delete non-leaf snapshot we need to update all
children snapshots and modify the disk sources for all affected disks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting snapshot we are starting block-commit job over all disks
that are part of the snapshot.
This operation may fail as it writes data changes to the backing qcow2
image so we need to wait for all the disks to finish the operation and
wait for correct signal from QEMU. If deleting active snapshot we will
get `ready` signal and for inactive snapshots we need to disable
autofinalize in order to get `pending` signal.
At this point if commit for any disk fails for some reason and we abort
the VM is still in consistent state and user can fix the reason why the
deletion failed.
After that we do `pivot` or `finalize` if it's active snapshot or not to
finish the block job. It still may fail but there is nothing else we can
do about it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In order to save some CPU cycles we will collect all the necessary data
to delete external snapshot before we even start. They will be later
used by code that deletes the snapshots and updates metadata when
needed.
With external snapshots we need data that libvirt gets from running QEMU
process so if the VM is not running we need to start paused QEMU process
for the snapshot deletion and kill at afterwards.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting external snapshots will require to run it as async domain job,
the same way as we do for snapshot creation.
For internal snapshots modify the job mask in order to forbid any other
job to be started.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting internal snapshot when the currently active disk image is
different than where the internal snapshot was taken doesn't work
correctly.
This applies to a running VM only as we are using QMP command and
talking to the QEMU process that is using different disk.
This works correctly when the VM is shut of as in this case we spawn
qemu-img process to delete the snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Prepare the validation function for external snapshot delete support.
There is one exception when deleting `children-only` snapshots. If the
snapshot tree is like this example:
snap1 (external)
|
+- snap2 (internal)
|
+- snap3 (internal)
|
+- snap4 (internal)
and user calls `snapshot-delete snap1 --children-only` the current
snapshot is external but all the children snapshots are internal only
and we are able to delete it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract the code deleting external snapshot metadata to separate
function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Previously the reparent happened before the actual snapshot deletion.
This change moves the code closer to the rest of the code handling
snapshot metadata when deletion happens. This makes the metadate
deletion happen after the data files are deleted.
Following patch will extract it into separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This simplifies the code a bit by reusing existing parts that deletes
a single snapshot.
The drawback of this change is that we will now call the re-parent bits
to keep the metadata in sync for every child even though it will get
deleted as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract code that deletes children of specific snapshot to separate
function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract code that deletes single snapshot to separate function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Move code around to make it clear what is called when deleting single
snapshot or children snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Looks up disk storage source within storage source chain using storage
source object instead of path to make it work with all disk types.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
QEMU emits this signal when the job finished its work and is about to be
finalized. If the job is started with autofinalize disabled the job
waits for user input to finalize the job.
This will be used by snapshot delete code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The created job will be needed by external snapshot delete code so
rework qemuBlockCommit to return that pointer.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
External snapshots will use this to synchronize qemu block jobs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Deleting external snapshots will require configuring autofinalize to
synchronize the block jobs for disks withing single snapshot in order to
be able safely abort of one of the jobs fails.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Upcoming snapshot deletion code will require that multiple commit jobs
are finished in sync. To allow aborting then if one fails we will need
to use manual finalization of the jobs.
This commit implements the monitor code for `job-finalize`.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow to use it while having async domain job active which we
will use when deleting external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow to use it while having async domain job active which we
will use when deleting external snapshots. At the same time we will need
to have the block job started as synchronous.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Move the code for finishing a job in the ready state to qemu_block.c.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Up until commit 629282d884, using mode=restrictive caused
virNumaSetupMemoryPolicy() to be called from qemuProcessHook(),
and that in turn resulted in virNumaNodesetIsAvailable() being
called and the nodeset being validated.
After that change, the only validation for the nodeset is the one
happening in qemuBuildMemoryBackendProps(), which is skipped when
using mode=restrictive.
Make sure virNumaNodesetIsAvailable() is called whenever a
nodeset has been provided by the user, regardless of the mode.
https://bugzilla.redhat.com/show_bug.cgi?id=2156289
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When post-copy migration fails, the domain stays running on the
destination with a VIR_DOMAIN_RUNNING_POSTCOPY_FAILED reason. Both the
state and the reason can later be rewritten in case the domain gets
paused for other reasons (such as an I/O error). Thus we need a separate
place to remember the post-copy migration failed to be able to resume
the migration.
https://bugzilla.redhat.com/show_bug.cgi?id=2111948
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parameter was only used to select which states correspond to an
active or failed post-copy migration. But these states are either
applicable to both operations or the check would just paper over a code
bug in case of an impossible combination of state and operation. By
dropping the check we can make the code simpler and also reuse existing
virDomainObjIsFailedPostcopy function and only check for active
post-copy states.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Convert to a switch instead of a bunch of 'if (type == ...).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The function currently didn't have a return value. Returning the
'virLockGuard' struct allows the callers to use automatic unlocking of
the mutex.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Require check of return value of the ACL checking functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that all code was refactored to use the new version we can remove
the old code.
For now the new close callbacks code has no error messages so
syntax-check forced me to remove the POTFILES entry for
virclosecallbacks.c
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The qemu driver uses connection close callbacks in more places requiring
more changes than other drivers, but luckily the changes are very
straightforward. The migration code was written in a way ensuring that
there's just one callback present so this can be preserved directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The rewrite is straightforward as bhyve registers only the
'bhyveProcessAutoDestroy' callback which by design doesn't need any
special handling (there's just one caller which can start the VM thus
implicitly there's only one possible registration for that function).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The rewrite is straightforward as LXC registers only the
'lxcProcessAutoDestroy' callback which by design doesn't need any
special handling (there's just one caller which can start the VM thus
implicitly there's only one possible registration for that function).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The new APIs store the list of callbacks for a VM inside the
virDomainObj and also allow registering multiple callbacks for a single
domain and also for multiple connections.
For now this code is dormant until each driver using the old APIs is not
refactored to use the new APIs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The new connect close callbacks for domains will be represented by a
virObject associated with the domain object itself.
To simplify handling the pointer to the close callback data will be done
by an immutable pointer allocated directly when allocating the
corresponding virDomainObj struct.
This patch adds the 'closecallbacks' field to virDomainObj and a
corresponding callback to allocate it into virDomainXMLOption.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The function can't fail so there's no point in returning anything.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Introduce a helper which will return a list of all domain objects inside
of the list without filtering and thus without the need to lock
individual members.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use the new style which doesn't require re-aligning the argument list
once you change the return type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove extraneous spaces and put comment on a single line.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
'virObjectNew' can't return NULL. If we pre-check the arguments we don't
need a cleanup label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Coverity scan reports:
"A time_t value is stored in an integer with too few bits to accommodate
it. The expression timeout is cast to unsigned int"
We are already casting and storing time_t timeout variable into unsigned int.
We can use time_t for timeout and cast it to unsigned long (should be big enough)
instead of unsigned int in sscanf, g_strdup_printf as required.
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
1.clear passwd in debug log
2.alignment
3.use the same variable name for function definition and declaration
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
These while loops exit directly due to break after entering.
Use if instead of these while loops.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Fix a misspelling in the documation of 'daemonCreateClientStream'.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In a recent commit I've introduced an umount() call. But the
function where the call lives is compiled on all OSes, not just
Linux. But umount() is Linux specific. Other OSes have unmount
(FreeBSD), or maybe something else. But since namespaces are
Linux specific, we can wrap the call in #ifdef __linux__ and not
care about other OSes.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When calling virConnectGetDomainCapabilities() (exposed as virsh
domcapabilities) users have option to specify whatever sub-set of
{ emulatorbin, arch, machine, virttype } they want. Then we have
a logic (hidden in virQEMUCapsCacheLookupDefault()) that picks
qemuCaps that satisfy values passed by user. And whatever was not
specified is then set to the default value as specified by picked
qemuCaps. For instance: if no machine type was provided but
emulatorbin was, then the machine type is set to the default one
as defined by the emulatorbin.
Or, when just virttype was set then the remaining three values
are set to their respective defaults. Except, we have a crasher
in this case:
# virsh domcapabilities --virttype hvf
error: Disconnected from qemu:///system due to end of file
error: failed to get emulator capabilities
error: End of file while reading data: Input/output error
This is because for 'hvf' virttype (at least my) QEMU does not
have any machine type. Therefore, @machine is set to NULL and the
rest of the code does not expect that.
What we can do about this is to validate all arguments. Well,
except for the emulatorbin which is obtained from passed
qemuCaps. This also fixes the issue when domcapabilities for a
virttype of a different driver are requested, or a different
arch.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When deciding whether to bind mount a path in domain's namespace,
we look at the QEMU mount table (/proc/$pid/mounts) and try to
match prefix of given path with one of mount points. Well, we
do that in a bit clumsy way. For instance, if there's
"/dev/hugepages" already mounted inside the namespace and we are
deciding whether to bind mount "/dev/hugepages1G/..." we decide
to skip over the path and NOT bind mount it. This is because
plain STRPREFIX() is used and yes, the former is prefix of the
latter. What we need to check also is whether the next character
after the prefix is slash.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our code relies on mount events propagating into the namespace we
create for a domain. However, there's one caveat. In v8.8.0-rc1~8
I've tried to make us detect differences in mount tables between
the namespace in which libvirtd runs and the domain namespace.
This is crucial for any mounts that happen after the domain was
started (for instance new hugetlbfs can be mounted on say
/dev/hugepages1G).
Therefore, we take a look into /proc/$(pgrep qemu)/mounts to see
what filesystems are mounted under /dev. Now, since we don't
umount the original /dev, just mount a tmpfs over it, we get all
the events (e.g. aforementioned hugetlbfs mount on
/dev/hugepages1G), but we are not really able to access it
because of the tmpfs that's placed on top. This then confuses our
algorithm for detecting which filesystems are mounted (the
algorithm is implemented in qemuDomainGetPreservedMounts()).
To break the link between host's and guest's /dev we just need to
umount() the original /dev in the namespace. Just before our
artificially created tmpfs is moved into its place.
Fixes: 46b03819ae
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151869#c6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Inside of qemuCaps (for the corresponding accelerator) we have
full host CPU expansion stored, among with supported Hyper-V
Enlightenments. To report them in the domain capabilities, we
just have to pick those starting with "hv-" and see if we know
them.
You may notice that neither of our domaincapsdata test shows any
enlightenment. This is because the test works by parsing
corresponding qemucapabilitiesdata/caps_*.xml file and none of
these store the full host CPU expansion (hostCPU.fullQEMU)
because that is runtime piece of information and not formatted
into virQEMUCaps XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1717611
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we have qemuMonitorGetCPUModelExpansion() aware of
Hyper-V Enlightenments, we can start querying it. Two conditions
need to be met:
1) KVM is in use,
2) Arch is either x86 or arm.
It may look like modifying the first call to
qemuMonitorGetCPUModelExpansion() inside of
virQEMUCapsProbeQMPHostCPU() would be sufficient but it is not.
We really need to ask QEMU for full expansion and the first call
does not guarantee that.
For the test data, I've just copied whatever
'query-cpu-model-expansion' returned earlier, therefore there are
no hv-* props. But that's okay - the full expansion is not stored
in cache (and thus not formatted in
tests/qemucapabilitiesdata/caps_*.replies files either). This is
purely runtime thing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This continues and finishes propagation of the @hv_passthrough
argument started in the previous commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Apart from setting @migratable prop to the
query-cpu-model-expansion command, we will need @hv-passthrough
so that we can query for expansion of Hyper-V Enlightenments
supported on the current host. The idea is to run:
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "full",
"model": {
"name": "host",
"props": {
"hv-passthrough": true
}
}
}
}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virDomainCapsEnumFormat() function does not return anything
but zero and none of its callers is interested in the failure
anyways. Switch its return type from integer to void.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We are formatting <enum/> element and its children using
virBufferAddLit(), virBufferAsprintf(), virBufferAdjustIndent(),
etc. Well, we can avoid that when switching to
virXMLFormatElement().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In a recent commit, when ditching virXPathULong() the parsing of
<selfvers/> was changed. But it was changed to virXMLPropUInt()
which is not correct because the value we're interested in is not
in an attribute but element itself.
Fixes: a3c7426839
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Since we really only need to handle key skipping in the top level object
the caller doesn't at this point even pass it to the array formatting
helper function. Remove the unused argument.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Skipping of a specific key is needed only for the top level object to
specially handle the object type. We must not pass it to any recursed
printing of nested objects as skipping keys there might be surprising
and also is unhandlable later when formatting the commandline.
Until now this did not pose a problem but was discovered when adding a
new netdev backend which has a nested config object which also has the
'type' key which was being skipped.
Modern usage will prefer JSON directly but fix the commandline generator
to prevent surprises.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The @hash variable inside of virQEMUCapsProbeQMPHostCPU() is used
only within a block, but declared at the beginning of the
function. Bring the variable declaration into the said block.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There's nothing qemu specific about
qemuDomainCapsFeatureFormatSimple() and in fact, the function
lives in hypervisor agnostic location and thus mustn't have qemu
prefix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After previous cleanup this function is no longer used and thus
can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When starting swtpm binary, the qemuSecurityStartTPMEmulator() is
called which sets seclabel on the TPM state and then uses
qemuSecurityCommandRun() to execute the swtpm binary with proper
seclabel. Well, the aim is to ditch
qemuSecurityStartTPMEmulator() because it entangles two distinct
operations. Just call functions for them separately.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If swtpm binary fails to start after successful exec() (e.g. it
fails to initialize itself), the seclabels set in
qemuSecurityStartTPMEmulator() are not restored. This is due to
lacking qemuSecurityRestoreTPMLabels() call in the error path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we have qemuSecurityRestoreTPMLabels() we might as well
have qemuSecuritySetTPMLabels(). The aim here is to remove
qemuSecurityStartTPMEmulator() which couples two separate things
into a single function call.
Therefore, introduce qemuSecuritySetTPMLabels() which does only
set seclabels on the TPM state.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The qemuSecurityCleanupTPMEmulator() function calls
virSecurityManagerRestoreTPMLabels() and thus the proper name is
qemuSecurityRestoreTPMLabels(). Rename it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, qemuSecurityCleanupTPMEmulator() returns nothing which
means a caller (well, there's only one - qemuExtTPMStop()) can't
produce a warning when restoring seclabels on TPM state failed.
True, qemuSecurityCleanupTPMEmulator() does report a warning
itself, but only in one specific error path.
Make the function return an integer, just like the rest of
qemuSecurity*Restore() functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemu is about to deprecate the '-no-hpet' option in favor of configuring
the timer via '-machine'.
Use the QEMU_CAPS_MACHINE_HPET capability to switch to the new syntax
and mask out the old QEMU_CAPS_NO_HPET capability at the same time to
prevent using the old syntax.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
The capability represents that qemu accepts the configuration of the
HPET timer via -machine hpet=on/off rather than the
soon-to-be-deprecated '-no-hpet' option.
The capability is detected from 'query-command-line-options' which
recently added the 'hpet' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
That way it actually fits with what the condition checks for.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
