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: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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: NCole Robinson <crobinso@redhat.com>

virutil.(c|h) is a very gross collection of random code. Remove the enum
handlers from there so we can limit the scope where virtutil.h is used.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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.htmlReviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

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: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

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: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

That is a job of libvirtd and virtlogd has a dependency on it, so that will
prevent it properly.  Doing it one extra time in virtlogd might also cause AVC
denials because it is not allowed to call that dbus method.

Caused by commit df34363d.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1547250Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Currently both virtlogd and virtlockd use a single worker thread for
dispatching RPC messages. Even this is overkill and their RPC message
handling callbacks all run in short, finite time and so blocking the
main loop is not an issue like you'd see in libvirtd with long running
QEMU commands.

By setting max_workers==0, we can turn off the worker thread and run
these daemons single threaded. This in turn fixes a serious problem in
the virtlockd daemon whereby it loses all fcntl() locks at re-exec due
to multiple threads existing. fcntl() locks only get preserved if the
process is single threaded at time of exec().
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NJim Fehlig <jfehlig@suse.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Add a virtlogd-admin-sock can serves the admin protocol for the virtlogd
daemon and define a virtlogd:///{system,session}  URI scheme for
connecting to it.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

With the current code it is neccessary to call

  virNetDaemonNewPostExecRestart()

and then for each server that needs restarting you are supposed
to call

  virNetDaemonAddSeverPostExecRestart()

This is fine if there's only ever one server, but as soon as you
have two servers it is impossible to use this design. The code
has no idea which servers were recorded in the JSON state doc,
nor in which order the hash table serialized its keys.

So this patch changes things so that we only call

  virNetDaemonNewPostExecRestart()

passing in a callback, which is invoked once for each server
found int he JSON state doc.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

We put the server into a hash table as we do with the other daemons,
there is no compelling reason why it should have another pointer
dedicated just to the server. Besides, the locking daemon doesn't have
it and virtlogd is essentially a copy paste of virtlockd.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Commit 94c465d0 refactored the logging setup phase but introduced an
issue, where the daemon ignores verbose mode when there are no outputs
defined and the default must be used. The problem is that the default
output was determined too early, thus ignoring the potential '--verbose'
option taking effect. This patch postpones the creation of the default
output to the very last moment when nothing else can change. Since the
default output is only created during the init phase, it's safe to leave
the pointer as NULL for a while, but it will be set eventually, thus not
affecting runtime.
Patch also adjusts both the other daemons.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1442947Signed-off-by: NErik Skultety <eskultet@redhat.com>

The log and lock protocol don't have an extra handshake to close the
connection. Instead they just close the socket. Unfortunately that
resulted into a lot of spurious garbage logged to the system log files:

2017-03-17 14:00:09.730+0000: 4714: error : virNetSocketReadWire:1800 : End of file while reading data: Input/output error

or in the journal as:

Mar 13 16:19:33 xxxx virtlogd[32360]: End of file while reading data: Input/output error

Use the new facility in the netserverclient to suppress the IO error
report from the virNetSocket layer.

Now that virLog{Get,Set}DefaultOutput routines are introduced we can wire them
up to the daemon's logging initialization code. Also, change the order of
operations a bit so that we still strictly honor our precedence of settings:
cmdline > env > config now that outputs and filters are not appended anymore.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Along with an empty string, it should also be possible for users to pass
NULL to the public APIs which in turn would trigger a routine(future
work) responsible for defining an appropriate default logging output
given the current circumstances.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Similar to outputs, parser should do parsing only, thus the 'define' logic
is going to be stripped from virLogParseAndDefineFilters by replacing calls to
this method to virLogSetFilters instead.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Since virLogParseAndDefineOutputs is going to be stripped from 'output defining'
logic, replace all relevant occurrences with virLogSetOutputs call to make the
change transparent to all original callers (daemons mostly).
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Right now virLogParse* functions are doing both parsing and defining of filters
and outputs which should be two separate operations. Since the naming is
apparently a bit poor this patch renames these functions to
virLogParseAndDefine* which eventually will be replaced by virLogSet*.
Additionally, virLogParse{Filter,Output} will be later (after the split) reused,
so that these functions do exactly what the their name suggests.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Currently virtlogd has a hardcoded max file size of 128kb
and max of 3 backups. This adds two new config parameters
to /etc/libvirt/virtlogd.conf to let these be customized.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Reorder code for setting default log level from cmdline prior
initialization of log outputs. Thus the --verbose option is reflected.

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325072

Convert to virGetLastErrorMessage() in the rest of the code

Admin API needs a way of addressing specific clients. Unlike servers, which we
are happy to address by names both because its name reflects its purpose (to
some extent) and we only have two of them (so far), naming clients doesn't make
any sense, since a) each client is an anonymous, i.e. not recognized after a
disconnect followed by a reconnect, b) we can't predict what kind of requests
it's going to send to daemon, and c) the are loads of them comming and going,
so the only viable option is to use an ID which is of a reasonably wide data
type.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Take setlocale/gettext error handling pattern from tools/virsh-*
and use it for all standalone binaries via a new shared
virGettextInitialize routine. The virsh* pattern differed slightly
from other callers. All users now consistently:

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror

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

