With watchdog action=dump the actual watchdog action is set to pause and
the daemon then proceeds to dump the process. After that the domain is
resumed. That was the case since the feature was added. However the
resuming of the domain might be unexpected, especially when compared to
HW watchdog, which will never run the guest from the point where it got
interrupted.
Document the pre-existing behaviour, since any change might be
unexpected as well. Change of behaviour would require new options like
dump+reset, dump+pause, etc. That option is still possible, but
orthogonal to this change.
Resolves: https://issues.redhat.com/browse/RHEL-753 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Laine Stump [Fri, 30 Aug 2024 16:37:05 +0000 (12:37 -0400)]
network: inhibit idle timeout of daemon if there are any active networks
When the daemons were split out from the monolithic libvirtd, the
network driver didn't implement "inhibit idle timeout if there are any
active objects" as was done for other drivers, so virtnetworkd would
always exit after 120 seconds of no incoming connections. This didn't
every cause any visible problem, although it did mean that anytime a
network API was called after an idle time > 120 seconds, that the
restarting virtnetworkd would flush and reload all the
iptables/nftables rules for any active networks.
This patch replicates what is done in the QEMU driver - an nactive is
added to the network driver object, along with an inhibitCallback; the
latter is passed into networkStateInitialize when the driver is
loaded, and the former is incremented for each already-active network,
then incremented/decremented each time a network is started or
stopped. If nactive transitions from 0 to 1 or 1 to 0, inhibitCallback
is called, and it "does the right stuff" to prevent/enable the idle
timeout.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Fri, 6 Sep 2024 22:08:05 +0000 (16:08 -0600)]
libxl: Reject VM config referencing nwfilters
The Xen libxl driver does not support nwfilter. Introduce a
deviceValidateCallback function with a check for nwfilters, returning
VIR_ERR_CONFIG_UNSUPPORTED if any are found. Also fail to start any
existing VMs referencing nwfilters.
Drivers generally ignore unrecognized XML configuration, but ignoring
a user's request to filter VM network traffic can be viewed as a
security issue.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Fri, 4 Oct 2024 22:43:02 +0000 (18:43 -0400)]
network: a different implementation of *un*setting firewalld zone when network is destroyed
(this is a remake of commit v10.7.0-78-g200f60b2e1, which was reverted
due to a regression in another patch it was dependent on. The new
implementation just adds the call to virFirewallDInterfaceUnsetZone()
into the existing networkRemoveFirewallRules() (but only if we had set
a zone when the network was first started).
Laine Stump [Fri, 4 Oct 2024 22:14:36 +0000 (18:14 -0400)]
network: a different way of supporting firewalld zone for mode='open' networks
Now that networkAddFirewallRules and networkRemoveFirewallRules() are
being called for mode='open' networks, we just need to move the code
that sets the zone outside of the if (mode != ...OPEN) clause, so that
it's done for all forward modes, with the exception of setting the
implied 'libvirt*' zones, which are set when no zone is specified for
all forward modes *except* 'open'.
This was previously done in commit v10.7.0-76-g1a72b83d56, but in a
manner that caused the zone to be unset whenever firewalld reloaded
its rules. That patch was reverted, and this new better patch takes
its place.
Laine Stump [Fri, 4 Oct 2024 21:17:59 +0000 (17:17 -0400)]
network: call network(Add|Remove)FirewallRules() for forward mode='open'
Previously networkAddFirewallRules() and networkRemoveFirewallRules()
were only called if the forward mode was none, 'route', or 'nat', so
those functions didn't check the forward mode. Although their current
contents shouldn't be executed for forward mode='open', soon they will
have extra functionality that should be executed for all the current
forward modes and also mode='open'.
This patch modifies all places either of the functions are called to
make sure they are called for mode='open' in addition to current modes
(by either adding 'case ..._OPEN:' to the case of a switch statement,
or just removing an 'if (mode != ...OPEN)' around the calls; to
balance out for that, it puts the entirety of the contents of both
functions inside if (mode != ...OPEN) to retain current behavior. (an
upcoming patch will add code outside that if clause).
debug log messages were also added to make it easier to test that the
right thing is being done in all cases.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Laine Stump [Fri, 4 Oct 2024 17:46:20 +0000 (13:46 -0400)]
Revert "network: support setting firewalld zone for bridge device of open networks"
This reverts commit 1a72b83d566df952033529001b0f88a66d7f4393. That
patch had made the incorrect assumption that the firewalld zone of a
bridge would not be changed/removed when firewalld reloaded its rules
(e.g. with "killall -HUP firewalld"). It turns out my memory was
faulty, and this *does* remove the bridge interface's zone, which
results in guest networking failure after a firewalld reload, until
the virtual network is restarted.
The functionality reverted as a result of this patch reversion will be
added back in an upcoming patch that keeps the zone setting in
networkAddFirewallRules() (rather than moving it into a separate
function) so that it is called every time the network's firewall rules
are reloaded (including the reload that happens in response to a
reload notification from firewalld).
Signed-off-by: Laine Stump Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Jim Fehlig [Thu, 15 Aug 2024 23:21:50 +0000 (17:21 -0600)]
qemu: Use consistent naming for save image format
The image format setting in qemu.conf is named 'save_image_format'. The
enum of supported format types is declared with name 'virQEMUSaveFormat'.
Let's be consistent and use 'format' instead of 'compressed' when referring
to the save image format.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jim Fehlig [Thu, 15 Aug 2024 22:52:57 +0000 (16:52 -0600)]
qemu: conf: Improve the foo_image_format setting descriptions
The current description of the various foo_image_format settings can
be construded to imply the setting is only used to control compression
of the image. Improve the documentation to clarify that format describes
the representation of guest memory blocks on disk, which includes
compression among other possible layouts.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Peter Krempa [Tue, 8 Oct 2024 13:06:17 +0000 (15:06 +0200)]
docs: Prohibit 'external' links within the webpage
Enforce that relative links are used within the page, so that local
installations don't require internet conection and/or don't redirect to
the web needlessly.
This is done by looking for any local link (barring exceptions) when
checking links with 'check-html-references.py'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 8 Oct 2024 13:07:03 +0000 (15:07 +0200)]
docs: newreposetup: Drop section about 'libvirt project server'
Now that most things were migrated out of the old server which hosted
the 'libvirt.org' web (now handles only 'https://download.libvirt.org')
which no longer even hosts the cgit web interface (any link redirects to
gitlab) the whole section now is obsolete. Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 8 Oct 2024 07:49:48 +0000 (09:49 +0200)]
docs: Use relative links within the web page
Replace full/external links which point to content within
'https://libvirt.org/' with relative links so that the web page works
fully locally.
This does not change the links in 'docs/manpages' as we want the
installed man page to work from everywhere (even when the local docs are
not installed) and the generated API docs which take links from the C
source.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 8 Oct 2024 11:38:34 +0000 (13:38 +0200)]
docs: Reject non-https external links
Add a '--require-https' switch to 'check-html-references' helper script
which will error out if any non-https external link is used from our web
and use it while builidng docs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2024 13:16:14 +0000 (15:16 +0200)]
qemu: snapshot: Allow internal snapshots with PFLASH nvram
With the new snapshot QMP command we can select which block device
backend receives the VM state and thus the main issue with internal
snapshots with pflash was addressed.
Thus we can relax the check and allow snapshots if the pflash nvram is
on qcow2.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Thu, 3 Oct 2024 11:24:31 +0000 (13:24 +0200)]
qemuSnapshotActiveInternalDeleteGetDevices: Add warning when deleting inconsistent snapshot
As explained in the commit which added the new internal snapshot
deletion code we don't want to do any form of strict checking whether
the libvirt metadata is consistent with the on-disk state as we didn't
historically do that.
In order to be able to spot the cases add a warning into the logs if
such state is encountered. While warnings are easy to miss it's the only
reasonable way to do that. Users will be encouraged to file an issue
with the information, without requiring them to enable debug logs as
the reproduction of that issue may include very old historical state.
The checker is deliberately added separately so that it can be easily
reverted once it's no longer needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Thu, 3 Oct 2024 11:23:20 +0000 (13:23 +0200)]
qemu snapshot: use QMP snapshot-delete for internal snapshots deletion
Switch to using the modern QMP command.
As the user visible logic when deleting internal snapshots using the old
'delvm' command was very lax in terms of catching inconsistencies
between the snapshot metadata and on-disk state we re-implement this
behaviour even using the new command. We could improve the validation
but that'd go at the cost of possible failures which users might not
expect.
As 'delvm' was simply ignoring any kind of failure the selection of
devices to delete the snapshot from is based on querying qemu first
which top level images do have the internal snapshot and then continuing
only on those.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemu snapshot: use QMP snapshot-save for internal snapshots creation
The usage of HMP commands are highly discouraged by qemu. Moreover,
current snapshot creation routine does not provide flexibility in
choosing target device for VM state snapshot.
This patch makes use of QMP commands snapshot-save and by
default chooses first writable non-shared qcow2 disk (if present)
as target for VM state.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 2 Oct 2024 12:24:31 +0000 (14:24 +0200)]
qemu: monitor: Store internal snapshot names from 'query-named-block-nodes'
Store the names of internal snapshots present in supported images in the
data we dump from 'query-named-block-nodes' so that the upcoming changes
to the internal snapshot code can access it.
To test this we use the bitmap detection test cases which can be easily
extended to dump this data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The 'snapshot-save/delete' QMP commands were introduced in QEMU 6.0.0,
so we add a compatible capability to check if target QEMU binary supports it.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemu: blockjob: Add job types for 'snapshot-save/delete'
The snapshot creation/deletion QMP commands use the qemu 'job' API
to signal completion thus we need to add corresponding job types.
As the job handles everything internally we don't store anything about
the job.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemu: monitor: Add plumbing for 'snaphot-save'/'snapshot-delete' QMP commands
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:46 +0000 (12:26 +0200)]
cpu_map: Drop vmx-invvpid-single-context from CPU models
QEMU calls the same feature differently, but translating the names in
libvirt does not make sense because the name in QEMU conflicts with
another feature. QEMU will not change the name for compatibility reasons
so we can just drop our invented name as it is not supported by QEMU.
Apart from this slightly different reason behind the feature being
unsupported by QEMU the situation is similar to vmx-ept-{uc,wb} dropped
in the previous patch and so is the implications.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:45 +0000 (12:26 +0200)]
cpu_map: Drop vmx-ept-{uc,wb} features from CPU models
Although QEMU knows and enables the corresponding MSR bits, it does not
allow users to configure them (there are no names attached to them).
They should have never been added to the CPU map and definitely not to
CPU models as the features will always be considered disabled regardless
on their actual state as QEMU will not report them.
While we cannot drop them completely for backward compatibility, we can
at least remove them from all CPU models.
This is effectively no change for CPU models where the features were
marked with added='yes' because migration source would always remove the
features from domain XML so not adding them to the live XML does not
hurt. On the other side the destination could not ever be surprised by
the features being suddenly enabled as QEMU never reports them, which
means libvirt considers them disabled all the time.
GraniteRapids CPU model is the only one which contains the feature ever
since it was introduced in libvirt, but it was never possible to migrate
a domain with such CPU. The source would always mark vmx-ept-wb as
disabled and the destination without the fixes in this series would drop
the feature from the XML completely as it is unsupported by QEMU and
disabled, but when probing for the actual CPU created by QEMU libvirt
would expect the feature to be enabled (as it is included in the CPU
model and not explicitly mentioned in the domain definition) and fail
the migration. There's nothing the source could do to workaround the
behavior on the destination and migration to older libvirt will still be
broken. But it's possible to migrate a domain with GraniteRapids to a
destination with this series applied from both old and new source.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:43 +0000 (12:26 +0200)]
qemu: Translate vmx-invvpid-single-context-noglobals CPU feature
This feature is called "vmx-invept-single-context-noglobals" in QEMU and
our CPU map even contains the appropriate alias. But we failed to
actually translate the name when talking to QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:42 +0000 (12:26 +0200)]
cpu-data.py: Properly handle aliases
The script is used to create data files for cputest from QEMU replies.
By ignoring aliases we might end up thinking a feature is not enabled by
QEMU just because its name differs from the primary one in the CPU map.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:41 +0000 (12:26 +0200)]
qemu: Do not drop unknown CPU features from domain XML
CPU features with policy='disable' which are unknown to QEMU may be
safely skipped when generating the -cpu command line, but we should
still keep them in the domain definition so that we can properly check
they are disabled after migrating the domain to a newer QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 8 Oct 2024 10:26:40 +0000 (12:26 +0200)]
qemu: Drop vmx-* from migratable CPU model only when origCPU is set
When qemuDomainMakeCPUMigratable is called with origCPU == NULL the code
just removed all vmx-* features marked as added in the specified CPU
model just like when origCPU is not NULL, but does not list any of the
vmx-* features. But this is wrong, we should not touch these features at
all when no origCPU is supplied, which happens when parsing XML passed
by a user (e.g., migration XML). Such XML is supposed to be generated by
libvirt as migration XML and contains only vmx-* features explicitly
requested by a user.
https://issues.redhat.com/browse/RHEL-52314
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
util: Look for newer name of cpu wait time statistic
It looks like linux changed the key for wait time in /proc/<pid>/sched
and /proc/<pid>/task/<tid>/sched files in commit ceeadb83aea2 (or around
that time) from se.statistics.wait_sum to just wait_sum. Similarly to
the previous change (from se.wait_sum) just look for the new name first.
Resolves: https://issues.redhat.com/browse/RHEL-60030 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Andrea Bolognani [Mon, 12 Aug 2024 15:07:54 +0000 (17:07 +0200)]
security: Allow skipping locking when labeling lock files
This is needed when migrating a guest that has persistent TPM
state: relabeling (which implies locking) needs to happen
before the swtpm process is started on the destination host,
but the lock file won't be released by the swtpm process
running on the source host before a handshake with the target
process has happened, creating a catch-22 scenario.
In order to make migration possible, make it so that locking
for lock files can be explicitly skipped. All other state
files are handled as usual.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Andrea Bolognani [Fri, 30 Aug 2024 12:25:25 +0000 (14:25 +0200)]
security: Always forget labels for TPM state directory
In the case of outgoing migration, we avoid restoring the
remembered labels for the TPM state directory because doing so
would risk cutting off storage access for the target node.
Even in that case though, we should still forget (unref) the
remembered labels: if we don't, the source node will keep
thinking that the state directory is in use.
Note that this change only affects the SELinux driver because
the DAC driver doesn't currently implement label remembering
for TPM state at all.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Fri, 2 Aug 2024 13:23:44 +0000 (15:23 +0200)]
qemu: migration: Don't remember seclabel for images shared from current host
In case when the user exports images from current host and there is an
incoming migration from a remote host, security label remembering would
be possible but would attempt to remember the label allowing access to
the image as the image is already used by a VM on remote host.
To prevent remembering the wrong label, we'll skip the remembering of
the label for any shared resource, so that the code behaves identically
regardless of how the image is accessed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Fri, 2 Aug 2024 13:23:43 +0000 (15:23 +0200)]
storage_source: Add field for skipping seclabel remembering
In case of incoming migration where a local directory is shared to other
hosts we'll need to avoid seclabel remembering as the code would
remember the seclabel already allowing access to the image.
As the decision requires a lot of information not available in the
security driver it would either require plumbing in unpleasant callbacks
able to pass in the data or alternatively we can mark this in the
'virStorageSource' struct.
This patch chose to do the latter approach by adding a field called
'seclabelSkipRemember' which will be filled before starting the process
in cases when it will be required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Fri, 2 Aug 2024 13:23:42 +0000 (15:23 +0200)]
security_(dac|selinux): Unref remembered security labels on outgoing migration
When 'qemuSecurityRestoreAllLabel' is called on outgoing migration it
skips the actual relabeling part of the images in dac/selinux drivers in
order to avoid cutting off access to the image.
As shared filesystems don't really support the trusted XATTR groups,
remembering of security labels never worked on those paths so we never
actually had remembered seclabels for images that could be migrated.
With recent changes we now support migration from local storage to
remote in case the admin declares it as shared. This means that in case
when the VM is started on local storage we'd actually store seclabels,
but when migrating out the XATTRs remembering the seclabels would not
actually be unref'd and thus the seclabels would leak.
As we can't know whether a remote host will be able to use the XATTRs or
not (but really it won't) and at the same time the destination side of
migration will actually call 'qemuSecuritySetAllLabel' setting/refing
it's own seclabels we really need to unref them on our side.
This patch adds the appropriate *RecallLabel() calls on the code paths
in which relabelling is skipped due to migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Fri, 2 Aug 2024 13:23:41 +0000 (15:23 +0200)]
virSecuritySELinuxRestoreImageLabelInt: Move FD image relabeling after 'migrated' check
Reorganize the code so that the 'migrated' flag isn't checked multiple
times and thus that it's more obvious what is happening when the
'migrated' flag is asserted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Up until this point, we have avoided setting labels for
incoming migration when the TPM state is stored on a shared
filesystem. This seems to make sense, because since the
underlying storage is shared surely the labels will be as
well.
There's one problem, though: when a guest is migrated, the
SELinux context for the destination process is different from
the one of the source process.
We haven't hit any issues with the current approach so far
because NFS doesn't support SELinux, so effectively it doesn't
matter whether relabeling happens or not: even if the SELinux
contexts of the source and target processes are different,
both will be able to access the storage.
Now that it's possible for the local admin to manually mark
exported directories as shared filesystems, however, things
can get problematic.
Consider the case in which one host (mig-one) exports its
local filesystem /srv/nfs/libvirt/swtpm via NFS, and at the
same time bind-mounts it to /var/lib/libvirt/swtpm; another
host (mig-two) mounts the same filesystem to the same
location, this time via NFS. Additionally, in order to
allow migration in both directions, on mig-one the
/var/lib/libvirt/swtpm directory is listed in the
shared_filesystems qemu.conf option.
When migrating from mig-one to mig-two, things work just fine;
going in the opposite direction, however, results in an error:
# virsh migrate cirros qemu+ssh://mig-one/system
error: internal error: QEMU unexpectedly closed the monitor (vm='cirros'):
qemu-system-x86_64: tpm-emulator: Setting the stateblob (type 1) failed with a TPM error 0x1f
qemu-system-x86_64: error while loading state for instance 0x0 of device 'tpm-emulator'
qemu-system-x86_64: load of migration failed: Input/output error
This is because the directory on mig-one is considered a
shared filesystem and thus labeling is skipped, resulting in
a SELinux denial.
The solution is quite simple: remove the check and always
relabel. We know that it's okay to do so not just because it
makes the error seen above go away, but also because no such
check currently exists for disks and other types of persistent
storage such as NVRAM files, which always get relabeled.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If the local admin has explicitly declared that a certain
filesystem is to be considered shared, we should treat it as
such.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
virFileIsSharedFS() is the function that ultimately decides
whether a filesystem should be considered shared, but the list
of manually configured shared filesystems is part of the QEMU
driver's configuration, so we need to pass the information
through several layers in order to make use of it.
Note that with this change the list is propagated all the way
through, but its contents are still ignored, so the behavior
remains the same for now.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As explained in the comment, this can help in scenarios where
a shared filesystem can't be detected as such by libvirt, by
giving the admin the opportunity to provide this information
manually.
https://issues.redhat.com/browse/RHEL-35752
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libxl_conf: Add check for unsupported graphics type
libxlMakeVfb always succeeds regardless of if the graphics type is
actually supported or not.
libxl_defbool_val is called in libxlMakeBuildInfoVfb which besides returning
the boolean value of the defbool also has an assertion that the defbool value
is not set to default. It is possible to fail this assertion if an
unsupported graphics type is used. In libxlMakeVfb, the VNC and SDL enable
defbools are still left in their default state if the graphics type falls
outside the two, which leads to this issue.
This patch adds a check to reject graphics types outside of SDL, VNC, and SPICE
very early on in libxlMakeVfb. As a safeguard, we also initialize both vnc
enable and sdl enable defbools as false early.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
libxl_conf: Fix config generation for multiple serial devices
Currently, an array of libxl_string_list (char **) or in other words,
a triple char pointer is initialized. This is dereferenced to a char ** type
and stored in serial_list, which is NULL at this point. There is an attempt to
reference an element of this serial_list when making a call to
libxlMakeChrdevStr which causes a segmentation fault.
To fix this, we simply allocate an array of char * instead of
libxl_string_list.
This patch also adds testcases to extend coverage over both single serial and
multiple serial cases.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 07:51:20 +0000 (09:51 +0200)]
qemu: Introduce and wire in 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES'
The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration
parameter allows users of migration to pass in a list of disks where
zero-detection (which avoids transferring the zeroed-blocks) should be
enabled for the migration connection. This comes at the cost of extra
CPU cycles needed to check each block if it's all-zero.
This is useful for storage backends where information about the
allocation state of a block is not available and thus without this the
image would become fully allocated on the destination.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 30 Sep 2024 08:38:58 +0000 (10:38 +0200)]
qemu: migration: Extract validation of disk target list
The migration code is checking the disk list provided via
VIR_MIGRATE_PARAM_MIGRATE_DISKS against existing disks. Extract it to a
helper function as we'll be passing another list of disk targets soon.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 11:56:06 +0000 (13:56 +0200)]
qemu: migration: Avoid use of 'nmigration_disks'
'migration_disks' is a NULL-terminated string list, so the code can be
converted to either iterate the string-list, use existing accessors or
check the presence of the pointers instead of checking the count.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 11:40:15 +0000 (13:40 +0200)]
qemu: migration: Don't log 'nmigrate_disks'
The actual number of disks to migrate is not important. The presence of
disks to migrate can be inferred from presence of the 'migrate_disks'
pointer which is logged.
Since 'nmigrate_disks' will eventually be removed remove the logging
right now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 11:33:40 +0000 (13:33 +0200)]
qemuMigrationSrcBeginPhaseBlockDirtyBitmaps: Use qemuMigrationAnyCopyDisk()
The function open-coded the checking whether a disk is being migrated
with non-shared storage and did so badly (not taking into account if
user doesn't explicitly provide list of disks to migrate).
Use the existing helper instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 11:01:22 +0000 (13:01 +0200)]
virTypedParamsGetStringList: Ensure that returned array is NULL if there are no matching fields
'virTypedParamsGetStringList' fills the returned array only with string
parameters with matching name. The filtering code though leaves the
possibility that all items are filtered out but the return array is
still (over)allocated.
Since 'virTypedParamsFilter()' now also allows filtering by type we can
move the filtering there ensuring that we always allocate the right
number of elements and more importantly the returned array will be NULL
if none elements are present.
Rework the code and adjust docs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 09:15:06 +0000 (11:15 +0200)]
virTypedParamsGetStringList: Refactor and adjust docs
Use automatic freeing, declare one variable per line and return early
when possible. As this is an internal helper there's no need to check
that the caller passed non-NULL @values.
Modify the documentation to be accurate and warn callers to not free the
strings just the array.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 08:55:00 +0000 (10:55 +0200)]
virTypedParamsFilter: Adjust return type and docs
The 'virTypedParamsFilter' function can't fail and thus it never returns
negative values. Change the return type to 'size_t' and adjust callers
to not check the return value for being negative.
Adjust the docs to hilight this and also the fact that the filtered
typed param list returned via @ret is not a deep copy and thus callers
must not use the common function to free it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 30 Sep 2024 12:16:58 +0000 (14:16 +0200)]
qemu: migration: Pre-create QCOW2 images for non-shared storage with 0 allocation
Specify that the <allocation> parameter for the newly-created qcow2
image is 0 so that only metadata gets preallocated. Otherwise the
storage driver code instructs qemu to use 'fallocate' preallocation mode
and considers the image fully allocated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 07:07:22 +0000 (09:07 +0200)]
qemu: blockjob: Clean out disk mirror data after concluding the job
The 'disk->mirrorJob' and 'disk->mirrorState' fields need to be cleared
after a blockjob, but should be kept around while 'disk->mirror' is
still in place. As 'disk->mirror' is cleared only after conclusion of
the job in 'qemuBlockJobEventProcessConcluded()' we should be resetting
them only afterwards.
Move the code later, but since the job is unregistered from the disk we
need to store the pointer to the disk before concluding the job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 27 Sep 2024 06:57:08 +0000 (08:57 +0200)]
qemu: blockjob: Update 'mirror' of a copy job before removing images
When concluding a job with a 'mirror' we first unplugged the appropriate
no-longer used images from qemu and then updated the definition.
Normally this wouldn't be a problem because for any other thread this is
done under the VM lock thus atomic. Unfortunately though, the AppArmor
security backend is using a VM XML to pass data to the helper process
and the state of the definition at that point was unsuitable to format a
valid XML thus making 'virt-aa-helper' report parsing failure.
Since we're removing the images the proper state of the VM definition
indeed should not include the mirror element any more at the point when
the images are removed.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/601 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Tue, 24 Sep 2024 13:27:23 +0000 (15:27 +0200)]
testutilsqemuschema: Support 'unstable' feature in QMP schema validator
The 'unstable' feature is present on any schema member which was not yet
finalized in qemu. Use it to refuse such fields/commands in qemu as they
are possibly subject to change.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Laine Stump [Mon, 30 Sep 2024 15:30:38 +0000 (11:30 -0400)]
qemu: fix regression in update-device for interfaces
Commit a37bd2a15b8f2e7aa09519c86fe1ba1e59ce113f eliminated a failure
to update *any* change in an interface that was connected via a
network that consisted of a pool of VFs using macvtap passthrough
mode. Unfortunately it caused a regression that results in failure to
update changes to bandwidth/vlan/trustGuestRxFilters in any interface
connected via a network that uses a bridge to connect tap devices.
This fixes that problem by narrowing the usage of the fix in the
earlier patch to only be done in the case that the the interface is
connected via a macvtap+passthrough network.
Signed-off-by: Laine Stump <laine@redhat.com> Fixes: a37bd2a15b8f2e7aa09519c86fe1ba1e59ce113f Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Andrea Bolognani [Fri, 27 Sep 2024 12:47:21 +0000 (14:47 +0200)]
qemu: Look for qemu-bridge-helper in more directories
Commit 0caacf47d7b423db9126660fb0382ed56cd077c1 recently
made it so the new path used for qemu-bridge-helper in Debian
would be allowed, but the logic used to actually figure out
the complete path for the helper was not updated accordingly.
Ján Tomko [Wed, 14 Aug 2024 15:55:17 +0000 (17:55 +0200)]
ci: adapt to 'dtrace' package split
Fedora has decided to separate dtrace out of the systemtap-sdt-devel
package: https://fedoraproject.org/wiki/Changes/Separate_dtrace_package
Similarly, these are split in OpenSUSE Tumbleweed, however in a
backward-compatbile way:
https://build.opensuse.org/package/show/openSUSE:Factory/systemtap
Require the new 'systemtap' package mapping, as well as the old
'dtrace'.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The model was defined with two CPU features that cannot be explicitly
configured in QEMU (it knows the MSR bits, but there's no name
associated with them). The features should have never existed in the CPU
map. While removing them from the list of features and existing CPU
models is not trivial (to avoid compatibility issues), we can at least
fix the SierraForest CPU model added in this release cycle.
The rest will be handled later in a separate series.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 18 Sep 2024 13:32:45 +0000 (15:32 +0200)]
qemu: Provide sane default for dump_guest_core
QEMU uses Linux extensions to madvise() to include/exclude guest
memory from core dump. These are obviously not available
everywhere. Currently, users have two options:
1) configure <memory dumpCore=''/> in domain XML, or
2) configure dump_guest_core in qemu.conf
While these work, they may harm user experience as "things just
don't work" out of the box. Provide sane default in
virQEMUDriverConfigNew() so neither of two options is required.
To have predictable results in tests, explicitly set
cfg->dumpGuestCore to false in qemuTestDriverInit() (which
creates cfg object for tests).
projects / libvirt.git / log