Commit Graph

42 Commits

Author SHA1 Message Date
Daniel Henrique Barboza
c5d86a9834 remote: remove unneeded cleanup labels
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-11-12 17:54:01 +01:00
Michal Privoznik
2b41a017ab remote: Use g_strdup_printf() instead of virAsprintf()
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2019-11-12 16:15:58 +01:00
Ján Tomko
b2d079c113 remote: use g_strdup instead of VIR_STRDUP
Replace all occurrences of
  if (VIR_STRDUP(a, b) < 0)
     /* effectively dead code */
with:
  a = g_strdup(b);

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-10-21 12:51:58 +02:00
Ján Tomko
45678bd70a Use g_autoptr instead of VIR_AUTOPTR
Since commit 44e7f02915
    util: rewrite auto cleanup macros to use glib's equivalent

VIR_AUTOPTR aliases to g_autoptr. Replace all of its use by the GLib
macro version.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-10-16 12:06:43 +02:00
Ján Tomko
67e72053c1 Use G_N_ELEMENTS instead of ARRAY_CARDINALITY
Prefer the GLib version of the macro.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-10-15 16:14:19 +02:00
Ján Tomko
6727ca6b2a remote: use G_GNUC_UNUSED
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-10-15 11:25:24 +02:00
Daniel P. Berrangé
b74a95d6a2 remote: convert methods using virIdentityPtr to auto free macros
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-10-14 10:54:42 +01:00
Daniel P. Berrangé
0ad9f12e75 remote: fix systemd IP socket activation with virtproxyd
We recently forbid the use of --listen with socket activation:

  commit 3a6a725b8f
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Aug 22 14:52:16 2019 +0100

      remote: forbid the --listen arg when systemd socket activation

In this change we forgot that virtproxyd doesn't have a --listen
parameter, and instead behaves as if it was always present. Thus
when systemd socket activation is present, we must disable this
built-in default

Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-09-30 12:04:29 +01:00
Michael Chapman
522b3d2b24 remote: fix registration of TLS socket
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
2019-09-17 10:35:53 +01:00
Daniel P. Berrangé
b1b878c512 util: activate directory override when used from library
The Perl bindings for libvirt use the test driver for unit tests. This
tries to load the cpu_map/index.xml file, and when run from an
uninstalled build will fail.

The problem is that virFileActivateDirOverride is called by our various
binaries like libvirtd, virsh, but is not called when a 3rd party app
uses libvirt.so

To deal with this we allow the LIBVIRT_DIR_OVERRIDE=1 env variable to be
set and make virInitialize look for this. The 'run' script will set it,
so now build using this script to run against an uninstalled tree we
will correctly resolve files to the source tree.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-09-10 11:03:35 +01:00
eater
ec78c9a0ff remote: fix UNIX socket path being incorrectly built for libvirtd
As a result of changes in

      commit d5f0c1b6dd
      Author: Daniel P. Berrangé <berrange@redhat.com>
      Date:   Thu Jul 18 12:30:22 2019 +0100

        remote: stop trying to print help as giant blocks of text

The socket path built would be libvirt//var/run/libvirt-sock
instead of /var/run/libvirt/libvirt-sock. Fortunately this only
affects users who have set the 'unix_sock_dir' config parameter
in /etc/libvirt/libvirtd.conf, which is pretty rare/unusual.

Signed-off-by: eater <=@eater.me>

Exception made for the psuedonym above since patch is considered
trivial & thus non-copyrightable material.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-09-09 11:01:16 +01:00
Daniel P. Berrangé
3a6a725b8f remote: forbid the --listen arg when systemd socket activation
When using systemd socket activation the --listen arg has no
effect. This is confusing to users upgrading from previous versions of
libvirt as their config is silently ignored. Turn use of --listen into a
fatal error when sockets are passed from systemd.

This helps the admin discover the change in behaviour and thus decide
whether to stick with socket activation or revert to previous behaviour.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-27 10:46:37 +01:00
Daniel P. Berrangé
d29c917ef4 src: honour the RUNSTATEDIR variable in all code
All code using LOCALSTATEDIR "/run" is updated to use RUNSTATEDIR
instead. The exception is the remote driver client which still
uses LOCALSTATEDIR "/run". The client needs to connect to remote
machines which may not be using /run, so /var/run is more portable
due to the /var/run -> /run symlink.

Some duplicate paths in the apparmor code are also purged.