Since servers know their name, there is no need to supply such
information twice.  Also defeats inconsistencies.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

At first I did not want to do this, but after trying to implement some
newer feaures in the admin API I realized we need that to make our lives
easier.  On the other hand they are not saved redundantly and the
virNetServer objects are still kept in a hash table.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Since the daemon can manage and add (at fresh start) multiple servers,
we also should be able to add them from a JSON state file in case of a
daemon restart, so post exec restart support for multiple servers is also
provided. Patch also updates virnetdaemontest accordingly.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

With some versions of GLibC / GCC, a variable called 'daemon'
will result in a warning about clashing with the function also
named 'daemon'. Rename it to 'dmn' to avoid the clash.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

The virtlogd daemon is launched with a 30 second timeout for
unprivileged users. Unfortunately the timeout is only inhibited
while RPC clients are connected, and they only connect for a
short while to open the log file descriptor. We need to hold
an inhibition for as long as the log file descriptor itself
is open.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Define a new RPC protocol for the virtlogd daemon that provides
for handling of logs. The initial RPC method defined allows a
client to obtain a file handle to use for writing to a log
file for a guest domain. The file handle passed back will not
actually refer to the log file, but rather an anonymous pipe.
The virtlogd daemon will forward I/O between them, ensuring
file rotation happens when required.

Initially the log setup is hardcoded to cap log files at
128 KB, and keep 3 backups when rolling over, which gives
a max usage of 512 KB per guest.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Copy the virtlockd codebase across to form the initial virlogd
code. Simple search & replace of s/lock/log/ and gut the remote
protocol & dispatcher. This gives us a daemon that starts up
and listens for connections, but does nothing with them.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Lets use wrapper functions virLockDaemonLock and
virLockDaemonUnlock instead of virMutexLock and virMutexUnlock.
This has no functional impact, but it's easier to read (at least
for me).
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

So we have this mechanism that on SIGUSR1 the virtlockd dumps its
internal state into a JSON file, reexec itself and the reloads
the internal state back. However, there's a bug in our
implementation:

  (gdb) signal SIGUSR1
  Continuing with signal SIGUSR1.
  [Thread 0x7fd094f7b700 (LWP 10602) exited]
  process 10600 is executing new program: /home/zippy/work/libvirt/libvirt.git/src/virtlockd
  warning: Could not load shared library symbols for linux-vdso.so.1.
  Do you need "set solib-search-path" or "set sysroot"?
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib64/libthread_db.so.1".
  [New Thread 0x7fb28bc3c700 (LWP 14501)]

  Program received signal SIGSEGV, Segmentation fault.
  0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
  288         if (*countptr + add < *countptr) {
  (gdb) bt
  #0  0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
  #1  0x00007fb29132a267 in virNetServerAddProgram (srv=0x0, prog=0x7fb2915d08b0) at rpc/virnetserver.c:661
  #2  0x00007fb29131f27f in main (argc=1, argv=0x7fff8f771298) at locking/lock_daemon.c:1445

Notice the NULL @srv passed to frame 2? Usually, the @srv
variable is initialized on fresh start. However, in case of
daemon reload, the code path that is responsible for initializing
the value was not triggered and therefore we crashed immediately.
Fix this by always setting the variable.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The internal representation of a JSON array counts the items in
size_t. However, for some reason, when asking for the count it's
reported as int. Firstly, we need the function to return a signed
type as it's returning -1 on an error. But, not every system has
integer the same size as size_t. Therefore, lets return ssize_t.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Now that we have virNetDaemon object holding all the data and being
capable of referencing multiple servers, having a duplicate reference to
a single server stored in virLockDaemon isn't necessary anymore. This
patch removes the above described element.

Since its introduction in 2011 (particularly in commit f4324e32),
the option doesn't work.  It just effectively disables all incoming
connections.  That's because the client private data that contain the
'keepalive_supported' boolean, are initialized to zeroes so the bool is
false and the only other place where the bool is used is when checking
whether the client supports keepalive.  Thus, according to the server,
no client supports keepalive.

Removing this instead of fixing it is better because a) apparently
nobody ever tried it since 2011 (4 years without one month) and b) we
cannot know whether the client supports keepalive until we get a ping or
pong keepalive packet.  And that won't happen until after we dispatched
the ConnectOpen call.

Another two reasons would be c) the keepalive_required was tracked on
the server level, but keepalive_supported was in private data of the
client as well as the check that was made in the remote layer, thus
making all other instances of virNetServer miss this feature unless they
all implemented it for themselves and d) we can always add it back in
case there is a request and a use-case for it.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This allows to have more servers in one daemon which helps isolating
some resources.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Some hypervisors like Xen do not have PIDs associated with domains.
Relax the requirement for PID != 0 in the locking code so it can
be used by hypervisors that do not represent domains as a process
running on the host.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>