Use the newly introduced close callback helpers to make the code look just a
bit cleaner and more importantly, to fix the following memleak regarding a
dangling virAdmConnect object reference caused by assigning NULL to the close
callback data once the catch-disconnect routine used the callback followed
by a comparison of NULL to the originally defined close callback (which at that
moment had already been NULL'd by remoteAdminClientCloseFunc) in
virAdmConnectCloseCallbackUnregister.

717 (88 direct, 629 indirect) bytes in 1 blocks are definitely lost record
 110 of 141
    at 0x4C2A988: calloc (vg_replace_malloc.c:711)
    by 0x530696F: virAllocVar (viralloc.c:560)
    by 0x53689E6: virObjectNew (virobject.c:193)
    by 0x5368B5E: virObjectLockableNew (virobject.c:219)
    by 0x4E3E7EE: virAdmConnectNew (datatypes.c:900)
    by 0x4E398BB: virAdmConnectOpen (libvirt-admin.c:220)
    by 0x10D3E3: vshAdmConnect (virt-admin.c:161)
    by 0x10D624: vshAdmReconnect (virt-admin.c:215)
    by 0x10DB0A: cmdConnect (virt-admin.c:353)
    by 0x11288F: vshCommandRun (vsh.c:1313)
    by 0x10FDB6: main (virt-admin.c:1439)

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

The only place we change the @conn object is actually virAdmConnectOpen
routine, thus at the moment we don't really need to lock it, given the fact that
what we're trying to do here is to change the closeCallback object which is a
lockable object itself, so that should be enough to avoid races.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NNitesh Konkar <nitkon12@linux.vnet.ibm.com>

Commit 30ce2f0e tried to fix the issue with an incorrect session URI to admin
server but it messed up the checks:

    if (geteuid == 0 && VIR_STRDUP(*uristr, "libvirtd:///system") < 0)
        return -1;
    else if (VIR_STRDUP(*uristr, "libvirtd:///session") < 0)
        return -1;

So if a client executed with root privileges tries to connect, its euid is
checked (true) and the correct URI is successfully copied to @uristr (false),
therefore the 'else' branch is taken and @uristr is replaced by the session URI
which for root results in:
    Failed to connect socket to '/root/.cache/libvirt/libvirt-admin-sock':
    No such file or directory
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Just like we decide on which URI we go with based on EUID for qemu in remote
driver, do a similar thing for admin except we do not spawn a daemon in this
case.

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

The original name 'admin_uri_default' was introduced to our code by commit
dbecb87f. However, at that time we already had a separate config file for
admin library but the commit mentioned above didn't properly adjust the
config's option name. The result is that when we're loading the config, we
check a non-existent config option (there's not much to do with the URIs
anyway, since we only allow local connection). Additionally, virt-admin's man
page documents, that the default URI can be altered by setting
admin_uri_default option. So the fix proposed by this patch leaves the
libvirt-admin.conf as is and adjusts the naming in the code as well as in the
virt-admin's man page.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Similarly to what virsh virt-login-shell do, call virAdmInitialize prior to
initializing an event loop and initializing the error handler. Commit 97973ebb
described and fixed an identical issue for libvirt_lxc.
Since virAdmInitialize becomes a public API after applying this patch,
the symbol is also added to public syms and the doc string of the method is
slightly enhanced analogically to virInitialize.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

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>

Enable retrieval of the number of maximum clients connected to all sockets
combined, as well as the number of maximum clients waiting for authentication,
in order to be successfully connected. These are the attributes configurable
through libvirtd.conf, however, it could be handy to not only know values for
these limits, but also the values for the current number of clients
connected and number of clients currently waiting for authentication which are
changing dynamically. This API does both, retrieves the limits as well as the
current dynamic values.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Once we're able to list and identify all clients connected to a specific
server, we can then support force-closing a connection. This patch introduces
a simple API calling virNetServerClientClose on a specific client, which
can be later extended easily, e.g. by sending an event once the client is
disconnected successfully.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Unlike the previous commit, we do actually support one client-side only flag
VIR_CONNECT_NO_ALIASES, so besides removing the check for flags this flag
has to be masked out before sending a message to the daemon, otherwise it
would trigger an error when checking flags on the daemon side.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Due to compatibility reasons these should be checked on the server side.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Since nparams can be technically negative, it is a good practice throughout
our code to check if nparams actually has a non-negative value. The same effect
would be achieved by converting our internal typed params serializer argument
to 'unsigned' type, but it definitely would not be the path of least resistance.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Expose a public API to retrieve some identity and connection information about
a client connected to the specified server on daemon. The identity info
retrieved is mostly connection transport dependent, i.e. there won't be any
socket address returned for a local (UNIX socket) connection, while on the
other hand, when connected through TLS or unencrypted TCP, obviously no UNIX
process identification will be present in the returned data. All supported
values that can be returned in typed params are exposed and documented in
include/libvirt/libvirt-admin.h
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>

Finally add public method to retrieve the list of currently connected clients
to a given server.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Besides ID, the object also stores static data like connection transport and
connection timestamp, since once obtained a list of all clients connected to a
server, from user's perspective, it would be nice to know whether a given
client is remote or local only and when did it connect to the daemon.
Along with the object introduction, all necessary client-side methods necessary
to work with the object are added as well.
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>

It does not have a suffix ByName because there are no other means of
looking up the server and since the name is known, this should be the
preferred one.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Resetting an error should be the first thing public API does.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Function virAdmConnectListServers() forgot to check for flags at all,
virAdmConnectOpen() on the other hand checked them but did no dispatch
the error.  virCheckFlags() should be used only when there should be no
other thing done after erroring out and since they are used on different
places then just public API, they cannot dispatch errors.  So let's use
virCheckFlagsGoto instead.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

We don't want to end up like with virDomainFree() and other, right?
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This API is merely a convenience API, i.e. when managing clients connected to
daemon's servers, we should know (convenience) which server the specific client
is connected to. This implies a client-side representation of a server along
with a basic API to let the administrating client know what servers are actually
available on the daemon.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Commmit df8192aa introduced admin related rename and some minor
(caused by automated approach, aka sed) and some more severe isues along with
it. First reason to revert is the inconsistency with libvirt library.
Although we deal with the daemon directly rather than with a specific
hypervisor, we still do have a connection. That being said, contributors might
get under the impression that AdmDaemonNew would spawn/start a new daemon
(since it's admin API, why not...), or AdmDaemonClose would do the exact
opposite or they might expect DaemonIsAlive report overall status of the daemon
which definitely isn't the case.
The second reason to revert this patch is renaming virt-admin client. The
client tool does not necessarily have to reflect the names of the API's it's
using in his internals. An example would be 's/vshAdmConnect/vshAdmDaemon'
where noone can be certain of what the latter function really does. The former
is quite expressive about some connection magic it performs, but the latter does
not say anything, especially when vshAdmReconnect and vshAdmDisconnect were
left untouched.

virAdmConnect was named after virConnect, but after some discussions,
most of the APIs called will be working with remote daemon and starting
them virAdmDaemon will make more sense.  Only possibly controversal name
is CloseCallback (de)registration, and connecting to the daemon (which
will still be Open/Close), but even this makes sense if one thinks about
the daemon being opened and closed, e.g. as file, etc.

This way all the APIs working with the daemon will start with
virAdmDaemon prefix, they will accept virAdmDaemonPtr as first parameter
and that will better suit with other namings as well (virDomain*,
virAdmServer*, etc.).

Because in virt-admin, the connection name does not refer to a struct
that would have a connect in its name, also adjust 'connname' in
clients.  And because it is not used anywhere in the vsh code, move it
from there into each client.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Introduce a new API to get libvirt version. It is worth noting, that
libvirt-admin and libvirt share the same version number. Unfortunately,
our existing API isn't generic enough to be used with virAdmConnectPtr
as well. Also this patch wires up this API to the virt-admin client
as a generic cmdVersion command.

As we need a client disconnect handler, we also need a mechanism to register
such handlers for a client. This patch introduced both the close callbacks and
also the client vshAdmCatchDisconnect handler to be registered with it. By
registering the handler we still need to make sure the client can react to
daemon's events like disconnect or keepalive, so asynchronous I/O event polling
is necessary to be enabled too.

Now that we introduced URI support in libvirt-admin, we should also support URI
aliases during connection establishment phase. After applying this patch,
virAdmConnectOpen will also support VIR_CONNECT_NO_ALIASES flag.

Since virt-admin should be able to connect to various admin servers
on hosted different daemons, we need to provide URI support to
libvirt-admin.

By moving the remote version into a separate module, we gain a slightly
better maintainability in the long run than just by leaving it in one
place with the existing libvirt-admin library which can start getting
pretty messy later on.

Since most of our APIs rely on an acive functional connection to a daemon and
we have such a mechanism in libvirt already, there's need to have such a way in
libvirt-admin as well. By introducing a new public API, this patch provides
support to check for an active connection.

Unfortunately, client side version retrieval API virGetVersion uses
one-time initialization (due to the fact we might not have initialized the
library by calling connect prior to this) which is not completely compatible
with admin initialization. This API is rather simplistic and reimplementing
it for admin might be the preferred method of reusing it. Note that even though
the method will be reimplemented, the version number is still the same for both
the libvirt and libvirt-admin library.

Running valgrind on a very simplistic program consisting only of
opening and closing admin connection (virAdmConnect{Open,Close}) shows a
leak in remoteAdminPrivNew, because the last reference to privateData is
not decremented, thus the object won't be disposed. This patch unrefs
the privateData object once we closed the active connection to daemon,
making further use of this connection  useless.

==24577==    at 0x4A089C7: calloc (in /usr/lib64/valgrind/vgpreload_***linux.so)
==24577==    by 0x4E8835F: virAllocVar (viralloc.c:560)
==24577==    by 0x4EDFA5C: virObjectNew (virobject.c:193)
==24577==    by 0x4EDFBD4: virObjectLockableNew (virobject.c:219)
==24577==    by 0x4C14DAF: remoteAdminPrivNew (libvirt-admin.c:152)
==24577==    by 0x4C1537E: virAdmConnectOpen (libvirt-admin.c:308)
==24577==    by 0x400BAD: main (listservers.c:39)

==24577== LEAK SUMMARY:
==24577==    definitely lost: 80 bytes in 1 blocks
==24577==    indirectly lost: 840 bytes in 6 blocks
==24577==      possibly lost: 0 bytes in 0 blocks
==24577==    still reachable: 12,179 bytes in 199 blocks
==24577==         suppressed: 0 bytes in 0 blocks

By trying to lead the way of clean includes, I sorted the lines
alphabetically and that is a problem for mingw builds with gnulib.
As 'configmake.h' defines DATADIR and 'datatypes.h' transitively
includes 'winsock.h' that uses 'DATADIR' as a name for a struct,
it's enough to reorder those.

Even though this might be worked around in gnulib later on, this
fixes the build for now.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This reverts commit 5792fabb.

I mistakenly pushed it along with the Admin API series.

Just one of the simplest functions that returns string "Clients: X"
where X is the number of connected clients to daemon's first
subserver (the original one), so it can be tested using virsh, ipython,
etc.

The subserver is gathered by incrementing its reference
counter (similarly to getting qemu capabilities), so there is no
deadlock with admin subserver in this API.

Here you can see how functions should be named in the client (virAdm*)
and server (adm*).

There is also a parameter @flags that must be 0, which helps testing
proper error propagation into the client.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Initial scratch of the admin library.  It has its own virAdmConnectPtr
that inherits from virAbstractConnectPtr and thus trivially supports
error reporting.

There's pkg-config file added and spec-file adjusted as well.

Since the library should be "minimalistic" and not depend on any other
library, the list of files is especially crafted for it.  Most of them
could've been put to it's own sub-libraries that would be LIBADD'd to
libvirt_util, libvirt_net_rpc and libvirt_setuid_rpc_client to minimize
the number of object files being built, but that's a refactoring that
isn't the orginal aim of this commit.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>