Tim Wiederhake [Wed, 19 May 2021 14:10:03 +0000 (16:10 +0200)]
virDomainAudioPulseAudioParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `latency`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jonathon Jongsma [Fri, 14 May 2021 21:29:00 +0000 (16:29 -0500)]
tests: nodedevxml2xmltest: test more mdev files
Add the rest of the mdev xml files to the xml2xml test, and include 2
new test cases: one that explicitly specifies 'manual' start, and one
that explicitly specifies 'auto' start.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Boris Fiuczynski [Fri, 14 May 2021 21:28:59 +0000 (16:28 -0500)]
nodedev: support auto-start property for mdevs
This adds a new element to the mdev capabilities xml schema that
represents the start policy for a defined mediated device. The actual
auto-start functionality is handled behind the scenes by mdevctl, but it
wasn't yet hooked up in libvirt.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jonathon Jongsma [Fri, 14 May 2021 21:28:58 +0000 (16:28 -0500)]
test: move nodedev xml2xml output to a separate dir
Currently, we're loading and parsing the xml from the input file, and
then formatting it and then comparing it directly back to the input
file. This works for now, but is severely limiting as it relies on the
input file being fully-specified and in the exact order as the output
xml format.
If optional elements are ommitted in the input XML, the output xml
may include default values for the ommitted elements and thus the output
will not match the input.
In order to allow more flexibility in testing, save the expected output
to a seprate 'out' directory similar to what most of the other xml2xml
tests are already doing.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 19 May 2021 09:58:21 +0000 (11:58 +0200)]
virsh-domain: Fix @ret handling in cmdSetmem and cmdSetmaxmem
These functions initialize @ret to true and only after something
fails either they call cleanup code (which consists only from
virshDomainFree()) and return false, or they set ret = false and
carry on (when the failure occurred close to cleanup code).
Switch them to the usual pattern in which ret is initialized to
failure, goto cleanup is used and ret is set to true only after
everything succeeded.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Tue, 18 May 2021 12:24:01 +0000 (14:24 +0200)]
virsh: Fix logic wrt to --current flag in cmdSetmem
In my commit of v7.1.0-rc1~376 I've simplified the logic of
handling @flags. My assumption back then was that calling
virDomainSetMemory() is equivalent to
virDomainSetMemoryFlags(flags = 0). But that is not the case,
because it is equivalent to virDomainSetMemoryFlags(flags =
VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old
API.
Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961118 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Mon, 17 May 2021 08:38:25 +0000 (10:38 +0200)]
qemuxml2argvtest: Limit 'disk-network-sheepdog' testcase to qemu-6.0.0
QEMU is dropping sheepdog support in 6.1 so we need to limit the test
case to the latest version supporting sheepdog as it won't be described
by the QMP schema any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 17 May 2021 08:51:15 +0000 (10:51 +0200)]
testQemuInfoSetArgs: Strip default machine alias only for 'latest' test cases
For the real-capabilities test cases testing 'latest' capabilities we
strip off the alias from 'pc' to the appropriate versioned machine type
to prevent update to all tests when bumping qemu capabilities.
Recenly we also started caching the capabilities to prevent re-parsing
the XML all the time. The commit adding the caching kept the alias
stripping prior to cache insertion, thus the cache contains the stripped
alias.
This leads to problem when a test case is added where the 'latest'
equals to the selected version.
Move the machine alias stripping after we create a local copy thus
stripping it only for 'latest' tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:51 +0000 (17:04 +0200)]
virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `number`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:50 +0000 (17:04 +0200)]
virDomainAudioOSSParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:49 +0000 (17:04 +0200)]
virDomainAudioCoreAudioParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:48 +0000 (17:04 +0200)]
virDomainChrDefParseTargetXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `port`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:47 +0000 (17:04 +0200)]
virDomainChrSourceReconnectDefParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `timeout`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:46 +0000 (17:04 +0200)]
virDomainDiskDefGeometryParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `cyls`, `heads` and `secs`.
Allowing negative numbers to be interpreted this way makes no sense for
these attributes.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Tim Wiederhake [Tue, 18 May 2021 15:04:44 +0000 (17:04 +0200)]
virDomainDeviceUSBMasterParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `startport`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Olaf Hering [Wed, 5 May 2021 14:06:32 +0000 (16:06 +0200)]
libxl: set vcpu affinity during domain creation
Since Xen 4.5 libxl allows to set affinities during domain creation.
This enables Xen to allocate the domain memory on NUMA systems close to
the specified pcpus.
Libvirt can now handle <domain/cputune/vcpupin> in domU.xml correctly.
Without this change, Xen will create the domU and assign NUMA memory and
vcpu affinities on its own. Later libvirt will adjust the affinity,
which may move the vcpus away from the assigned NUMA node.
Signed-off-by: Olaf Hering <olaf@aepfle.de> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
qemu_process: Drop needless check in qemuProcessNeedMemoryBackingPath()
The aim of this function is to return whether domain definition
and/or memory device that user intents to hotplug needs a private
path inside cfg->memoryBackingDir. The rule for the memory device
that's being hotplug includes checking whether corresponding
guest NUMA node needs memoryBackingDir. Well, while the rationale
behind makes sense it is not necessary to check for that really -
just a few lines above every guest NUMA node was checked exactly
for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemu_process: Deduplicate code in qemuProcessNeedHugepagesPath()
The aim of qemuProcessNeedHugepagesPath() is to return whether
guest needs private path inside HugeTLBFS mounts (deducted from
domain definition @def) or whether the memory device that user is
hotplugging in needs the private path (deducted from the @mem
argument). The actual creation of the path is done in the only
caller qemuProcessBuildDestroyMemoryPaths().
The rule for the first case (@def) and the second case (@mem) is
the same (domain has a DIMM device that has HP requested) and is
written twice. Move the logic into a function to deduplicate the
code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jim Fehlig [Mon, 17 May 2021 17:52:25 +0000 (11:52 -0600)]
tests: libxl: Mock xs_open and xs_close
The Xen-related unit tests are failing against the recently released
Xen 4.15. Xen commit 90c9f9f4dd changed the implementation of
libxl_ctx_alloc to use xs_open instead of xs_daemon_open. libvirt has
already mocked xs_daemon-{open,close} and others to allow using libxl
in confined build environments. This patch adds xs_{open,close} to the
list of functions mocked in libxlmock.c
Andrea Bolognani [Fri, 14 May 2021 09:03:42 +0000 (11:03 +0200)]
meson: Add yajl kludge
If this looks familiar, that's because it's literally *the
same code* that we used to work around *the same issue* in
readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)
Note that the issue only really affects people building from
source on Apple Silicon: on Intel, Homebrew installs header
files under directories that are part of the default search
path, which explains why our CI pipeline never ran into it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
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>
projects / libvirt.git / log