Convert the field, and fill in missing cases to switch()
statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field, adjust the XML parsers to use virXMLPropEnum()
and fill in missing cases to switch() statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field, adjust the XML parser to use
virXMLPropEnumDefault() and fill in missing cases to switch()
statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The point of virLXCControllerSetupHostdevCaps() is to access
.caps union member of given <hostdev/> source. And it does so in
the switch, but then, when reporting an error the .subsys member
is accessed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Avoid logging multiline debug logs so that the function which attempts
to extract a non-debug log error message can work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Generated using the following semantic patch:
@@
type T;
identifier X;
@@
- T X;
+ T X = { 0 };
... when exists
(
- memset(&X, 0, sizeof(X));
|
- memset(&X, 0, sizeof(T));
)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
In a few places, where a capabilities <hostdev/> is processed, a
wrong union member is access: def->source.subsys.type instead of
def->source.caps.type. Fortunately, both union members have .type
as the very first member so no real harm is done. Nevertheless,
we should access the correct union member.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When commit bac6b266fb added this "functionality" this was the only
naming I could think of, but after discussion with Dan we found the name
'null' fits a bit better, so change it before we make a release with the
old name.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This represents an interface connected to a VMWare Distributed Switch,
previously obscured as a dummy interface.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim of 'restrictive' numatune mode is to rely solely on
CGroups to have QEMU running on configured NUMA nodes. However,
we were never setting the cpuset controller when a domain was
starting up. We are doing so only when
virDomainSetNumaParameters() is called (aka live pinning).
This is obviously wrong. Fortunately, fix is simple as
'restrictive' is similar to 'strict' - every location where
VIR_DOMAIN_NUMATUNE_MEM_STRICT occurs can be audited and
VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE case can be added.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2070380
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Adding an exception for the whole file usually defeats the purpose of a
syntax check and is also likely to get forgotten once the file is
removed.
In case of the suggestion of using 'safewrite' instead of write even the
comment for safewrite states that the function needs to be used only in
certain cases.
Remove the blanket exceptions for files and use an exclude string
instead. The only instance where we keep the full file exception is for
src/libvirt-stream.c as there are multiple uses in example code in
comments where I couldn't find a nicer targetted wapproach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the _virDomainTimerDef structure we have @present member which
is like virTristateBool, except it's an integer and has values
shifted by one. This is harder to read. Retype the member to
virTristateBool which we are familiar with.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
And make callers check the return value as well. This helps error out early for
invalid environment variables.
That is desirable because it could lead to deadlocks. This can happen when
resetting logging after fork() reports translated errors because gettext
functions are not reentrant. Well, it is not limited to resetting logging after
fork(), it can be any translation at that phase, but parsing environment
variables is easy to make fail on purpose to show the result, it can also happen
just due to a typo.
Before this commit it is possible to deadlock the daemon on startup
with something like:
LIBVIRT_LOG_FILTERS='1:*' LIBVIRT_LOG_OUTPUTS=1:stdout libvirtd
where filters are used to enable more logging and hence make the race less rare
and outputs are set to invalid
Combined with the previous patches this changes
the following from:
...
<deadlock>
to:
...
libvirtd: initialisation failed
The error message is improved in future commits and is also possible thanks to
this patch.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Unfortunately, this fix breakes machinectl in a very nasty way,
for instance 'machinectl shell' drops into the host shell. It's
worse than being unable to start a container with CGroupsV1.
Revert until a proper fix is figured out.
This reverts commit 1b9ce05ce2.
References: https://gitlab.com/libvirt/libvirt/-/issues/182
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an
early close of all clients of lxc_controller. Here, libvirtd itself is a client
of this controller, and the client connection is used to notify libvirtd if a
reboot of the container is required. However, the client connection was closed
before such a status could be sent to libvirtd. To fix this bug, we will
immediately send the reboot or shutdown status of the container to libvirtd,
and only after client disconnect will we trigger virNetDaemonQuit.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/237
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773
Signed-off-by: Joachim Falk <joachim.falk@gmx.de>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With cgroup v1 I'm seeing LXC container startup failures:
$ sudo virt-install --connect lxc:/// --name test-container --memory 128
--boot init=/bin/sh
Starting install...
ERROR error from service:
GDBus.Error:org.freedesktop.machine1.NoMachineForPID: PID 2145047 does
not belong to any known machine
libvirt 7.0.0 works but 7.1.0+ does not. The root error seems to predate
that, showing up in syslog, but commit 9c1693eff made it fatal:
commit 9c1693eff4
Author: Pavel Hrdina <phrdina@redhat.com>
Date: Fri Feb 5 16:17:35 2021 +0100
vircgroup: use DBus call to systemd for some APIs
The error comes from virSystemdGetMachineByPID. The PID that shows up in
the above error message does not match the leader PID as reported by
machinectl.
This change fixes the error. Things seem to continue to work with
cgroupsv2 after this change.
https://gitlab.com/libvirt/libvirt/-/issues/182
Tested-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Use virAppendElement instead of virInsertElementsN to implement
VIR_APPEND_ELEMENT which allows us to remove error handling as the
only relevant errors were removed when switching to aborting memory
allocation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is the bug I'm facing. I deliberately configured a container
so that the source of a <filesystem/> to passthrough doesn't
exist. The start fails with:
lxcContainerPivotRoot:669 : Failed to create /non-existent/path/.oldroot: Permission denied
which is expected. But what is NOT expected is that CGroup
hierarchy is left behind. This is because the controller sets up
the CGroup hierarchy, user namespace, moves interfaces, etc. and
finally checks whether container setup (done in a separate
process) succeeded. Only after all this the error is propagated
to the LXC driver. The driver aborts the startup and tries to
perform the cleanup, but this is missing CGroups because those
weren't detected yet.
Ideally, whenever a function fails, it tries to unroll back so
that is has no artifacts left behind (look at all those frees/FD
closes/etc. at end of functions). But with CGroups it is
different - the controller process can't clean up after itself,
because it is still running inside that CGroup.
Therefore, what we have to do is to let the driver detect CGroups
as soon as they are created, and proceed with controller
execution only after that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Currently, there is only a single pipe passed to lxc_controller
and it is used by lxc_controller to signal to the LXC driver that
the container is set up and ready to run. However, in the next
commit we will need to signal that the LXC driver has done its
part of startup process and thus the controller can proceed.
Unfortunately, virCommand handshake can't be used for this,
because it's already used to read controller's PID.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Future commits will want to reuse the handshakeFd and thus it
mustn't be closed in virLXCControllerDaemonHandshake(). Do the
closing explicitly afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The lxc_controller has a structure that's keeping its internal
state, including so called handshakeFd which is the write end of
a pipe that's used to signal to the LXC driver that the container
is set up and ready to run. However, the struct member is not
initialized to -1, so if anything fails before it is set then the
virLXCControllerFree() function tries to close FD 0 (stdin).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Generated by the following spatch:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In short, virXXXPtr type is going away. With big bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Generated using the following spatch:
@@
expression path;
@@
- virFileMakePath(path)
+ g_mkdir_with_parents(path, 0777)
However, 14 occurrences were not replaced, e.g. in
virHostdevManagerNew(). I don't really understand why.
Fixed by hand afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only reason why virstoragefile.h needs to be included in virfile.h
is that virFileNBDDeviceAssociate() takes virStorageFileFormat argument.
The function doesn't need the enum value as it converts the value to
string and uses only that.
Change the argument to string which will allow us to remove that
include.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.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>
The function only returns zero or aborts, so it might as well be void.
This has the added benefit of simplifying the code that calls it.
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This patch adds new schema and adds support for parsing and formatting
domain configurations that include vdpa devices.
vDPA network devices allow high-performance networking in a virtual
machine by providing a wire-speed data path. These devices require a
vendor-specific host driver but the data path follows the virtio
specification.
When a device on the host is bound to an appropriate vendor-specific
driver, it will create a chardev on the host at e.g. /dev/vhost-vdpa-0.
That chardev path can then be used to define a new interface with
type='vdpa'.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
As preparation for g_autoptr() we need to change the function to take
only virCgroupPtr.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Split those initializations that depend on a statement
above them.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refer to the notion of mount propagation instead which describes
the actual behaviour more clearly.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The two sides of a PTY can be referred to as primary and secondary
TTYs.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Given our supported platform matrix, we can safely assume that
all the mount constants we need are defined by the system
headers.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since 5b82f7f3, @path should have been placed inside the for loop
since it'd need to be free'd for each pass through the loop; otherwise,
we'd leak like a sieve.
Found by Coverity.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
