virDomainControllerDefParseXML() does a lot of checks with
virDomainPCIControllerOpts parameters that can be moved to
virDomainControllerDefValidate, sharing the logic with other use
cases that does not rely on XML parsing.
'pseries-default-phb-numa-node' parse error was changed to reflect
the error that is being thrown by qemuValidateDomainDeviceDefController()
via deviceValidateCallback, that is executed before
virDomainControllerDefValidate().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will add more validations to this function. Let's move
it to domain_validate.c beforehand.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move this check to a new virDomainDefTunablesValidate(), which
is called by virDomainDefValidateInternal().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This check is not tied to XML parsing and can be moved to
virDomainSmartcardDefValidate().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will move a validation to virDomainSmartcardDefValidate(),
but this function can't be moved alone to domain_validate.c without
making virDomainChrSourceDefValidate(), from domain_conf.c, public.
Given that the idea is to eventually move all validations to domain_validate.c
anyways, let's move all ChrSource related validations in a single punch.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function isn't doing XML validation of any sort. Rename it to
be compatible with its actual use.
While we're at it, change the VIR_ERR_XML_ERROR error being thrown
in the function to VIR_ERR_CONFIG_UNSUPPORTED.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The 'tray' check isn't a XML parse specific code and can be pushed
to the validate callback, in virDomainDiskDefValidate().
'vendor' and 'product' string sizes are already checked by the
domaincommon.rng schema, but can be of use in the validate callback
since not all scenarios will go through the XML parsing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will add more validations to the function. Let's move
it beforehand to domain_validate.c.
virSecurityDeviceLabelDefValidateXML() is still used inside
domain_conf.c, so make it public for now until its current
caller (virDomainChrSourceDefValidate()) is also moved to
domain_validate.c.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
These checks are not related to XML parsing and can be moved to the
validate callback. Errors were changed from VIR_ERR_XML_ERROR to
VIR_ERR_CONFIG_UNSUPPORTED.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We'll add more video validations into the function in the next
patch. Let's move it beforehand to domain_validate.c.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This check isn't exclusive to XML parsing. Let's move it to
virDomainDefVideoValidate() in domain_validate.c
We don't have a failure test for this scenario, so a new test called
'video-multiple-primaries' was added to test this failure case.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This patch creates a new function, virDomainDefBootValidate(), to host
the validation of boot menu timeout and rebootTimeout outside of parse
time. The checks in virDomainDefParseBootXML() were changed to throw
VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside
domain_conf.c, let's put this new function in a new domain_validate.c
file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit 926563dc3a which refactored the function call deleting the
snapshot's on disk state introduced a logic bug, which skips over the
deletion of libvirt metadata after the disk state deletion is done.
To fix it we must not return early.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/109
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We already calculated the guest area, which is what is subject
to minimum size requirements, a few lines earlier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation callback always fetched a fresh copy of 'qemuCaps' to use
for validation which is wrong in cases when the VM is already running,
such as device hotplug. The newly-fetched qemuCaps may contain flags
which weren't originally in use when starting the VM e.g. on a libvirtd
upgrade.
Since the post-parse/validation machinery has a per-run 'parseOpaque'
field filled with qemuCaps of the actual process we can reuse the caps
in cases when we get them.
The code still fetches a fresh copy if parseOpaque doesn't have a
per-run copy to preserve existing functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The validation callbacks always fetch latest qemuCaps so it won't ever
be NULL. Remove the tautological conditions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainDefPostParse infrastructure has apart from the global opaque
data also per-run data, but this was not duplicated into the validation
callbacks.
This is important when drivers want to use correct run-state for the
validation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This makes the dockerfile name match the output container name
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This introduces Fedora 33 and removes some redundant packages.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit f5e8715a8b added logic which adds some fake job info when qemu
didn't return anything but in such case the job type would not be set.
Since we already have the proper job type recorded in qemuBlockJobDataPtr
which the caller fetched, we can use this it and also remove the lookup
from the disk which was necessary prior to the conversion to
qemuBlockJobDataPtr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Test slices on top of nvme-backed disks.
Note that the changes in seemingly irrelevant parts of the output are
due to re-naming the nodenames.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to other disk-related stuff, the index is useful when you want
to refer to the image in APIs such as virDomainSetBlockThreshold.
For internal use we also need to parse it inside of the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nodename may be asociated to a disk backup job, add support to looking
up in that chain too. This is specifically useful for the
BLOCK_WRITE_THRESHOLD event which can be registered for any nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nodename may be asociated to a disk backup job, add support to looking
up in that chain too. This is specifically useful for the
BLOCK_WRITE_THRESHOLD event which can be registered for any nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's a technical detail in qemu that QCOW2 is needed for a pull-mode
backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virStorageFileChainLookup' reports an error when the lookup of the
backing chain entry is unsuccessful. Since we possibly use it multiple
times when looking up backing for 'disk->mirror' the function can report
error which won't be actually reported.
Replace the call to virStorageFileChainLookup by lookup in the chain by
index.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use dummy variable to fill 'src' so that access to it doesn't need to be
conditionalized and use temporary variable for 'disk' rather than
dereferencing the array multiple times.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When removing SSH keys via set-user-sshkeys virsh command, then
files to remove are read from passed file. But when
experimenting, I've passed /dev/null as the file which resulted
in API checks which caught that @keys argument of
virDomainAuthorizedSSHKeysSet() can't be NULL. This is because if
the file is empty then its content is an empty string and thus
the buffer the file was read in to is not NULL.
Long story short, error is reported correctly, but it's not
necessary to go through public API to catch it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In v6.10.0-rc1~104 I've added a virsh command that exposes
virDomainAuthorizedSSHKeysSet() API under "set-user-sshkeys"
command. The command accepts mutually exclusive "--reset" and
"--remove" options (among others). While the former controls the
VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND flag, the latter
controls the VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE flag.
These flags are also mutually exclusive. But the code that sets
them has a logical error which may result in both flags being
set. In fact, this results in user being not able to set just the
remove flag.
Fixes: 87d12effbe
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1904674
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We previous added code for passing FDs which was explicitly derived from
gnulib's passfd code:
commit 17460825f3
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Fri Jan 17 11:57:17 2020 +0000
src: implement APIs for passing FDs over UNIX sockets
This is a simplified variant of gnulib's passfd module
without the portability code that we do not require.
while the license was unchanged, we mistakenly failed to copy the FSF
copyright header which is required by the license terms.
Reported-by: Bruno Haible <bruno@clisp.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since Xen 4.2 libxl expects device_model_override="/path" instead of
device_model="/path". Adjust the code to parse this as <emulator>.
While libxl also recognizes device_model_version="", this knob is not
required for libvirt. A runtime detection exists in libvirt to select
either "qemu-xen" or "qemu-xen-traditional".
Since qemu-xen-traditional is marked as supported just for stubdoms
there is no need to handle it.
Test data files with 'device_model' were adjusted to use
'device_model_override' instead.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Attempting to create a lxc domain with <seclabel type='none'/> fails
virsh --connect lxc:/// create distro_nosec.xml
error: Failed to create domain from distro_nosec.xml
error: unsupported configuration: Security driver model '(null)' is not available
Commit 638ffa2228 adjusted the logic for setting a driver's default
security model.
The lxc driver does not set a default security driver model in the XML
parser config, causing seclabels of type='none' to have a null model.
The lxc driver's security manager is initialized in lxcStateInitialize()
by calling lxcSecurityInit(). Use the model of this manager as the
default in the XML parser config.
For the record, this is a regression caused by commit 638ffa2228, which
changed the logic for setting a driver's default security model. The
qemu driver was adjusted accordingly, but a similar change was missed
in the lxc driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a feature is added (or removed) in a QEMU CPU model version, we
get to see the QEMU pretty name for the feature, not the name of
the macro.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The meson build system is configured to only ever build shared
libraries, so we delete the -static sub-RPMs.
The few driver conditionals are deleted as there was never any
scenario in which their value changed.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The comment that
> For instance, qemu-ga doesn't support guest time synchronization on
> Windows guests, but Linux ones.
Was correct at the time, but has since been addressed by
qemu/qemu@105fad6bb2, which added support for set-time without a time
argument, as used by `virsh domtime --sync` by libvirt-guests.sh. I can
confirm that `virsh domtime --sync` works correctly on a Windows 10
guest, as does `SYNC_TIME=1`. (Note that there can be a significant
delay between when the command completes and when the guest time
finishes synchronizing due to QEMU GA calling `w32tm` with `/nowait`,
which complicates testing.)
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
In v6.0.0-rc1~439 (and friends) we tried to cache NUMA
capabilities because we assumed they are immutable. And to some
extent they are (NUMA hotplug is not a thing, is it). However,
our capabilities contain also some runtime info that can change,
e.g. hugepages pool allocation sizes or total amount of memory
per node (host side memory hotplug might change the value).
Because of the caching we might not be reporting the correct
runtime info in 'virsh capabilities'.
The NUMA caps are used in three places:
1) 'virsh capabilities'
2) domain startup, when parsing numad reply
3) parsing domain private data XML
In cases 2) and 3) we need NUMA caps to construct list of
physical CPUs that belong to NUMA nodes from numad reply. And
while this may seem static, it's not really because of possible
CPU hotplug on physical host.
There are two possible approaches:
1) build a validation mechanism that would invalidate the
cached NUMA caps, or
2) drop the caching and construct NUMA caps from scratch on
each use.
In this commit, the latter approach is implemented, because it's
easier.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819058
Fixes: 1a1d848694
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
If the job has finished, but we didn't yet process the completion fake
that it's still incomplete so that apps which decided to poll
qemuDomainGetBlockJobInfo rather than use events can be sure that the
XML update was completed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Replace qemuMonitorGetBlockJobInfo by qemuMonitorGetAllBlockJobInfo and
hash table lookup. This basically open-codes qemuMonitorGetBlockJobInfo,
but it will be removed in next patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