There's no functional change by default yet since both expressions
expand to the same value.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-27 10:23:13 +01:00
Daniel P. Berrangé
b7ed8ce981 remote: introduce virtproxyd daemon to handle IP connectivity
The libvirtd daemon provides the traditional libvirt experience where
all the drivers are in a single daemon, and is accessible over both
local UNIX sockets and remote IP sockets.

In the new world we're having a set of per-driver daemons which will
primarily be accessed locally via their own UNIX sockets.

We still, however, need to allow for case of applications which will
connect to libvirt remotely. These remote connections can be done as
TCP/TLS sockets, or by SSH tunnelling to the UNIX socket.

In the later case, the old libvirt.so clients will only know about
the path to the old libvirtd socket /var/run/libvirt/libvirt-sock,
and not the new driver sockets /var/run/libvirt/virtqemud-sock.

It is also not desirable to expose the main driver specific daemons
over IP directly to minimize their attack service.

Thus the virtproxyd daemon steps into place, to provide TCP/TLS sockets,
and back compat for the old libvirtd UNIX socket path(s). It will then
forward all RPC calls made to the appropriate driver specific daemon.

Essentially it is equivalent to the old libvirtd with absolutely no
drivers registered except for the remote driver (and other stateless
drivers in libvirt.so).

We could have modified libvirtd so none of the drivers are registed
to get the same end result. We could even add a libvirtd.conf parameter
to control whether the drivers are loaded to enable users to switch back
to the old world if we discover bugs in the split-daemon model. Using a
new daemon though has some advantages

 - We can make virtproxyd and the virtXXXd per-driver daemons all
   have "Conflicts: libvirtd.service" in their systemd unit files.
   This will guarantee that libvirtd is never started at the same
   time, as this would result in two daemons running the same driver.
   Fortunately drivers use locking to protect themselves, but it is
   better to avoid starting a daemon we know will conflict.

 - It allows us to break CLI compat to remove the --listen parameter.
   Both listen_tcp and listen_tls parameters in /etc/libvirtd/virtd.conf
   will default to zero. Either TLS or TCP can be enabled exclusively
   though virtd.conf without requiring the extra step of adding --listen.

 - It allows us to set a strict SELinux policy over virtproxyd. For
   back compat the libvirtd policy must continue to allow all drivers
   to run. We can't easily give a second policy to libvirtd which
   locks it down. By introducing a new virtproxyd we can set a strict
   policy for that daemon only.

 - It gets rid of the weird naming of having a daemon with "lib" in
   its name. Now all normal daemons libvirt ships will have "virt"
   as their prefix not "libvirt".

 - Distros can more easily choose their upgrade path. They can
   ship both sets of daemons in their packages, and choose to
   either enable libvirtd, or enable the per-driver daemons and
   virtproxyd out of the box. Users can easily override this if
   desired by just tweaking which systemd units are active.

After some time we can deprecate use of libvirtd and after some more
time delete it entirely, leaving us in a pretty world filled with
prancing unicorns.

The main downside with introducing a new daemon, and with the
per-driver daemons in general, is figuring out the correct upgrade
path.

The conservative option is to leave libvirtd running if it was
an existing installation. Only use the new daemons & virtproxyd
on completely new installs.

The aggressive option is to disable libvirtd if already running
and activate all the new daemons.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:31 +01:00
Daniel P. Berrangé
4ce29411fc remote: in per-driver daemons ensure that state initialize succeeds
When running in libvirtd, we are happy for any of the drivers to simply
skip their initialization in virStateInitialize, as other drivers are
still potentially useful.

When running in per-driver daemons though, we want the daemon to abort
startup if the driver cannot initialize itself, as the daemon will be
useless without it.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:31 +01:00
Daniel P. Berrangé
2204585167 remote: conditionalize IP socket usage in libvirtd daemon
Prepare for reusing libvirtd source to create other daemons by making
the use of IP sockets conditionally defined by the make rules.

The main libvirtd daemon will retain IP listen ability, but all the
driver specific daemons will be local UNIX sockets only. Apps needing
IP connectivity will connect via the libvirtd daemon which will proxy
to the driver specfic daemon.

Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:31 +01:00
Daniel P. Berrangé
5d3475c8dc remote: conditionalize driver loading in libvirtd daemon
Prepare for reusing libvirtd source to create other daemons by making
the driver(s) to load conditionally defined by the make rules.

If nothing is set, all drivers will be loaded, ignoring any missing ones
as historically done.

