Add missing ABI stability check for blockio properties for disk
devices.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previous commits were all about empty strings and empty JSON
arrays. Introduce a test case for "[]" to make sure we pare it
correctly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
After 'mdevctl' was ran, its stdout is captured in @output which
is then compared against NULL and if it is NULL a negative value
is returned (to indicate error to the caller). But this is
effectively a dead code, because virCommand (specifically
virCommandProcessIO()) makes sure both stdout and stderr buffers
are properly '\0' terminated. Therefore, this can never evaluate
to true. Also, if there really is no output from 'mdevctl' (which
was handled in one of earlier commits, but let just assume it
wasn't), then we should not error out and treat such scenario as
'no mdevs defined/active'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
We have virMdevctlListDefined() to list defined mdevs, and
virMdevctlListActive() to list active mdevs. Both have the same
body except for one boolean argument passed to
nodeDeviceGetMdevctlListCommand(). Join the two functions under
virMdevctlList() name and introduce @defined argument that is
then just passed to the cmd line builder function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
As explained earlier, 'mdevctl' can output nothing. Add a test
case to nodedevmdevctltest which covers this situation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
It is possible for 'mdevctl' to output nothing, an empty string
(e.g. when no mediated devices are defined on the host). What is
weird is that when passing '--defined' then 'mdevctl' outputs an
empty JSON array instead. Nevertheless, we should accept both and
treat them the same, i.e. as no mediated devices.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/523
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The mdevctl-list-empty test case is there to test whether an
empty JSON array "[]" is handled correctly by mdevctl handling
code. Well, mdevctl can output both, an empty JSON array or no
output at all.
Therefore, rename "mdevctl-list-empty" test case to
"mdevctl-list-empty-array" which is more descriptive and also
frees up slot for actual empty output (handled in next commits).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The whole purpose of virCloseRangeInit() is to be called
somewhere during initialization (ideally before first virExec()
or virCommandRun()), so that the rest of the code already knows
kernel capabilities. While I can put the call somewhere into
remote_daemon.c (when a daemon initializes), we might call
virCommand*() even from client library (i.e. no daemon).
Therefore, put it into virGlobalInit() with the rest of
initialization code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
This is brand new way of closing FDs before exec(). We need to
close all FDs except those we want to explicitly pass to avoid
leaking FDs into the child. Historically, we've done this by
either iterating over all opened FDs and closing them one by one
(or preserving them), or by iterating over an FD interval [2 ...
N] and closing them one by one followed by calling closefrom(N +
1). This is a lot of syscalls.
That's why Linux kernel developers introduced new close_from
syscall. It closes all FDs within given range, in a single
syscall. Since we keep list of FDs we want to preserve and pass
to the child process, we can use this syscall to close all FDs
in between. We don't even need to care about opened FDs.
Of course, we have to check whether the syscall is available and
fall back to the old implementation if it isn't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
We have two version of mass FD closing: one for FreeBSD (because
it has closefrom()) and the other for everything else. But now
that we have closefrom() wrapper even for Linux, we can unify
these two.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
It is handy to close all FDs from given FD to infinity. On
FreeBSD the libc even has a function for that: closefrom(). It
was ported to glibc too, but not musl. At least glibc
implementation falls back to calling:
close_range(from, ~0U, 0);
Now that we have a wrapper for close_range() we implement
closefrom() trivially.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Linux gained new close_range() syscall (in v5.9) that allows
closing a range of FDs in a single syscall. Ideally, we would use
it to close FDs when spawning a process (e.g. via virCommand
module).
Glibc has close_range() wrapper over the syscall, which falls
back to iterative closing of all FDs inside the range if running
under older kernel. We don't wane that as in that case we might
just close opened FDs (see Linux version of
virCommandMassClose()). And musl doesn't have close_range() at
all. Therefore, call syscall directly.
Now, mass close of FDs happens in a fork()-ed off child. While it
could detect whether the kernel does support close_range(), it
has no way of passing this info back to the parent and thus each
child would need to query it again and again.
Since this can't change while we are running we can cache the
information - hence virCloseRangeInit().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
As advertised earlier, now that the _virDomainMemoryDef struct is
cleaned up, we can shorten some names as their placement within
unions define their use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The _virDomainMemoryDef struct is getting a bit messy. It has
various members and only some of them are valid for given model.
Worse, some are re-used for different models. We tried to make
this more bearable by putting a comment next to each member
describing what models the member is valid for, but that gets
messy too.
Therefore, do what we do elsewhere: introduce an union of structs
and move individual members into their respective groups.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The _virDomainMemoryDef struct is getting a bit messy. It has
various members and only some of them are valid for given model.
Worse, some are re-used for different models. We tried to make
this more bearable by putting a comment next to each member
describing what models the member is valid for, but that gets
messy too.
Therefore, do what we do elsewhere: introduce an union of structs
and move individual members into their respective groups.
This allows us to shorten some names (e.g. nvdimmPath or
sourceNodes) as their purpose is obvious due to their placement.
But to make this commit as small as possible, that'll be
addressed later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When guest acknowledges change in size of virtio-mem (portion
that's exposed to the guest), QEMU emits
MEMORY_DEVICE_SIZE_CHANGE event. We process it in
processMemoryDeviceSizeChange(). So far, QEMU emits the even only
for virtio-mem (as that's the only memory device model that
allows live changes to its size). Nevertheless, if this ever
changes, validate the memory model upon processing the event as
the rest of the code blindly expects virtio-mem model.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is similar to one of my previous commits. Simply speaking,
users can specify address where a memory device is mapped to. And
as such, we should include it when looking up corresponding
device in domain definition (e.g. on device hot unplug).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuDomainChangeMemoryLiveValidateChange() function is called
when a live memory device change is requested (via
virDomainUpdateDeviceFlags()). Currently, the only model that is
allowed to change is VIRTIO_MEM (and the only value that's
allowed to change is requestedsize). The aim of the function is
to check whether the change user requested follows this rule. And
in accordance with defensive programming I made the function
check all virDomainMemoryDef struct members. Even those which are
unused for VIRTIO_MEM model.
Drop these checks as the respective members will be inaccessible
soon (as the struct is refined).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As of v7.9.0-rc1~296 users have ability to adjust what portion of
virtio-mem is exposed to the guest. Then, as of v9.4.0-rc2~5 they
have ability to set address where the memory is mapped. But due
to a missing check it was possible to feed
virDomainUpdateDeviceFlags() API with memory device XML that
changes the address. This is of course not possible and should be
forbidden. Add the missing check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Conceptually, from host POV there's no difference between NVDIMM
and VIRTIO_PMEM. Both expose a file to the guest (which is used
as a permanent storage). Other secdriver treat NVDIMM and
VIRTIO_PMEM the same. Thus, modify virt-aa-helper so that is
appends virtio-pmem backing path into the domain profile too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, inside of virt-aa-helper code the domain definition is
parsed and then all def->mems are iterated over and for NVDIMM
models corresponding nvdimmPath is set label on. Conceptually,
this code works (except the label should be set for VIRTIO_PMEM
model too, but that is addressed in the next commit), but it can
be written in more extensible way. Firstly, there's no need to
check whether def->mems[i] is not NULL because we're inside a
for() loop that's counting through def->nmems. Secondly, we can
have a helper variable ('mem') to make the code more readable
(just like we do in other loops). Then, we can use switch() to
allow compiler warn us on new memory model.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When running libvirt from the build directory with the 'run' script, it
will run as unconfined_t. This can result in unexpected behavior when
selinux is enforcing due to the fact that the selinux policies are
written assuming that libvirt is running with the
system_u:system_r:virtd_t context. This patch adds a new --selinux
option to the run script. When this option is specified, it will launch
the specified binary using the 'runcon' utility to set its selinux
context to the one mentioned above. Since this may require root
privileges, setting the selinux context is not the default behavior and
must be enabled with the command line switch.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Since its introduction in 4d1b771fbb
it has only been used to differentiate between START and non-START.
Last use of QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH was removed by:
commit f709377301
qemu: Fix qemuDomainObjTaint with virtlogd
QEMU_DOMAIN_LOG_CONTEXT_MODE_STOP is unused since:
commit cf3ea0769c
qemu: process: Append the "shutting down" message using the new APIs
Now, the only caller passes QEMU_DOMAIN_LOG_CONTEXT_MODE_START.
Assume that's always the case and remove the 'mode' argument.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the support to revert external snapshots is implemented we can
drop this check.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With the introduction of external snapshot revert support we need to
error out in some cases when trying to delete some snapshots.
If users reverts to non-leaf snapshots and would try to delete it after
the revert is done it would not work currently as this operation would
require using block-stream which is not implemented for now as in this
case the snapshot has two children so the disk files have multiple
overlays.
Similarly if user reverts to non-leaf snapshot and would try to delete
snapshot that is non-leaf but not in currently active snapshot chain we
would still need to use block-commit operation. The issue here is that
in order to do that we would have to start new qemu process with
different domain definition than what is currently used by the domain.
If the current domain would be running it would complicate things even
more so this operation is not yet supported.
If user creates new snapshot after reverting to non-leaf snapshot it
creates a new branch. Deleting snapshot with multiple children will
require block-stream which is not implemented for now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There will be more external snapshot checks introduced by following
patch so group them together.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With introduction of external snapshot revert we will have to update
backing store of qcow images not actively used be QEMU manually.
The need for this patch comes from the fact that we stop and start QEMU
process therefore after revert not all existing snapshots will be known
to that QEMU process due to reverting to non-leaf snapshot or having
multiple branches.
We need to loop over all existing snapshots and check all disks to see
if they happen to have the image we are deleting as backing store and
update them to point to the new image except for images currently used
by the running QEMU process doing the merge operation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We only need the domain definition from domain object. This will allow
us to use it from snapshot code where we need to pass different domain
definition.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This new helper will allow us to check if we are able to delete external
snapshot after user did revert to non-leaf snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When user creates a new snapshot after reverting to non-leaf snapshot we
no longer need to store the temporary overlays as they will be part of
the VM XMLs stored in the newly created snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When deleting external snapshot and parent snapshot is the currently
active snapshot as user reverted to it we need to properly update the
parent snapshot metadata.
After the delete is done the new overlay files will be the currently
used files created when snapshot revert was done, replacing the original
overlay files.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When block commit is not needed we can just simply unlink the
disk files.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In this case there is no need to run block commit and using qemu process
at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Before external snapshot revert every delete operation did block commit
in order to delete a snapshot. But now when user reverts to non-leaf
snapshot deleting leaf snapshot will not have any overlay files so we
can just simply delete the snapshot images.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This part of code is about to grow to make deletion work when user
reverts to non-leaf snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The new name reflects that we prepare data for external snapshot
deletion and the old name will be used later for different part of code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When reverting to external snapshot we need to create new overlay qcow2
files from the disk files the VM had when the snapshot was taken.
There are some specifics and limitations when reverting to a snapshot:
1) When reverting to last snapshot we need to first create new overlay
files before we can safely delete the old overlay files in case the
creation fails so we have still recovery option when we error out.
These new files will not have the suffix as when the snapshot was
created as renaming the original files in order to use the same file
names as when the snapshot was created would add unnecessary
complexity to the code.
2) When reverting to any snapshot we will always create overlay files
for every disk the VM had when the snapshot was done. Otherwise we
would have to figure out if there is any other qcow2 image already
using any of the VM disks as backing store and that itself might be
extremely complex and in some cases impossible.
3) When reverting from any state the current overlay files will be
always removed as that VM state is not meant to be saved. It's the
same as with internal snapshots. If user want's to keep the current
state before reverting they need to create a new snapshot. For now
this will only work if the current snapshot is the last.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Both creating and deleting snapshot are using VIR_ASYNC_JOB_SNAPSHOT but
reverting is using VIR_ASYNC_JOB_START. Let's unify it to make it
consistent for all snapshot operations.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We will need to reuse the functionality when reverting external
snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
To create new overlay files when external snapshot revert support is
introduced we will be using different domain definition than what is
currently used by the domain.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract creation of qcow2 files for external snapshots to separate
function as we will need it for external snapshot revert code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When creating external snapshot this function is called only when the VM
is not running so there is only one definition to care about. However,
it will be used by external snapshot revert code for active and inactive
definition and they may be different if a disk was (un)plugged only for
the active or inactive definition.
The current code would crash so use virDomainDiskByName() to get the
correct disk from the domain definition based on the disk name and make
sure it exists.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Extract the code that updates disks in domain definition while creating
external snapshots. We will use it later in the external snapshot revert
code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This new option will be used by external snapshot revert code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This new element will hold the new disk overlay created when reverting
to non-leaf snapshot in order to remember the files libvirt created.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit <ef3f3884a2432958bdd4ea0ce45509d47a91a453> introduced new
argument for virDomainSnapshotAlignDisks() that allows passing alternate
domain definition in case the snapshot parent.dom is NULL.
In case of redefining snapshot it will not hit the part of code that
unconditionally uses parent.dom as there will not be need to generate
default external file names.
It should be still fixed to make it safe. Future external snapshot
revert code will use this to generate default file names and in this
case it would crash.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Convert all cases using DO_TEST() to use DO_TEST_CAPS_LATEST() and
remove DO_TEST() to prevent further use.
Most of the changes are related to CPU being present in the output XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
