Commit Graph

1435 Commits

Author SHA1 Message Date
Michal Privoznik
1051c23b51 lxc: Let the driver detect CGroups earlier
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>
2021-04-22 14:10:47 +02:00
Michal Privoznik
5aba8d5438 lxc: Pass another pipe to lxc_controller
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>
2021-04-22 13:57:46 +02:00
Michal Privoznik
bfe2d857f2 lxc_controller: Move closing of handshakeFd out of virLXCControllerDaemonHandshake()
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>
2021-04-22 13:53:05 +02:00
Michal Privoznik
da61e92739 lxc_controller: Initialize ctrl->handshakeFd properly
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>
2021-04-22 13:52:21 +02:00
Luke Yue
6e91cbfdad Replace AbsPath judgement method with g_path_is_absolute()
The g_path_is_absolute() considers more situations
than just a simply "path[0] == '/'".

Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12

Signed-off-by: Luke Yue <lukedyue@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-04-21 10:02:09 +02:00
Michal Privoznik
69a4cd9249 lxc: Format --handshakefd for controller cmd fully
The command line argument is called --hanshakefd (check out
lxc_controller.c:main()). But the command line builder puts only
--handshake. This works, because there is no other argument
sharing the prefix.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-04-19 11:21:40 +02:00
Michal Privoznik
c8238579fb lib: Drop internal virXXXPtr typedefs
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>
2021-04-13 17:00:38 +02:00
Michal Privoznik
5f9330e724 lib: Undo some g_steal_pointer() changes
Recently, a few commits back I've switched bunch of code to
g_steal_pointer() using coccinelle. Problem was that the semantic
patch used was slightly off:

  @@
  expression a, b;
  @@

  + b = g_steal_pointer(&a);
  - b = a;
    ... when != a
  - a = NULL;

Problem is that, "... when != a" is supposed to jump over those
lines, which don't contain expression a. My idea was to replace
the following pattern too:

  ptrX = ptrY;
  if (something(ptrZ) < 0) goto error;
  ptrY = NULL;

But what I missed is that the following pattern is also matched
and replaced:

  ptrX = ptrY;
  if (something(ptrX) < 0) goto error;
  ptrY = NULL;

This is not necessarily correct - as demonstrated by our hotplug
code. The real problem is ambiguous memory ownership transfer
(functions which add device to domain def take ownership only on
success), but to not tackle the real issue let's revert those
parts.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-03-26 10:11:57 +01:00
Michal Privoznik
b0f78d626a lib: Fix calling of virNetworkUpdate() driver callback
The order in which virNetworkUpdate() accepts @section and
@command arguments is not the same as in which it passes them
onto networkUpdate() callback. Until recently, it did not really
matter, because calling the API on client side meant arguments
were encoded in reversed order (compared to the public API), but
then on the server it was fixed again - because the server
decoded RPC (still swapped), called public API (still swapped)
and in turn called the network driver callback (with reversing
the order - so magically fixing the order).

Long story short, if the public API is called even number of
times those swaps cancel each other out. The problem is when the
API is called an odd numbed of times - which happens with split
daemons and the right URI. There's one call in the client (e.g.
virsh net-update), the other in a hypervisor daemon (say
virtqemud) which ends up calling the API in the virnetworkd.

The fix is obvious - fix the order in which arguments are passed
to the callback.

But, to maintain compatibility with older, yet unfixed, daemons
new connection feature is introduced. The feature is detected
just before calling the callback and allows client to pass
arguments in correct order (talking to fixed daemon) or in
reversed order (talking to older daemon).

Unfortunately, older client talking to newer daemon can't be
fixed. Let's hope that it's less frequent scenario.