If MODULE_NAME is set only one driver will be loaded and that one must
succeed.

Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:31 +01:00
Daniel P. Berrangé
74d76d6478 remote: conditionalize daemon name in libvirtd daemon
Prepare for reusing libvirtd source to create other daemons by making
the daemon name conditionally defined by the make rules.

Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:31 +01:00
Daniel P. Berrangé
d5f0c1b6dd remote: stop trying to print help as giant blocks of text
The remote daemon tries to print out its help text in a couple of giant
blocks of text. This has already lead to duplication of the text for the
privileged vs unprivileged execution mode. With the introduction of more
daemons, this text is going to be duplicated many more times with small
variations. This is very unfriendly to translators as they have to
translate approximately the same text many times with small tweaks.

Splitting the text up into individual strings to print means that each
piece will only need translating once. It also gets rid of all the
layout information from the translated strings, so avoids the problem of
translators breaking formatting by mistake.

Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-09 14:06:22 +01:00
Daniel P. Berrangé
80d387c62f logging: pass binary name not logfile name when enabling logging
Instead of having each caller pass in the desired logfile name, pass in
the binary name instead. The logging code can then just derive a logfile
name by appending ".log".

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-07-19 14:33:18 +01:00
Daniel P. Berrangé
e2f012b8f0 remote: fix handling of systemd activation wrt socket ordering
The current libvirtd code for systemd socket activation assumes socket
FDs are passed in the order unix-rw, unix-ro, unix-admin.  There is in
fact no ordering guarantee made by systemd. Applications are expected
to check the address or name associated with each FD to figure out its
identity.

This rewrites libvirtd to make use of the new systemd activation APIs
to make it robust wrt socket ordering changes.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-07-12 16:55:40 +01:00
Daniel P. Berrangé
e9095c328a remote: simplify libvirtd code for deciding if SASL auth is needed
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-07-12 16:55:40 +01:00
Daniel P. Berrangé
5a148ce846 remote: delete the avahi mDNS support
Libvirtd has long had integration with avahi for advertising libvirtd
using mDNS when TCP/TLS listening is enabled. For a long time the
virt-manager application had support for auto-detecting libvirtds
on the local network using mDNS, but this was removed last year

  commit fc8f8d5d7e3ba80a0771df19cf20e84a05ed2422
  Author: Cole Robinson <crobinso@redhat.com>
  Date:   Sat Oct 6 20:55:31 2018 -0400

    connect: Drop avahi support

    Libvirtd can advertise itself over avahi. The feature is disabled by
    default though and in practice I hear of no one actually using it
    and frankly I don't think it's all that useful

    The 'Open Connection' wizard has a disproportionate amount of code
    devoted to this feature, but I don't think it's useful or worth
    maintaining, so let's drop it

I've never heard of any other applications having support for using
mDNS to detect libvirtd instances. Though it is theoretically possible
something exists out there, it is clearly going to be a niche use case
in the virt ecosystem as a whole.

By removing avahi integration we can cut down the dependency chain for
the basic libvirtd install and reduce our code maint burden.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-06-21 12:59:42 +01:00
Daniel P. Berrangé
e10310d641 remote: drop code for migrating config files from pre-XDG dir layout
The unprivileged libvirtd daemon switched to use the XDG dir layout in
the 0.9.13 release, and included code for moving config files from the
old location. The chances of someone upgrading libvirt from <= 0.9.12
directly to libvirt >= 5.5.0 is close enough to zero that we can
reasonably drop the back compat code.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-06-21 12:59:06 +01:00
Cole Robinson
1d31526b52 Always put _LAST enums on second line of VIR_ENUM_IMPL
Standardize on putting the _LAST enum value on the second line
of VIR_ENUM_IMPL invocations. Later patches that add string labels
to VIR_ENUM_IMPL will push most of these to the second line anyways,
so this saves some noise.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-04-11 12:47:23 -04:00
Michal Privoznik
c6266ddb02 daemon: Register secret driver before storage driver
The order in which drivers are registered is important because
their stateInitialize and stateAutoStart callback are called in
that order. Well, stateAutoStart is going away and therefore if
there is some dependency between two drivers (e.g. when
initializing storage driver expects secret driver to be available
already), the registration of such drivers must happen in correct
order.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-03-06 10:20:31 +01:00
Cole Robinson
6a4d938dd3 Require a semicolon for VIR_ENUM_IMPL calls
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_IMPL calls.

Move the verify() statement to the end of the macro and drop
the semicolon, so the compiler will require callers to add a
semicolon.

