On Thu, Apr 16, 2015 at 04:46:44PM +0200, Martin Kletzander wrote: > Initial scratch of the admin library. It has its own virAdmConnectPtr > that inherits from virAbstractConnectPtr and thus trivially supports > error reporting. See my note earlier about error reporting on the connection being a bad idea due to lack of thread safety. > Configuration option --with-admin is added to control whether the admin > library should be built or not (set to 'yes' by default). Is there a compelling reason why we'd need/want to be able to disable building of the admin library ? As a comparison we unconditionally build libvirt-qemu.so & libvirt-lxc.so > diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h > new file mode 100644 > index 0000000..7fe03cf > --- /dev/null > +++ b/include/libvirt/libvirt-admin.h > @@ -0,0 +1,62 @@ > +/* > + * libvirt-admin.h: Admin interface for libvirt > + * Summary: Interfaces for handling server-related tasks > + * Description: Provides the interfaces of the libvirt library to operate > + * with the server itself, not any hypervisors. > + * > + * Copyright (C) 2014-2015 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + * Author: Martin Kletzander <mkletzan@xxxxxxxxxx> > + */ > + > +#ifndef __VIR_ADMIN_H__ > +# define __VIR_ADMIN_H__ > + > +# include "internal.h" > + > +# ifdef __cplusplus > +extern "C" { > +# endif > + > + > +/** > + * virAdmConnect: > + * > + * a virAdmConnect is a private structure representing a connection to > + * libvirt daemon. > + */ > +typedef struct _virAdmConnect virAdmConnect; > + > +/** > + * virAdmConnectPtr: > + * > + * a virAdmConnectPtr is pointer to a virAdmConnect private structure, > + * this is the type used to reference a connection to the daemon > + * in the API. > + */ > +typedef virAdmConnect *virAdmConnectPtr; > + > +virAdmConnectPtr virAdmConnectOpen(unsigned int flags); How does this figure out which libvirtd daemon to connect to ? Presumably you've hardcoded it based on the UID you're running as ? I think for future proofing we should probably define a URI syntax for this. eg admin:///system admin:///session And allow an optional parameter for the socket path, for people who have built their daemon with an unusual --prefix arg. > +libvirt_admin_la_SOURCES += \ > + datatypes.c \ > + util/viralloc.c \ > + util/viratomic.c \ > + util/virauth.c \ > + util/virauthconfig.c \ > + util/virbitmap.c \ > + util/virbuffer.c \ > + util/vircommand.c \ > + util/virerror.c \ > + util/virevent.c \ > + util/vireventpoll.c \ > + util/virfile.c \ > + util/virhash.c \ > + util/virhashcode.c \ > + util/virjson.c \ > + util/virkeyfile.c \ > + util/virlog.c \ > + util/virobject.c \ > + util/virpidfile.c \ > + util/virprocess.c \ > + util/virrandom.c \ > + util/virseclabel.c \ > + util/virsocketaddr.c \ > + util/virstorageencryption.c \ > + util/virstoragefile.c \ > + util/virstring.c \ > + util/virthread.c \ > + util/virtime.c \ > + util/virtypedparam.c \ > + util/viruri.c \ > + util/virutil.c \ > + util/viruuid.c \ > + util/virxml.c \ > + remote/remote_protocol.c \ > + rpc/virnetmessage.h \ > + rpc/virnetmessage.c \ > + rpc/virnetsocket.c \ > + rpc/virnetsshsession.c \ > + rpc/virkeepalive.c \ > + rpc/virnetclient.c \ > + rpc/virnetclientprogram.c \ > + rpc/virnetclientstream.c \ > + rpc/virnetprotocol.c \ > + rpc/virnettlscontext.c \ > + rpc/virnetsaslcontext.c > + > +libvirt_admin_la_LDFLAGS = \ > + $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_ADMIN_SYMBOL_FILE) \ > + -version-info $(LIBVIRT_VERSION_INFO) \ > + $(AM_LDFLAGS) \ > + $(CYGWIN_EXTRA_LDFLAGS) \ > + $(MINGW_EXTRA_LDFLAGS) > + > +libvirt_admin_la_LIBADD = \ > + $(CYGWIN_EXTRA_LIBADD) > + > +libvirt_admin_la_CFLAGS = \ > + $(AM_CFLAGS) \ > + -I$(srcdir)/remote \ > + -I$(srcdir)/rpc \ > + -I$(srcdir)/admin > + > +libvirt_admin_la_CFLAGS += \ > + $(CAPNG_CFLAGS) \ > + $(YAJL_CFLAGS) \ > + $(SSH2_CFLAGS) \ > + $(SASL_CFLAGS) \ > + $(GNUTLS_CFLAGS) > + > +libvirt_admin_la_LIBADD += \ > + $(CAPNG_LIBS) \ > + $(YAJL_LIBS) \ > + $(DEVMAPPER_LIBS) \ > + $(LIBXML_LIBS) \ > + $(SSH2_LIBS) \ > + $(SASL_LIBS) \ > + $(GNUTLS_LIBS) > + > +if WITH_DTRACE_PROBES > +libvirt_admin_la_LIBADD += libvirt_probes.lo > +endif WITH_DTRACE_PROBES > + > +endif WITH_ADMIN > + > # Empty source list - it merely links a bunch of convenience libs together > libvirt_la_SOURCES = > libvirt_la_LDFLAGS = \ > diff --git a/src/datatypes.c b/src/datatypes.c > index b21113e..83cee7e 100644 > --- a/src/datatypes.c > +++ b/src/datatypes.c > @@ -59,6 +59,10 @@ static void virStreamDispose(void *obj); > static void virStorageVolDispose(void *obj); > static void virStoragePoolDispose(void *obj); > > +virClassPtr virAdmConnectClass; > + > +static void virAdmConnectDispose(void *obj); > + > static int > virDataTypesOnceInit(void) > { > @@ -88,6 +92,8 @@ virDataTypesOnceInit(void) > DECLARE_CLASS(virStorageVol); > DECLARE_CLASS(virStoragePool); > > + DECLARE_CLASS_CONNECT(virAdmConnect); > + > #undef DECLARE_CLASS_COMMON > #undef DECLARE_CLASS_LOCKABLE > #undef DECLARE_CLASS_CONNECT > @@ -804,3 +810,27 @@ virDomainSnapshotDispose(void *obj) > VIR_FREE(snapshot->name); > virObjectUnref(snapshot->domain); > } > + > + > +virAdmConnectPtr > +virAdmConnectNew(void) > +{ > + virAdmConnectPtr ret; > + > + if (virDataTypesInitialize() < 0) > + return NULL; > + > + if (!(ret = virGetAbstractConnect(virAdmConnectClass))) > + return NULL; > + > + return ret; > +} > + > +static void > +virAdmConnectDispose(void *obj) > +{ > + virAdmConnectPtr conn = obj; > + > + if (conn->privateDataFreeFunc) > + conn->privateDataFreeFunc(conn->privateData); > +} > diff --git a/src/datatypes.h b/src/datatypes.h > index 9f95811..b240e4c 100644 > --- a/src/datatypes.h > +++ b/src/datatypes.h > @@ -42,6 +42,8 @@ extern virClassPtr virStreamClass; > extern virClassPtr virStorageVolClass; > extern virClassPtr virStoragePoolClass; > > +extern virClassPtr virAdmConnectClass; > + > # define virCheckConnectReturn(obj, retval) \ > do { \ > if (!virObjectIsClass(obj, virConnectClass)) { \ > @@ -296,6 +298,26 @@ extern virClassPtr virStoragePoolClass; > dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__); \ > } while (0) > > +# define virCheckAdmConnectReturn(obj, retval) \ > + do { \ > + if (!virObjectIsClass(obj, virAdmConnectClass)) { \ > + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ > + __FILE__, __FUNCTION__, __LINE__, \ > + __FUNCTION__); \ > + virDispatchError(NULL); \ > + return retval; \ > + } \ > + } while (0) > +# define virCheckAdmConnectGoto(obj, label) \ > + do { \ > + if (!virObjectIsClass(obj, virAdmConnectClass)) { \ > + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ > + __FILE__, __FUNCTION__, __LINE__, \ > + __FUNCTION__); \ > + goto label; \ > + } \ > + } while (0) > + > /** > * VIR_DOMAIN_DEBUG: > * @dom: domain > @@ -365,6 +387,19 @@ struct _virConnect { > }; > > /** > + * _virAdmConnect: > + * > + * Internal structure associated to an admin connection > + */ > +struct _virAdmConnect { > + virAbstractConnect object; > + > + void *privateData; > + virFreeCallback privateDataFreeFunc; > +}; > + > + > +/** > * _virDomain: > * > * Internal structure associated to a domain > @@ -545,4 +580,6 @@ virNWFilterPtr virGetNWFilter(virConnectPtr conn, > virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, > const char *name); > > +virAdmConnectPtr virAdmConnectNew(void); > + > #endif /* __VIR_DATATYPES_H__ */ > diff --git a/src/internal.h b/src/internal.h > index 4d473af..1fbcfc2 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -58,6 +58,7 @@ > # include "libvirt/libvirt.h" > # include "libvirt/libvirt-lxc.h" > # include "libvirt/libvirt-qemu.h" > +# include "libvirt/libvirt-admin.h" > # include "libvirt/virterror.h" > > # include "c-strcase.h" > diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c > new file mode 100644 > index 0000000..e47089d > --- /dev/null > +++ b/src/libvirt-admin.c > @@ -0,0 +1,337 @@ > +/* > + * libvirt-admin.c > + * > + * Copyright (C) 2014-2015 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + * Author: Martin Kletzander <mkletzan@xxxxxxxxxx> > + */ > + > +#include <config.h> > + > +#include <rpc/rpc.h> > + > +#include "internal.h" > +#include "configmake.h" > +#include "datatypes.h" > +#include "viralloc.h" > +#include "virlog.h" > +#include "virstring.h" > +#include "virutil.h" > +#include "viruuid.h" > +#include "virnetclient.h" > + > +#define VIR_FROM_THIS VIR_FROM_ADMIN > +#define LIBVIRTD_ADMIN_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock" > + > +VIR_LOG_INIT("libvirt-admin"); > + > + > +typedef struct _remoteAdminPriv remoteAdminPriv; > +typedef remoteAdminPriv *remoteAdminPrivPtr; > + > +struct _remoteAdminPriv { > + virMutex lock; > + int counter; > + int localUses; > + virNetClientPtr client; > + virNetClientProgramPtr program; > +}; > + > +static void > +remoteAdminLock(remoteAdminPrivPtr priv) > +{ > + virMutexLock(&priv->lock); > +} > + > +static void > +remoteAdminUnlock(remoteAdminPrivPtr priv) > +{ > + virMutexUnlock(&priv->lock); > +} > + > +static void > +remoteAdminPrivFree(void *opaque) > +{ > + remoteAdminPrivPtr priv = opaque; > + > + virObjectUnref(priv->program); > + virObjectUnref(priv->client); > + > + virMutexDestroy(&priv->lock); > + VIR_FREE(priv); > +} > + > +static int > +callFull(virAdmConnectPtr conn ATTRIBUTE_UNUSED, > + remoteAdminPrivPtr priv, > + int *fdin, > + size_t fdinlen, > + int **fdout, > + size_t *fdoutlen, > + int proc_nr, > + xdrproc_t args_filter, char *args, > + xdrproc_t ret_filter, char *ret) > +{ > + int rv; > + virNetClientProgramPtr prog; > + int counter = priv->counter++; > + virNetClientPtr client = priv->client; > + priv->localUses++; > + > + /* We hav nothing else right now, but we can still add more > + * programs in the future */ > + prog = priv->program; > + > + /* Unlock, so that if we get any async events/stream data > + * while processing the RPC, we don't deadlock when our > + * callbacks for those are invoked > + */ > + remoteAdminUnlock(priv); > + rv = virNetClientProgramCall(prog, > + client, > + counter, > + proc_nr, > + fdinlen, fdin, > + fdoutlen, fdout, > + args_filter, args, > + ret_filter, ret); > + remoteAdminLock(priv); > + priv->localUses--; > + > + return rv; > +} > + > +static int > +call(virAdmConnectPtr conn, > + unsigned int flags, > + int proc_nr, > + xdrproc_t args_filter, char *args, > + xdrproc_t ret_filter, char *ret) > +{ > + virCheckFlags(0, -1); > + > + return callFull(conn, conn->privateData, > + NULL, 0, NULL, NULL, proc_nr, > + args_filter, args, ret_filter, ret); > +} > + > +#include "admin_protocol.h" > +#include "admin_client.h" > + > +static bool virAdmGlobalError; > +static virOnceControl virAdmGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER; > + > +static remoteAdminPrivPtr > +remoteAdminPrivNew(void) > +{ > + remoteAdminPrivPtr priv = NULL; > + > + /* initialize private data */ > + if (VIR_ALLOC(priv) < 0) > + goto error; > + > + if (virMutexInit(&priv->lock) < 0) { > + VIR_FREE(priv); > + goto error; > + } > + > + priv->localUses = 1; > + if (!(priv->client = virNetClientNewUNIX(LIBVIRTD_ADMIN_UNIX_SOCKET, > + false, NULL))) > + goto error; > + > + if (!(priv->program = virNetClientProgramNew(ADMIN_PROGRAM, > + ADMIN_PROTOCOL_VERSION, > + NULL, 0, NULL))) > + goto error; > + > + if (virNetClientAddProgram(priv->client, priv->program) < 0) > + goto error; > + > + return priv; > + error: > + remoteAdminPrivFree(priv); > + return NULL; > +} > + > +static void > +virAdmGlobalInit(void) > +{ > + /* It would be nice if we could trace the use of this call, to > + * help diagnose in log files if a user calls something other than > + * virAdmConnectOpen first. But we can't rely on VIR_DEBUG working > + * until after initialization is complete, and since this is > + * one-shot, we never get here again. */ > + if (virThreadInitialize() < 0 || > + virErrorInitialize() < 0) > + goto error; > + > + virLogSetFromEnv(); > + > + if (!bindtextdomain(PACKAGE, LOCALEDIR)) > + goto error; > + > + return; > + error: > + virAdmGlobalError = true; > +} > + > +/** > + * virAdmInitialize: > + * > + * Initialize the library. > + * > + * Returns 0 in case of success, -1 in case of error > + */ > +static int > +virAdmInitialize(void) > +{ > + if (virOnce(&virAdmGlobalOnce, virAdmGlobalInit) < 0) > + return -1; > + > + if (virAdmGlobalError) > + return -1; > + > + return 0; > +} > + > +/** > + * virAdmConnectOpen: > + * @flags: unused, must be 0 > + * > + * Opens connection to admin interface of the daemon. > + * > + * Returns @virAdmConnectPtr object or NULL on error > + */ > +virAdmConnectPtr > +virAdmConnectOpen(unsigned int flags) > +{ > + virAdmConnectPtr conn = NULL; > + remoteAdminPrivPtr priv = NULL; > + > + if (virAdmInitialize() < 0) > + goto error; > + > + VIR_DEBUG("flags=%x", flags); > + virResetLastError(); > + > + if (!(conn = virAdmConnectNew())) > + goto error; > + > + if (!(priv = remoteAdminPrivNew())) > + goto error; > + > + remoteAdminLock(priv); > + conn->privateData = priv; > + conn->privateDataFreeFunc = remoteAdminPrivFree; > + > + { > + admin_connect_open_args args = { flags }; > + > + VIR_DEBUG("Trying to open admin connection"); > + if (call(conn, 0, ADMIN_PROC_CONNECT_OPEN, > + (xdrproc_t) xdr_admin_connect_open_args, (char *) &args, > + (xdrproc_t) xdr_void, (char *) NULL) == -1) > + goto error; > + } > + > + remoteAdminUnlock(priv); > + > + return conn; > + error: > + virDispatchError(NULL); > + if (priv) > + remoteAdminUnlock(priv); > + virObjectUnref(conn); > + return NULL; > +} > + > +/** > + * virAdmConnectClose: > + * @conn: pointer to admin connection to close > + * > + * This function closes the connection to the Hypervisor. This should > + * not be called if further interaction with the Hypervisor are needed > + * especially if there is running domain which need further monitoring by > + * the application. > + * > + * Connections are reference counted; the count is explicitly > + * increased by the initial open (virConnectOpen, virConnectOpenAuth, > + * and the like) as well as virConnectRef; it is also temporarily > + * increased by other API that depend on the connection remaining > + * alive. The open and every virConnectRef call should have a > + * matching virConnectClose, and all other references will be released > + * after the corresponding operation completes. > + * > + * Returns a positive number if at least 1 reference remains on > + * success. The returned value should not be assumed to be the total > + * reference count. A return of 0 implies no references remain and > + * the connection is closed and memory has been freed. A return of -1 > + * implies a failure. > + * > + * It is possible for the last virConnectClose to return a positive > + * value if some other object still has a temporary reference to the > + * connection, but the application should not try to further use a > + * connection after the virConnectClose that matches the initial open. > + */ > +int > +virAdmConnectClose(virAdmConnectPtr conn) > +{ > + VIR_DEBUG("conn=%p", conn); > + > + virResetLastError(); > + if (!conn) > + return 0; > + > + virCheckAdmConnectReturn(conn, -1); > + > + if (!virObjectUnref(conn)) > + return 0; > + return 1; > +} > + > + > +/** > + * virAdmConnectRef: > + * @conn: the connection to hold a reference on > + * > + * Increment the reference count on the connection. For each > + * additional call to this method, there shall be a corresponding > + * call to virConnectClose to release the reference count, once > + * the caller no longer needs the reference to this object. > + * > + * This method is typically useful for applications where multiple > + * threads are using a connection, and it is required that the > + * connection remain open until all threads have finished using > + * it. I.e., each new thread using a connection would increment > + * the reference count. > + * > + * Returns 0 in case of success, -1 in case of failure > + */ > +int > +virAdmConnectRef(virAdmConnectPtr conn) > +{ > + VIR_DEBUG("conn=%p refs=%d", conn, > + conn ? conn->object.parent.parent.u.s.refs : 0); > + > + virResetLastError(); > + virCheckAdmConnectReturn(conn, -1); > + > + virObjectRef(conn); > + > + return 0; > +} > diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms > new file mode 100644 > index 0000000..292433f > --- /dev/null > +++ b/src/libvirt_admin.syms > @@ -0,0 +1,18 @@ > +# > +# Officially exported symbols, for which header > +# file definitions are installed in /usr/include/libvirt > +# from libvirt-admin.h > +# > +# Versions here are *fixed* to match the libvirt version > +# at which the symbol was introduced. This ensures that > +# a new client app requiring symbol foo() can't accidentally > +# run with old libvirt-admin.so not providing foo() - the global > +# soname version info can't enforce this since we never > +# change the soname > +# > +LIBVIRT_ADMIN_1.2.3 { Either you've been working on this a really long time, or this version was a typo. Either way, when we are ready to merge the admin library, that might be a nice reason to bump our version to 1.3.x, since we've been on 1.2.x a long time. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list