So far clients were closed when disposing the daemon, after the state
driver cleanup. This was leading to libvirtd crashing at shutdown due
to missing driver.

Moving the client close in virNetServerClose() fixes the problem.
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Use the return value of virObjectRef directly. This way, it's easier
for another reader to identify the reason why the additional reference
is required.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>

Prior to commit 2737aaaf, we allowed every client to connect successfully,
however, if accepting a client would eventually lead to an overcommit of the
limits, we would disconnect it immediately with "Too many active clients,
dropping connection from...". Recent changes refactored the code in a way, that
it is not possible for the client-related callback to be dispatched and the
client to be accepted if the limits wouldn't permit to do so, therefore a check
if a connection should be dropped due to limits violation has become a dead
code that could be removed.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Commit 2737aaaf changed our policy for accepting new clients in a way, that
instead of accepting new clients only to disconnect them immediately, since
that would overcommit the limit, we temporarily disable polling for the
dedicated file descriptor, so any new connection will queue on the socket.
Commit 8b1f0469 then added the possibility to change the limits during runtime
but it didn't re-enable polling for the previously disabled file descriptor,
thus any new connection would still continue to queue on the socket. This patch
forces an update of the services each time the limits were changed in some way.

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

So far, virNetServerCheckLimits was only used to possibly re-enable accepting
new clients that might have previously been disabled due to client limits
violation (max_clients, max_anonymous_clients). This patch refactors
virNetServerAddClient, which is currently the only place where the services get
disabled, in order to use the virNetServerCheckLimits helper instead of
checking the limits by itself.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Since virNetServerAddClient checks for the limits in order to temporarily
suspend the services, thus not accepting any more clients, there is no reason
why virNetServerCheckLimits, which is only responsible for re-enabling
previously disabled services according to the limits, could not do both. To be
able to do that however, it needs to be moved up in the file since it's static
(and because it's just a helper and there's only one caller it should remain
 static).
Signed-off-by: NErik Skultety <eskultet@redhat.com>

The original naming was just a leftover that should have been fixed in commit
8b1f0469.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Use it in virNetServerClientGetInfo to switch back to using
the URI-format (separated by ':') instead of the SASL format
(separated by ';').

Also use it in the error message reported by virNetServerAddClient.

Opposite operation to virAdmServerGetClientLimits. Understandably though,
setting values for current number of clients connected or still waiting
for authentication does not make sense, since changes to these values are event
dependent, i.e. a client connects - counter is increased. Thus only the limits
to maximum clients connected and waiting for authentication can be set. Should
a request for other controls to be set arrive (provided such a setting will
be first introduced to the config), the set of configuration controls can be
later expanded (thanks to typed params). This patch also introduces a
constraint that the maximum number of clients waiting for authentication has to
be less than the overall maximum number of clients connected and any attempt to
violate this constraint will be denied.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Add some trivial getters for client related attributes to virnetserver before
any admin method can be introduced.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Just like with server-related APIs, before any of client-based APIs can be
called, a reference to a client-side client object needs to be obtained. For
this purpose, a lookup method should exist. Apart from the client retrieval
logic, a new error code for non-existent client had to be added as well.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

For now, the list copy is done simply by locking the whole server, walking the
original and increasing the refcount on each object. We may want to change
the list to a lockable object (like list of domains) later in the future if
we discover some performance issues related to locking the whole server in
order to walk the whole list of clients, possibly issuing some 'ForEach'
callback.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

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>

Since threadpool increments the current number of threads according to current
load, i.e. how many jobs are waiting in the queue. The count however, is
constrained by max and min limits of workers. The logic of this new API works
like this:
    1) setting the minimum
        a) When the limit is increased, depending on the current number of
           threads, new threads are possibly spawned if the current number of
           threads is less than the new minimum limit
        b) Decreasing the minimum limit has no possible effect on the current
           number of threads
    2) setting the maximum
        a) Icreasing the maximum limit has no immediate effect on the current
           number of threads, it only allows the threadpool to spawn more
           threads when new jobs, that would otherwise end up queued, arrive.
        b) Decreasing the maximum limit may affect the current number of
           threads, if the current number of threads is less than the new
           maximum limit. Since there may be some ongoing time-consuming jobs
           that would effectively block this API from killing any threads.
           Therefore, this API is asynchronous with best-effort execution,
           i.e. the necessary number of workers will be terminated once they
           finish their previous job, unless other workers had already
           terminated, decreasing the limit to the requested value.
    3) setting priority workers
        - both increase and decrease in count of these workers have an
          immediate impact on the current number of workers, new ones will be
          spawned or some of them get terminated respectively.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