Fixes: 574b9bc66b
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2021-03-25 10:10:23 +01:00
Michal Privoznik
0c30e7221c lib: Use g_steal_pointer() more
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>
2021-03-24 13:57:51 +01:00
Jiri Denemark
7d2fd6ef01 Do not check return value of VIR_EXPAND_N
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2021-03-22 12:44:18 +01:00
Michal Privoznik
9d3cd0c1d4 lib: Put some variable declarations on individual lines
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>
2021-03-15 09:38:18 +01:00
Michal Privoznik
7f482a67e4 lib: Replace virFileMakePath() with g_mkdir_with_parents()
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>
2021-03-04 20:52:23 +01:00
Pavel Hrdina
e95489d813 cgroup: use virCgroupSetCpuShares instead of virCgroupSetupCpuShares
Now that we enforce the cpu.shares range kernel will no longer silently
change the value that libvirt configures so there is no need to read
the value back to get the actual configuration.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-03-04 11:13:28 +01:00
Peter Krempa
e2ac76f707 lxc_process: Remove OOM handling from logging setup
'virLogGetFilters' doesn't return failure and 'virLogGetOutputs' reports
it's own errors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-02 09:50:19 +01:00
Kristina Hanicova
78879650e4 Remove redundant variables/labels
In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and
virNetDevIPRouteParseXML()

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-01 18:50:22 -05:00
Kristina Hanicova
5fa51adcf2 Use g_autoptr instead of virNetDevIPRouteFree if possible
In files: src/conf/domain_conf: in virDomainNetIPInfoParseXML(),
src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/vz/vz_sdk: in prlsdkGetRoutes(), src/conf/networkcommon_conf:
in virNetDevIPRouteCreate()

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-03-01 18:50:22 -05:00
Peter Krempa
10157731f4 Replace virStringSplit with g_strsplit
Our implementation was heavily inspired by the glib version so it's a
drop-in replacement.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
2f9b2c0cdd Replace virStringListLength where actual lenght is not needed
Some callers don't need to know the actual lenght of the list but only
care whether the required element is present or the list is non-empty.
Don't calculate the list length in those cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
56cedfcf38 Replace virStringListHasString by g_strv_contains
The glib variant doesn't accept NULL list, but there's just one caller
where it wasn't checked explicitly, thus there's no need for our own
wrapper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Jim Fehlig
b9a063cd8e build: Remove unused 'conflicts' key from virt_daemon_unit
The 'conflict' key in a virt_daemon_unit dictionary is not used when
generating systemd service and socket files. The comment associated
with the key claims the default is 'true', and a few build files
needlessly set it to 'true' when defining their virt_daemon_unit.
Remove the 'conflict' key and its use in the affect build files.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 09:30:41 -07:00
Laine Stump
7f37110f2f use g_autoptr for all virConnectPtrs used with virGetConnectNetwork()
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-01-08 11:34:59 -05:00
Laine Stump
c2b2cdf746 call virDomainNetNotifyActualDevice() for all interface types
Now that this function can be called regardless of interface type (and
whether or not we have a conn for the network driver), let's actually
call it for all interface types. This will assure that we re-connect
any disconnected bridge devices for <interface type='bridge'> as
mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1730084#c26
(until now we've only been reconnecting bridge devices for <interface
type='network'>)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-08 11:34:49 -05:00
Laine Stump
2d0bac9d58 lxc: eliminate leaked and dangling pointers in virLXCProcessSetupInterfaceTap
The two scenarios were found by Coverity after a seemingly-unrelated
change to virLXCProcessSetupInterfaceTap() (in commit ecfc2d5f43), and
explained by John Ferlan here:

https://www.redhat.com/archives/libvir-list/2020-December/msg00810.html

To re-explain:

a) On entry to virLXCProcessSetupInterfaceTap() if net->ifname != NULL
   then a copy of net->ifname is made into parentVeth, and a reference
   to *that* pointer is sent down to virNetDevVethCreate().

b) If parentVeth (aka net->ifname) is a template name (e.g. "blah%d"),
   then virNetDevVethCreate() calls virNetDevGenerateName(), and if
   virNetDevGenerateName() successfully generates a usable name
   (e.g. "blah27") then it will free the original template string
   (which is pointed to by net->ifname and by parentVeth), then
   replace the pointer in parentVeth with a pointer to the new
   string. Note that net->ifname still points to the now-freed
   template string.

c) returning back up to virLXCProcessSetupInterfaceTap(), we check if
   net->ifname == NULL - it *isn't* (still contains stale pointer to
   template string), so we don't replace it with the pointer to the new
   string that is in parentVeth.

d) Result: the new string is leaked once we return from
   virLXCProcessSetupInterfaceTap(), while there is a dangling pointer
   to the old string in net->ifname.

There is also a leak if there is a failure somewhere between steps (b)
and (c) above - the failure cleanup in virNetDevVethCreate() will only
free the newly-generated parentVeth string if the original pointer was
NULL (narrator: "It wasn't."). But it's a new string allocated by
virNetDevGenerateName(), not the original string from net->ifname, so
it really does need to be freed.