While we are touching these call sites, standardize on putting
the closing parenth on its own line, as discussed here:
https://www.redhat.com/archives/libvir-list/2019-January/msg00750.html

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-03 17:46:29 -05:00
Cole Robinson
7662194bf3 Require a semicolon to VIR_ENUM_DECL calls
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>), and we have a mix of semicolon and
non-semicolon usage through the code. Let's standardize on using
a semicolon for VIR_ENUM_DECL calls.

Drop the semicolon from the final statement of the macro, so
the compiler will require callers to add a semicolon.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-03 17:46:29 -05:00
Michal Privoznik
29682196d8 Drop UML driver
The driver is unmaintained, untested and severely broken for
quite some time now. Since nobody even reported any issue with it
let us drop it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-12-17 17:52:46 +01:00
Daniel P. Berrangé
600462834f Remove all Author(s): lines from source file headers
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.

In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.

With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to  find the
author of a particular bit of code.

This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.

The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.

Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-12-13 16:08:38 +00:00
Erik Skultety
5165ff0971 src: More cleanup of some system headers already contained in internal.h
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
2018-09-20 10:16:39 +02:00
Ján Tomko
9ed59012d3 Revert "remote: daemon: Make sure that JSON symbols are properly loaded at startup"
This reverts commit 3251fc9c9b.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

https://bugzilla.redhat.com/show_bug.cgi?id=1614569

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-08-13 15:50:00 +02:00
Peter Krempa
3251fc9c9b remote: daemon: Make sure that JSON symbols are properly loaded at startup
Explicitly call virJSONInitialize at startup of the libvirt daemon so
that we are sure that the symbols in the compat library are properly
loaded. This will prevent any random failure from happening later on
when the daemon would want to use the JSON parser.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
2018-08-01 14:32:40 +02:00
Daniel P. Berrangé
f14c37ce4c nwfilter: convert virt drivers to use public API for nwfilter bindings
Remove the callbacks that the nwfilter driver registers with the domain
object config layer. Instead make the current helper methods call into
the public API for creating/deleting nwfilter bindings.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-06-26 18:17:23 +01:00
Michal Privoznik
234ce7d02f src: Drop most of #ifdef WITH_GNUTLS
Now that GnuTLS is a requirement, we can drop a lot of
conditionally built code. However, not all ifdef-s can go because
we still want libvirt_setuid to build without gnutls.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-06-05 14:32:36 +02:00
Daniel P. Berrangé
a91eaec152 remote: honour errors from virDriverLoadModule
The libvirtd daemon currently ignores the return status of
virDriverLoadModule entirely. This is way too loose, resulting in many
important problems going undiagnosed, resulting in a libvirtd that may
never work correctly. We should only ignore a non-existant module, and
pass back any fatal errors.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-04-24 17:01:49 +01:00
Daniel P. Berrangé
8a062f5fe3 driver: add option to make missing drivers a fatal problem
Currently the driver module loading code does not report an error if the
driver module is physically missing on disk. This is useful for distro
packaging optional pieces. When the daemons are split up into one daemon
per driver, we will expect module loading to always succeed. If a driver
is not desired, the entire daemon should not be installed.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-04-24 17:01:48 +01:00
Daniel P. Berrangé
87680332a3 remote: stop trying to load Xen driver module
The Xen driver was recently deleted, but libvirtd has left over code
that tries to use it. Fortunately this is dead code because WITH_XEN
will never be defined anymore.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-04-24 16:58:40 +01:00
Daniel P. Berrangé
da1ade7a52 remote: remove some __sun conditionals
The libvirtd daemon has some arbitrary logic to drop privileges, but
only on Solaris platforms. This was added during Xen days, when Xen was
the only driver running in libvirtd. There's no expectation or testing
that this works with the new libxl stack, nor whether dropping
privileges breaks any of the secondary drivers. Finally, we'll be
splitting drivers out into their own independant daemons, so this won't
be applicable to libvirtd in future anyway.

The remote driver client meanwhile arbitrarily disables daemon
auto-spawn when connecting as non-root, breaking a key feature of
libvirt unprivileged connections.

Since we've not had any contributions for Solaris since circa 2012
and we don't do any CI testing we should consider this platform
unmaintained and thus reasonable to remove this cruft. If someone steps
forward to maintain Solaris again, this code would need re-evaluating to
come up with something more targetted.

