'qemuSnapshotDeleteBlockJobIsRunning' returns only 0 and 1. Convert it
to bool and remove the dead code handling -1 return in the caller.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/682
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The migration parameter causes zero detection to be enabled and zero
blocks are *not* transferred to the destination. This means that users
must provide pre-cleared images that read all zero, otherwise the
non-zero blocks on destination which reside in places where the source
has zero blocks would be kept intact corrupting the image.
As not transferring and overwriting the zero blocks is what the feature
is supposed to do the users need to provide the proper environment.
Document the requirement, both in API and in the virsh man page for the
'--migrate-disks-detect-zeroes' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The idea of migration with VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES
populated is to sparsify the image. The QEMU NBD client as it was
configured in commit 621f879adf would
signal to the destination to do thick allocation of holes which would
result in a non-sparse image for any backend except a qcow2 image which
I used to test it.
Switch to VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP and
VIR_DOMAIN_DISK_DISCARD_UNMAP which tells the NBD client (and that in
turn the NBD server) to preserve the sparse blocks it detected from the
image.
Fixes: 621f879adf
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
major() is a macro defined in sys/sysmacros.h so luckily the code works,
but it's very confusing. Let's rename the local variable to make the
difference between it and the macro more obvious. And while touching the
line we can also initialize it to make sure "clever" analyzers do not
think it may be used uninitialized.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
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>
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>
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>
(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).
Replaces: 200f60b2e1
Resolves: https://issues.redhat.com/browse/RHEL-61576
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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.
Replaces: 1a72b83d56
Resolves: https://issues.redhat.com/browse/RHEL-61576
Re-Resolves: https://gitlab.com/libvirt/libvirt/-/issues/215
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
This reverts commit 1a72b83d56. 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>
This reverts commit 200f60b2e1. The same
functionality will be re-added in a different way in an upcoming patch.
Signed-off-by: Laine Stump
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
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>
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>
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>
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>
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>
The documentation about remote access to libvirt was pointing to a blog
post about usage of the 'ssh-agent' to avoid being asked for passwords.
The blog/host is now unfortunately defunct thus the link is dead.
Replace the link by the official man page of the 'ssh-agent' program.
This convenietnly removes the last non-'https' link on our web.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
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>
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>
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>
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>
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>
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>
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Callers must handle the return value of this function as the VM might
have died. Add compiler annotation to force it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
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>
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>
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>
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>
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>
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>
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>
By not attempting to lock the lock file, which would fail.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
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