Michal Privoznik [Mon, 17 May 2021 19:34:55 +0000 (21:34 +0200)]
qemusecuritytest: Honour EXIT_AM_SKIP
There is a case where qemusecuritytest is skipped - on MacOS and
MinGW. In such case, EXIT_AM_SKIP should be returned. However,
my recent patch of 5d99b157bc completely missed that and made the
test return EXIT_FAILURE even though the test exited early
without performing any test case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 17 May 2021 16:01:11 +0000 (18:01 +0200)]
viridentity: Fix ref/unref imbalance in VIR_IDENTITY_AUTORESTORE
The basic use case of VIR_IDENTITY_AUTORESTORE() is in
conjunction with virIdentityElevateCurrent(). What happens is
that virIdentityElevateCurrent() gets current identity (which
increases the refcounter of thread local virIdentity object) and
returns a pointer to it. Later, when the variable goes out of
scope the virIdentityRestoreHelper() is called which calls
virIdentitySetCurrent() over the old identity. But this means
that the refcounter is increased again.
Therefore, we have to explicitly decrease the refcounter by
calling g_object_unref().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 17 May 2021 10:40:00 +0000 (12:40 +0200)]
virnumamock: Allow CPU-less NUMA nodes
The original virNumaGetNodeCPUs() returns an empty virBitmap if
given NUMA node has no CPUs. But that's not how our mock behaves
- it looks under $fakesysfs/node/node$N/cpulist only to find an
empty file which is then passed to virBitmapParseUnlimited()
which threats such input as error.
Fortunately, we don't have any fake sysfs data where this path is
hit, but we might soon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 17 May 2021 12:48:36 +0000 (14:48 +0200)]
driver: Don't leak saved error in virGetConnectGeneric()
Recently, a new code was added to virGetConnectGeneric() that
saves the original error into a variable so that it's not lost in
virConnectClose() called under the 'error' label.
However, the error saving code uses virSaveLastError() +
virSetError() combo which leaks the memory allocated for the
error copy. Using virErrorPreserveLast() + virErrorRestore() does
the same job without the memleak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Sun, 16 May 2021 16:20:56 +0000 (18:20 +0200)]
testutils: Document and enforce @func callback retvals for virTestMain()
When a test has a wrapper over main() (e.g. because it's
preloading some mock libraries). the main() is renamed to
something else (usually mymain()), and main() is generated by
calling one of VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros.
This has a neat side effect - if mymain() returns an error a
short summary is printed, e.g.:
Some tests failed. Run them using:
VIR_TEST_DEBUG=1 VIR_TEST_RANGE=5-6 ./virtest
However, this detection only works if EXIT_FAILURE is returned by
mymain(). Document and enforce this limitation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Sun, 16 May 2021 16:14:53 +0000 (18:14 +0200)]
tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0
When using VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros, the
retval of mymain() will become retval of main(). Hence, mymain()
should use EXIT_FAILURE and EXIT_SUCCESS return values for
greater portability. Another reason is that otherwise our summary
printing of failed tests doesn't work (see following commit for
more info).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Sun, 16 May 2021 16:21:10 +0000 (18:21 +0200)]
testutils: Drop libtool binary name handling
Back in the old days, we used to use libtool to run compiled
libraries. That meant we had to deal with "lt-" prefix for our
binaries. With meson that's no longer the case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
A secret can be marked with the "private" attribute. The intent was that
it is not possible for any libvirt client to be able to read the secret
value, it would only be accesible from within libvirtd. eg the QEMU
driver can read the value to launch a guest.
With the modular daemons, the QEMU, storage and secret drivers are all
running in separate daemons. The QEMU and storage drivers thus appear to
be normal libvirt client's from the POV of the secret driver, and thus
they are not able to read a private secret. This is unhelpful.
With the previous patches that introduced a "system token" to the
identity object, we can now distinguish APIs invoked by libvirt daemons
from those invoked by client applications.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
src: elevate current identity privilege when fetching secret
When fetching the value of a private secret, we need to use an elevated
identity otherwise the secret driver will deny access.
When using the modular daemons, the elevated identity needs to be active
before the secret driver connection is opened, and it will apply to all
APIs calls made on that conncetion.
When using the monolithic daemon, the identity at time of opening the
connection is ignored, and the elevated identity needs to be active
precisely at the time the virSecretGetValue API call is made.
After acquiring the secret value, the elevated identity should be
cleared.
This sounds complex, but is fairly straightfoward with the automatic
cleanup callbacks.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The drivers can all call virGetConnectXXX to open a connection to a
secondary driver. For example, when creating a encrypted storage volume,
the storage driver has to open a secret driver connection, or when
starting a guest, the QEMU driver has to open the network driver to
lookup a virtual network.
When using monolithic libvirtd, the connection has the same effective
identity as the client, since everything is still in the same process.
When using the modular daemons, however, the remote daemon sees the
identity of the calling daemon. This is a mistake as it results in
the modular daemons seeing the client with elevated privileges.
We need to pass on the current identity explicitly when opening the
secondary drivers. This is the same thing that is done by daemon RPC
dispatcher code when it is directly forwarding top level API calls
from virtproxyd and other daemons.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: helper to temporary elevate privileges of the current identity
When talking to the secret driver, the callers inside libvirt daemons
need to be able to run with an elevated privileges that prove the API
calls are made by a libvirt daemon, not an end user application.
The virIdentityElevateCurrent method will take the current identity
and, if not already present, add the system token. The old current
identity is returned to the caller. With the VIR_IDENTITY_AUTORESTORE
annotation, the old current identity will be restored upon leaving
the codeblock scope.
... early work with regular privileges ...
if (something needing elevated privs) {
VIR_IDENTITY_AUTORESTORE virIdentity *oldident =
virIdentityElevateCurrent();
if (!oldident)
return -1;
... do something with elevated privileges ...
}
... later work with regular privileges ...
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When creating the system identity set the system token. The system
token is currently stored in a local path
/var/run/libvirt/common/system.token
Obviously with only traditional UNIX DAC in effect, this is largely
security through obscurity, if the client is running at the same
privilege level as the daemon. It does, however, reliably distinguish
an unprivileged client from the system daemons.
With a MAC system like SELinux though, or possible use of containers,
access can be further restricted.
A possible future improvement for Linux would be to populate the
kernel keyring with a secret for libvirt daemons to share.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: introduce concept of a system token into identities
We want a way to distinguish between calls from a libvirt daemon, and a
regular client application when both are running as the same user
account. This is not possible with the current set of attributes
recorded against an identity, as there is nothing that is common to all
of the modular libvirt daemons, while distinct to all other processes.
We thus introduce the idea of a system token, which is simply a random
hex string that is only known by the libvirt daemons, to be recorded
against the system identity.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Neal Gompa [Tue, 11 May 2021 23:22:23 +0000 (19:22 -0400)]
rpm: Set version information for libvirt-admin virtual name
The libvirt-daemon package now provides the 'libvirt-admin' virtual
name, but the Provides stanza doesn't declare version information,
which breaks things depending on that package using a versioned
dependency. Fix this by setting the version-release of libvirt to
that name to mimic the previous state.
Fixes: 2244ac168d42c3fa424bae6d33ecdbb8726da7c2 Signed-off-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
gitlab: avoid building libvirt twice and running syntax-check twice
In the distros using RPMs, we build libvirt once as a side effect
of running "ninja dist", and once via rpmbuild.
In addition "ninja dist" will run all tests including the "syntax-check"
suite, despite use having a separate "codestyle" job for for that.
There is no way to pass "--no-suite" when creating the dist, but if we
switch to invoking "meson dist", we can skip the build+test part
entirely using "--no-tests".
When doing this we then run explicit "meson compile" and "meson test"
commands for the distros that don't build the RPMs, and in the latter
case we can now skip the "syntax-check" suite.
The RPM builds already skipped the "syntax-check" suite.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gitlab: run "meson" directly instead of "ninja" indirection
The "dist" and "test" targets in ninja end up calling back into
the equivalent meson commands. The meson commands support various
arguments that are not accessible when invoked via ninja, so it
is preferrable to use meson directly.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
"meson test" will only print a list of which tests fail by default,
so we were sending the full test log to stdout on failure. This makes
it really hard to see the errors though as the test log has all
succcesful tests too.
"ninja test" will print the same as "meson test", following by details
of each failure.
It does this using the "--print-errorlog" flag, so lets use that in
the codestyle job.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Tue, 11 May 2021 15:13:59 +0000 (17:13 +0200)]
spec: Reintroduce supported_platform variable
The rewritten checks, which made it possible to drop the
variable, are in fact not equivalent to the original ones,
and rewriting them once again so that they are would make
them unwieldy. Let's go back to how things were.
Tim Wiederhake [Mon, 10 May 2021 12:48:34 +0000 (14:48 +0200)]
virNetworkDHCPLeaseTimeDef: Make expiry unsigned long long
The width of `unsigned long` differs on 32 bit and 64 bit architectures.
There is no compelling reason why the maximum DHCP lease time should
depend on the architecture.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Once this commit merges, the above wiki should point to this kbase
document.
NB: I've intentionally left out the example for pull-based full backups.
I'll tackle it once QMP `x-blockdev-reopen` comes out of experimental
mode in upstream QEMU. Then pull-based can be described for both full
and and differntial backups.
Overall, future documents should cover:
- full backups using both push- and pull-mode
- differential backups using both push- and pull-mode
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Mon, 10 May 2021 13:07:09 +0000 (15:07 +0200)]
qemu_firmware: don't error out for unknown firmware features
When QEMU introduces new firmware features libvirt will fail until we
list that feature in our code as well which doesn't sound right.
We should simply ignore the new feature until we add a proper support
for it.
Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virCapabilitiesHostNUMAInitReal: Bring variables into loop
Some variables are needed only inside for() loop. They were
declared at the beginning of the function because of VIR_FREE()
calls, but since they are auto-freed they can be declared inside
the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 29 Apr 2021 16:58:40 +0000 (18:58 +0200)]
virCapabilitiesHostNUMAAddCell: Take double pointer
What this function really does it takes ownership of all pointers
passed (well, except for the first one - caps - to which it
registers new NUMA node). But since all info is passed as a
single pointer it's hard to tell (and use g_auto*). Let's use
double pointers to make the ownership transfer obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @cpus variable is an array of structs in which each item
contains a virBitmap member. As such it is not enough to just
VIR_FREE() the array - each bitmap has to be freed too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virCapabilitiesHostNUMAFormat: Swap order of arguments
The rest of virCapabilities format functions take virBuffer as
the first argument and struct to format as the second. Also, they
accept NULL (as the second argument). Fix
virCapabilitiesHostNUMAFormat() so that it follows this logic.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 10 May 2021 08:33:02 +0000 (10:33 +0200)]
securityselinuxhelper: Fix retval of setcon_raw() and security_disable()
The securityselinuxhelper is a mock that's replacing libselinux
APIs with our own implementation to achieve deterministic
results. Our implementation uses env vars (among other things) to
hold internal state. For instance, "FAKE_SELINUX_CONTEXT" and
"FAKE_SELINUX_DISABLED" variables are used. However, as we were
switching from setenv() to g_setenv() we also changed the set of
possible retvals from setcon_raw() and security_disable().
Previously, the retval of setenv() was used directly which
returns 0 on success and -1 on error. But g_setenv() has
different retval semantics: it returns 1 on success and 0 on
error.
This discrepancy can be observed by running viridentitytest where
case #2 reports an error ("!") - because setcon_raw() returns 1.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virthread: Make sure virOnce() returns -1 on error
Since its introduction in v0.9.1~65 the virOnce() was expected to
follow the usual retval logic (0 for success, a negative number
for failure). However, that was never the case.
On the other hand, looking into glibc and musl the pthread_once()
never returns anything other than zero (uclibc-ng seems to not
implement pthread_once()), therefore we never really hit any
problem. But for code cleanliness (and to match POSIX
documentation), let's change to code so that our retval logic is
honoured.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 6 May 2021 12:35:22 +0000 (14:35 +0200)]
virXMLPropEnum: Always initialize '@result'
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 15:02:50 +0000 (17:02 +0200)]
virDomainVideoDefParseXML: Fix parsing of 'backend'
Commit 8391cfbc2dbc converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'backend' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 15:02:50 +0000 (17:02 +0200)]
virDomainAudioCommonParse: Fix parsing of 'format'
Commit 38180f87f5b converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'format' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
In two cases the code needed to be adjusted to preserve functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 14:01:32 +0000 (16:01 +0200)]
virDomainBackupDiskDefParseXML: Fill default backup state after parsing it
Set the backup mode to VIR_TRISTATE_BOOL_YES after virXMLPropTristateBool
left it set to VIR_TRISTATE_BOOL_ABSENT. This will allow fixing
virXMLPropTristateBool to always initialize @result.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 12:35:22 +0000 (14:35 +0200)]
virXMLPropInt: Always initialize '@result'
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
This is done by adding a @defaultResult argument to virXMLPropInt since
many places have a non-0 default.
In certain cases such as in virDomainControllerDefParseXML we pass the
value from the original value, which will still trigger compiler checks
if unused while preserving the existing functionality of keeping the
previous value.
This commit fixes 3 uses of uninitialized value parsed by this function:
in virDomainDiskSourceNetworkParse introduced by 38dc25989c5
in virDomainChrSourceDefParseTCP introduced by fa48004af5b
in virDomainGraphicsListenDefParseXML introduced by 0b20fd3754c
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 12:00:16 +0000 (14:00 +0200)]
virXMLPropUInt: Always initialize @result
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 13:07:04 +0000 (15:07 +0200)]
virDomainVcpuParse: Assign default vcpus count based on return value of virXMLPropUInt
Assign the vcpu count when virXMLPropUInt returns '0' meaning that the
cpu count was not present in the XML. This will allow to always
initialize the value of @result in virXMLPropUInt to prevent use of
uninitialized values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 12:00:16 +0000 (14:00 +0200)]
virXMLPropULongLong: Always initialize @result
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 6 May 2021 11:53:18 +0000 (13:53 +0200)]
util: xml: Extract implementation of xml property -> enum parsing to a common helper
virXMLPropTristateBool/virXMLPropTristateSwitch/virXMLPropEnum can be
implemented using the same internal code. Extract it into a new function
called virXMLPropEnumInternal, which will also simplify adding versions
of these functions with a custom default value.
This way we'll be able to always initialize @result so that unused value
bugs can be prevented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
projects / libvirt.git / log