Michal Privoznik [Thu, 28 May 2020 10:38:43 +0000 (12:38 +0200)]
conf: Move and rename virDomainParseScaledValue()
There is nothing domain specific about the function, thus it
should not have virDomain prefix. Also, the fact that it is a
static function makes it impossible to use from other files.
Move the function to virxml.c and drop the 'Domain' infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 27 May 2020 12:24:56 +0000 (14:24 +0200)]
qemuxml2xmltest: Add "numatune-distance" test case
This test case checks that expanding NUMA distance works. On
input we accept if only distance from A to B is specified. On the
output we format the B to A distance too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Wed, 8 Jul 2020 07:13:42 +0000 (09:13 +0200)]
qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
This is okay for the command line visitor, but the property is declared
as boolean in qemu and thus it will not work when using QMP.
Modify the type to boolean. This changes the command line, but
fortunately the command line visitor in qemu parses both 'yes' and 'on'
as true for the property.
Erik Skultety [Fri, 3 Jul 2020 12:26:13 +0000 (14:26 +0200)]
qemu: Use virQEMUCapsCacheLookupDefault instead of lookup by arch
Firstly, SEV is present only on AMD, so we can safely assume x86.
Secondly, the problem with looking up capabilities in the cache by arch
is that it's using virHashSearch with a callback to find the right
capabilities and get the binary name from it as well, but since the
cache is empty, it will return NULL and we won't get the corresponding
binary name out of the lookup either. Then, during the cache validation
we try to create a new cache entry for the emulator, but since we don't
have the binary name, nothing gets created.
Therefore, virQEMUCapsCacheLookupDefault is used to fix this issue,
because it doesn't rely on the capabilities cache to construct the
emulator binary name.
The semantics of the backup operation don't strictly require that all
disks being backed up are part of the same incremental part (when a disk
was checkpointed/backed up separately or in a different VM), or even
they may not have a previous checkpoint at all (e.g. when the disk
was freshly hotplugged to the vm).
In such cases we can still create a common checkpoint for all of them
and backup differences according to configuration.
This patch adds a per-disk configuration of the checkpoint to do the
incremental backup from via the 'incremental' attribute and allows
perform full backups via the 'backupmode' attribute.
Note that no changes to the qemu driver are necessary to take advantage
of this as we already obey the per-disk 'incremental' field.
Peter Krempa [Tue, 7 Jul 2020 14:38:00 +0000 (16:38 +0200)]
backupxml2xmltest: Call 'virDomainBackupAlignDisks' before formatting output
Call the post-processing function so that we can validate that it does
the correct thing.
virDomainBackupAlignDisks requires disk definitions to be present so
let's fake them by copying disks from the backup definition and add one
extra disk 'vdextradisk'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Fixes: 193ad364062407c3fcd3267f0f135d8960b53020 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Wed, 1 Jul 2020 10:25:42 +0000 (12:25 +0200)]
conf: backup: Add 'tls' attribute for 'server' element
Allow enabling TLS for the NBD server used to do pull-mode backups. Note
that documentation already mentions 'tls', so this just implements the
schema and XML bits.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Wed, 1 Jul 2020 14:58:29 +0000 (16:58 +0200)]
conf: checkpoint: Add a flag storing whether disk 'size' is valid
Avoid printing '0' size in case when we weren't able to determine the
backup size by adding a flag whether the size is valid and interlock
printing of the field according to the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Thu, 25 Jun 2020 10:49:45 +0000 (12:49 +0200)]
conf: backup: Don't explicitly forbid backup of read-only disk
Users may want to use this to create a full backup or even incremental
if the checkpoints are pre-existing. We still will not allow to create a
checkpoint on a read-only disk as that makes no sense.
Peter Krempa [Tue, 30 Jun 2020 15:56:08 +0000 (17:56 +0200)]
virQEMUDriverConfigLoadSpecificTLSEntry: Split up fetching of server-only config options
The '*_tls_x509_verify' options are relevant only when we are going to
expose a server socket as client sockets always enable verification.
Split up the macro to separate the common bits from the server bits so
that when we'll later extend support of 'nbd' and 'vxhs' disks which are
client only we can reuse the existing macros.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Laine Stump [Fri, 19 Jun 2020 02:33:28 +0000 (22:33 -0400)]
util: remove OOM error log from virGetHostnameImpl()
The strings allocated in virGetHostnameImpl() are all allocated via
g_strdup(), which will exit on OOM anyway, so the call to
virReportOOMError() is redundant, and removing it allows slight
modification to the code, in particular the cleanup label can be
eliminated.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Fri, 19 Jun 2020 21:40:17 +0000 (17:40 -0400)]
network: fix memory leak in networkBuildDhcpDaemonCommandLine()
hostsfilestr was not being freed. This will be turned into g_autofree
in an upcoming patch converting a lot more of the same file to using
g_auto*, but I wanted to make a separate patch for this first so the
other patch is simpler to review (and to make backporting easier).
Laine Stump [Thu, 18 Jun 2020 23:16:33 +0000 (19:16 -0400)]
use g_autoptr for all xmlBuffers
AUTOPTR_CLEANUP_FUNC is set to xmlBufferFree() in util/virxml.h (This
is actually new - added accidentally (but fortunately harmlessly!) in
commit 257aba2dafe. I had added it along with the hunks in this patch,
then decided to remove it and submit separately, but missed taking out
the hunk in virxml.h)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Thu, 18 Jun 2020 16:49:09 +0000 (12:49 -0400)]
conf, vmx: check for OOM after calling xmlBufferCreate()
Although libvirt itself uses g_malloc0() and friends, which exit when
there isn't enouogh memory, libxml2 uses standard malloc(), which just
returns NULL on OOM - this means we must check for NULL on return from
any libxml2 functions that allocate memory.
xmlBufferCreate(), for example, might return NULL, and we don't always
check for it. This patch adds checks where it isn't already done.
(NB: Although libxml2 has a provision for changing behavior on OOM (by
calling xmlMemSetup() to change what functions are used to
allocating/freeing memory), we can't use that, since parts of libvirt
code end up in libvirt.so, which is linked and called directly by
applications that may themselves use libxml2 (and may have already set
their own alternate malloc()), e.g. drivers like esx which live totally
in the library rather than a separate process.)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Mon, 29 Jun 2020 17:00:36 +0000 (19:00 +0200)]
cirrus: Generate jobs dynamically
Instead of having static job definitions for FreeBSD and macOS,
use a generic template for both and fill in the details that are
actually different, such as the list of packages to install, in
the GitLab CI job, right before calling cirrus-run.
The target-specific information are provided by lcitool, so that
keeping them up to date is just a matter of running the refresh
script when necessary.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
The root cause flaw is that non-root libvirtd is using /etc/libvirt for
its hooks. Traditionally that has been harmless though since we checked
whether we could access the hook file and degraded gracefully. We need
the same access check for iterating over the hook directory.
Long term we should make it possible to have an unprivileged hook dir
under $HOME.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the recent update of Fedora rawhide I've noticed
virnettlssessiontest and virnettlscontexttest failing with:
Our own certificate servercertreq-ctx.pem failed validation
against cacertreq-ctx.pem: The certificate uses an insecure
algorithm
This is result of Fedora changes to support strong crypto [1]. RSA
with 1024 bit key is viewed as legacy and thus insecure. Generate
a new private key then. Moreover, switch to EC which is not only
shorter but also not deprecated that often as RSA. Generated
using the following command:
Andrea Bolognani [Fri, 26 Jun 2020 18:11:58 +0000 (20:11 +0200)]
ci: Run all jobs, for all branches, all the time
After recent changes (increasing the parallelism of the pipeline
by reducing the number of stages, introducing FreeBSD builds that
take longer than any other job), the difference between running
the full pipeline or a reduced one has basically disappeared: in
both cases, the completion time is around 25-35 minutes depending
on whether containers need to be rebuilt and how many shared
runners are available.
Reduce the complexity of our .gitlab-ci.yml and make things
simpler for contributors by simply always running all jobs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Sat, 27 Jun 2020 07:09:39 +0000 (09:09 +0200)]
domain_conf: Remove zPCI validation from formatter
In 076591009ad a validation code was added to
virDomainDeviceInfoFormat() which reports an error if zPCI
address entered in was incomplete. But, there are two problems
with this approach.
The first problem is the placement of the code - it doesn't
belong into XML formatter rather than XML validator.
The second one is that at the point of formatting XML the post
parse callback has run and thus filled in required info.
Therefore this check can never do something useful and instead of
moving it into validator, it's removed completely.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Sat, 27 Jun 2020 08:49:37 +0000 (10:49 +0200)]
qemu_validate: Fix how qemuValidateDomainDeviceDefZPCIAddress() is called
To make the code future proof, the rest of the
qemuValidateDomainDeviceDefAddress() has to be executed (even
though there is nothing there yet) instead of returning directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
If initializing test monitor in testQemuHotplugCpuPrepare()
fails, the control jumps to error label where
testQemuHotplugCpuDataFree() is called. But since the data->mon
is NULL due to aforementioned failure,
qemuMonitorTestGetMonitor() dereferences a NULL pointer leading
to a SIGSEGV.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
domain_conf.c: skip checking ZPCI address is incomplete if not present
Commit 076591009ad1 ("conf: fix zPCI address auto-generation on
s390") is doing a check for virZPCIDeviceAddressIsIncomplete()
prior to checking if the device has a ZPCI address at all. This
results in errors like these when starting libvirt:
error : virDomainDeviceInfoFormat:7527 : internal error:
Missing uid or fid attribute of zPCI address
Fix it by moving virZPCIDeviceAddressIsIncomplete() after the
check done by virZPCIDeviceAddressIsPresent().
Fixes: 076591009ad11ec108521b52a4945d0f895fa160 Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
1. Test for auto-generating uids while specifying valid fids
2. Test for auto-generating fids while specifying valid uids
3. Test for parse error while specifying a valid fid and an invalid
uid
4. Test for parse error while specifying two ZPCI devices with same
uid and fid addresses
5. Test for parse error when both uid and fid are set to zero
6. Test for error while specifying uid and not providing ZPCI
capability.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
we expect uid='0x0001' (or the next available uid for the domain).
However, we get a parsing error:
$ virsh define zpci.xml
error: XML error: Invalid PCI address uid='0x0000', must be > 0x0000
and <= 0xffff
Secondly, when the uid is specified explicitly with the invalid
numerical value '0x0000', we actually expect the parsing error above.
However, the domain is being defined and the uid value is silently
changed to a valid value.
The first issue is a bug and the second one is undesired behaviour, and
both issues are related to how we (in-band) signal invalid values for
uid and fid. So let's fix the XML parsing to do validation based on what
is actually specified in the XML.
The first issue is also related to the current code behaviour, which
is, if either uid or fid is specified by the user, it is incorrectly
assumed that both uid and fid are specified. This bug is fixed by
identifying when the user specified ZPCI address is incomplete and
auto-generating the missing ZPCI address.
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Thu, 25 Jun 2020 14:34:30 +0000 (16:34 +0200)]
qemuxml2xmltest: Set dummy non-hypervisor drivers
When parsing domain XML post parse callbacks are run and one of
them might try and call API from a non-hypervisor driver (e.g.
just like qemuDomainDeviceNetDefPostParse() is doing - it calls a
network API). To avoid this in the test suite, set dummy drivers,
which renders all non-hypervisor APIs return error.
This mimics what qemuxml2argvtest does.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jonathon Jongsma [Thu, 25 Jun 2020 20:18:23 +0000 (15:18 -0500)]
tests: ensure failure if input file doesn't exist
When using the DO_TEST_PARSE_ERROR() macro, a failure to parse the input
file is considered a successful test. However, if the input file is
totally missing, that should be distinguished from a parsing error and
not be treated as a test success.
The function virDomainDefParseFile() simply returns NULL for any parse
failure, including a missing file. So we need to explicitly check
whether the file exists first, and fail the test if it is missing.
Jonathon Jongsma [Thu, 25 Jun 2020 20:18:22 +0000 (15:18 -0500)]
qemu: ramfb video device doesn't support PCI address
Although a ramfb video device is not a PCI device, we don't currently
report an error for ramfb device definitions containing a PCI address.
However, a guest configured with such a device will fail to start:
# virsh start test1
error: Failed to start domain test1
error: internal error: qemu unexpectedly closed the monitor: 2020-06-16T05:23:02.759221Z qemu-kvm: -device ramfb,id=video0,bus=pcie.0,addr=0x1: Device 'ramfb' can't go on PCIE bus
A better approach is to reject any device definitions that contain PCI
addresses. While this is a change in behavior, any existing
configurations were non-functional.
Michal Privoznik [Thu, 25 Jun 2020 07:27:30 +0000 (09:27 +0200)]
qemuDomainDeviceNetDefPostParse: Switch order of conditions
A few commits back (in v6.4.0-131-gbdb8f2e418) the post parse
function for domain interface was changed so that it doesn't fill
in model for hostdev types of interfaces (including network type
interfaces which would end up hostdevs).
While the idea is sound, the execution can be a bit better:
virDomainNetResolveActualType() which is used to determine
runtime type of given interface is heavy gun - it connects to
network driver, fetches network XML, parses it. This all is
followed by check whether the interface doesn't already have
model set (from domain XML).
If we switch the order of these two checks then the short circuit
evaluation will ensure the expensive check is done only if really
needed.
This commit in fact fixes qemuxml2xmltest which due to lacking
fake network driver tries to connect to network:///session and
start the virtnetworkd. Fortunately, because of v6.3.0-25-gf28fbb05d3 it fails to do so and
virDomainNetResolveActualType() returns -1. The only reason we
don't see the test failing is because our input XMLs have model
and thus we are saved by the latter (now former) check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Andrea Bolognani [Thu, 25 Jun 2020 12:28:25 +0000 (14:28 +0200)]
ci: Refresh Dockerfiles
All targets get pip3, which is now part of the base system, and
the number of native packages included in the MinGW containers is
significantly reduced.
The corresponding libvirt-ci commit is 4ff697ba0b5d.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Wed, 24 Jun 2020 09:41:28 +0000 (11:41 +0200)]
kbase: incrementalbackupinternals: Replace bash with pseudocode
Simplify the docs and reduce maintenance burden by just describing the
algorithm by a pseudo-language. Users are encouraged to use libvirt
anyways and projects such as oVirt which do some management of storage
themselves are unlikely to use bash anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Wed, 24 Jun 2020 09:26:29 +0000 (11:26 +0200)]
kbase: incrementalbackupinternals: Add secion on bitmap handling in shell
Add a section that outlines usage of tools to handle bitmaps and
introduce terms corresponding to the output of qemu-img to be used in
further sections.
With this we can simplify the section about checking bitmap health as we
don't have to explain the qemu-img output but can refer to the newly
defined terms.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>