There's various __sun conditionals in the Xen driver code, but those are
not touched. This is all for the legacy Xen driver, which will be
entirely removed at some point in future, so not benefit to hacking out
just the Solaris parts.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-23 15:10:25 +00:00
Marc Hartmayer
9f24004b82 remote: remove unneeded global variables
Remove unneeded global variables and convert them into local variables
where they're needed.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-03-15 17:12:28 -04:00
Jim Fehlig
33c6eb9689 libvirtd: fix potential deadlock when reloading
It is possible to deadlock libvirtd when concurrently starting a domain
and restarting the daemon. Threads involved in the deadlock are

Thread 4 (Thread 0x7fc13b53e700 (LWP 64084)):
/lib64/libpthread.so.0
    at util/virthread.c:154
    at qemu/qemu_monitor.c:1083
    cmd=0x7fc110017700, scm_fd=-1, reply=0x7fc13b53d318) at
qemu/qemu_monitor_json.c:305
cmd=0x7fc110017700,
    reply=0x7fc13b53d318) at qemu/qemu_monitor_json.c:335
    at qemu/qemu_monitor_json.c:1298
    at qemu/qemu_monitor.c:1697
    vm=0x7fc110003d00, asyncJob=QEMU_ASYNC_JOB_START) at qemu/qemu_process.c:1763
vm=0x7fc110003d00,
    asyncJob=6, logCtxt=0x7fc1100089c0) at qemu/qemu_process.c:1835
    vm=0x7fc110003d00, asyncJob=6, logCtxt=0x7fc1100089c0) at
qemu/qemu_process.c:2180
driver=0x7fc12004e1e0,
    vm=0x7fc110003d00, asyncJob=QEMU_ASYNC_JOB_START, incoming=0x0, snapshot=0x0,
    vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, flags=17) at qemu/qemu_process.c:6111
driver=0x7fc12004e1e0,
    vm=0x7fc110003d00, updatedCPU=0x0, asyncJob=QEMU_ASYNC_JOB_START,
migrateFrom=0x0,
    migrateFd=-1, migratePath=0x0, snapshot=0x0,
vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
    flags=17) at qemu/qemu_process.c:6334
    xml=0x7fc110000ed0 "<!--\nWARNING: THIS IS AN AUTO-GENERATED FILE.
CHANGES TO IT ARE LIKELY TO BE\nOVERWRITTEN AND LOST. Changes to this xml
configuration should be made using:\n  virsh edit testvv\nor other
applicati"..., flags=0) at qemu/qemu_driver.c:1776
...

Thread 1 (Thread 0x7fc143c66880 (LWP 64081)):
/lib64/libpthread.so.0
    at util/virthread.c:122
conf/nwfilter_conf.c:159
sig=0x7ffe0a831e30,
    opaque=0x0) at remote/remote_daemon.c:724
    opaque=0x558c5328b230) at rpc/virnetdaemon.c:654
    at util/vireventpoll.c:508
rpc/virnetdaemon.c:858
remote/remote_daemon.c:1496
(gdb) thr 1
[Switching to thread 1 (Thread 0x7fc143c66880 (LWP 64081))]
/lib64/libpthread.so.0
(gdb) f 1
    at util/virthread.c:122
122	    pthread_rwlock_wrlock(&m->lock);
(gdb) p updateLock
$1 = {lock = {__data = {__lock = 0, __nr_readers = 1, __readers_wakeup = 0,
      __writer_wakeup = 0, __nr_readers_queued = 0, __nr_writers_queued = 1,
__writer = 0,
      __shared = 0, __rwelision = 0 '\000', __pad1 = "\000\000\000\000\000\000",
      __pad2 = 0, __flags = 0},
    __size = "\000\000\000\000\001", '\000' <repeats 15 times>, "\001",
'\000' <repeats 34 times>, __align = 4294967296}}

Reloading of the nwfilter driver is stuck waiting for a write lock, which
already has a reader (from qemuDomainCreateXML) in the critical section.
Since the reload occurs in the context of the main event loop thread,
libvirtd becomes deadlocked. The deadlock can be avoided by offloading
the reload work to a thread.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-12 10:30:13 -06:00
Daniel P. Berrangé
91fd3dc932 daemon: move libvirtd code into src/remote/ directory
Having a daemon/ directory makes little sense from a code structure
point of view, as 90% of the code that is built into libvirtd already
lives in the src/ directory. The virtlockd and virlogd daemons also live
entirely in src/{locking,logging} directories. This moves the source
code for libvirtd into src/remote/, alongside the client code.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-02-22 12:21:08 +00:00