New API to retrieve current server workerpool specs. Since it uses typed
parameters, more specs to retrieve can be further included in the pool of
supported ones.
Signed-off-by: NErik Skultety <eskultet@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>

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>

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 reverts commit 6bda9f8a.

I mistakenly pushed it along with the Admin API series.

This function accesses the number of connected clients while properly
locking the server it returns the data about.
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>

If virNetServerMDNSAddEntry() fails when adding a service to a server,
it doesn't decrease the number of services.  Hence access to their
members segfaults (e.g. when free()-ing the sruct).
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

As opposed to 'static'; by exporting it (privately).
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Since we don't have syntax-check for this, it has to be checked
manually.  Let's hope this is the only place it happened.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This only affected the servers that re-exec themselves, which is only
virtlockd and it didn't do any mess, so this is mostly a clenaup.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

C guarantees that static variables are zero-initialized.  Some older
compilers (and also gcc -fno-zero-initialized-in-bss) create larger
binaries if you explicitly zero-initialize a static variable.

* src/conf/nwfilter_conf.c: Fix initialization.
* src/cpu/cpu_x86.c: Likewise.
* src/interface/interface_backend_netcf.c: Likewise.
* src/locking/lock_daemon.c: Likewise.
* src/locking/lock_driver_lockd.c: Likewise.
* src/locking/lock_driver_sanlock.c: Likewise.
* src/network/bridge_driver.c: Likewise.
* src/node_device/node_device_udev.c: Likewise.
* src/nwfilter/nwfilter_learnipaddr.c: Likewise.
* src/rpc/virnetserver.c: Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxGenSecurityLabel): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

We have these configuration knobs, like max_clients and
max_anonymous_clients. They limit the number of clients
connected.  Whenever the limit is reached, the daemon stops
accepting new ones and resumes if one of the connected clients
disconnects. If that's the case, a debug message is printed into
the logs. And when the daemon starts over to accept new clients
too. However, the problem is the messages have debug priority.
This may be unfortunate, because if the daemon stops accepting
new clients all of a sudden, and users don't have debug logs
enabled they have no idea what's going on. Raise the messages
level to INFO at least.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

A earlier commit changed the global log buffer so that it only
records messages that are explicitly requested via the log
filters setting. This removes the performance burden, and
improves the signal/noise ratio for messages in the global
buffer. At the same time though, it is somewhat pointless, since
all the recorded log messages are already going to be sent to an
explicit log output like syslog, stderr or the journal. The
global log buffer is thus just duplicating this data on stderr
upon crash.

The log_buffer_size config parameter is left in the augeas
lens to prevent breakage for users on upgrade. It is however
completely ignored hereafter.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Coverity spotted a use of possibly undefined variable. If a server is
restarting as an result of update, the JSON file that keeps current
value of some variables will not contain the new variables. This is
the case of @max_anonymous_clients too. We are correctly querying if
there's "max_anonymous_clients" in the JSON, however, we are not
setting a sane default if there's none.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

This config tunable allows users to determine the maximum number of
accepted but yet not authenticated users.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The counter gets incremented on each unauthenticated client added to the
server and decremented whenever the client authenticates.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Systemd does not forget about the cases, where client service needs to
wait for daemon service to initialize and start accepting new clients.
Setting a dependency in client is not enough as systemd doesn't know
when the daemon has initialized itself and started accepting new
clients. However, it offers a mechanism to solve this. The daemon needs
to call a special systemd function by which the daemon tells "I'm ready
to accept new clients". This is exactly what we need with
libvirtd-guests (client) and libvirtd (daemon). So now, with this
change, libvirt-guests.service is invoked not any sooner than
libvirtd.service calls the systemd notify function.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

There are still two places where we are using 1bit width unsigned
integer to store a boolean. There's no real need for this and these
occurrences can be replaced with 'bool'.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Currently, even if max_client limit is hit, we accept() incoming
connection request, but close it immediately. This has disadvantage of
not using listen() queue. We should accept() only those clients we
know we can serve and let all other wait in the (limited) queue.