The solution is to make a copy of the entire original string into a
g_autofree pointer, then iff everything is successful we g_free() the
original net->ifname and replace it by stealing the string returned by
virNetDevVethCreate().

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-07 19:41:27 -05:00
Laine Stump
84617bf2f8 lxc: remove unnecessary call to virNetDevReserveName()
In all cases *except* when parsing status XML as libvirt is being
restarted, the XML parser will delete any manually specified interface
name (aka "<target dev='blah'/>" aka net->ifname) that could have been
generated by virNetDevGenerateName(). This means that during the setup
when a domain is being started (e.g. during
virLXCProcessSetupInterfaceTap()) it is pointless to call
virNetDevReserveName() with any setting of net->ifname that has come
from the XML parser - it is guaranteed to not fit the pattern of any
auto-generated name, and so the call is just a NOP anyway.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-07 19:41:27 -05:00
Pavel Hrdina
e1894cf490 virfile: refactor virFileNBDDeviceAssociate
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>
2021-01-06 13:15:17 +01:00
Michal Privoznik
64edf25c35 lxd_domain: Require that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE is zero
Our parser code relies on the fact that
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE has value of zero and thus
uses g_new0().  But strictly speaking, this is not mandated by
the enum typedef. Fix that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2020-12-17 09:42:37 +01:00
Michal Privoznik
fe983e4c50 lxc: Rework lxcDomainDefNamespaceParse()
While fixing our schema for <lxc:namespace/> I've looked into the
parser and realized it could use some treating.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2020-12-17 09:42:32 +01:00
Michal Privoznik
6ac44c6334 lxc: Allow NULL argument to lxcDomainDefNamespaceFree()
As all other free functions, NULL should be accepted. Even though
there currently is no caller that would pass NULL, there will be
in future patches.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2020-12-17 09:42:08 +01:00
Laine Stump
4252318bb3 lxc: skip the netdev autogenerated name counter past existing devices
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-12-16 21:32:12 -05:00
Shi Lei
ecfc2d5f43 lxc: fix a memory leak
In virLXCProcessSetupInterfaceTap, containerVeth needs to be freed on
failure.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2020-12-16 14:43:51 -05:00
Shi Lei
2dd0fb492f netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName
Simplify virNetDevVethCreate by using common GenerateName/ReserveName
functions.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2020-12-15 13:35:39 -05:00
Laine Stump
84dc367e2a lxc: don't try to reserve macvtap name for LXC domains
Commit 729a06c41 added code to the LXC driver (patterned after similar
code in the QEMU driver) that called
virNetDevMacVlanReserveName(net->ifname) for all type='direct'
interfaces during a libvirtd restart, to prevent other domains from
attempting to use a macvtap device name that was already in use by a
domain.

But, unlike a QEMU domain, when an LXC domain creates a macvtap
device, that device is almost immediately moved into the namespace of
the container (and it's then renamed, but that part isn't
important). Because of this, the LXC driver doesn't keep track (in
net->ifname) of the name used to create the device (as the QEMU driver
does).

The result of this is that if libvirtd is restarted while there is an
active LXC domain that has <interface type='direct'>, libvirtd will
segfault (since virNetDevMacVLanReserveName() doesn't check for a NULL
pointer).

The fix is to just not call that function in the case of the LXC
driver, since it is pointless anyway.

Fixes: 729a06c41a
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2020-12-15 09:27:55 -05:00
Daniel Henrique Barboza
69f30cfc67 domain_conf: move net device validation to domain_validate.c
The next objective is to move virDomainDeviceDefValidate() to
domain_validate.c. First let's move all the static helpers.

