While the virsh option definitions specify (either explicitly after
recent refactors, or implicitly before) whether an argument is
positional or not, the actual parser is way more lax and actually and
allows also arguments which were considered/documented as non-positional
to be filled positionally unless VSH_OFLAG_REQ_OPT is used in the flags.
This creates situations such as 'snapshot-create-as' which has the
following docs:
SYNOPSIS
snapshot-create-as <domain> [--name <string>] [--description <string>]
[--print-xml] [--no-metadata] [--halt] [--disk-only]
[--reuse-external] [--quiesce] [--atomic] [--live] [--validate]
[--memspec <string>] [[--diskspec] <string>]...
Thus showing as if '--name' and '--description' required the option, but
in fact the following happens when only positionals are passed:
$ virsh snapshot-create-as --print-xml 1 2 3 4 5
<domainsnapshot>
<name>2</name>
<description>3</description>
<disks>
<disk name='4'/>
<disk name='5'/>
</disks>
</domainsnapshot>
In the above example e.g. '--memspec' is not populated.
This disconnect makes it impossible to refactor the parser itself and
allows users to write buggy interactions with virsh.
In order to address this we'll be annotating every single of these
unwanted positional options as such so that this doesn't happen in the
future, while still preserving the quirk in the parser.
This patch introduces a tool which outputs list of options which are not
marked as positional but are lacking the VSH_OFLAG_REQ_OPT flag.
This tool will be removed once all the offenders found by it will be
addressed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The macro is used in just one place and the definition of the option is
going to be modified. Inline the macro.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The macro is used in one place only and the command definition will be
altered. Inline it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming patches will need to tweak some of the properties of the
command. Since the macro is used in just two places expand it inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
- move the check that completer_flags are 0 if no completer is set
into a common place and remove duplication
- add check that _BOOL arguments are not positional
- add missing checks to _ALIAS
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All virsh commands in non-quiet mode append another separator line thus
having two is unnecessary and in quiet mode it still has a trailing
blank line. Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
XML metadata for snapshot contains only single list of disk overlays
from the moment when the snapshot was taken. When user creates multiple
branches of snapshots the parent snapshot will still list only the
original disk overlays. This may cause an issue in a specific scenario:
s1
|
+- s2
+- s3 (active)
For this snapshot topology when we delete s2 metadata for s1 are not
updated. Now when we delete s1 the code operated with incorrect
overlays from s1 metadata in order to update s3 metadata resulting in no
changes to s3 metadata.
Now when user tries to delete s3 it fails with following error:
error: Failed to delete snapshot s3
error: operation failed: snapshot VM disk source and parent disk source are not the same
For the actual deletion there is a code to figure out the correct disk
source but it was not used to update metadata as well. Due to reasons
how block commit in libvirt works we need to create a copy of that disk
source in order to have it available when updating metadata as the
original source will be freed at that point.
Resolves: https://issues.redhat.com/browse/RHEL-26276
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Calling this function when deleting internal snapshot isn't required
because with internal snapshots all changes are done within the file
itself so there is no file deletion and no need to update snapshot
metadata.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The PCI VPD (Vital Product Data) may be missing or the kernel can report
presence but not actually have the data. Also the data is specified by
the device vendor and thus may be invalid in some cases.
To avoid log spamming, since the only usage in the node device driver is
ignoring errors, remove all error reporting.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/607
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The VPD parsing is fragile and depends on hardware vendor's adherance to
standards. Since libvirt only ever uses this data to report it in the
nodedev XML which ignores any errors there's no much point in having
error reporting which I've added recently.
Turn the errors into VIR_DEBUG statements in preparation for upcoming
patch which completely removes the expectation to report errors.
This effectively reverts commits dfc85658bd and f85a382a0e.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As this command was introduced in this release add the flag requiring to
pass optionname.
This is needed to actually disallow positional parsing of the value
despite documenting that the flag name is required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.
Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/
Co-authored-by: Weblate <noreply@weblate.org>
Signed-off-by: Fedora Weblate Translation <i18n@lists.fedoraproject.org>
If the original code detected a missing or null boot index in the
new XML, it automatically added the current value. This
autocompletion was incorrect because it was impossible to
distinguish between user intent and user error - changing the
boot order itself is forbidden and should always be an error.
Resolves: https://issues.redhat.com/browse/RHEL-23416
Fixes: aa3e07caec
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Change the log level for pauses of guests due to watchdog timeouts
or io errors from debug to warn to enhance the visibility of such
events.
Signed-off-by: Lennart Fricke <lennart.fricke@drehpunkt.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add domaincapstest qemuxml2argvtest qemuxml2xmltest
related test cases for loongarch.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Implement method for loongarch to get host info, such as
cpu frequency, system info, etc.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Implement support for loongarch64 in the QEMU driver.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Add loongarch cpu support, Define new cpu type 'loongarch64'
and implement it's driver functions.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
While the C API entry points will validate non-negative lengths
for various parameters, the RPC server de-serialization code
will need to allocate memory for arrays before entering the C
API. These allocations will thus happen before the non-negative
length check is performed.
Passing a negative length to the g_new0 function will usually
result in a crash due to the negative length being treated as
a huge positive number.
This was found and diagnosed by ALT Linux Team with AFLplusplus.
CVE-2024-2494
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Found-by: Alexandr Shashkin <dutyrok@altlinux.org>
Co-developed-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Current entries should always be listed before obsolete ones.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
The single caller for each function passes the same value
for @src and @parent, which means that we don't really need
the additional API.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Each one only has a single, trivial caller.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
It was clearly copied over from the SELinux driver without
updating its name in the process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
ch_driver expects path to be of a dir for save/restore. So, update
the documentation at global API as well.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Save & Restore are supported without any network and hostdev config
defined. So, add a validation for it before performing save.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are chances that libvirt process is killed and it resulting in
stale managed save dirs. So check for it, and cleanup it there's any.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Following callbacks have been implemented
* domainRestore
* domainRestoreFlags
The path parameter to these callbacks has to be of the directory where
libvirt has performed save. Additionally, call restore in `domainCreate`
if the domain has managedsave.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Create libvirt managed saveDir and pass it to CH to save the VM
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implemented save callbacks. CH's vmm.snapshot API is called to save the
domain state. The path passed to these callbacks has to be of directory
as CH takes dir as input to snapshot and saves multiple files under it.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pass virCHDriverConfig to VirCHMonitorNew instead of just stateDir so
that the cfg can be used for any additional purposes.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implement folowing API calls from CH monitor
* vmm.snapshot -> to save a domain
* vmm.restore -> to restore saved domain
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virProcessSetScheduler() uses not just sched_setscheduler() but
also sched_get_priority_{min,max}(). Currently we assume that
the former being available implies that the latter are as well,
but that's not the case for at least GNU/Hurd.
Make sure all functions are actually available before
attempting to use them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit c07cf0a686 replaced this check with one for the
presence of cpu_set_t.
The idea at the time was that only sched_{get,set}affinity()
were visible by default, while making cpu_set_t visible required
defining _WITH_CPU_SET_T. So libvirt would detect the function
and attempt to use it, but the code would not compile because
the necessary data type had not been made accessible.
The commit in question brought three FreeBSD commits as evidence
of this. While [1] and [2] do indeed seem to support this
explanation, [3] from just a few days later made it so that not
just cpu_set_t, but also the functions, required user action to
be visible. This arguably would have made the change unnecessary.
However, [4] from roughly a month later changed things once
again: it completely removed _WITH_CPU_SET_T, making both the
functions and the data type visible by default.
This is the status quo that seems to have persisted until
today. If one were to check any recent FreeBSD build job
performed as part of our CI pipeline, for example [5] and [6]
for FreeBSD 13 and 14 respectively, they would be able to
confirm that in both cases cpu_set_t is detected as available.
Since there is no longer a difference between the availability
of the functions and that of the data type, go back to what we
had before.
This has the interesting side-effect of fixing a bug
introduced by the commit in question.
When detection was changed from the function to the data type,
most uses of WITH_SCHED_GETAFFINITY were replaced with uses of
WITH_DECL_CPU_SET_T, but not all of them: specifically, those
that decided whether qemuProcessInitCpuAffinity() would be
actually implemented or replaced with a no-op stub were not
updated, which means that we've been running the stub version
everywhere except on FreeBSD ever since.
The code has been copied to the Cloud Hypervisor driver in
the meantime, which is similarly affected. Now that we're
building the actual implementation, we need to add virnuma.h
to the includes.
As a nice bonus this also makes things work correctly on
GNU/Hurd, where cpu_set_t is available but
sched_{get,set}affinity() are non-working stubs.
[1] https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2
[2] https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb
[3] https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a
[4] https://cgit.freebsd.org/src/commit/?id=5e04571cf3cf4024be926976a6abf19626df30be
[5] https://gitlab.com/libvirt/libvirt/-/jobs/6266401204
[6] https://gitlab.com/libvirt/libvirt/-/jobs/6266401205
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
FreeBSD 14 implements sched_{get,set}affinity() for
compatibility with Linux, but we should still use the native
syscalls instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