The net device validation functions are used across multiple
drivers, so let's move them separately first.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-12-14 09:32:31 -03:00
Jim Fehlig
cf4e7e620a lxc: Set default security model in XML parser config
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>
2020-12-07 10:41:15 -07:00
Michal Privoznik
b0d3053a2b lxc: Cleanup after failed startup
If starting an container fails, the virLXCProcessStop() is
called. But since vm->def->id is not set until libvirt_lxc is
spawned (the domain's ID is PID of that process),
virLXCProcessStop() returns early as virDomainObjIsActive()
returns false. But doing so leaves behind resources reserved for
the containers during the startup process. Most notably, hostdevs
are not re-attached to the host, the domain's transient XML is
not removed, etc.

To resolve this, virLXCProcessCleanup() is called in this case.
However, it is modified to accept @flags which allows caller to
run only specific cleanups (depending how far in container
creation the failure occurred). There is plenty of cleanups which
don't need this guard because either they detect a NULL pointer
or try to release an unique resource.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2020-12-07 10:12:32 +01:00
Michal Privoznik
b7d4e6b67e lib: Replace VIR_AUTOSTRINGLIST with GStrv
Glib provides g_auto(GStrv) which is in-place replacement of our
VIR_AUTOSTRINGLIST.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-12-02 15:43:07 +01:00
Matt Coleman
736c9ea02d domain_conf: make virDomainDiskInsert() void
This function always returns zero, so it might as well be void.

Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-11 12:53:11 +01:00
Matt Coleman
b1423cf1ea domain_conf: make virDomainDiskSetSource() void
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>
2020-11-05 16:31:16 +01:00
Jonathon Jongsma
08f8fd8413 conf: Add support for vDPA network devices
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>
2020-10-20 14:46:52 -04:00
Pavel Hrdina
a9bb02cfc9 lxc: use g_autoptr for virCgroup
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
2020-10-09 16:24:42 +02:00
Pavel Hrdina
5ad8272888 util: vircgroup: change virCgroupFree to take only virCgroupPtr
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>
2020-10-09 16:24:35 +02:00
Peter Krempa
cb6fdb0125 virBitmapNew: Don't check return value
Remove return value check from all callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-10-05 15:38:47 +02:00
Ján Tomko
e4116eaa44 rpc: require write acl for guest agent in virDomainInterfaceAddresses
CVE-2020-25637

Add a requirement for domain:write if source is set to
VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-09-30 11:42:28 +02:00
Pavel Hrdina
bf50ada8c1 src/lxc/lxc_controller: convert to use GLib DBus
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-09-17 18:20:05 +02:00
Martin Kletzander
9514e24984 Do not report error when setting affinity is allowed to fail
Suggested-by: Ján Tomko <jtomko@redhat.com>

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-09-07 11:35:36 +02:00
Ján Tomko
6fab37da59 Prefer https: everywhere where possible
Use https: links for websites that support them.

The URIs which are used as namespace identifiers
are left alone.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
2020-09-01 21:58:46 +02:00
Laine Stump
d7f38beb2e util: replace macvtap name reservation bitmap with a simple counter
There have been some reports that, due to libvirt always trying to
assign the lowest numbered macvtap / tap device name possible, a new
guest would sometimes be started using the same tap device name as
previously used by another guest that is in the process of being
destroyed *as the new guest is starting.

In some cases this has led to, for example, the old guest's
qemuProcessStop() code deleting a port from an OVS switch that had
just been re-added by the new guest (because the port name is based on
only the device name using the port). Similar problems can happen (and
I believe have) with nwfilter rules and bandwidth rules (which are
both instantiated based on the name of the tap device).

A couple patches have been previously proposed to change the ordering
of startup and shutdown processing, or to put a mutex around
everything related to the tap/macvtap device name usage, but in the
end no matter what you do there will still be possible holes, because
the device could be deleted outside libvirt's control (for example,
regular tap devices are automatically deleted when the qemu process
terminates, and that isn't always initiated by libvirt but could
instead happen completely asynchronously - libvirt then has no control
over the ordering of shutdown operations, and no opportunity to
protect it with a mutex.)

But this only happens if a new device is created at the same time as
one is being deleted. We can effectively eliminate the chance of this
happening if we end the practice of always looking for the lowest
numbered available device name, and instead just keep an integer that
is incremented each time we need a new device name. At some point it
will need to wrap back around to 0 (in order to avoid the IFNAMSIZ 15
character limit if nothing else), and we can't guarantee that the new
name really will be the *least* recently used name, but "math"
suggests that it will be *much* less common that we'll try to re-use
the *most* recently used name.

This patch implements such a counter for macvtap/macvlan, replacing
the existing, and much more complicated, "ID reservation" system. The
counter is set according to whatever macvtap/macvlan devices are
already in use by guests when libvirtd is started, incremented each
time a new device name is needed, and wraps back to 0 when either
INT_MAX is reached, or when the resulting device name would be longer
than IFNAMSIZ-1 characters (which actually is what happens when the
template for the device name is "maccvtap%d"). The result is that no
macvtap name will be re-used until the host has created (and possibly
destroyed) 99,999,999 devices.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-09-01 14:16:36 -04:00
Ján Tomko
52cd849e62 VIR_XPATH_NODE_AUTORESTORE: remove semicolon from users
Since the macro no longer includes the 'ignore_value'
statement, stop putting another empty statement after it.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-08-25 19:03:12 +02:00
Ján Tomko
96b4f38603 Move debug statements after declarations
Many of our functions start with a DEBUG statement.
Move the statements after declarations to appease
our coding style.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-08-25 19:03:11 +02:00