[libvirt] [PATCH 00/13] Admin API

This is a working part of the administration API with some usability helpers. We're still missing documentation and proper client (for which virsh needs to be split), but apart from the last three patches, this series is ready to go in if we decide that the documentation and client work can be done later on. Martin Kletzander (13): util: add virJSONValueCopy Move daemon-related parts of virNetServer to virNetDaemon Teach gendispatch how to handle admin dispatching files Add admin protocol Build client headers for admin protocol Add admin error domain Add libvirt-admin library Add XML files with admin API specification Add configuration options for permissions on daemon's admin socket Add support for admin API in libvirt daemon rpc: Add virNetServerGetNClients admin: Add virAdmHello function Example virt-admin .gitignore | 5 + Makefile.am | 4 +- cfg.mk | 9 +- configure.ac | 11 +- daemon/Makefile.am | 34 +- daemon/admin_server.c | 139 +++++++ daemon/admin_server.h | 36 ++ daemon/libvirtd-config.c | 5 +- daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 205 +++++++--- daemon/libvirtd.conf | 8 + daemon/libvirtd.h | 14 +- daemon/test_libvirtd.aug.in | 1 + docs/Makefile.am | 23 +- docs/apibuild.py | 10 +- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + include/libvirt/Makefile.am | 4 +- include/libvirt/libvirt-admin.h | 63 ++++ include/libvirt/virterror.h | 3 +- libvirt-admin.pc.in | 13 + libvirt.spec.in | 10 + po/POTFILES.in | 4 + src/Makefile.am | 127 ++++++- src/admin/admin_protocol.x | 78 ++++ src/admin_protocol-structs | 18 + src/datatypes.c | 30 ++ src/datatypes.h | 37 ++ src/internal.h | 1 + src/libvirt-admin.c | 412 ++++++++++++++++++++ src/libvirt_admin.syms | 19 + src/libvirt_private.syms | 1 + src/libvirt_remote.syms | 29 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/gendispatch.pl | 130 ++++--- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 536 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + src/util/virerror.c | 1 + src/util/virjson.c | 65 +++- src/util/virjson.h | 4 +- tests/confdata/libvirtd.conf | 6 + tests/confdata/libvirtd.out | 5 + tests/jsontest.c | 111 ++++++ tools/virt-admin/Makefile.am | 70 ++++ tools/virt-admin/virt-admin.c | 72 ++++ tools/virt-admin/virt-admin.pod | 43 +++ 53 files changed, 2735 insertions(+), 685 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h create mode 100644 tools/virt-admin/Makefile.am create mode 100644 tools/virt-admin/virt-admin.c create mode 100644 tools/virt-admin/virt-admin.pod -- 2.4.0

Faster version of virJSONValueFromString(virJSONValueToString()). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virjson.c | 65 ++++++++++++++++++++++++++- src/util/virjson.h | 4 +- tests/jsontest.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 15e71d5..883331f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1582,6 +1582,7 @@ virJSONValueArrayAppend; virJSONValueArrayGet; virJSONValueArraySize; virJSONValueArraySteal; +virJSONValueCopy; virJSONValueFree; virJSONValueFromString; virJSONValueGetArrayAsBitmap; diff --git a/src/util/virjson.c b/src/util/virjson.c index c8d761f..40ec613 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1,7 +1,7 @@ /* * virjson.c: JSON object parsing/formatting * - * Copyright (C) 2009-2010, 2012-2013 Red Hat, Inc. + * Copyright (C) 2009-2010, 2012-2015 Red Hat, Inc. * Copyright (C) 2009 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1233,6 +1233,69 @@ virJSONValueObjectForeachKeyValue(virJSONValuePtr object, } +virJSONValuePtr +virJSONValueCopy(virJSONValuePtr in) +{ + size_t i; + virJSONValuePtr out = NULL; + + if (!in) + return NULL; + + switch ((virJSONType) in->type) { + case VIR_JSON_TYPE_OBJECT: + out = virJSONValueNewObject(); + if (!out) + return NULL; + for (i = 0; i < in->data.object.npairs; i++) { + virJSONValuePtr val = NULL; + if (!(val = virJSONValueCopy(in->data.object.pairs[i].value))) + goto error; + if (virJSONValueObjectAppend(out, in->data.object.pairs[i].key, + val) < 0) { + virJSONValueFree(val); + goto error; + } + } + break; + case VIR_JSON_TYPE_ARRAY: + out = virJSONValueNewArray(); + if (!out) + return NULL; + for (i = 0; i < in->data.array.nvalues; i++) { + virJSONValuePtr val = NULL; + if (!(val = virJSONValueCopy(in->data.array.values[i]))) + goto error; + if (virJSONValueArrayAppend(out, val) < 0) { + virJSONValueFree(val); + goto error; + } + } + break; + + /* No need to error out in the following cases */ + case VIR_JSON_TYPE_STRING: + out = virJSONValueNewString(in->data.string); + break; + case VIR_JSON_TYPE_NUMBER: + out = virJSONValueNewNumber(in->data.number); + break; + case VIR_JSON_TYPE_BOOLEAN: + out = virJSONValueNewBoolean(in->data.boolean); + break; + case VIR_JSON_TYPE_NULL: + out = virJSONValueNewNull(); + break; + } + + return out; + + error: + virJSONValueFree(out); + return NULL; +} + + #if WITH_YAJL static int virJSONParserInsertValue(virJSONParserPtr parser, diff --git a/src/util/virjson.h b/src/util/virjson.h index 9bb7461..e871b2e 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -1,7 +1,7 @@ /* * virjson.h: JSON object parsing/formatting * - * Copyright (C) 2009, 2012-2013 Red Hat, Inc. + * Copyright (C) 2009, 2012-2015 Red Hat, Inc. * Copyright (C) 2009 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -165,4 +165,6 @@ int virJSONValueObjectForeachKeyValue(virJSONValuePtr object, virJSONValueObjectIteratorFunc cb, void *opaque); +virJSONValuePtr virJSONValueCopy(virJSONValuePtr in); + #endif /* __VIR_JSON_H_ */ diff --git a/tests/jsontest.c b/tests/jsontest.c index f070649..c080946 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -119,6 +119,91 @@ testJSONAddRemove(const void *data) static int +testJSONCopy(const void *data) +{ + const struct testInfo *info = data; + virJSONValuePtr json = NULL; + virJSONValuePtr jsonCopy = NULL; + char *result = NULL; + char *resultCopy = NULL; + int ret = -1; + + json = virJSONValueFromString(info->doc); + if (!json) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to parse %s\n", info->doc); + ret = -1; + goto cleanup; + } + + jsonCopy = virJSONValueCopy(json); + if (!jsonCopy) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to copy JSON data\n"); + ret = -1; + goto cleanup; + } + + result = virJSONValueToString(json, false); + if (!result) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to format original JSON data\n"); + ret = -1; + goto cleanup; + } + + resultCopy = virJSONValueToString(json, false); + if (!resultCopy) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to format copied JSON data\n"); + ret = -1; + goto cleanup; + } + + if (STRNEQ(result, resultCopy)) { + if (virTestGetVerbose()) + virtTestDifference(stderr, result, resultCopy); + ret = -1; + goto cleanup; + } + + VIR_FREE(result); + VIR_FREE(resultCopy); + + result = virJSONValueToString(json, true); + if (!result) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to format original JSON data\n"); + ret = -1; + goto cleanup; + } + + resultCopy = virJSONValueToString(json, true); + if (!resultCopy) { + if (virTestGetVerbose()) + fprintf(stderr, "Failed to format copied JSON data\n"); + ret = -1; + goto cleanup; + } + + if (STRNEQ(result, resultCopy)) { + if (virTestGetVerbose()) + virtTestDifference(stderr, result, resultCopy); + ret = -1; + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(result); + VIR_FREE(resultCopy); + virJSONValueFree(json); + virJSONValueFree(jsonCopy); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -180,6 +265,32 @@ mymain(void) DO_TEST_FULL("add and remove", AddRemove, "[ 1 ]", NULL, false); + DO_TEST_FULL("copy and free", Copy, + "{\"return\": [{\"name\": \"quit\"}, {\"name\": \"eject\"}," + "{\"name\": \"change\"}, {\"name\": \"screendump\"}," + "{\"name\": \"stop\"}, {\"name\": \"cont\"}, {\"name\": " + "\"system_reset\"}, {\"name\": \"system_powerdown\"}, " + "{\"name\": \"device_add\"}, {\"name\": \"device_del\"}, " + "{\"name\": \"cpu\"}, {\"name\": \"memsave\"}, {\"name\": " + "\"pmemsave\"}, {\"name\": \"migrate\"}, {\"name\": " + "\"migrate_cancel\"}, {\"name\": \"migrate_set_speed\"}," + "{\"name\": \"client_migrate_info\"}, {\"name\": " + "\"migrate_set_downtime\"}, {\"name\": \"netdev_add\"}, " + "{\"name\": \"netdev_del\"}, {\"name\": \"block_resize\"}," + "{\"name\": \"balloon\"}, {\"name\": \"set_link\"}, {\"name\":" + "\"getfd\"}, {\"name\": \"closefd\"}, {\"name\": \"block_passwd\"}," + "{\"name\": \"set_password\"}, {\"name\": \"expire_password\"}," + "{\"name\": \"qmp_capabilities\"}, {\"name\": " + "\"human-monitor-command\"}, {\"name\": \"query-version\"}," + "{\"name\": \"query-commands\"}, {\"name\": \"query-chardev\"}," + "{\"name\": \"query-block\"}, {\"name\": \"query-blockstats\"}, " + "{\"name\": \"query-cpus\"}, {\"name\": \"query-pci\"}, {\"name\":" + "\"query-kvm\"}, {\"name\": \"query-status\"}, {\"name\": " + "\"query-mice\"}, {\"name\": \"query-vnc\"}, {\"name\": " + "\"query-spice\"}, {\"name\": \"query-name\"}, {\"name\": " + "\"query-uuid\"}, {\"name\": \"query-migrate\"}, {\"name\": " + "\"query-balloon\"}], \"id\": \"libvirt-2\"}", NULL, true); + DO_TEST_PARSE("almost nothing", "[]"); DO_TEST_PARSE_FAIL("nothing", ""); -- 2.4.0

On Tue, May 19, 2015 at 10:19:35PM -0700, Martin Kletzander wrote:
Faster version of virJSONValueFromString(virJSONValueToString()).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virjson.c | 65 ++++++++++++++++++++++++++- src/util/virjson.h | 4 +- tests/jsontest.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 2 deletions(-)
ACK 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 :|

This allows to have more servers in one daemon which helps isolating some resources. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3e7f87c..22ba6cb 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1,7 +1,7 @@ /* * libvirtd.c: daemon start of day, guest process & i/o management * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -49,7 +49,7 @@ #include "viralloc.h" #include "virconf.h" #include "virnetlink.h" -#include "virnetserver.h" +#include "virnetdaemon.h" #include "remote.h" #include "virhook.h" #include "viraudit.h" @@ -774,14 +774,14 @@ daemonSetupPrivs(void) #endif -static void daemonShutdownHandler(virNetServerPtr srv, +static void daemonShutdownHandler(virNetDaemonPtr dmn, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - virNetServerQuit(srv); + virNetDaemonQuit(dmn); } -static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, +static void daemonReloadHandler(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -797,15 +797,15 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, VIR_WARN("Error while reloading drivers"); } -static int daemonSetupSignals(virNetServerPtr srv) +static int daemonSetupSignals(virNetDaemonPtr dmn) { - if (virNetServerAddSignalHandler(srv, SIGINT, daemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGINT, daemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGQUIT, daemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGQUIT, daemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGTERM, daemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGTERM, daemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGHUP, daemonReloadHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGHUP, daemonReloadHandler, NULL) < 0) return -1; return 0; } @@ -813,12 +813,12 @@ static int daemonSetupSignals(virNetServerPtr srv) static void daemonInhibitCallback(bool inhibit, void *opaque) { - virNetServerPtr srv = opaque; + virNetDaemonPtr dmn = opaque; if (inhibit) - virNetServerAddShutdownInhibition(srv); + virNetDaemonAddShutdownInhibition(dmn); else - virNetServerRemoveShutdownInhibition(srv); + virNetDaemonRemoveShutdownInhibition(dmn); } @@ -828,26 +828,26 @@ static DBusConnection *systemBus; static void daemonStopWorker(void *opaque) { - virNetServerPtr srv = opaque; + virNetDaemonPtr dmn = opaque; - VIR_DEBUG("Begin stop srv=%p", srv); + VIR_DEBUG("Begin stop dmn=%p", dmn); ignore_value(virStateStop()); - VIR_DEBUG("Completed stop srv=%p", srv); + VIR_DEBUG("Completed stop dmn=%p", dmn); /* Exit libvirtd cleanly */ - virNetServerQuit(srv); + virNetDaemonQuit(dmn); } /* We do this in a thread to not block the main loop */ -static void daemonStop(virNetServerPtr srv) +static void daemonStop(virNetDaemonPtr dmn) { virThread thr; - virObjectRef(srv); - if (virThreadCreate(&thr, false, daemonStopWorker, srv) < 0) - virObjectUnref(srv); + virObjectRef(dmn); + if (virThreadCreate(&thr, false, daemonStopWorker, dmn) < 0) + virObjectUnref(dmn); } @@ -856,14 +856,14 @@ handleSessionMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *opaque) { - virNetServerPtr srv = opaque; + virNetDaemonPtr dmn = opaque; - VIR_DEBUG("srv=%p", srv); + VIR_DEBUG("dmn=%p", dmn); if (dbus_message_is_signal(message, DBUS_INTERFACE_LOCAL, "Disconnected")) - daemonStop(srv); + daemonStop(dmn); return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } @@ -874,14 +874,14 @@ handleSystemMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *opaque) { - virNetServerPtr srv = opaque; + virNetDaemonPtr dmn = opaque; - VIR_DEBUG("srv=%p", srv); + VIR_DEBUG("dmn=%p", dmn); if (dbus_message_is_signal(message, "org.freedesktop.login1.Manager", "PrepareForShutdown")) - daemonStop(srv); + daemonStop(dmn); return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } @@ -890,22 +890,22 @@ handleSystemMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED, static void daemonRunStateInit(void *opaque) { - virNetServerPtr srv = opaque; + virNetDaemonPtr dmn = opaque; virIdentityPtr sysident = virIdentityGetSystem(); virIdentitySetCurrent(sysident); /* Since driver initialization can take time inhibit daemon shutdown until we're done so clients get a chance to connect */ - daemonInhibitCallback(true, srv); + daemonInhibitCallback(true, dmn); /* Start the stateful HV drivers * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will * seriously delay OS bootup process */ - if (virStateInitialize(virNetServerIsPrivileged(srv), + if (virStateInitialize(virNetDaemonIsPrivileged(dmn), daemonInhibitCallback, - srv) < 0) { + dmn) < 0) { VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); @@ -916,17 +916,17 @@ static void daemonRunStateInit(void *opaque) #ifdef HAVE_DBUS /* Tie the non-priviledged libvirtd to the session/shutdown lifecycle */ - if (!virNetServerIsPrivileged(srv)) { + if (!virNetDaemonIsPrivileged(dmn)) { sessionBus = virDBusGetSessionBus(); if (sessionBus != NULL) dbus_connection_add_filter(sessionBus, - handleSessionMessageFunc, srv, NULL); + handleSessionMessageFunc, dmn, NULL); systemBus = virDBusGetSystemBus(); if (systemBus != NULL) { dbus_connection_add_filter(systemBus, - handleSystemMessageFunc, srv, NULL); + handleSystemMessageFunc, dmn, NULL); dbus_bus_add_match(systemBus, "type='signal',sender='org.freedesktop.login1', interface='org.freedesktop.login1.Manager'", NULL); @@ -934,20 +934,20 @@ static void daemonRunStateInit(void *opaque) } #endif /* Only now accept clients from network */ - virNetServerUpdateServices(srv, true); + virNetDaemonUpdateServices(dmn, true); cleanup: - daemonInhibitCallback(false, srv); - virObjectUnref(srv); + daemonInhibitCallback(false, dmn); + virObjectUnref(dmn); virObjectUnref(sysident); virIdentitySetCurrent(NULL); } -static int daemonStateInit(virNetServerPtr srv) +static int daemonStateInit(virNetDaemonPtr dmn) { virThread thr; - virObjectRef(srv); - if (virThreadCreate(&thr, false, daemonRunStateInit, srv) < 0) { - virObjectUnref(srv); + virObjectRef(dmn); + if (virThreadCreate(&thr, false, daemonRunStateInit, dmn) < 0) { + virObjectUnref(dmn); return -1; } return 0; @@ -1098,6 +1098,7 @@ daemonUsage(const char *argv0, bool privileged) } int main(int argc, char **argv) { + virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; char *remote_config_file = NULL; int statuswrite = -1; @@ -1352,6 +1353,12 @@ int main(int argc, char **argv) { goto cleanup; } + if (!(dmn = virNetDaemonNew()) || + virNetDaemonAddServer(dmn, srv) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + /* Beyond this point, nothing should rely on using * getuid/geteuid() == 0, for privilege level checks. */ @@ -1406,11 +1413,10 @@ int main(int argc, char **argv) { if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); - virNetServerAutoShutdown(srv, - timeout); + virNetDaemonAutoShutdown(dmn, timeout); } - if ((daemonSetupSignals(srv)) < 0) { + if ((daemonSetupSignals(dmn)) < 0) { ret = VIR_DAEMON_ERR_SIGNAL; goto cleanup; } @@ -1465,7 +1471,7 @@ int main(int argc, char **argv) { } /* Initialize drivers & then start accepting new clients from network */ - if (daemonStateInit(srv) < 0) { + if (daemonStateInit(dmn) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1487,7 +1493,7 @@ int main(int argc, char **argv) { #endif /* Run event loop. */ - virNetServerRun(srv); + virNetDaemonRun(dmn); ret = 0; @@ -1499,7 +1505,8 @@ int main(int argc, char **argv) { virObjectUnref(remoteProgram); virObjectUnref(lxcProgram); virObjectUnref(qemuProgram); - virNetServerClose(srv); + virNetDaemonClose(dmn); + virObjectUnref(dmn); virObjectUnref(srv); virNetlinkShutdown(); if (statuswrite != -1) { diff --git a/docs/internals.html.in b/docs/internals.html.in index 398d02e..c30f52f 100644 --- a/docs/internals.html.in +++ b/docs/internals.html.in @@ -82,7 +82,9 @@ <ul> <li>Daemon Startup <p>The daemon initialization processing will declare itself - as a server via a virNetServerNew() call, then use + as a daemon via a virNetDaemonNew() call, then creates new server + using virNetServerNew() and adds that server to the main daemon + struct with virNetDaemonAddServer() call. It will then use virDriverLoadModule() to find/load all known drivers, set up an RPC server program using the <code>remoteProcs[]</code> table via a virNetServerProgramNew() call. The table is the diff --git a/docs/internals/rpc.html.in b/docs/internals/rpc.html.in index f2ed64f..a48a4c0 100644 --- a/docs/internals/rpc.html.in +++ b/docs/internals/rpc.html.in @@ -532,6 +532,13 @@ calls in parallel, with dispatch across multiple worker threads. </dd> + <dt><code>virNetDaemonPtr</code> (virnetdaemon.h)</dt> + <dd>The virNetDaemon APIs are used to manage a daemon process. A + deamon is a process that might expose one or more servers. It + handles most process-related details, network-related should + be part of the underlying server. + </dd> + <dt><code>virNetServerMDNSPtr</code> (virnetservermdns.h)</dt> <dd>The virNetServerMDNS APIs are used to advertise a server across the local network, enabling clients to automatically diff --git a/po/POTFILES.in b/po/POTFILES.in index bb0f6e1..ebb0482 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -132,6 +132,7 @@ src/rpc/virkeepalive.c src/rpc/virnetclient.c src/rpc/virnetclientprogram.c src/rpc/virnetclientstream.c +src/rpc/virnetdaemon.c src/rpc/virnetmessage.c src/rpc/virnetsaslcontext.c src/rpc/virnetsocket.c diff --git a/src/Makefile.am b/src/Makefile.am index 579421d..7e95cf1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2521,6 +2521,7 @@ libvirt_net_rpc_server_la_SOURCES = \ rpc/virnetserverservice.h rpc/virnetserverservice.c \ rpc/virnetserverclient.h rpc/virnetserverclient.c \ rpc/virnetservermdns.h rpc/virnetservermdns.c \ + rpc/virnetdaemon.h rpc/virnetdaemon.c \ rpc/virnetserver.h rpc/virnetserver.c libvirt_net_rpc_server_la_CFLAGS = \ $(AVAHI_CFLAGS) \ diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 6b520b5..ac12807 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -57,6 +57,22 @@ virNetClientStreamSendPacket; virNetClientStreamSetError; +# rpc/virnetdaemon.h +virNetDaemonAddShutdownInhibition; +virNetDaemonAddSignalHandler; +virNetDaemonAutoShutdown; +virNetDaemonClose; +virNetDaemonGetServer; +virNetDaemonIsPrivileged; +virNetDaemonNew; +virNetDaemonNewPostExecRestart; +virNetDaemonPreExecRestart; +virNetDaemonQuit; +virNetDaemonRemoveShutdownInhibition; +virNetDaemonRun; +virNetDaemonUpdateServices; + + # rpc/virnetmessage.h virNetMessageClear; virNetMessageDecodeHeader; @@ -79,18 +95,16 @@ xdr_virNetMessageError; # rpc/virnetserver.h virNetServerAddProgram; virNetServerAddService; -virNetServerAddShutdownInhibition; -virNetServerAddSignalHandler; -virNetServerAutoShutdown; virNetServerClose; -virNetServerIsPrivileged; +virNetServerHasClients; virNetServerKeepAliveRequired; virNetServerNew; virNetServerNewPostExecRestart; virNetServerPreExecRestart; -virNetServerQuit; -virNetServerRemoveShutdownInhibition; -virNetServerRun; +virNetServerProcessClients; +virNetServerStart; +virNetServerTrackCompletedAuth; +virNetServerTrackPendingAuth; virNetServerUpdateServices; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 042ff94..f9afc8c 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1,7 +1,7 @@ /* * lock_daemon.c: lock management daemon * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -41,6 +41,7 @@ #include "virlog.h" #include "viralloc.h" #include "virconf.h" +#include "rpc/virnetdaemon.h" #include "rpc/virnetserver.h" #include "virrandom.h" #include "virhash.h" @@ -60,6 +61,7 @@ VIR_LOG_INIT("locking.lock_daemon"); struct _virLockDaemon { virMutex lock; + virNetDaemonPtr dmn; virNetServerPtr srv; virHashTablePtr lockspaces; virLockSpacePtr defaultLockspace; @@ -118,6 +120,7 @@ virLockDaemonFree(virLockDaemonPtr lockd) return; virObjectUnref(lockd->srv); + virObjectUnref(lockd->dmn); virHashFree(lockd->lockspaces); virLockSpaceFree(lockd->defaultLockspace); @@ -155,6 +158,10 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; + if (!(lockd->dmn = virNetDaemonNew()) || + virNetDaemonAddServer(lockd->dmn, lockd->srv) < 0) + goto error; + if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, virLockDaemonLockSpaceDataFree))) goto error; @@ -230,18 +237,21 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) } } - if (!(child = virJSONValueObjectGet(object, "server"))) { + if (!(child = virJSONValueObjectGet(object, "daemon"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing server data from JSON file")); + _("Missing daemon data from JSON file")); goto error; } - if (!(lockd->srv = virNetServerNewPostExecRestart(child, - virLockDaemonClientNew, - virLockDaemonClientNewPostExecRestart, - virLockDaemonClientPreExecRestart, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) + goto error; + + if (!(lockd->srv = virNetDaemonAddServerPostExec(lockd->dmn, + virLockDaemonClientNew, + virLockDaemonClientNewPostExecRestart, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; return lockd; @@ -529,32 +539,32 @@ virLockDaemonVersion(const char *argv0) } static void -virLockDaemonShutdownHandler(virNetServerPtr srv, +virLockDaemonShutdownHandler(virNetDaemonPtr dmn, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - virNetServerQuit(srv); + virNetDaemonQuit(dmn); } static void -virLockDaemonExecRestartHandler(virNetServerPtr srv, +virLockDaemonExecRestartHandler(virNetDaemonPtr dmn, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { execRestart = true; - virNetServerQuit(srv); + virNetDaemonQuit(dmn); } static int -virLockDaemonSetupSignals(virNetServerPtr srv) +virLockDaemonSetupSignals(virNetDaemonPtr dmn) { - if (virNetServerAddSignalHandler(srv, SIGINT, virLockDaemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGINT, virLockDaemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGQUIT, virLockDaemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGQUIT, virLockDaemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGTERM, virLockDaemonShutdownHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGTERM, virLockDaemonShutdownHandler, NULL) < 0) return -1; - if (virNetServerAddSignalHandler(srv, SIGUSR1, virLockDaemonExecRestartHandler, NULL) < 0) + if (virNetDaemonAddSignalHandler(dmn, SIGUSR1, virLockDaemonExecRestartHandler, NULL) < 0) return -1; return 0; } @@ -966,7 +976,7 @@ virLockDaemonPostExecRestart(const char *state_file, static int virLockDaemonPreExecRestart(const char *state_file, - virNetServerPtr srv, + virNetDaemonPtr dmn, char **argv) { virJSONValuePtr child; @@ -982,10 +992,10 @@ virLockDaemonPreExecRestart(const char *state_file, if (!(object = virJSONValueNewObject())) goto cleanup; - if (!(child = virNetServerPreExecRestart(srv))) + if (!(child = virNetDaemonPreExecRestart(dmn))) goto cleanup; - if (virJSONValueObjectAppend(object, "server", child) < 0) { + if (virJSONValueObjectAppend(object, "daemon", child) < 0) { virJSONValueFree(child); goto cleanup; } @@ -1350,11 +1360,11 @@ int main(int argc, char **argv) { if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); - virNetServerAutoShutdown(lockDaemon->srv, + virNetDaemonAutoShutdown(lockDaemon->dmn, timeout); } - if ((virLockDaemonSetupSignals(lockDaemon->srv)) < 0) { + if ((virLockDaemonSetupSignals(lockDaemon->dmn)) < 0) { ret = VIR_LOCK_DAEMON_ERR_SIGNAL; goto cleanup; } @@ -1366,6 +1376,7 @@ int main(int argc, char **argv) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; } + if (virNetServerAddProgram(lockDaemon->srv, lockProgram) < 0) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; @@ -1389,12 +1400,12 @@ int main(int argc, char **argv) { /* Start accepting new clients from network */ - virNetServerUpdateServices(lockDaemon->srv, true); - virNetServerRun(lockDaemon->srv); + virNetDaemonUpdateServices(lockDaemon->dmn, true); + virNetDaemonRun(lockDaemon->dmn); if (execRestart && virLockDaemonPreExecRestart(state_file, - lockDaemon->srv, + lockDaemon->dmn, argv) < 0) ret = VIR_LOCK_DAEMON_ERR_REEXEC; else diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c index 8a6d18f..106e820 100644 --- a/src/locking/lock_daemon_config.c +++ b/src/locking/lock_daemon_config.c @@ -28,7 +28,7 @@ #include "viralloc.h" #include "virerror.h" #include "virlog.h" -#include "rpc/virnetserver.h" +#include "rpc/virnetdaemon.h" #include "configmake.h" #include "virstring.h" #include "virutil.h" diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index bad646c..1b479db 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -1,7 +1,7 @@ /* * lock_daemon_dispatch.c: lock management daemon dispatch * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -22,7 +22,7 @@ #include <config.h> -#include "rpc/virnetserver.h" +#include "rpc/virnetdaemon.h" #include "rpc/virnetserverclient.h" #include "virlog.h" #include "virstring.h" diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index ba44e09..cc735a1 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2010-2015 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_controller.c: linux container process controller @@ -65,7 +65,7 @@ #include "virprocess.h" #include "virnuma.h" #include "virdbus.h" -#include "rpc/virnetserver.h" +#include "rpc/virnetdaemon.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -93,7 +93,7 @@ struct _virLXCControllerConsole { size_t fromContLen; char fromContBuf[1024]; - virNetServerPtr server; + virNetDaemonPtr daemon; }; typedef struct _virLXCController virLXCController; @@ -125,8 +125,7 @@ struct _virLXCController { virSecurityManagerPtr securityManager; - /* Server socket */ - virNetServerPtr server; + virNetDaemonPtr daemon; bool firstClient; virNetServerClientPtr client; virNetServerProgramPtr prog; @@ -149,7 +148,7 @@ static void virLXCControllerQuitTimer(int timer ATTRIBUTE_UNUSED, void *opaque) virLXCControllerPtr ctrl = opaque; VIR_DEBUG("Triggering event loop quit"); - virNetServerQuit(ctrl->server); + virNetDaemonQuit(ctrl->daemon); } @@ -280,7 +279,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) if (ctrl->timerShutdown != -1) virEventRemoveTimeout(ctrl->timerShutdown); - virObjectUnref(ctrl->server); + virObjectUnref(ctrl->daemon); virLXCControllerFreeFuse(ctrl); virCgroupFree(&ctrl->cgroup); @@ -296,7 +295,7 @@ static int virLXCControllerAddConsole(virLXCControllerPtr ctrl, { if (VIR_EXPAND_N(ctrl->consoles, ctrl->nconsoles, 1) < 0) return -1; - ctrl->consoles[ctrl->nconsoles-1].server = ctrl->server; + ctrl->consoles[ctrl->nconsoles-1].daemon = ctrl->daemon; ctrl->consoles[ctrl->nconsoles-1].hostFd = hostFd; ctrl->consoles[ctrl->nconsoles-1].hostWatch = -1; @@ -846,6 +845,7 @@ static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client, static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) { + virNetServerPtr srv = NULL; virNetServerServicePtr svc = NULL; char *sockpath; @@ -853,13 +853,13 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl->name) < 0) return -1; - if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, - 0, -1, 0, false, - NULL, - virLXCControllerClientPrivateNew, - NULL, - virLXCControllerClientPrivateFree, - ctrl))) + if (!(srv = virNetServerNew(0, 0, 0, 1, + 0, -1, 0, false, + NULL, + virLXCControllerClientPrivateNew, + NULL, + virLXCControllerClientPrivateFree, + ctrl))) goto error; if (virSecurityManagerSetSocketLabel(ctrl->securityManager, ctrl->def) < 0) @@ -880,7 +880,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) if (virSecurityManagerClearSocketLabel(ctrl->securityManager, ctrl->def) < 0) goto error; - if (virNetServerAddService(ctrl->server, svc, NULL) < 0) + if (virNetServerAddService(srv, svc, NULL) < 0) goto error; virObjectUnref(svc); svc = NULL; @@ -891,14 +891,19 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) virLXCMonitorNProcs))) goto error; - virNetServerUpdateServices(ctrl->server, true); + if (!(ctrl->daemon = virNetDaemonNew()) || + virNetDaemonAddServer(ctrl->daemon, srv) < 0) + goto error; + + virNetDaemonUpdateServices(ctrl->daemon, true); VIR_FREE(sockpath); return 0; error: VIR_FREE(sockpath); - virObjectUnref(ctrl->server); - ctrl->server = NULL; + virObjectUnref(srv); + virObjectUnref(ctrl->daemon); + ctrl->daemon = NULL; virObjectUnref(svc); return -1; } @@ -926,7 +931,7 @@ static bool wantReboot; static virMutex lock = VIR_MUTEX_INITIALIZER; -static void virLXCControllerSignalChildIO(virNetServerPtr server, +static void virLXCControllerSignalChildIO(virNetDaemonPtr daemon, siginfo_t *info ATTRIBUTE_UNUSED, void *opaque) { @@ -937,7 +942,7 @@ static void virLXCControllerSignalChildIO(virNetServerPtr server, ret = waitpid(-1, &status, WNOHANG); VIR_DEBUG("Got sig child %d vs %lld", ret, (unsigned long long)ctrl->initpid); if (ret == ctrl->initpid) { - virNetServerQuit(server); + virNetDaemonQuit(daemon); virMutexLock(&lock); if (WIFSIGNALED(status) && WTERMSIG(status) == SIGHUP) { @@ -996,7 +1001,7 @@ static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr consol VIR_DEBUG(":fail"); virReportSystemError(errno, "%s", _("Unable to add epoll fd")); - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); goto cleanup; } console->hostEpoll = events; @@ -1008,7 +1013,7 @@ static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr consol virReportSystemError(errno, "%s", _("Unable to remove epoll fd")); VIR_DEBUG(":fail"); - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); goto cleanup; } console->hostEpoll = 0; @@ -1034,7 +1039,7 @@ static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr consol virReportSystemError(errno, "%s", _("Unable to add epoll fd")); VIR_DEBUG(":fail"); - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); goto cleanup; } console->contEpoll = events; @@ -1046,7 +1051,7 @@ static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr consol virReportSystemError(errno, "%s", _("Unable to remove epoll fd")); VIR_DEBUG(":fail"); - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); goto cleanup; } console->contEpoll = 0; @@ -1075,7 +1080,7 @@ static void virLXCControllerConsoleEPoll(int watch, int fd, int events, void *op continue; virReportSystemError(errno, "%s", _("Unable to wait on epoll")); - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); goto cleanup; } @@ -1188,7 +1193,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu virEventRemoveHandle(console->contWatch); virEventRemoveHandle(console->hostWatch); console->contWatch = console->hostWatch = -1; - virNetServerQuit(console->server); + virNetDaemonQuit(console->daemon); virMutexUnlock(&lock); } @@ -1208,7 +1213,7 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl) int rc = -1; size_t i; - if (virNetServerAddSignalHandler(ctrl->server, + if (virNetDaemonAddSignalHandler(ctrl->daemon, SIGCHLD, virLXCControllerSignalChildIO, ctrl) < 0) @@ -1254,7 +1259,7 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl) } } - virNetServerRun(ctrl->server); + virNetDaemonRun(ctrl->daemon); err = virGetLastError(); if (!err || err->code == VIR_ERR_OK) @@ -2228,7 +2233,7 @@ virLXCControllerEventSendExit(virLXCControllerPtr ctrl, VIR_DEBUG("Waiting for client to complete dispatch"); ctrl->inShutdown = true; virNetServerClientDelayedClose(ctrl->client); - virNetServerRun(ctrl->server); + virNetDaemonRun(ctrl->daemon); } VIR_DEBUG("Client has gone away"); return 0; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c @@ -0,0 +1,746 @@ +/* + * virnetdaemon.c + * + * Copyright (C) 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@redhat.com> + */ + +#include <config.h> + +#include <unistd.h> +#include <string.h> +#include <fcntl.h> + +#include "virnetdaemon.h" +#include "virlog.h" +#include "viralloc.h" +#include "virerror.h" +#include "virthread.h" +#include "virthreadpool.h" +#include "virutil.h" +#include "virfile.h" +#include "virnetserver.h" +#include "virnetservermdns.h" +#include "virdbus.h" +#include "virstring.h" +#include "virsystemd.h" + +#ifndef SA_SIGINFO +# define SA_SIGINFO 0 +#endif + +#define VIR_FROM_THIS VIR_FROM_RPC + +VIR_LOG_INIT("rpc.netserver"); + +typedef struct _virNetDaemonSignal virNetDaemonSignal; +typedef virNetDaemonSignal *virNetDaemonSignalPtr; + +struct _virNetDaemonSignal { + struct sigaction oldaction; + int signum; + virNetDaemonSignalFunc func; + void *opaque; +}; + +struct _virNetDaemon { + virObjectLockable parent; + + bool privileged; + + size_t nsignals; + virNetDaemonSignalPtr *signals; + int sigread; + int sigwrite; + int sigwatch; + + size_t nservers; + virNetServerPtr *servers; + virJSONValuePtr srvObject; + + bool quit; + + unsigned int autoShutdownTimeout; + size_t autoShutdownInhibitions; + bool autoShutdownCallingInhibit; + int autoShutdownInhibitFd; +}; + + +static virClassPtr virNetDaemonClass; + +void +virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetDaemon) + + +virNetDaemonPtr +virNetDaemonNew(void) +{ + virNetDaemonPtr dmn; + struct sigaction sig_action; + + if (virNetDaemonInitialize() < 0) + return NULL; + + if (!(dmn = virObjectLockableNew(virNetDaemonClass))) + return NULL; + + dmn->sigwrite = dmn->sigread = -1; + dmn->privileged = geteuid() == 0; + dmn->autoShutdownInhibitFd = -1; + + if (virEventRegisterDefaultImpl() < 0) + goto error; + + memset(&sig_action, 0, sizeof(sig_action)); + sig_action.sa_handler = SIG_IGN; + sigaction(SIGPIPE, &sig_action, NULL); + + return dmn; + + error: + virObjectUnref(dmn); + return NULL; +} + + +int +virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr srv) +{ + int ret = -1; + + virObjectLock(dmn); + + if (VIR_APPEND_ELEMENT(dmn->servers, dmn->nservers, srv) < 0) + goto cleanup; + + ret = dmn->nservers - 1; + cleanup: + virObjectUnlock(dmn); + return ret; +} + + +/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function. + */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID); + virObjectUnlock(dmn); + + return srv; +} + +virNetServerPtr +virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + virNetServerClientPrivNew clientPrivNew, + virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, + virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, + virFreeCallback clientPrivFree, + void *clientPrivOpaque) +{ + virJSONValuePtr object = NULL; + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + + if (!dmn->srvObject) + goto error; + + if (virJSONValueIsArray(dmn->srvObject)) { + object = virJSONValueArraySteal(dmn->srvObject, 0); + if (virJSONValueArraySize(dmn->srvObject) == 0) { + virJSONValueFree(dmn->srvObject); + dmn->srvObject = NULL; + } + } else { + object = dmn->srvObject; + dmn->srvObject = NULL; + } + + srv = virNetServerNewPostExecRestart(object, + clientPrivNew, + clientPrivNewPostExecRestart, + clientPrivPreExecRestart, + clientPrivFree, + clientPrivOpaque); + + if (!srv || VIR_APPEND_ELEMENT(dmn->servers, dmn->nservers, srv) < 0) + goto error; + + virJSONValueFree(object); + virObjectUnlock(dmn); + return srv; + + error: + virObjectUnlock(dmn); + virObjectUnref(srv); + virJSONValueFree(object); + return NULL; +} + + +virNetDaemonPtr +virNetDaemonNewPostExecRestart(virJSONValuePtr object) +{ + virNetDaemonPtr dmn = NULL; + virJSONValuePtr servers = virJSONValueObjectGet(object, "servers"); + + if (!(dmn = virNetDaemonNew())) + goto error; + + if (!servers) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed servers data in JSON document")); + goto error; + } + + if (!(dmn->srvObject = virJSONValueCopy(servers))) + goto error; + + return dmn; + error: + virObjectUnref(dmn); + return NULL; +} + + +virJSONValuePtr +virNetDaemonPreExecRestart(virNetDaemonPtr dmn) +{ + virJSONValuePtr object, srvArray = NULL; + size_t i; + + virObjectLock(dmn); + + if (!(object = virJSONValueNewObject())) + goto error; + + if (!(srvArray = virJSONValueNewArray()) || + virJSONValueObjectAppend(object, "servers", srvArray) < 0) + goto error; + + for (i = 0; i < dmn->nservers; i++) { + virJSONValuePtr srvJSON = NULL; + srvJSON = virNetServerPreExecRestart(dmn->servers[i]); + + if (!srvJSON) + goto error; + + if (virJSONValueArrayAppend(srvArray, srvJSON) < 0) { + virJSONValueFree(srvJSON); + goto error; + } + } + + virObjectUnlock(dmn); + + return object; + + error: + virJSONValueFree(object); + virJSONValueFree(srvArray); + virObjectUnlock(dmn); + return NULL; +} + + +bool +virNetDaemonIsPrivileged(virNetDaemonPtr dmn) +{ + bool priv; + virObjectLock(dmn); + priv = dmn->privileged; + virObjectUnlock(dmn); + return priv; +} + + +void +virNetDaemonAutoShutdown(virNetDaemonPtr dmn, + unsigned int timeout) +{ + virObjectLock(dmn); + + dmn->autoShutdownTimeout = timeout; + + virObjectUnlock(dmn); +} + + +#if defined(HAVE_DBUS) && defined(DBUS_TYPE_UNIX_FD) +static void +virNetDaemonGotInhibitReply(DBusPendingCall *pending, + void *opaque) +{ + virNetDaemonPtr dmn = opaque; + DBusMessage *reply; + int fd; + + virObjectLock(dmn); + dmn->autoShutdownCallingInhibit = false; + + VIR_DEBUG("dmn=%p", dmn); + + reply = dbus_pending_call_steal_reply(pending); + if (reply == NULL) + goto cleanup; + + if (dbus_message_get_args(reply, NULL, + DBUS_TYPE_UNIX_FD, &fd, + DBUS_TYPE_INVALID)) { + if (dmn->autoShutdownInhibitions) { + dmn->autoShutdownInhibitFd = fd; + } else { + /* We stopped the last VM since we made the inhibit call */ + VIR_FORCE_CLOSE(fd); + } + } + dbus_message_unref(reply); + + cleanup: + virObjectUnlock(dmn); +} + + +/* As per: http://www.freedesktop.org/wiki/Software/systemd/inhibit */ +static void +virNetDaemonCallInhibit(virNetDaemonPtr dmn, + const char *what, + const char *who, + const char *why, + const char *mode) +{ + DBusMessage *message; + DBusPendingCall *pendingReply; + DBusConnection *systemBus; + + VIR_DEBUG("dmn=%p what=%s who=%s why=%s mode=%s", + dmn, NULLSTR(what), NULLSTR(who), NULLSTR(why), NULLSTR(mode)); + + if (!(systemBus = virDBusGetSystemBus())) + return; + + /* Only one outstanding call at a time */ + if (dmn->autoShutdownCallingInhibit) + return; + + message = dbus_message_new_method_call("org.freedesktop.login1", + "/org/freedesktop/login1", + "org.freedesktop.login1.Manager", + "Inhibit"); + if (message == NULL) + return; + + dbus_message_append_args(message, + DBUS_TYPE_STRING, &what, + DBUS_TYPE_STRING, &who, + DBUS_TYPE_STRING, &why, + DBUS_TYPE_STRING, &mode, + DBUS_TYPE_INVALID); + + pendingReply = NULL; + if (dbus_connection_send_with_reply(systemBus, message, + &pendingReply, + 25*1000)) { + dbus_pending_call_set_notify(pendingReply, + virNetDaemonGotInhibitReply, + dmn, NULL); + dmn->autoShutdownCallingInhibit = true; + } + dbus_message_unref(message); +} +#endif + +void +virNetDaemonAddShutdownInhibition(virNetDaemonPtr dmn) +{ + virObjectLock(dmn); + dmn->autoShutdownInhibitions++; + + VIR_DEBUG("dmn=%p inhibitions=%zu", dmn, dmn->autoShutdownInhibitions); + +#if defined(HAVE_DBUS) && defined(DBUS_TYPE_UNIX_FD) + if (dmn->autoShutdownInhibitions == 1) + virNetDaemonCallInhibit(dmn, + "shutdown", + _("Libvirt"), + _("Virtual machines need to be saved"), + "delay"); +#endif + + virObjectUnlock(dmn); +} + + +void +virNetDaemonRemoveShutdownInhibition(virNetDaemonPtr dmn) +{ + virObjectLock(dmn); + dmn->autoShutdownInhibitions--; + + VIR_DEBUG("dmn=%p inhibitions=%zu", dmn, dmn->autoShutdownInhibitions); + + if (dmn->autoShutdownInhibitions == 0) + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + virObjectUnlock(dmn); +} + + + +static sig_atomic_t sigErrors; +static int sigLastErrno; +static int sigWrite = -1; + +static void +virNetDaemonSignalHandler(int sig, siginfo_t * siginfo, + void* context ATTRIBUTE_UNUSED) +{ + int origerrno; + int r; + siginfo_t tmp; + + if (SA_SIGINFO) + tmp = *siginfo; + else + memset(&tmp, 0, sizeof(tmp)); + + /* set the sig num in the struct */ + tmp.si_signo = sig; + + origerrno = errno; + r = safewrite(sigWrite, &tmp, sizeof(tmp)); + if (r == -1) { + sigErrors++; + sigLastErrno = errno; + } + errno = origerrno; +} + +static void +virNetDaemonSignalEvent(int watch, + int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, + void *opaque) +{ + virNetDaemonPtr dmn = opaque; + siginfo_t siginfo; + size_t i; + + virObjectLock(dmn); + + if (saferead(dmn->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo)) { + virReportSystemError(errno, "%s", + _("Failed to read from signal pipe")); + virEventRemoveHandle(watch); + dmn->sigwatch = -1; + goto cleanup; + } + + for (i = 0; i < dmn->nsignals; i++) { + if (siginfo.si_signo == dmn->signals[i]->signum) { + virNetDaemonSignalFunc func = dmn->signals[i]->func; + void *funcopaque = dmn->signals[i]->opaque; + virObjectUnlock(dmn); + func(dmn, &siginfo, funcopaque); + return; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected signal received: %d"), siginfo.si_signo); + + cleanup: + virObjectUnlock(dmn); +} + +static int +virNetDaemonSignalSetup(virNetDaemonPtr dmn) +{ + int fds[2] = { -1, -1 }; + + if (dmn->sigwrite != -1) + return 0; + + if (pipe2(fds, O_CLOEXEC|O_NONBLOCK) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create signal pipe")); + return -1; + } + + if ((dmn->sigwatch = virEventAddHandle(fds[0], + VIR_EVENT_HANDLE_READABLE, + virNetDaemonSignalEvent, + dmn, NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to add signal handle watch")); + goto error; + } + + dmn->sigread = fds[0]; + dmn->sigwrite = fds[1]; + sigWrite = fds[1]; + + return 0; + + error: + VIR_FORCE_CLOSE(fds[0]); + VIR_FORCE_CLOSE(fds[1]); + return -1; +} + +int +virNetDaemonAddSignalHandler(virNetDaemonPtr dmn, + int signum, + virNetDaemonSignalFunc func, + void *opaque) +{ + virNetDaemonSignalPtr sigdata = NULL; + struct sigaction sig_action; + + virObjectLock(dmn); + + if (virNetDaemonSignalSetup(dmn) < 0) + goto error; + + if (VIR_EXPAND_N(dmn->signals, dmn->nsignals, 1) < 0) + goto error; + + if (VIR_ALLOC(sigdata) < 0) + goto error; + + sigdata->signum = signum; + sigdata->func = func; + sigdata->opaque = opaque; + + memset(&sig_action, 0, sizeof(sig_action)); + sig_action.sa_sigaction = virNetDaemonSignalHandler; + sig_action.sa_flags = SA_SIGINFO; + sigemptyset(&sig_action.sa_mask); + + sigaction(signum, &sig_action, &sigdata->oldaction); + + dmn->signals[dmn->nsignals-1] = sigdata; + + virObjectUnlock(dmn); + return 0; + + error: + VIR_FREE(sigdata); + virObjectUnlock(dmn); + return -1; +} + + +static void +virNetDaemonAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, + void *opaque) +{ + virNetDaemonPtr dmn = opaque; + + virObjectLock(dmn); + + if (!dmn->autoShutdownInhibitions) { + VIR_DEBUG("Automatic shutdown triggered"); + dmn->quit = true; + } + + virObjectUnlock(dmn); +} + +void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn); +} + +void +virNetDaemonRun(virNetDaemonPtr dmn) +{ + int timerid = -1; + bool timerActive = false; + size_t i; + + virObjectLock(dmn); + + if (dmn->srvObject) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Not all servers restored, cannot run server")); + goto cleanup; + } + + for (i = 0; i < dmn->nservers; i++) { + if (virNetServerStart(dmn->servers[i]) < 0) + goto cleanup; + } + + dmn->quit = false; + + if (dmn->autoShutdownTimeout && + (timerid = virEventAddTimeout(-1, + virNetDaemonAutoShutdownTimer, + dmn, NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to register shutdown timeout")); + goto cleanup; + } + + /* We are accepting connections now. Notify systemd + * so it can start dependent services. */ + virSystemdNotifyStartup(); + + VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); + while (!dmn->quit) { + /* A shutdown timeout is specified, so check + * if any drivers have active state, if not + * shutdown after timeout seconds + */ + if (dmn->autoShutdownTimeout) { + if (timerActive) { + for (i = 0; i < dmn->nservers; i++) { + if (virNetServerHasClients(dmn->servers[i])) { + VIR_DEBUG("Deactivating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, -1); + timerActive = false; + break; + } + } + } else { + for (i = 0; i < dmn->nservers; i++) { + if (!virNetServerHasClients(dmn->servers[i])) { + VIR_DEBUG("Activating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, + dmn->autoShutdownTimeout * 1000); + timerActive = true; + break; + } + } + } + } + + virObjectUnlock(dmn); + if (virEventRunDefaultImpl() < 0) { + virObjectLock(dmn); + VIR_DEBUG("Loop iteration error, exiting"); + break; + } + virObjectLock(dmn); + + for (i = 0; i < dmn->nservers; i++) + virNetServerProcessClients(dmn->servers[i]); + } + + cleanup: + virObjectUnlock(dmn); +} + + +void +virNetDaemonQuit(virNetDaemonPtr dmn) +{ + virObjectLock(dmn); + + VIR_DEBUG("Quit requested %p", dmn); + dmn->quit = true; + + virObjectUnlock(dmn); +} + + +void +virNetDaemonClose(virNetDaemonPtr dmn) +{ + size_t i; + + if (!dmn) + return; + + virObjectLock(dmn); + + for (i = 0; i < dmn->nservers; i++) + virNetServerClose(dmn->servers[i]); + + virObjectUnlock(dmn); +} diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h new file mode 100644 index 0000000..9e176d6 --- /dev/null +++ b/src/rpc/virnetdaemon.h @@ -0,0 +1,82 @@ +/* + * virnetdaemon.h + * + * Copyright (C) 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@redhat.com> + */ + +#ifndef __VIR_NET_DAEMON_H__ +# define __VIR_NET_DAEMON_H__ + +# include <signal.h> + +# ifdef WITH_GNUTLS +# include "virnettlscontext.h" +# endif +# include "virobject.h" +# include "virjson.h" +# include "virnetserverprogram.h" +# include "virnetserverclient.h" +# include "virnetserverservice.h" +# include "virnetserver.h" + +virNetDaemonPtr virNetDaemonNew(void); + +int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr); + +virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + virNetServerClientPrivNew clientPrivNew, + virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, + virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, + virFreeCallback clientPrivFree, + void *clientPrivOpaque); + +virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object); + +virJSONValuePtr virNetDaemonPreExecRestart(virNetDaemonPtr dmn); + +typedef int (*virNetDaemonAutoShutdownFunc)(virNetDaemonPtr dmn, void *opaque); + +bool virNetDaemonIsPrivileged(virNetDaemonPtr dmn); + +void virNetDaemonAutoShutdown(virNetDaemonPtr dmn, + unsigned int timeout); + +void virNetDaemonAddShutdownInhibition(virNetDaemonPtr dmn); +void virNetDaemonRemoveShutdownInhibition(virNetDaemonPtr dmn); + +typedef void (*virNetDaemonSignalFunc)(virNetDaemonPtr dmn, siginfo_t *info, void *opaque); + +int virNetDaemonAddSignalHandler(virNetDaemonPtr dmn, + int signum, + virNetDaemonSignalFunc func, + void *opaque); + +void virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled); + +void virNetDaemonRun(virNetDaemonPtr dmn); + +void virNetDaemonQuit(virNetDaemonPtr dmn); + +void virNetDaemonClose(virNetDaemonPtr dmn); + +virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID); + +#endif /* __VIR_NET_DAEMON_H__ */ diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 42427dc..df844d9 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1,7 +1,7 @@ /* * virnetserver.c: generic network RPC server * - * Copyright (C) 2006-2012, 2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -23,40 +23,19 @@ #include <config.h> -#include <unistd.h> -#include <string.h> -#include <fcntl.h> - #include "virnetserver.h" #include "virlog.h" #include "viralloc.h" #include "virerror.h" #include "virthread.h" #include "virthreadpool.h" -#include "virutil.h" -#include "virfile.h" #include "virnetservermdns.h" -#include "virdbus.h" #include "virstring.h" -#include "virsystemd.h" - -#ifndef SA_SIGINFO -# define SA_SIGINFO 0 -#endif #define VIR_FROM_THIS VIR_FROM_RPC VIR_LOG_INIT("rpc.netserver"); -typedef struct _virNetServerSignal virNetServerSignal; -typedef virNetServerSignal *virNetServerSignalPtr; - -struct _virNetServerSignal { - struct sigaction oldaction; - int signum; - virNetServerSignalFunc func; - void *opaque; -}; typedef struct _virNetServerJob virNetServerJob; typedef virNetServerJob *virNetServerJobPtr; @@ -72,14 +51,6 @@ struct _virNetServer { virThreadPoolPtr workers; - bool privileged; - - size_t nsignals; - virNetServerSignalPtr *signals; - int sigread; - int sigwrite; - int sigwatch; - char *mdnsGroupName; virNetServerMDNSPtr mdns; virNetServerMDNSGroupPtr mdnsGroup; @@ -100,17 +71,10 @@ struct _virNetServer { unsigned int keepaliveCount; bool keepaliveRequired; - bool quit; - #ifdef WITH_GNUTLS virNetTLSContextPtr tls; #endif - unsigned int autoShutdownTimeout; - size_t autoShutdownInhibitions; - bool autoShutdownCallingInhibit; - int autoShutdownInhibitFd; - virNetServerClientPrivNew clientPrivNew; virNetServerClientPrivPreExecRestart clientPrivPreExecRestart; virFreeCallback clientPrivFree; @@ -259,8 +223,8 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, } -static int virNetServerAddClient(virNetServerPtr srv, - virNetServerClientPtr client) +int virNetServerAddClient(virNetServerPtr srv, + virNetServerClientPtr client) { virObjectLock(srv); @@ -356,7 +320,6 @@ virNetServerPtr virNetServerNew(size_t min_workers, void *clientPrivOpaque) { virNetServerPtr srv; - struct sigaction sig_action; if (virNetServerInitialize() < 0) return NULL; @@ -376,13 +339,10 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->keepaliveInterval = keepaliveInterval; srv->keepaliveCount = keepaliveCount; srv->keepaliveRequired = keepaliveRequired; - srv->sigwrite = srv->sigread = -1; srv->clientPrivNew = clientPrivNew; srv->clientPrivPreExecRestart = clientPrivPreExecRestart; srv->clientPrivFree = clientPrivFree; srv->clientPrivOpaque = clientPrivOpaque; - srv->privileged = geteuid() == 0; - srv->autoShutdownInhibitFd = -1; if (VIR_STRDUP(srv->mdnsGroupName, mdnsGroupName) < 0) goto error; @@ -394,15 +354,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, goto error; } - if (virEventRegisterDefaultImpl() < 0) - goto error; - - memset(&sig_action, 0, sizeof(sig_action)); - sig_action.sa_handler = SIG_IGN; - sigaction(SIGPIPE, &sig_action, NULL); - return srv; - error: virObjectUnref(srv); return NULL; @@ -484,7 +436,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - if (!(srv = virNetServerNew(min_workers, max_workers, + if (!(srv = virNetServerNew(min_workers, max_clients, priority_workers, max_clients, max_anonymous_clients, keepaliveInterval, keepaliveCount, @@ -499,7 +451,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - n = virJSONValueArraySize(services); + n = virJSONValueArraySize(services); if (n < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Malformed services data in JSON document")); @@ -532,7 +484,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - n = virJSONValueArraySize(clients); + n = virJSONValueArraySize(clients); if (n < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Malformed clients data in JSON document")); @@ -679,286 +631,6 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) } -bool virNetServerIsPrivileged(virNetServerPtr srv) -{ - bool priv; - virObjectLock(srv); - priv = srv->privileged; - virObjectUnlock(srv); - return priv; -} - - -void virNetServerAutoShutdown(virNetServerPtr srv, - unsigned int timeout) -{ - virObjectLock(srv); - - srv->autoShutdownTimeout = timeout; - - virObjectUnlock(srv); -} - - -#if defined(HAVE_DBUS) && defined(DBUS_TYPE_UNIX_FD) -static void virNetServerGotInhibitReply(DBusPendingCall *pending, - void *opaque) -{ - virNetServerPtr srv = opaque; - DBusMessage *reply; - int fd; - - virObjectLock(srv); - srv->autoShutdownCallingInhibit = false; - - VIR_DEBUG("srv=%p", srv); - - reply = dbus_pending_call_steal_reply(pending); - if (reply == NULL) - goto cleanup; - - if (dbus_message_get_args(reply, NULL, - DBUS_TYPE_UNIX_FD, &fd, - DBUS_TYPE_INVALID)) { - if (srv->autoShutdownInhibitions) { - srv->autoShutdownInhibitFd = fd; - } else { - /* We stopped the last VM since we made the inhibit call */ - VIR_FORCE_CLOSE(fd); - } - } - dbus_message_unref(reply); - - cleanup: - virObjectUnlock(srv); -} - - -/* As per: http://www.freedesktop.org/wiki/Software/systemd/inhibit */ -static void virNetServerCallInhibit(virNetServerPtr srv, - const char *what, - const char *who, - const char *why, - const char *mode) -{ - DBusMessage *message; - DBusPendingCall *pendingReply; - DBusConnection *systemBus; - - VIR_DEBUG("srv=%p what=%s who=%s why=%s mode=%s", - srv, NULLSTR(what), NULLSTR(who), NULLSTR(why), NULLSTR(mode)); - - if (!(systemBus = virDBusGetSystemBus())) - return; - - /* Only one outstanding call at a time */ - if (srv->autoShutdownCallingInhibit) - return; - - message = dbus_message_new_method_call("org.freedesktop.login1", - "/org/freedesktop/login1", - "org.freedesktop.login1.Manager", - "Inhibit"); - if (message == NULL) - return; - - dbus_message_append_args(message, - DBUS_TYPE_STRING, &what, - DBUS_TYPE_STRING, &who, - DBUS_TYPE_STRING, &why, - DBUS_TYPE_STRING, &mode, - DBUS_TYPE_INVALID); - - pendingReply = NULL; - if (dbus_connection_send_with_reply(systemBus, message, - &pendingReply, - 25*1000)) { - dbus_pending_call_set_notify(pendingReply, - virNetServerGotInhibitReply, - srv, NULL); - srv->autoShutdownCallingInhibit = true; - } - dbus_message_unref(message); -} -#endif - -void virNetServerAddShutdownInhibition(virNetServerPtr srv) -{ - virObjectLock(srv); - srv->autoShutdownInhibitions++; - - VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); - -#if defined(HAVE_DBUS) && defined(DBUS_TYPE_UNIX_FD) - if (srv->autoShutdownInhibitions == 1) - virNetServerCallInhibit(srv, - "shutdown", - _("Libvirt"), - _("Virtual machines need to be saved"), - "delay"); -#endif - - virObjectUnlock(srv); -} - - -void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) -{ - virObjectLock(srv); - srv->autoShutdownInhibitions--; - - VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); - - if (srv->autoShutdownInhibitions == 0) - VIR_FORCE_CLOSE(srv->autoShutdownInhibitFd); - - virObjectUnlock(srv); -} - - - -static sig_atomic_t sigErrors; -static int sigLastErrno; -static int sigWrite = -1; - -static void -virNetServerSignalHandler(int sig, siginfo_t * siginfo, - void* context ATTRIBUTE_UNUSED) -{ - int origerrno; - int r; - siginfo_t tmp; - - if (SA_SIGINFO) - tmp = *siginfo; - else - memset(&tmp, 0, sizeof(tmp)); - - /* set the sig num in the struct */ - tmp.si_signo = sig; - - origerrno = errno; - r = safewrite(sigWrite, &tmp, sizeof(tmp)); - if (r == -1) { - sigErrors++; - sigLastErrno = errno; - } - errno = origerrno; -} - -static void -virNetServerSignalEvent(int watch, - int fd ATTRIBUTE_UNUSED, - int events ATTRIBUTE_UNUSED, - void *opaque) -{ - virNetServerPtr srv = opaque; - siginfo_t siginfo; - size_t i; - - virObjectLock(srv); - - if (saferead(srv->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo)) { - virReportSystemError(errno, "%s", - _("Failed to read from signal pipe")); - virEventRemoveHandle(watch); - srv->sigwatch = -1; - goto cleanup; - } - - for (i = 0; i < srv->nsignals; i++) { - if (siginfo.si_signo == srv->signals[i]->signum) { - virNetServerSignalFunc func = srv->signals[i]->func; - void *funcopaque = srv->signals[i]->opaque; - virObjectUnlock(srv); - func(srv, &siginfo, funcopaque); - return; - } - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected signal received: %d"), siginfo.si_signo); - - cleanup: - virObjectUnlock(srv); -} - -static int virNetServerSignalSetup(virNetServerPtr srv) -{ - int fds[2] = { -1, -1 }; - - if (srv->sigwrite != -1) - return 0; - - if (pipe2(fds, O_CLOEXEC|O_NONBLOCK) < 0) { - virReportSystemError(errno, "%s", - _("Unable to create signal pipe")); - return -1; - } - - if ((srv->sigwatch = virEventAddHandle(fds[0], - VIR_EVENT_HANDLE_READABLE, - virNetServerSignalEvent, - srv, NULL)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to add signal handle watch")); - goto error; - } - - srv->sigread = fds[0]; - srv->sigwrite = fds[1]; - sigWrite = fds[1]; - - return 0; - - error: - VIR_FORCE_CLOSE(fds[0]); - VIR_FORCE_CLOSE(fds[1]); - return -1; -} - -int virNetServerAddSignalHandler(virNetServerPtr srv, - int signum, - virNetServerSignalFunc func, - void *opaque) -{ - virNetServerSignalPtr sigdata = NULL; - struct sigaction sig_action; - - virObjectLock(srv); - - if (virNetServerSignalSetup(srv) < 0) - goto error; - - if (VIR_EXPAND_N(srv->signals, srv->nsignals, 1) < 0) - goto error; - - if (VIR_ALLOC(sigdata) < 0) - goto error; - - sigdata->signum = signum; - sigdata->func = func; - sigdata->opaque = opaque; - - memset(&sig_action, 0, sizeof(sig_action)); - sig_action.sa_sigaction = virNetServerSignalHandler; - sig_action.sa_flags = SA_SIGINFO; - sigemptyset(&sig_action.sa_mask); - - sigaction(signum, &sig_action, &sigdata->oldaction); - - srv->signals[srv->nsignals-1] = sigdata; - - virObjectUnlock(srv); - return 0; - - error: - VIR_FREE(sigdata); - virObjectUnlock(srv); - return -1; -} - - int virNetServerAddService(virNetServerPtr srv, virNetServerServicePtr svc, @@ -1021,22 +693,6 @@ int virNetServerSetTLSContext(virNetServerPtr srv, #endif -static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, - void *opaque) -{ - virNetServerPtr srv = opaque; - - virObjectLock(srv); - - if (!srv->autoShutdownInhibitions) { - VIR_DEBUG("Automatic shutdown triggered"); - srv->quit = true; - } - - virObjectUnlock(srv); -} - - static void virNetServerUpdateServicesLocked(virNetServerPtr srv, bool enabled) @@ -1085,127 +741,16 @@ virNetServerCheckLimits(virNetServerPtr srv) } } -void virNetServerRun(virNetServerPtr srv) -{ - int timerid = -1; - bool timerActive = false; - size_t i; - - virObjectLock(srv); - - if (srv->mdns && - virNetServerMDNSStart(srv->mdns) < 0) - goto cleanup; - - srv->quit = false; - - if (srv->autoShutdownTimeout && - (timerid = virEventAddTimeout(-1, - virNetServerAutoShutdownTimer, - srv, NULL)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to register shutdown timeout")); - goto cleanup; - } - - /* We are accepting connections now. Notify systemd - * so it can start dependent services. */ - virSystemdNotifyStartup(); - - VIR_DEBUG("srv=%p quit=%d", srv, srv->quit); - while (!srv->quit) { - /* A shutdown timeout is specified, so check - * if any drivers have active state, if not - * shutdown after timeout seconds - */ - if (srv->autoShutdownTimeout) { - if (timerActive) { - if (srv->clients) { - VIR_DEBUG("Deactivating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, -1); - timerActive = false; - } - } else { - if (!srv->clients) { - VIR_DEBUG("Activating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, - srv->autoShutdownTimeout * 1000); - timerActive = true; - } - } - } - - virObjectUnlock(srv); - if (virEventRunDefaultImpl() < 0) { - virObjectLock(srv); - VIR_DEBUG("Loop iteration error, exiting"); - break; - } - virObjectLock(srv); - - reprocess: - for (i = 0; i < srv->nclients; i++) { - /* Coverity 5.3.0 couldn't see that srv->clients is non-NULL - * if srv->nclients is non-zero. */ - sa_assert(srv->clients); - if (virNetServerClientWantClose(srv->clients[i])) - virNetServerClientClose(srv->clients[i]); - if (virNetServerClientIsClosed(srv->clients[i])) { - virNetServerClientPtr client = srv->clients[i]; - - VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients); - - if (virNetServerClientNeedAuth(client)) - virNetServerTrackCompletedAuthLocked(srv); - - virNetServerCheckLimits(srv); - - virObjectUnlock(srv); - virObjectUnref(client); - virObjectLock(srv); - - goto reprocess; - } - } - } - - cleanup: - virObjectUnlock(srv); -} - - -void virNetServerQuit(virNetServerPtr srv) -{ - virObjectLock(srv); - - VIR_DEBUG("Quit requested %p", srv); - srv->quit = true; - - virObjectUnlock(srv); -} - void virNetServerDispose(void *obj) { virNetServerPtr srv = obj; size_t i; - VIR_FORCE_CLOSE(srv->autoShutdownInhibitFd); - for (i = 0; i < srv->nservices; i++) virNetServerServiceToggle(srv->services[i], false); virThreadPoolFree(srv->workers); - for (i = 0; i < srv->nsignals; i++) { - sigaction(srv->signals[i]->signum, &srv->signals[i]->oldaction, NULL); - VIR_FREE(srv->signals[i]); - } - VIR_FREE(srv->signals); - VIR_FORCE_CLOSE(srv->sigread); - VIR_FORCE_CLOSE(srv->sigwrite); - if (srv->sigwatch > 0) - virEventRemoveHandle(srv->sigwatch); - for (i = 0; i < srv->nservices; i++) virObjectUnref(srv->services[i]); VIR_FREE(srv->services); @@ -1278,3 +823,62 @@ size_t virNetServerTrackCompletedAuth(virNetServerPtr srv) virObjectUnlock(srv); return ret; } + +bool +virNetServerHasClients(virNetServerPtr srv) +{ + bool ret; + + virObjectLock(srv); + ret = !!srv->nclients; + virObjectUnlock(srv); + + return ret; +} + +void +virNetServerProcessClients(virNetServerPtr srv) +{ + size_t i; + + virObjectLock(srv); + + reprocess: + for (i = 0; i < srv->nclients; i++) { + /* Coverity 5.3.0 couldn't see that srv->clients is non-NULL + * if srv->nclients is non-zero. */ + sa_assert(srv->clients); + if (virNetServerClientWantClose(srv->clients[i])) + virNetServerClientClose(srv->clients[i]); + if (virNetServerClientIsClosed(srv->clients[i])) { + virNetServerClientPtr client = srv->clients[i]; + + VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients); + + if (virNetServerClientNeedAuth(client)) + virNetServerTrackCompletedAuthLocked(srv); + + virNetServerCheckLimits(srv); + + virObjectUnlock(srv); + virObjectUnref(client); + virObjectLock(srv); + + goto reprocess; + } + } + + virObjectUnlock(srv); +} + +int +virNetServerStart(virNetServerPtr srv) +{ + /* + * Do whatever needs to be done before starting. + */ + if (!srv->mdns) + return 0; + + return virNetServerMDNSStart(srv->mdns); +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 8c5ae07..3e312c0 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -1,7 +1,7 @@ /* * virnetserver.h: generic network RPC server * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -24,8 +24,6 @@ #ifndef __VIR_NET_SERVER_H__ # define __VIR_NET_SERVER_H__ -# include <signal.h> - # ifdef WITH_GNUTLS # include "virnettlscontext.h" # endif @@ -35,6 +33,9 @@ # include "virobject.h" # include "virjson.h" +typedef struct _virNetServer virNetServer; +typedef virNetServer *virNetServerPtr; + virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t priority_workers, @@ -56,24 +57,9 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, virFreeCallback clientPrivFree, void *clientPrivOpaque); -virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv); - -typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); - -bool virNetServerIsPrivileged(virNetServerPtr srv); - -void virNetServerAutoShutdown(virNetServerPtr srv, - unsigned int timeout); - -void virNetServerAddShutdownInhibition(virNetServerPtr srv); -void virNetServerRemoveShutdownInhibition(virNetServerPtr srv); - -typedef void (*virNetServerSignalFunc)(virNetServerPtr srv, siginfo_t *info, void *opaque); +void virNetServerClose(virNetServerPtr srv); -int virNetServerAddSignalHandler(virNetServerPtr srv, - int signum, - virNetServerSignalFunc func, - void *opaque); +virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv); int virNetServerAddService(virNetServerPtr srv, virNetServerServicePtr svc, @@ -87,18 +73,18 @@ int virNetServerSetTLSContext(virNetServerPtr srv, virNetTLSContextPtr tls); # endif -void virNetServerUpdateServices(virNetServerPtr srv, - bool enabled); - -void virNetServerRun(virNetServerPtr srv); - -void virNetServerQuit(virNetServerPtr srv); - -void virNetServerClose(virNetServerPtr srv); - bool virNetServerKeepAliveRequired(virNetServerPtr srv); size_t virNetServerTrackPendingAuth(virNetServerPtr srv); size_t virNetServerTrackCompletedAuth(virNetServerPtr srv); -#endif +int virNetServerAddClient(virNetServerPtr srv, + virNetServerClientPtr client); +bool virNetServerHasClients(virNetServerPtr srv); +void virNetServerProcessClients(virNetServerPtr srv); + +void virNetServerUpdateServices(virNetServerPtr srv, bool enabled); + +int virNetServerStart(virNetServerPtr srv); + +#endif /* __VIR_NET_SERVER_H__ */ diff --git a/src/rpc/virnetserverprogram.h b/src/rpc/virnetserverprogram.h index c1ae17e..0ccc372 100644 --- a/src/rpc/virnetserverprogram.h +++ b/src/rpc/virnetserverprogram.h @@ -28,6 +28,9 @@ # include "virnetserverclient.h" # include "virobject.h" +typedef struct _virNetDaemon virNetDaemon; +typedef virNetDaemon *virNetDaemonPtr; + typedef struct _virNetServer virNetServer; typedef virNetServer *virNetServerPtr; -- 2.4.0

On Tue, May 19, 2015 at 10:19:36PM -0700, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
I like the split of this, but I'm concerned we might have a regression in the exec-restart feature. eg if we start an exec-restart with old libvirt and complete it with new libvirt, I'm not sure we're backwards compatible in handling the JSON file data. I see we don't have any unit testing to let us check that, so I'm going to write a unit test for exactly this. 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 :|

On 20.05.2015 07:19, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c
+ + +static virClassPtr virNetDaemonClass; + +void
static
+virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +}
+/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function.
No, it's not.
+ */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID);
Did you forget virObjectLock(srv) here?
+ virObjectUnlock(dmn); + + return srv; +} +
+void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn);
Hm... I guess we need something slightly different. While we may want one service to stop accepting new clients, we may want the other one to still accept new ones. But since this is a big split, and so far a daemon will have only one server, it can be saved for a follow up patch.
+} +
Besides Dan's finding, looking okay. ACK. Michal

On Wed, May 20, 2015 at 06:11:14PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c
+ + +static virClassPtr virNetDaemonClass; + +void
static
+virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +}
+/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function.
No, it's not.
+ */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID);
Did you forget virObjectLock(srv) here?
+ virObjectUnlock(dmn); + + return srv; +} +
+void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn);
Hm... I guess we need something slightly different. While we may want one service to stop accepting new clients, we may want the other one to still accept new ones. But since this is a big split, and so far a daemon will have only one server, it can be saved for a follow up patch.
+} +
Besides Dan's finding, looking okay. ACK.
Just to be clear, not having back-compat for the exec-restart is a NACK. So we need to confirm upgrades definitely work before we can merge this refactoring. 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 :|

On Wed, May 20, 2015 at 05:16:04PM +0100, Daniel P. Berrange wrote:
On Wed, May 20, 2015 at 06:11:14PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c
+ + +static virClassPtr virNetDaemonClass; + +void
static
+virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +}
+/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function.
No, it's not.
+ */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID);
Did you forget virObjectLock(srv) here?
+ virObjectUnlock(dmn); + + return srv; +} +
+void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn);
Hm... I guess we need something slightly different. While we may want one service to stop accepting new clients, we may want the other one to still accept new ones. But since this is a big split, and so far a daemon will have only one server, it can be saved for a follow up patch.
+} +
Besides Dan's finding, looking okay. ACK.
Just to be clear, not having back-compat for the exec-restart is a NACK. So we need to confirm upgrades definitely work before we can merge this refactoring.
The case you described now will fail, yes. I didn't want to pollute to code with that at first, but then forgot about it (you can se part of it is implemented. But as the first version had it, I can add it here as well, that's no problem. And looking at the code after the rewrite it won't be that big of a mess as it was before. The combined diff for both the virnetdaemon and the locking daemon (and one bug found plus the static keyword) looks like this and it works now: diff --git i/src/locking/lock_daemon.c w/src/locking/lock_daemon.c index f9afc8c..ecbe03a 100644 --- i/src/locking/lock_daemon.c +++ w/src/locking/lock_daemon.c @@ -237,10 +237,18 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) } } - if (!(child = virJSONValueObjectGet(object, "daemon"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing daemon data from JSON file")); - goto error; + if (virJSONValueObjectHasKey(object, "daemon")) { + if (!(child = virJSONValueObjectGet(object, "daemon"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed daemon data from JSON file")); + goto error; + } + } else { + if (!(child = virJSONValueObjectGet(object, "server"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing server data from JSON file")); + goto error; + } } if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) diff --git i/src/rpc/virnetdaemon.c w/src/rpc/virnetdaemon.c index 8d42638..ff06146 100644 --- i/src/rpc/virnetdaemon.c +++ w/src/rpc/virnetdaemon.c @@ -84,7 +84,7 @@ struct _virNetDaemon { static virClassPtr virNetDaemonClass; -void +static void virNetDaemonDispose(void *obj) { virNetDaemonPtr dmn = obj; @@ -240,7 +240,7 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, clientPrivFree, clientPrivOpaque); - if (!srv || VIR_APPEND_ELEMENT(dmn->servers, dmn->nservers, srv) < 0) + if (!srv || VIR_APPEND_ELEMENT_COPY(dmn->servers, dmn->nservers, srv) < 0) goto error; virJSONValueFree(object); @@ -260,17 +260,18 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object) { virNetDaemonPtr dmn = NULL; virJSONValuePtr servers = virJSONValueObjectGet(object, "servers"); + bool new_version = virJSONValueObjectHasKey(object, "servers"); if (!(dmn = virNetDaemonNew())) goto error; - if (!servers) { + if (new_version && !servers) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Malformed servers data in JSON document")); goto error; } - if (!(dmn->srvObject = virJSONValueCopy(servers))) + if (!(dmn->srvObject = virJSONValueCopy(new_version ? servers : object))) goto error; return dmn; --

On Wed, May 20, 2015 at 11:38:16AM -0700, Martin Kletzander wrote:
On Wed, May 20, 2015 at 05:16:04PM +0100, Daniel P. Berrange wrote:
On Wed, May 20, 2015 at 06:11:14PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c
+ + +static virClassPtr virNetDaemonClass; + +void
static
+virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +}
+/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function.
No, it's not.
+ */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID);
Did you forget virObjectLock(srv) here?
+ virObjectUnlock(dmn); + + return srv; +} +
+void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn);
Hm... I guess we need something slightly different. While we may want one service to stop accepting new clients, we may want the other one to still accept new ones. But since this is a big split, and so far a daemon will have only one server, it can be saved for a follow up patch.
+} +
Besides Dan's finding, looking okay. ACK.
Just to be clear, not having back-compat for the exec-restart is a NACK. So we need to confirm upgrades definitely work before we can merge this refactoring.
The case you described now will fail, yes. I didn't want to pollute to code with that at first, but then forgot about it (you can se part of it is implemented. But as the first version had it, I can add it here as well, that's no problem. And looking at the code after the rewrite it won't be that big of a mess as it was before. The combined diff for both the virnetdaemon and the locking daemon (and one bug found plus the static keyword) looks like this and it works now:
diff --git i/src/locking/lock_daemon.c w/src/locking/lock_daemon.c index f9afc8c..ecbe03a 100644 --- i/src/locking/lock_daemon.c +++ w/src/locking/lock_daemon.c @@ -237,10 +237,18 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) } }
- if (!(child = virJSONValueObjectGet(object, "daemon"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing daemon data from JSON file")); - goto error; + if (virJSONValueObjectHasKey(object, "daemon")) { + if (!(child = virJSONValueObjectGet(object, "daemon"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed daemon data from JSON file")); + goto error; + } + } else { + if (!(child = virJSONValueObjectGet(object, "server"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing server data from JSON file")); + goto error; + } }
if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) diff --git i/src/rpc/virnetdaemon.c w/src/rpc/virnetdaemon.c index 8d42638..ff06146 100644 --- i/src/rpc/virnetdaemon.c +++ w/src/rpc/virnetdaemon.c @@ -84,7 +84,7 @@ struct _virNetDaemon {
static virClassPtr virNetDaemonClass;
-void +static void virNetDaemonDispose(void *obj) { virNetDaemonPtr dmn = obj; @@ -240,7 +240,7 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, clientPrivFree, clientPrivOpaque);
- if (!srv || VIR_APPEND_ELEMENT(dmn->servers, dmn->nservers, srv) < 0) + if (!srv || VIR_APPEND_ELEMENT_COPY(dmn->servers, dmn->nservers, srv) < 0) goto error;
virJSONValueFree(object); @@ -260,17 +260,18 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object) { virNetDaemonPtr dmn = NULL; virJSONValuePtr servers = virJSONValueObjectGet(object, "servers"); + bool new_version = virJSONValueObjectHasKey(object, "servers");
if (!(dmn = virNetDaemonNew())) goto error;
- if (!servers) { + if (new_version && !servers) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Malformed servers data in JSON document")); goto error; }
- if (!(dmn->srvObject = virJSONValueCopy(servers))) + if (!(dmn->srvObject = virJSONValueCopy(new_version ? servers : object))) goto error;
return dmn;
Ok, I've just sent a patch which adds tests for virNetServer load/save JSON so we can verify your change now. You'll need to update the test to deal with the changed API obviously and add new JOSN data files for the new format. 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 :|

On Wed, May 20, 2015 at 06:11:14PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
This allows to have more servers in one daemon which helps isolating some resources.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 101 ++--- docs/internals.html.in | 4 +- docs/internals/rpc.html.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_remote.syms | 28 +- src/locking/lock_daemon.c | 63 ++-- src/locking/lock_daemon_config.c | 2 +- src/locking/lock_daemon_dispatch.c | 4 +- src/lxc/lxc_controller.c | 65 ++-- src/rpc/virnetdaemon.c | 746 +++++++++++++++++++++++++++++++++++++ src/rpc/virnetdaemon.h | 82 ++++ src/rpc/virnetserver.c | 526 ++++---------------------- src/rpc/virnetserver.h | 46 +-- src/rpc/virnetserverprogram.h | 3 + 15 files changed, 1074 insertions(+), 605 deletions(-) create mode 100644 src/rpc/virnetdaemon.c create mode 100644 src/rpc/virnetdaemon.h
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c new file mode 100644 index 0000000..8d42638 --- /dev/null +++ b/src/rpc/virnetdaemon.c
+ + +static virClassPtr virNetDaemonClass; + +void
static
+virNetDaemonDispose(void *obj) +{ + virNetDaemonPtr dmn = obj; + size_t i; + + VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); + + for (i = 0; i < dmn->nsignals; i++) { + sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL); + VIR_FREE(dmn->signals[i]); + } + VIR_FREE(dmn->signals); + VIR_FORCE_CLOSE(dmn->sigread); + VIR_FORCE_CLOSE(dmn->sigwrite); + if (dmn->sigwatch > 0) + virEventRemoveHandle(dmn->sigwatch); + + for (i = 0; i < dmn->nservers; i++) + virObjectUnref(dmn->servers[i]); + VIR_FREE(dmn->servers); + + virJSONValueFree(dmn->srvObject); +} + +static int +virNetDaemonOnceInit(void) +{ + if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(), + "virNetDaemon", + sizeof(virNetDaemon), + virNetDaemonDispose))) + return -1; + + return 0; +}
+/* + * Separate function merely for the purpose of unified error + * reporting. + */ +static virNetServerPtr +virNetDaemonGetServerInternal(virNetDaemonPtr dmn, + int subServerID) +{ + if (subServerID < 0 || subServerID >= dmn->nservers) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid server ID: %d"), + subServerID); + return NULL; + } + + return virObjectRef(dmn->servers[subServerID]); +} + +/* + * The server is locked after this function.
No, it's not.
I see I forgot to reply on few things here. You're right, that's a leftover.
+ */ +virNetServerPtr +virNetDaemonGetServer(virNetDaemonPtr dmn, + int subServerID) +{ + virNetServerPtr srv = NULL; + + virObjectLock(dmn); + srv = virNetDaemonGetServerInternal(dmn, subServerID);
Did you forget virObjectLock(srv) here?
Nope, the beauty of this is that just refs up the server and returns it. Similarly to virQEMUDriverGetCapabilities().
+ virObjectUnlock(dmn); + + return srv; +} +
+void +virNetDaemonUpdateServices(virNetDaemonPtr dmn, + bool enabled) +{ + size_t i; + + virObjectLock(dmn); + for (i = 0; i < dmn->nservers; i++) + virNetServerUpdateServices(dmn->servers[i], enabled); + virObjectUnlock(dmn);
Hm... I guess we need something slightly different. While we may want one service to stop accepting new clients, we may want the other one to still accept new ones. But since this is a big split, and so far a daemon will have only one server, it can be saved for a follow up patch.
What you describe is still possible, you can call virNetServerUpdateServices() on one server, nobody is saying you cannot. This function just abstracts starting all servers together into one function.

Since this is just a new option for gendispatch, it looks more like a cleanup. The only differences handled by it are connect pointers, private pointers and API naming customs. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 128 ++++++++++++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 50 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index cb8e157..5097e13 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -49,6 +49,8 @@ my $procprefix = shift or die "missing procedure prefix argument"; my $protocol = shift or die "missing protocol argument"; my @autogen; +my $connect_ptr = $structprefix eq "admin" ? "virAdmConnectPtr" : "virConnectPtr"; +my $prefix = ($structprefix eq "admin") ? "adm" : "vir"; sub fixup_name { my $name = shift; @@ -78,9 +80,12 @@ sub fixup_name { # Convert name_of_call to NameOfCall. sub name_to_ProcName { my $name = shift; + my $forcefix = $structprefix eq "admin"; my @elems; - if ($name =~ /_/ || (lc $name) eq "open" || (lc $name) eq "close") { + + if ($forcefix || $name =~ /_/ || + (lc $name) eq "open" || (lc $name) eq "close") { @elems = split /_/, $name; @elems = map lc, @elems; @elems = map ucfirst, @elems; @@ -104,6 +109,19 @@ sub name_to_TypeName { return $typename; } +sub push_privconn { + my $args = shift; + + if (!@$args) { + if ($structprefix eq "admin") { + push(@$args, "priv->dmn"); + } else { + push(@$args, "priv->conn"); + } + } +} + + # Read the input file (usually remote_protocol.x) and form an # opinion about the name, args and return type of each RPC. my ($name, $ProcName, $id, $flags, %calls, @calls, %opts); @@ -506,16 +524,11 @@ elsif ($mode eq "server") { " virObjectUnref(snapshot);\n" . " virObjectUnref(dom);"); } elsif ($args_member =~ m/^(?:remote_string|remote_uuid) (\S+)<\S+>;/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } - + push_privconn(\@args_list); push(@args_list, "args->$1.$1_val"); push(@args_list, "args->$1.$1_len"); } elsif ($args_member =~ m/^(?:opaque|remote_nonnull_string) (\S+)<\S+>;(.*)$/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); my $cast = ""; my $arg_name = $1; @@ -532,9 +545,7 @@ elsif ($mode eq "server") { push(@args_list, "${cast}args->$arg_name.${arg_name}_val"); push(@args_list, "args->$arg_name.${arg_name}_len"); } elsif ($args_member =~ m/^(?:unsigned )?int (\S+)<\S+>;/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); push(@args_list, "args->$1.$1_val"); push(@args_list, "args->$1.$1_len"); @@ -556,35 +567,25 @@ elsif ($mode eq "server") { # just make all other array types fail die "unhandled type for argument value: $args_member"; } elsif ($args_member =~ m/^remote_uuid (\S+);/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); push(@args_list, "(unsigned char *) args->$1"); } elsif ($args_member =~ m/^remote_string (\S+);/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); push(@vars_list, "char *$1"); push(@optionals_list, "$1"); push(@args_list, "$1"); } elsif ($args_member =~ m/^remote_nonnull_string (\S+);/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); push(@args_list, "args->$1"); } elsif ($args_member =~ m/^(unsigned )?int (\S+);/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); push(@args_list, "args->$2"); } elsif ($args_member =~ m/^(unsigned )?hyper (\S+);/) { - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); my $arg_name = $2; @@ -819,9 +820,7 @@ elsif ($mode eq "server") { die "multi-return-value without insert@<offset> annotation: $call->{ret}"; } - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); my $struct_name = $call->{ProcName}; $struct_name =~ s/Get//; @@ -871,7 +870,12 @@ elsif ($mode eq "server") { foreach my $var (@vars_list) { print " $var;\n"; } - print " struct daemonClientPrivate *priv =\n"; + + if ($structprefix eq "admin") { + print " struct daemonAdmClientPrivate *priv =\n"; + } else { + print " struct daemonClientPrivate *priv =\n"; + } print " virNetServerClientGetPrivateData(client);\n"; if ($call->{streamflag} ne "none") { @@ -880,7 +884,13 @@ elsif ($mode eq "server") { } print "\n"; - print " if (!priv->conn) {\n"; + + if ($structprefix eq "admin") { + print " if (!priv->dmn) {\n"; + } else { + print " if (!priv->conn) {\n"; + } + print " virReportError(VIR_ERR_INTERNAL_ERROR, \"%s\", _(\"connection not open\"));\n"; print " goto cleanup;\n"; print " }\n"; @@ -919,18 +929,15 @@ elsif ($mode eq "server") { } if ($rettype eq "void") { - print " if (vir$call->{ProcName}("; + print " if ($prefix$call->{ProcName}("; print join(', ', @args_list); print ") < 0)\n"; print " goto cleanup;\n"; print "\n"; } elsif (!$multi_ret) { - my $prefix = ""; my $proc_name = $call->{ProcName}; - if (! @args_list) { - push(@args_list, "priv->conn"); - } + push_privconn(\@args_list); if ($structprefix eq "qemu" && $call->{ProcName} =~ /^(Connect)?Domain/) { @@ -949,7 +956,7 @@ elsif ($mode eq "server") { } if ($single_ret_by_ref) { - print " if (vir$prefix$proc_name("; + print " if ($prefix$proc_name("; print join(', ', @args_list); if (defined $single_ret_var) { @@ -958,7 +965,7 @@ elsif ($mode eq "server") { print ") < 0)\n"; } else { - print " if (($single_ret_var = vir$prefix$proc_name("; + print " if (($single_ret_var = $prefix$proc_name("; print join(', ', @args_list); print "))$single_ret_check)\n"; } @@ -1241,14 +1248,14 @@ elsif ($mode eq "client") { } if ($is_first_arg and $priv_src eq "conn") { - unshift(@args_list, "virConnectPtr conn"); + unshift(@args_list, "$connect_ptr conn"); } $is_first_arg = 0; } } - if (! @args_list) { + if (($structprefix ne "admin") && !@args_list) { push(@args_list, "virConnectPtr conn"); } @@ -1442,7 +1449,11 @@ elsif ($mode eq "client") { my $proc = $call->{ProcName}; my $extra = $structprefix; $extra =~ s/^(\w)/uc $1/e; - $proc =~ s/^(Domain)(.*)/$1 . $extra . $2/e; + if ($structprefix eq "admin") { + $proc = $extra . $proc; + } else { + $proc =~ s/^(Domain)(.*)/$1 . $extra . $2/e; + } print "remote$proc("; } @@ -1451,7 +1462,11 @@ elsif ($mode eq "client") { print ")\n"; print "{\n"; print " $single_ret_var;\n"; - print " struct private_data *priv = $priv_src->privateData;\n"; + if ($structprefix eq "admin") { + print " remoteAdminPrivPtr priv = $priv_src->privateData;\n"; + } else { + print " struct private_data *priv = $priv_src->privateData;\n"; + } foreach my $var (@vars_list) { print " $var;\n"; @@ -1466,7 +1481,11 @@ elsif ($mode eq "client") { } print "\n"; - print " remoteDriverLock(priv);\n"; + if ($structprefix eq "admin") { + print " virObjectLock(priv);\n"; + } else { + print " remoteDriverLock(priv);\n"; + } if ($call->{streamflag} ne "none") { print "\n"; @@ -1546,8 +1565,12 @@ elsif ($mode eq "client") { $callflags = "REMOTE_CALL_LXC"; } + if ($structprefix ne "admin") { + $priv_src = "$priv_src, priv"; + } + print "\n"; - print " if (call($priv_src, priv, $callflags, $call->{constname},\n"; + print " if (call($priv_src, $callflags, $call->{constname},\n"; print " (xdrproc_t)xdr_$argtype, (char *)$call_args,\n"; print " (xdrproc_t)xdr_$rettype, (char *)$call_ret) == -1) {\n"; @@ -1622,7 +1645,12 @@ elsif ($mode eq "client") { print join("\n", @free_list); - print " remoteDriverUnlock(priv);\n"; + if ($structprefix eq "admin") { + print " virObjectUnlock(priv);\n"; + } else { + print " remoteDriverUnlock(priv);\n"; + } + print " return rv;\n"; print "}\n"; } @@ -1682,7 +1710,7 @@ elsif ($mode eq "client") { next if $call->{acl}->[0] eq "none"; if ($mode eq "aclsym") { - my $apiname = "vir" . $call->{ProcName}; + my $apiname = $prefix . $call->{ProcName}; if ($structprefix eq "qemu") { $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { @@ -1722,7 +1750,7 @@ elsif ($mode eq "client") { } } - my $apiname = "vir" . $call->{ProcName}; + my $apiname = $prefix . $call->{ProcName}; if ($structprefix eq "qemu") { $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { @@ -1735,7 +1763,7 @@ elsif ($mode eq "client") { $object =~ s/^(\w)/uc $1/e; $object =~ s/_(\w)/uc $1/e; $object =~ s/Nwfilter/NWFilter/; - my $objecttype = "vir" . $object . "DefPtr"; + my $objecttype = $prefix . $object . "DefPtr"; $apiname .= $action . "ACL"; if ($arg eq "interface") { @@ -1743,7 +1771,7 @@ elsif ($mode eq "client") { } my @argdecls; - push @argdecls, "virConnectPtr conn"; + push @argdecls, "$connect_ptr conn"; if ($object ne "Connect") { if ($object eq "StorageVol") { push @argdecls, "virStoragePoolDefPtr pool"; @@ -1834,7 +1862,7 @@ elsif ($mode eq "client") { sub generate_aclapi { my $call = shift; - my $apiname = "vir" . $call->{ProcName}; + my $apiname = $prefix . $call->{ProcName}; if ($structprefix eq "qemu") { $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
Since this is just a new option for gendispatch, it looks more like a cleanup. The only differences handled by it are connect pointers, private pointers and API naming customs.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 128 ++++++++++++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 50 deletions(-)
Wow, I've always viewed gendispatch.pl as a write only. You proved me wrong. ACK Michal

For now there are only CONNECT_OPEN and CONNECT_CLOSE procedures. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + src/Makefile.am | 13 +++++++++- src/admin/admin_protocol.x | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 9 +++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs diff --git a/.gitignore b/.gitignore index 1a5cf8e..b60c162 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,7 @@ /src/access/viraccessapichecklxc.h /src/access/viraccessapicheckqemu.c /src/access/viraccessapicheckqemu.h +/src/admin/admin_protocol.[ch] /src/esx/*.generated.* /src/hyperv/*.generated.* /src/libvirt*.def diff --git a/src/Makefile.am b/src/Makefile.am index 7e95cf1..d625874 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -392,6 +392,16 @@ REMOTE_DRIVER_SOURCES = \ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ $(REMOTE_DRIVER_GENERATED) +ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x + +ADMIN_PROTOCOL_GENERATED = \ + admin/admin_protocol.c \ + admin/admin_protocol.h + +EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) +BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) +MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED) + # Ensure that we don't change the struct or member names or member ordering # in remote_protocol.x The embedded perl below needs a few comments, and # presumes you know what pdwtags output looks like: @@ -2093,7 +2103,8 @@ RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \ $(srcdir)/remote/lxc_protocol.x \ $(srcdir)/remote/qemu_protocol.x \ $(srcdir)/lxc/lxc_monitor_protocol.x \ - $(srcdir)/locking/lock_protocol.x + $(srcdir)/locking/lock_protocol.x \ + $(srcdir)/admin/admin_protocol.x libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES) > $@ diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x new file mode 100644 index 0000000..63f6a53 --- /dev/null +++ b/src/admin/admin_protocol.x @@ -0,0 +1,65 @@ +/* -*- c -*- + * admin_protocol.x: private protocol for communicating between + * remote_internal driver and libvirtd. This protocol is + * internal and may change at any time. + * + * 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@redhat.com> + */ + +%#include "remote_protocol.h" + + +/*----- Protocol. -----*/ +struct admin_connect_open_args { + unsigned int flags; +}; + + +/* Define the program number, protocol version and procedure numbers here. */ +const ADMIN_PROGRAM = 0x06900690; +const ADMIN_PROTOCOL_VERSION = 1; + +enum admin_procedure { + /* Each function must be preceded by a comment providing one or + * more annotations: + * + * - @generate: none|client|server|both + * + * Whether to generate the dispatch stubs for the server + * and/or client code. + * + * - @readstream: paramnumber + * - @writestream: paramnumber + * + * The @readstream or @writestream annotations let daemon and src/remote + * create a stream. The direction is defined from the src/remote point + * of view. A readstream transfers data from daemon to src/remote. The + * <paramnumber> specifies at which offset the stream parameter is inserted + * in the function parameter list. + */ + /** + * @generate: client + */ + ADMIN_PROC_CONNECT_OPEN = 1, + + /** + * @generate: client + */ + ADMIN_PROC_CONNECT_CLOSE = 2 +}; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs new file mode 100644 index 0000000..bc1d489 --- /dev/null +++ b/src/admin_protocol-structs @@ -0,0 +1,9 @@ +/* -*- c -*- */ +struct admin_connect_open_args { + unsigned int flags; +}; + +enum admin_procedure { + ADMIN_PROC_CONNECT_OPEN = 1, + ADMIN_PROC_CONNECT_CLOSE = 2, +}; -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
For now there are only CONNECT_OPEN and CONNECT_CLOSE procedures.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + src/Makefile.am | 13 +++++++++- src/admin/admin_protocol.x | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 9 +++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs
diff --git a/.gitignore b/.gitignore index 1a5cf8e..b60c162 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,7 @@ /src/access/viraccessapichecklxc.h /src/access/viraccessapicheckqemu.c /src/access/viraccessapicheckqemu.h +/src/admin/admin_protocol.[ch] /src/esx/*.generated.* /src/hyperv/*.generated.* /src/libvirt*.def diff --git a/src/Makefile.am b/src/Makefile.am index 7e95cf1..d625874 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -392,6 +392,16 @@ REMOTE_DRIVER_SOURCES = \ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ $(REMOTE_DRIVER_GENERATED)
+ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x + +ADMIN_PROTOCOL_GENERATED = \ + admin/admin_protocol.c \ + admin/admin_protocol.h + +EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) +BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) +MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED) + # Ensure that we don't change the struct or member names or member ordering # in remote_protocol.x The embedded perl below needs a few comments, and # presumes you know what pdwtags output looks like: @@ -2093,7 +2103,8 @@ RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \ $(srcdir)/remote/lxc_protocol.x \ $(srcdir)/remote/qemu_protocol.x \ $(srcdir)/lxc/lxc_monitor_protocol.x \ - $(srcdir)/locking/lock_protocol.x + $(srcdir)/locking/lock_protocol.x \ + $(srcdir)/admin/admin_protocol.x
libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES) > $@ diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x new file mode 100644 index 0000000..63f6a53 --- /dev/null +++ b/src/admin/admin_protocol.x @@ -0,0 +1,65 @@ +/* -*- c -*- + * admin_protocol.x: private protocol for communicating between + * remote_internal driver and libvirtd. This protocol is + * internal and may change at any time. + * + * Copyright (C) 2014-2015 Red Hat, Inc.
2014?
+ * + * 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@redhat.com> + */ + +%#include "remote_protocol.h" + + +/*----- Protocol. -----*/ +struct admin_connect_open_args { + unsigned int flags; +}; + + +/* Define the program number, protocol version and procedure numbers here. */ +const ADMIN_PROGRAM = 0x06900690; +const ADMIN_PROTOCOL_VERSION = 1; + +enum admin_procedure { + /* Each function must be preceded by a comment providing one or + * more annotations: + * + * - @generate: none|client|server|both + * + * Whether to generate the dispatch stubs for the server + * and/or client code. + * + * - @readstream: paramnumber + * - @writestream: paramnumber + * + * The @readstream or @writestream annotations let daemon and src/remote + * create a stream. The direction is defined from the src/remote point + * of view. A readstream transfers data from daemon to src/remote. The + * <paramnumber> specifies at which offset the stream parameter is inserted + * in the function parameter list. + */ + /** + * @generate: client + */ + ADMIN_PROC_CONNECT_OPEN = 1, + + /** + * @generate: client + */ + ADMIN_PROC_CONNECT_CLOSE = 2 +}; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs new file mode 100644 index 0000000..bc1d489 --- /dev/null +++ b/src/admin_protocol-structs @@ -0,0 +1,9 @@ +/* -*- c -*- */ +struct admin_connect_open_args { + unsigned int flags; +}; + +enum admin_procedure { + ADMIN_PROC_CONNECT_OPEN = 1, + ADMIN_PROC_CONNECT_CLOSE = 2, +};
ACK Michal

On Wed, May 20, 2015 at 06:11:04PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
For now there are only CONNECT_OPEN and CONNECT_CLOSE procedures.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + src/Makefile.am | 13 +++++++++- src/admin/admin_protocol.x | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 9 +++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs
diff --git a/.gitignore b/.gitignore index 1a5cf8e..b60c162 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,7 @@ /src/access/viraccessapichecklxc.h /src/access/viraccessapicheckqemu.c /src/access/viraccessapicheckqemu.h +/src/admin/admin_protocol.[ch] /src/esx/*.generated.* /src/hyperv/*.generated.* /src/libvirt*.def diff --git a/src/Makefile.am b/src/Makefile.am index 7e95cf1..d625874 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -392,6 +392,16 @@ REMOTE_DRIVER_SOURCES = \ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ $(REMOTE_DRIVER_GENERATED)
+ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x + +ADMIN_PROTOCOL_GENERATED = \ + admin/admin_protocol.c \ + admin/admin_protocol.h + +EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) +BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) +MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED) + # Ensure that we don't change the struct or member names or member ordering # in remote_protocol.x The embedded perl below needs a few comments, and # presumes you know what pdwtags output looks like: @@ -2093,7 +2103,8 @@ RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \ $(srcdir)/remote/lxc_protocol.x \ $(srcdir)/remote/qemu_protocol.x \ $(srcdir)/lxc/lxc_monitor_protocol.x \ - $(srcdir)/locking/lock_protocol.x + $(srcdir)/locking/lock_protocol.x \ + $(srcdir)/admin/admin_protocol.x
libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES) > $@ diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x new file mode 100644 index 0000000..63f6a53 --- /dev/null +++ b/src/admin/admin_protocol.x @@ -0,0 +1,65 @@ +/* -*- c -*- + * admin_protocol.x: private protocol for communicating between + * remote_internal driver and libvirtd. This protocol is + * internal and may change at any time. + * + * Copyright (C) 2014-2015 Red Hat, Inc.
2014?
That's when I started ;) I'll remove that if you want, I don't really care much about the copyrights.

On 20.05.2015 20:42, Martin Kletzander wrote:
On Wed, May 20, 2015 at 06:11:04PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
For now there are only CONNECT_OPEN and CONNECT_CLOSE procedures.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + src/Makefile.am | 13 +++++++++- src/admin/admin_protocol.x | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 9 +++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x new file mode 100644 index 0000000..63f6a53 --- /dev/null +++ b/src/admin/admin_protocol.x @@ -0,0 +1,65 @@ +/* -*- c -*- + * admin_protocol.x: private protocol for communicating between + * remote_internal driver and libvirtd. This protocol is + * internal and may change at any time. + * + * Copyright (C) 2014-2015 Red Hat, Inc.
2014?
That's when I started ;) I'll remove that if you want, I don't really care much about the copyrights.
Me neither. It was more of a question than anything. I don't care whether you keep it as is or modify it. Michal

On 20.05.2015 18:11, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
For now there are only CONNECT_OPEN and CONNECT_CLOSE procedures.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + src/Makefile.am | 13 +++++++++- src/admin/admin_protocol.x | 65 ++++++++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 9 +++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs
ACK
I guess my ACK was premature. This needs to be squashed in for struct test to work: diff --git a/src/Makefile.am b/src/Makefile.am index 7e4554d..bd3f211 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -429,7 +429,7 @@ MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED) # The alternation of the following regexps matches both cases. r1 = /\* \d+ \*/ r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor)) +struct_prefix = (remote_|qemu_|lxc_|keepalive|admin_|vir(Net|LockSpace|LXCMonitor)) # Depending on configure options, libtool creates one or both of # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index bc1d489..3ac31fa 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,9 +1,8 @@ /* -*- c -*- */ struct admin_connect_open_args { - unsigned int flags; + u_int flags; }; - enum admin_procedure { - ADMIN_PROC_CONNECT_OPEN = 1, - ADMIN_PROC_CONNECT_CLOSE = 2, + ADMIN_PROC_CONNECT_OPEN = 1, + ADMIN_PROC_CONNECT_CLOSE = 2, }; Michal

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + cfg.mk | 5 ++++- src/Makefile.am | 9 ++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b60c162..e5e0788 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,7 @@ /src/access/viraccessapichecklxc.h /src/access/viraccessapicheckqemu.c /src/access/viraccessapicheckqemu.h +/src/admin/admin_client.h /src/admin/admin_protocol.[ch] /src/esx/*.generated.* /src/hyperv/*.generated.* diff --git a/cfg.mk b/cfg.mk index 796ed80..ce13f0e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1070,13 +1070,16 @@ bracket-spacing-check: sc_po_check: \ $(srcdir)/daemon/remote_dispatch.h \ $(srcdir)/daemon/qemu_dispatch.h \ - $(srcdir)/src/remote/remote_client_bodies.h + $(srcdir)/src/remote/remote_client_bodies.h \ + $(srcdir)/src/admin/admin_client.h $(srcdir)/daemon/remote_dispatch.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C daemon remote_dispatch.h $(srcdir)/daemon/qemu_dispatch.h: $(srcdir)/src/remote/qemu_protocol.x $(MAKE) -C daemon qemu_dispatch.h $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C src remote/remote_client_bodies.h +$(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x + $(MAKE) -C src admin/admin_client.h # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$ diff --git a/src/Makefile.am b/src/Makefile.am index d625874..e8dce78 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -396,7 +396,14 @@ ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x ADMIN_PROTOCOL_GENERATED = \ admin/admin_protocol.c \ - admin/admin_protocol.h + admin/admin_protocol.h \ + admin/admin_client.h + +admin/admin_client.h: $(srcdir)/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ + admin ADMIN $(ADMIN_PROTOCOL) \ + > $(srcdir)/admin/admin_client.h EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + cfg.mk | 5 ++++- src/Makefile.am | 9 ++++++++- 3 files changed, 13 insertions(+), 2 deletions(-)
ACK Michal

Just the addition of VIR_FROM_ADMIN to the enum of error domains. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- include/libvirt/virterror.h | 3 ++- src/util/virerror.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 9c5b069..6325030 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copyright (C) 2006, 2010-2012 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -126,6 +126,7 @@ typedef enum { VIR_FROM_POLKIT = 60, /* Error from polkit code */ VIR_FROM_THREAD = 61, /* Error from thread utils */ + VIR_FROM_ADMIN = 62, /* Error from admin backend */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/src/util/virerror.c b/src/util/virerror.c index 73dae95..fae627b 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -133,6 +133,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Polkit", /* 60 */ "Thread jobs", + "Admin Interface", ) -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
Just the addition of VIR_FROM_ADMIN to the enum of error domains.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- include/libvirt/virterror.h | 3 ++- src/util/virerror.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
ACK This is where I'm gonna stop for today. I've found some issues in 07/13 regarding spec file. But I don't have much time left today, so I'll resume the review tomorrow. Michal

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: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 1 + configure.ac | 4 +- include/libvirt/Makefile.am | 4 +- include/libvirt/libvirt-admin.h | 62 +++++++ libvirt-admin.pc.in | 13 ++ libvirt.spec.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 106 ++++++++++- src/datatypes.c | 30 ++++ src/datatypes.h | 37 ++++ src/internal.h | 1 + src/libvirt-admin.c | 386 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin.syms | 18 ++ src/rpc/gendispatch.pl | 4 +- 14 files changed, 669 insertions(+), 5 deletions(-) create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms diff --git a/cfg.mk b/cfg.mk index ce13f0e..f1b5024 100644 --- a/cfg.mk +++ b/cfg.mk @@ -304,6 +304,7 @@ sc_flags_usage: $(srcdir)/include/libvirt/virterror.h \ $(srcdir)/include/libvirt/libvirt-qemu.h \ $(srcdir)/include/libvirt/libvirt-lxc.h \ + $(srcdir)/include/libvirt/libvirt-admin.h \ | grep -c '\(long\|unsigned\) flags')" != 4 && \ { echo '$(ME): new API should use "unsigned int flags"' 1>&2; \ exit 1; } || : diff --git a/configure.ac b/configure.ac index 6122fa5..0409f9d 100644 --- a/configure.ac +++ b/configure.ac @@ -821,7 +821,6 @@ if test "$with_libvirtd" = "yes" ; then fi AM_CONDITIONAL([WITH_LIBVIRTD], [test "$with_libvirtd" = "yes"]) - old_LIBS="$LIBS" old_CFLAGS="$CFLAGS" LIBXENSERVER_LIBS="" @@ -2358,6 +2357,7 @@ WIN32_EXTRA_CFLAGS= dnl libvirt.syms is generated in builddir, but libvirt_qemu.syms is in git; dnl hence the asymmetric naming of these two symbol files. LIBVIRT_SYMBOL_FILE=libvirt.syms +LIBVIRT_ADMIN_SYMBOL_FILE='$(srcdir)/libvirt_admin.syms' LIBVIRT_LXC_SYMBOL_FILE='$(srcdir)/libvirt_lxc.syms' LIBVIRT_QEMU_SYMBOL_FILE='$(srcdir)/libvirt_qemu.syms' MSCOM_LIBS= @@ -2388,6 +2388,7 @@ case "$host" in # Also set the symbol file to .def, so src/Makefile generates libvirt.def # from libvirt.syms and passes libvirt.def instead of libvirt.syms to the linker LIBVIRT_SYMBOL_FILE=libvirt.def + LIBVIRT_ADMIN_SYMBOL_FILE=libvirt_admin.def LIBVIRT_LXC_SYMBOL_FILE=libvirt_lxc.def LIBVIRT_QEMU_SYMBOL_FILE=libvirt_qemu.def # mingw's ld has the --version-script parameter, but it requires a .def file @@ -2403,6 +2404,7 @@ AC_SUBST([CYGWIN_EXTRA_LIBADD]) AC_SUBST([MINGW_EXTRA_LDFLAGS]) AC_SUBST([WIN32_EXTRA_CFLAGS]) AC_SUBST([LIBVIRT_SYMBOL_FILE]) +AC_SUBST([LIBVIRT_ADMIN_SYMBOL_FILE]) AC_SUBST([LIBVIRT_LXC_SYMBOL_FILE]) AC_SUBST([LIBVIRT_QEMU_SYMBOL_FILE]) AC_SUBST([VERSION_SCRIPT_FLAGS]) diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am index 8aee5d7..2b64b0d 100644 --- a/include/libvirt/Makefile.am +++ b/include/libvirt/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2011, 2013 Red Hat, Inc. +## Copyright (C) 2005-2011, 2013-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 @@ -34,6 +34,8 @@ virinc_HEADERS = libvirt.h \ libvirt-qemu.h \ virterror.h +virinc_HEADERS += libvirt-admin.h + install-exec-hook: $(mkinstalldirs) $(DESTDIR)$(virincdir) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h new file mode 100644 index 0000000..b3cfc93 --- /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@redhat.com> + */ + +#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(const char *name, unsigned int flags); +int virAdmConnectClose(virAdmConnectPtr conn); + +int virAdmConnectRef(virAdmConnectPtr conn); + +# ifdef __cplusplus +} +# endif + +#endif /* __VIR_ADMIN_H__ */ diff --git a/libvirt-admin.pc.in b/libvirt-admin.pc.in new file mode 100644 index 0000000..76126ae --- /dev/null +++ b/libvirt-admin.pc.in @@ -0,0 +1,13 @@ +prefix=@prefix@ +exec_prefix=@exec_prefix@ +libdir=@libdir@ +includedir=@includedir@ +datarootdir=@datarootdir@ + +libvirt_admin_api=@datadir@/libvirt/api/libvirt-admin-api.xml + +Name: libvirt-admin +Version: @VERSION@ +Description: libvirt admin library +Libs: -L${libdir} -lvirt-admin +Cflags: -I${includedir} diff --git a/libvirt.spec.in b/libvirt.spec.in index 4195518..afcfe31 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2206,6 +2206,12 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper %endif +%if %{with_admin} +%files admin +%defattr(-, root, root) +%{_libdir}/%{name}/libvirt_admin.so +%endif + %files client -f %{name}.lang %defattr(-, root, root) %doc COPYING COPYING.LESSER @@ -2298,6 +2304,7 @@ exit 0 %{_includedir}/libvirt/libvirt-stream.h %{_includedir}/libvirt/libvirt-qemu.h %{_includedir}/libvirt/libvirt-lxc.h +%{_includedir}/libvirt/libvirt-admin.h %{_libdir}/pkgconfig/libvirt.pc %{_libdir}/pkgconfig/libvirt-qemu.pc %{_libdir}/pkgconfig/libvirt-lxc.pc diff --git a/po/POTFILES.in b/po/POTFILES.in index ebb0482..4afa2f9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ src/interface/interface_backend_netcf.c src/interface/interface_backend_udev.c src/internal.h src/libvirt.c +src/libvirt-admin.c src/libvirt-domain.c src/libvirt-domain-snapshot.c src/libvirt-host.c diff --git a/src/Makefile.am b/src/Makefile.am index e8dce78..1241d6d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -501,6 +501,7 @@ PROTOCOL_STRUCTS = \ $(srcdir)/virkeepaliveprotocol-structs \ $(srcdir)/lxc_monitor_protocol-structs \ $(srcdir)/lock_protocol-structs \ + $(srcdir)/admin_protocol-structs \ $(NULL) if WITH_REMOTE @@ -522,6 +523,9 @@ $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) +$(srcdir)/admin_protocol-struct: \ + $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo + $(PDWTAGS) else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be @@ -534,6 +538,7 @@ check-drivername: $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \ $(srcdir)/driver.h \ $(srcdir)/libvirt_public.syms \ + $(srcdir)/libvirt_admin.syms \ $(srcdir)/libvirt_qemu.syms \ $(srcdir)/libvirt_lxc.syms @@ -1092,6 +1097,7 @@ USED_SYM_FILES = $(srcdir)/libvirt_private.syms GENERATED_SYM_FILES = \ $(ACCESS_DRIVER_SYM_FILES) \ libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def \ + libvirt_admin.def \ $(NULL) if WITH_TEST @@ -1803,7 +1809,8 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ - $(ACCESS_DRIVER_POLKIT_POLICY) + $(ACCESS_DRIVER_POLKIT_POLICY) \ + $(libvirt_admin_la_SOURCES) check-local: check-augeas @@ -2000,6 +2007,7 @@ EXTRA_DIST += \ libvirt_public.syms \ libvirt_lxc.syms \ libvirt_qemu.syms \ + libvirt_admin.syms \ $(SYM_FILES) \ $(NULL) @@ -2043,6 +2051,102 @@ libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms chmod a-w $@-tmp && \ mv $@-tmp libvirt_lxc.def +libvirt_admin.def: $(srcdir)/libvirt_admin.syms + $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ + printf 'EXPORTS\n' > $@-tmp && \ + sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ + -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ + chmod a-w $@-tmp && \ + mv $@-tmp libvirt_admin.def + +lib_LTLIBRARIES += libvirt-admin.la +libvirt_admin_la_SOURCES = \ + libvirt-admin.c \ + $(ADMIN_PROTOCOL_GENERATED) + +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 + # 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 39f83d9..12bcfc1 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) { @@ -86,6 +90,8 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool); + DECLARE_CLASS_LOCKABLE(virAdmConnect); + #undef DECLARE_CLASS_COMMON #undef DECLARE_CLASS_LOCKABLE #undef DECLARE_CLASS @@ -803,3 +809,27 @@ virDomainSnapshotDispose(void *obj) VIR_FREE(snapshot->name); virObjectUnref(snapshot->domain); } + + +virAdmConnectPtr +virAdmConnectNew(void) +{ + virAdmConnectPtr ret; + + if (virDataTypesInitialize() < 0) + return NULL; + + if (!(ret = virObjectLockableNew(virAdmConnectClass))) + return NULL; + + return ret; +} + +static void +virAdmConnectDispose(void *obj) +{ + virAdmConnectPtr conn = obj; + + if (conn->privateDataFreeFunc) + conn->privateDataFreeFunc(conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index f1d01d5..c498cb0 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -41,6 +41,8 @@ extern virClassPtr virStreamClass; extern virClassPtr virStorageVolClass; extern virClassPtr virStoragePoolClass; +extern virClassPtr virAdmConnectClass; + # define virCheckConnectReturn(obj, retval) \ do { \ if (!virObjectIsClass(obj, virConnectClass)) { \ @@ -295,6 +297,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 @@ -369,6 +391,19 @@ struct _virConnect { }; /** + * _virAdmConnect: + * + * Internal structure associated to an admin connection + */ +struct _virAdmConnect { + virObjectLockable object; + + void *privateData; + virFreeCallback privateDataFreeFunc; +}; + + +/** * _virDomain: * * Internal structure associated to a domain @@ -549,4 +584,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 52e40d1..7c042e0 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..11b6fe3 --- /dev/null +++ b/src/libvirt-admin.c @@ -0,0 +1,386 @@ +/* + * 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@redhat.com> + */ + +#include <config.h> + +#include <rpc/rpc.h> + +#include "internal.h" +#include "configmake.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virlog.h" +#include "virnetclient.h" +#include "virobject.h" +#include "virstring.h" +#include "viruri.h" +#include "virutil.h" +#include "viruuid.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +#define LIBVIRTD_ADMIN_SOCK_NAME "/libvirt-admin-sock" +#define LIBVIRTD_ADMIN_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt" LIBVIRTD_ADMIN_SOCK_NAME + +VIR_LOG_INIT("libvirt-admin"); + + +typedef struct _remoteAdminPriv remoteAdminPriv; +typedef remoteAdminPriv *remoteAdminPrivPtr; + +struct _remoteAdminPriv { + virObjectLockable parent; + + int counter; + virNetClientPtr client; + virNetClientProgramPtr program; +}; + +static virClassPtr remoteAdminPrivClass; + +static void +remoteAdminPrivDispose(void *opaque) +{ + remoteAdminPrivPtr priv = opaque; + + virObjectUnref(priv->program); + virObjectUnref(priv->client); +} + + +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 = priv->program; + int counter = priv->counter++; + virNetClientPtr client = priv->client; + + /* 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 + */ + virObjectRef(priv); + virObjectUnlock(priv); + + rv = virNetClientProgramCall(prog, + client, + counter, + proc_nr, + fdinlen, fdin, + fdoutlen, fdout, + args_filter, args, + ret_filter, ret); + + virObjectLock(priv); + virObjectUnref(priv); + + 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 void +remoteAdminPrivFree(void *opaque) +{ + virAdmConnectPtr conn = opaque; + + remoteAdminConnectClose(conn); +} + +static remoteAdminPrivPtr +remoteAdminPrivNew(const char *sock_path) +{ + remoteAdminPrivPtr priv = NULL; + + if (!(priv = virObjectLockableNew(remoteAdminPrivClass))) + goto error; + + if (!(priv->client = virNetClientNewUNIX(sock_path, 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: + virObjectUnref(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; + + if (!(remoteAdminPrivClass = virClassNew(virClassForObjectLockable(), + "remoteAdminPriv", + sizeof(remoteAdminPriv), + remoteAdminPrivDispose))) + 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; +} + +static char * +getSocketPath(const char *name) +{ + char *rundir = virGetUserRuntimeDirectory(); + char *sock_path = NULL; + size_t i = 0; + virURIPtr uri = NULL; + + if (name) { + if (!(uri = virURIParse(name))) + goto error; + + if (STRNEQ(uri->scheme, "admin") || + uri->server || uri->user || uri->fragment) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid connection name '%s'"), name); + goto error; + } + + for (i = 0; i < uri->paramsCount; i++) { + virURIParamPtr param = &uri->params[i]; + + if (STREQ(param->name, "socket")) { + VIR_FREE(sock_path); + if (VIR_STRDUP(sock_path, param->value) < 0) + goto error; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown URI parameter '%s'"), param->name); + goto error; + } + } + } + + if (!sock_path) { + if (!uri || !uri->path || STREQ(uri->path, "/system")) { + if (VIR_STRDUP(sock_path, LIBVIRTD_ADMIN_UNIX_SOCKET) < 0) + goto error; + } else if (STREQ_NULLABLE(uri->path, "/session")) { + if (!rundir) + goto error; + + if (virAsprintf(&sock_path, + "%s%s", rundir, LIBVIRTD_ADMIN_SOCK_NAME) < 0) + goto error; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid URI path '%s'"), uri->path); + goto error; + } + } + + cleanup: + VIR_FREE(rundir); + virURIFree(uri); + return sock_path; + + error: + VIR_FREE(sock_path); + goto cleanup; +} + +/** + * virAdmConnectOpen: + * @name: uri of the daemon to connect to, NULL for default + * @flags: unused, must be 0 + * + * Opens connection to admin interface of the daemon. + * + * Returns @virAdmConnectPtr object or NULL on error + */ +virAdmConnectPtr +virAdmConnectOpen(const char *name, unsigned int flags) +{ + char *sock_path = NULL; + virAdmConnectPtr conn = NULL; + + if (virAdmInitialize() < 0) + goto error; + + VIR_DEBUG("flags=%x", flags); + virResetLastError(); + + if (!(conn = virAdmConnectNew())) + goto error; + + if (!(sock_path = getSocketPath(name))) + goto error; + + if (!(conn->privateData = remoteAdminPrivNew(sock_path))) + goto error; + + conn->privateDataFreeFunc = remoteAdminPrivFree; + + if (remoteAdminConnectOpen(conn, flags) < 0) + goto error; + + cleanup: + VIR_FREE(sock_path); + return conn; + + error: + virDispatchError(NULL); + virObjectUnref(conn); + conn = NULL; + goto cleanup; +} + +/** + * virAdmConnectClose: + * @conn: pointer to admin connection to close + * + * This function closes the admin 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 virAdmConnectOpen, as well as virAdmConnectRef; it is also temporarily + * increased by other API that depend on the connection remaining alive. The + * open and every virAdmConnectRef call should have a matching + * virAdmConnectClose, 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 virAdmConnectClose 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 + * virAdmConnectClose 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 virAdmConnectClose 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.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..ea6a8cc --- /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.3.0 { + global: + virAdmInitialize; + virAdmConnectOpen; + virAdmConnectClose; +}; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5097e13..dda04a9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1255,8 +1255,8 @@ elsif ($mode eq "client") { } } - if (($structprefix ne "admin") && !@args_list) { - push(@args_list, "virConnectPtr conn"); + if (!@args_list) { + push(@args_list, "$connect_ptr conn"); } # handle return values of the function -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
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: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 1 + configure.ac | 4 +- include/libvirt/Makefile.am | 4 +- include/libvirt/libvirt-admin.h | 62 +++++++ libvirt-admin.pc.in | 13 ++ libvirt.spec.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 106 ++++++++++- src/datatypes.c | 30 ++++ src/datatypes.h | 37 ++++ src/internal.h | 1 + src/libvirt-admin.c | 386 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin.syms | 18 ++ src/rpc/gendispatch.pl | 4 +- 14 files changed, 669 insertions(+), 5 deletions(-) create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms
diff --git a/libvirt-admin.pc.in b/libvirt-admin.pc.in new file mode 100644 index 0000000..76126ae --- /dev/null +++ b/libvirt-admin.pc.in @@ -0,0 +1,13 @@ +prefix=@prefix@ +exec_prefix=@exec_prefix@ +libdir=@libdir@ +includedir=@includedir@ +datarootdir=@datarootdir@ + +libvirt_admin_api=@datadir@/libvirt/api/libvirt-admin-api.xml + +Name: libvirt-admin +Version: @VERSION@ +Description: libvirt admin library +Libs: -L${libdir} -lvirt-admin +Cflags: -I${includedir}
This file should be put into AC_CONFIG_FILES() too. And into EXTRA_DIST
diff --git a/libvirt.spec.in b/libvirt.spec.in index 4195518..afcfe31 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2206,6 +2206,12 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper %endif
+%if %{with_admin}
This doesn't make much sense. libvirt-admin is build unconditionally. Moreover, the macro {with_admin} is never defined.
+%files admin +%defattr(-, root, root) +%{_libdir}/%{name}/libvirt_admin.so
Nope. It's installed under different path.
+%endif
Unfortunately, you haven't defined admin package. So this won't fly.
+ %files client -f %{name}.lang %defattr(-, root, root) %doc COPYING COPYING.LESSER @@ -2298,6 +2304,7 @@ exit 0 %{_includedir}/libvirt/libvirt-stream.h %{_includedir}/libvirt/libvirt-qemu.h %{_includedir}/libvirt/libvirt-lxc.h +%{_includedir}/libvirt/libvirt-admin.h %{_libdir}/pkgconfig/libvirt.pc %{_libdir}/pkgconfig/libvirt-qemu.pc %{_libdir}/pkgconfig/libvirt-lxc.pc diff --git a/po/POTFILES.in b/po/POTFILES.in index ebb0482..4afa2f9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ src/interface/interface_backend_netcf.c src/interface/interface_backend_udev.c src/internal.h src/libvirt.c +src/libvirt-admin.c src/libvirt-domain.c src/libvirt-domain-snapshot.c src/libvirt-host.c diff --git a/src/Makefile.am b/src/Makefile.am index e8dce78..1241d6d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -501,6 +501,7 @@ PROTOCOL_STRUCTS = \ $(srcdir)/virkeepaliveprotocol-structs \ $(srcdir)/lxc_monitor_protocol-structs \ $(srcdir)/lock_protocol-structs \ + $(srcdir)/admin_protocol-structs \ $(NULL)
if WITH_REMOTE @@ -522,6 +523,9 @@ $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) +$(srcdir)/admin_protocol-struct: \ + $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo + $(PDWTAGS)
else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be @@ -534,6 +538,7 @@ check-drivername: $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \ $(srcdir)/driver.h \ $(srcdir)/libvirt_public.syms \ + $(srcdir)/libvirt_admin.syms \ $(srcdir)/libvirt_qemu.syms \ $(srcdir)/libvirt_lxc.syms
@@ -1092,6 +1097,7 @@ USED_SYM_FILES = $(srcdir)/libvirt_private.syms GENERATED_SYM_FILES = \ $(ACCESS_DRIVER_SYM_FILES) \ libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def \ + libvirt_admin.def \ $(NULL)
if WITH_TEST @@ -1803,7 +1809,8 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ - $(ACCESS_DRIVER_POLKIT_POLICY) + $(ACCESS_DRIVER_POLKIT_POLICY) \ + $(libvirt_admin_la_SOURCES)
This is not necessary. libvirt-admin is build unconditionally.
check-local: check-augeas
@@ -2000,6 +2007,7 @@ EXTRA_DIST += \ libvirt_public.syms \ libvirt_lxc.syms \ libvirt_qemu.syms \ + libvirt_admin.syms \ $(SYM_FILES) \ $(NULL)
@@ -2043,6 +2051,102 @@ libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms chmod a-w $@-tmp && \ mv $@-tmp libvirt_lxc.def
+libvirt_admin.def: $(srcdir)/libvirt_admin.syms + $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ + printf 'EXPORTS\n' > $@-tmp && \ + sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ + -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ + chmod a-w $@-tmp && \ + mv $@-tmp libvirt_admin.def
This pattern repeats itself already. Maybe one day we can turn it into a general rule how to make .def from .syms.
+ +lib_LTLIBRARIES += libvirt-admin.la +libvirt_admin_la_SOURCES = \ + libvirt-admin.c \ + $(ADMIN_PROTOCOL_GENERATED) + +libvirt_admin_la_SOURCES += \
Spurious space after =.
+ 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 \
This drags in (de)serializers for all the public APIs we have. I guess you have it here just becase of serializers for some basic types (e.g. string). Well, if we introduce a separate libvirt_admin.x file, rpcgen will generate the serializers again, and just for the types we need. So I think it's safe to drop this line (and libvirt-admin.so will lose some weight). Then, I wonder if we need to recompile nearly all utils/ again. Can't we just link libvirt_utils.so in?
+ 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
SSH, TLS and SASL? It's going to be a local socket only. I guess we don't need them. Or is it some kind of black magic of dependencies?
+ +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) +
diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms new file mode 100644 index 0000000..ea6a8cc --- /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.3.0 { + global: + virAdmInitialize; + virAdmConnectOpen; + virAdmConnectClose;
I wonder if we should introduce (and implement) virAdmConnectRef. For instance, if you have a multithreaded application, you open the connection, and then spawn threads. But for some reason, you want to have virAdmConnectClose in threads. Therefore each thread should increase the reference counter on the connection objects, so the first one to call the close() won't close the connection for the others.
+}; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5097e13..dda04a9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1255,8 +1255,8 @@ elsif ($mode eq "client") { } }
- if (($structprefix ne "admin") && !@args_list) { - push(@args_list, "virConnectPtr conn"); + if (!@args_list) { + push(@args_list, "$connect_ptr conn"); }
# handle return values of the function
This needs to be squashed in at least: diff --git a/Makefile.am b/Makefile.am index 4aafe94..c4178d5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,7 @@ EXTRA_DIST = \ libvirt.spec libvirt.spec.in \ mingw-libvirt.spec.in \ libvirt.pc.in \ + libvirt-admin.pc.in \ libvirt-qemu.pc.in \ libvirt-lxc.pc.in \ autobuild.sh \ diff --git a/configure.ac b/configure.ac index 0409f9d..af1fa82 100644 --- a/configure.ac +++ b/configure.ac @@ -2793,6 +2793,7 @@ AC_CONFIG_FILES([\ gnulib/lib/Makefile \ gnulib/tests/Makefile \ libvirt.pc \ + libvirt-admin.pc \ libvirt-qemu.pc \ libvirt-lxc.pc \ src/libvirt.pc \ diff --git a/libvirt.spec.in b/libvirt.spec.in index afcfe31..10010cf 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1216,6 +1216,15 @@ Includes the Sanlock lock manager plugin for the QEMU driver %endif +%package admin +Summary: Plugin to control daemon itself +Group: Development/Libraries + +%description admin +This package contains set of APIs for runtime daemon +administration, for instance adjustment of worker pool size, +logging etc. + %prep %setup -q @@ -2206,11 +2215,9 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper %endif -%if %{with_admin} %files admin %defattr(-, root, root) -%{_libdir}/%{name}/libvirt_admin.so -%endif +%{_libdir}/libvirt-admin.so.* %files client -f %{name}.lang %defattr(-, root, root) @@ -2286,6 +2293,7 @@ exit 0 %defattr(-, root, root) %{_libdir}/libvirt.so +%{_libdir}/libvirt-admin.so %{_libdir}/libvirt-qemu.so %{_libdir}/libvirt-lxc.so %dir %{_includedir}/libvirt diff --git a/src/Makefile.am b/src/Makefile.am index 1241d6d..7e4554d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1809,8 +1809,7 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ - $(ACCESS_DRIVER_POLKIT_POLICY) \ - $(libvirt_admin_la_SOURCES) + $(ACCESS_DRIVER_POLKIT_POLICY) check-local: check-augeas @@ -2064,7 +2063,7 @@ libvirt_admin_la_SOURCES = \ libvirt-admin.c \ $(ADMIN_PROTOCOL_GENERATED) -libvirt_admin_la_SOURCES += \ +libvirt_admin_la_SOURCES += \ datatypes.c \ util/viralloc.c \ util/viratomic.c \ @@ -2098,7 +2097,6 @@ libvirt_admin_la_SOURCES += \ util/virutil.c \ util/viruuid.c \ util/virxml.c \ - remote/remote_protocol.c \ rpc/virnetmessage.h \ rpc/virnetmessage.c \ rpc/virnetsocket.c \ Otherwise looking good. Michal

On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
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: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 1 + configure.ac | 4 +- include/libvirt/Makefile.am | 4 +- include/libvirt/libvirt-admin.h | 62 +++++++ libvirt-admin.pc.in | 13 ++ libvirt.spec.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 106 ++++++++++- src/datatypes.c | 30 ++++ src/datatypes.h | 37 ++++ src/internal.h | 1 + src/libvirt-admin.c | 386 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin.syms | 18 ++ src/rpc/gendispatch.pl | 4 +- 14 files changed, 669 insertions(+), 5 deletions(-) create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms
diff --git a/libvirt-admin.pc.in b/libvirt-admin.pc.in new file mode 100644 index 0000000..76126ae --- /dev/null +++ b/libvirt-admin.pc.in @@ -0,0 +1,13 @@ +prefix=@prefix@ +exec_prefix=@exec_prefix@ +libdir=@libdir@ +includedir=@includedir@ +datarootdir=@datarootdir@ + +libvirt_admin_api=@datadir@/libvirt/api/libvirt-admin-api.xml + +Name: libvirt-admin +Version: @VERSION@ +Description: libvirt admin library +Libs: -L${libdir} -lvirt-admin +Cflags: -I${includedir}
This file should be put into AC_CONFIG_FILES() too. And into EXTRA_DIST
diff --git a/libvirt.spec.in b/libvirt.spec.in index 4195518..afcfe31 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2206,6 +2206,12 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper %endif
+%if %{with_admin}
This doesn't make much sense. libvirt-admin is build unconditionally. Moreover, the macro {with_admin} is never defined.
+%files admin +%defattr(-, root, root) +%{_libdir}/%{name}/libvirt_admin.so
Nope. It's installed under different path.
+%endif
Unfortunately, you haven't defined admin package. So this won't fly.
Me and specfiles, we were never huge friends.
+ %files client -f %{name}.lang %defattr(-, root, root) %doc COPYING COPYING.LESSER @@ -2298,6 +2304,7 @@ exit 0 %{_includedir}/libvirt/libvirt-stream.h %{_includedir}/libvirt/libvirt-qemu.h %{_includedir}/libvirt/libvirt-lxc.h +%{_includedir}/libvirt/libvirt-admin.h %{_libdir}/pkgconfig/libvirt.pc %{_libdir}/pkgconfig/libvirt-qemu.pc %{_libdir}/pkgconfig/libvirt-lxc.pc diff --git a/po/POTFILES.in b/po/POTFILES.in index ebb0482..4afa2f9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ src/interface/interface_backend_netcf.c src/interface/interface_backend_udev.c src/internal.h src/libvirt.c +src/libvirt-admin.c src/libvirt-domain.c src/libvirt-domain-snapshot.c src/libvirt-host.c diff --git a/src/Makefile.am b/src/Makefile.am index e8dce78..1241d6d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -501,6 +501,7 @@ PROTOCOL_STRUCTS = \ $(srcdir)/virkeepaliveprotocol-structs \ $(srcdir)/lxc_monitor_protocol-structs \ $(srcdir)/lock_protocol-structs \ + $(srcdir)/admin_protocol-structs \ $(NULL)
if WITH_REMOTE @@ -522,6 +523,9 @@ $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) +$(srcdir)/admin_protocol-struct: \ + $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo + $(PDWTAGS)
else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be @@ -534,6 +538,7 @@ check-drivername: $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \ $(srcdir)/driver.h \ $(srcdir)/libvirt_public.syms \ + $(srcdir)/libvirt_admin.syms \ $(srcdir)/libvirt_qemu.syms \ $(srcdir)/libvirt_lxc.syms
@@ -1092,6 +1097,7 @@ USED_SYM_FILES = $(srcdir)/libvirt_private.syms GENERATED_SYM_FILES = \ $(ACCESS_DRIVER_SYM_FILES) \ libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def \ + libvirt_admin.def \ $(NULL)
if WITH_TEST @@ -1803,7 +1809,8 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ - $(ACCESS_DRIVER_POLKIT_POLICY) + $(ACCESS_DRIVER_POLKIT_POLICY) \ + $(libvirt_admin_la_SOURCES)
This is not necessary. libvirt-admin is build unconditionally.
check-local: check-augeas
@@ -2000,6 +2007,7 @@ EXTRA_DIST += \ libvirt_public.syms \ libvirt_lxc.syms \ libvirt_qemu.syms \ + libvirt_admin.syms \ $(SYM_FILES) \ $(NULL)
@@ -2043,6 +2051,102 @@ libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms chmod a-w $@-tmp && \ mv $@-tmp libvirt_lxc.def
+libvirt_admin.def: $(srcdir)/libvirt_admin.syms + $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ + printf 'EXPORTS\n' > $@-tmp && \ + sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ + -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ + chmod a-w $@-tmp && \ + mv $@-tmp libvirt_admin.def
This pattern repeats itself already. Maybe one day we can turn it into a general rule how to make .def from .syms.
Done.
+ +lib_LTLIBRARIES += libvirt-admin.la +libvirt_admin_la_SOURCES = \ + libvirt-admin.c \ + $(ADMIN_PROTOCOL_GENERATED) + +libvirt_admin_la_SOURCES += \
Spurious space after =.
+ 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 \
This drags in (de)serializers for all the public APIs we have. I guess you have it here just becase of serializers for some basic types (e.g. string). Well, if we introduce a separate libvirt_admin.x file, rpcgen will generate the serializers again, and just for the types we need. So I think it's safe to drop this line (and libvirt-admin.so will lose some weight).
Yes, but I have to rename that to something else than remote_string because that would collide in the libvirt daemon. That would mean I have to add each of the new names to gendispatch.pl. And so on and so on, just to get rid of some (de)serializers in the client library. I'll do that for remote_string for now, but if there are more than that, we should probably put those into another file. Well, you'll see how much bigger the diff will be even now.
Then, I wonder if we need to recompile nearly all utils/ again. Can't we just link libvirt_utils.so in?
Well, I wanted to make it lightweight even when it increases compilation time as it looks like we are constantly OK with that (setuid_rpc_client, vbox libs, etc.), but as I said in the commit message, I'd like to see a commit that minimizes the files being compiled.
+ 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
SSH, TLS and SASL? It's going to be a local socket only. I guess we don't need them. Or is it some kind of black magic of dependencies?
No magic, this is just our ugly way of handling WITH_GNUTLS nd WITH_SASL that drags dependencies around. I started the cleanup for this, but since it's pretty deeply engrained in the code, I haven't really found the time (and energy) to finish it.
+ +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) +
diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms new file mode 100644 index 0000000..ea6a8cc --- /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.3.0 { + global: + virAdmInitialize; + virAdmConnectOpen; + virAdmConnectClose;
I wonder if we should introduce (and implement) virAdmConnectRef. For instance, if you have a multithreaded application, you open the connection, and then spawn threads. But for some reason, you want to have virAdmConnectClose in threads. Therefore each thread should increase the reference counter on the connection objects, so the first one to call the close() won't close the connection for the others.
It is implemented right in this series. I just forgot to put it here ;)
+}; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5097e13..dda04a9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1255,8 +1255,8 @@ elsif ($mode eq "client") { } }
- if (($structprefix ne "admin") && !@args_list) { - push(@args_list, "virConnectPtr conn"); + if (!@args_list) { + push(@args_list, "$connect_ptr conn"); }
# handle return values of the function
This needs to be squashed in at least:
That itself won't work. But I squashed way more. And created another commit or two. Stay tuned!

On 22.05.2015 07:22, Martin Kletzander wrote:
On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
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: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 1 + configure.ac | 4 +- include/libvirt/Makefile.am | 4 +- include/libvirt/libvirt-admin.h | 62 +++++++ libvirt-admin.pc.in | 13 ++ libvirt.spec.in | 7 + po/POTFILES.in | 1 + src/Makefile.am | 106 ++++++++++- src/datatypes.c | 30 ++++ src/datatypes.h | 37 ++++ src/internal.h | 1 + src/libvirt-admin.c | 386 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin.syms | 18 ++ src/rpc/gendispatch.pl | 4 +- 14 files changed, 669 insertions(+), 5 deletions(-) create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms
+ +lib_LTLIBRARIES += libvirt-admin.la +libvirt_admin_la_SOURCES = \ + libvirt-admin.c \ + $(ADMIN_PROTOCOL_GENERATED) + +libvirt_admin_la_SOURCES += \
Spurious space after =.
+ 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 \
This drags in (de)serializers for all the public APIs we have. I guess you have it here just becase of serializers for some basic types (e.g. string). Well, if we introduce a separate libvirt_admin.x file, rpcgen will generate the serializers again, and just for the types we need. So I think it's safe to drop this line (and libvirt-admin.so will lose some weight).
Yes, but I have to rename that to something else than remote_string because that would collide in the libvirt daemon. That would mean I have to add each of the new names to gendispatch.pl. And so on and so on, just to get rid of some (de)serializers in the client library.
I'll do that for remote_string for now, but if there are more than that, we should probably put those into another file. Well, you'll see how much bigger the diff will be even now.
I see, so you need it for remote_string. I did what I suggested and saw a linking error later in the series (when the remote_string was added). So I guess we can split remote_protocol into two pieces, one containing just general type definitions, other containing libvirt APIs. Although this would mean yet another commits in this already big series. So I'm okay to save it to a follow up patches. Michal

On Thu, May 21, 2015 at 10:22:26PM -0700, Martin Kletzander wrote:
On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
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.
+ 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 \
This drags in (de)serializers for all the public APIs we have. I guess you have it here just becase of serializers for some basic types (e.g. string). Well, if we introduce a separate libvirt_admin.x file, rpcgen will generate the serializers again, and just for the types we need. So I think it's safe to drop this line (and libvirt-admin.so will lose some weight).
Yes, but I have to rename that to something else than remote_string because that would collide in the libvirt daemon. That would mean I have to add each of the new names to gendispatch.pl. And so on and so on, just to get rid of some (de)serializers in the client library.
I'll do that for remote_string for now, but if there are more than that, we should probably put those into another file. Well, you'll see how much bigger the diff will be even now.
Then, I wonder if we need to recompile nearly all utils/ again. Can't we just link libvirt_utils.so in?
Well, I wanted to make it lightweight even when it increases compilation time as it looks like we are constantly OK with that (setuid_rpc_client, vbox libs, etc.), but as I said in the commit message, I'd like to see a commit that minimizes the files being compiled.
I don't think you are actually making this lightweight as you intend. In reality anyone who is using libvirt-admin.so will likely already have libvirt.so in memory. So by recompiling the files you are causing two copies of the same code to be kept in RAM. So if libvirt-admin.so just linked to libvirt.so, then we would in fact be more lightweight than this, and avoid the recompilation. The recompilation for the setuid binary is a special case because we have security requirements that trump the extra overhead that the duplication implies both at compile time and runtime. So I don't think the recompilation is really justified in the case of libvirt-admin.so One day though, I do think we should perhaps turn things like libvirt_util.la, libvirt_rpc.la, etc into a formally installed .so's that we can dynamically link to from other libvirt parts. We'd /not/ include any header files, as these .so's would be solely for libvirt usage, not application usage. I think this would ultimately better achieve what you're trying todo here. 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 :|

On Fri, May 22, 2015 at 12:59:44PM +0100, Daniel P. Berrange wrote:
On Thu, May 21, 2015 at 10:22:26PM -0700, Martin Kletzander wrote:
On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
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.
+ 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 \
This drags in (de)serializers for all the public APIs we have. I guess you have it here just becase of serializers for some basic types (e.g. string). Well, if we introduce a separate libvirt_admin.x file, rpcgen will generate the serializers again, and just for the types we need. So I think it's safe to drop this line (and libvirt-admin.so will lose some weight).
Yes, but I have to rename that to something else than remote_string because that would collide in the libvirt daemon. That would mean I have to add each of the new names to gendispatch.pl. And so on and so on, just to get rid of some (de)serializers in the client library.
I'll do that for remote_string for now, but if there are more than that, we should probably put those into another file. Well, you'll see how much bigger the diff will be even now.
Then, I wonder if we need to recompile nearly all utils/ again. Can't we just link libvirt_utils.so in?
Well, I wanted to make it lightweight even when it increases compilation time as it looks like we are constantly OK with that (setuid_rpc_client, vbox libs, etc.), but as I said in the commit message, I'd like to see a commit that minimizes the files being compiled.
I don't think you are actually making this lightweight as you intend.
In reality anyone who is using libvirt-admin.so will likely already have libvirt.so in memory. So by recompiling the files you are causing two copies of the same code to be kept in RAM. So if libvirt-admin.so just linked to libvirt.so, then we would in fact be more lightweight than this, and avoid the recompilation.
OK, I though that shouldn't be the case, so it is installable separably, but since we allow only local socket for now, you're right that there's libvirt.so already loaded.
The recompilation for the setuid binary is a special case because we have security requirements that trump the extra overhead that the duplication implies both at compile time and runtime.
So I don't think the recompilation is really justified in the case of libvirt-admin.so
One day though, I do think we should perhaps turn things like libvirt_util.la, libvirt_rpc.la, etc into a formally installed .so's that we can dynamically link to from other libvirt parts. We'd /not/ include any header files, as these .so's would be solely for libvirt usage, not application usage. I think this would ultimately better achieve what you're trying todo here.
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 :|

No online docs are build from it since it doesn't really fit into our document structure and new page will need to be created for it, but this is at least a heads-up commit for easier parsing in order to build some documentation (or python bindings) later on. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + docs/Makefile.am | 23 +++++++++++++++++++---- docs/apibuild.py | 10 +++++++++- libvirt.spec.in | 1 + 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index e5e0788..0b40f4a 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,7 @@ /docs/apibuild.py.stamp /docs/devhelp/libvirt.devhelp /docs/hvsupport.html.in +/docs/libvirt-admin-*.xml /docs/libvirt-api.xml /docs/libvirt-lxc-*.xml /docs/libvirt-qemu-*.xml diff --git a/docs/Makefile.am b/docs/Makefile.am index f43da93..b7b49cb 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -128,8 +128,16 @@ lxc_xml = \ libvirt-lxc-api.xml \ libvirt-lxc-refs.xml +admin_xml = \ + libvirt-admin-api.xml \ + libvirt-admin-refs.xml + apidir = $(pkgdatadir)/api -api_DATA = libvirt-api.xml libvirt-qemu-api.xml libvirt-lxc-api.xml +api_DATA = \ + libvirt-api.xml \ + libvirt-qemu-api.xml \ + libvirt-lxc-api.xml \ + libvirt-admin-api.xml fig = \ libvirt-net-logical.fig \ @@ -149,7 +157,7 @@ EXTRA_DIST= \ hacking1.xsl hacking2.xsl wrapstring.xsl \ $(dot_html) $(dot_html_in) $(gif) $(apihtml) $(apipng) \ $(devhelphtml) $(devhelppng) $(devhelpcss) $(devhelpxsl) \ - $(xml) $(qemu_xml) $(lxc_xml) $(fig) $(png) $(css) \ + $(xml) $(qemu_xml) $(lxc_xml) $(admin_xml) $(fig) $(png) $(css) \ $(patches) $(dot_php_in) $(dot_php_code_in) $(dot_php)\ $(internals_html_in) $(internals_html) \ sitemap.html.in aclperms.htmlinc \ @@ -174,6 +182,7 @@ all-am: web api: $(srcdir)/libvirt-api.xml $(srcdir)/libvirt-refs.xml qemu_api: $(srcdir)/libvirt-qemu-api.xml $(srcdir)/libvirt-qemu-refs.xml lxc_api: $(srcdir)/libvirt-lxc-api.xml $(srcdir)/libvirt-lxc-refs.xml +admin_api: $(srcdir)/libvirt-admin-api.xml $(srcdir)/libvirt-admin-refs.xml web: $(dot_html) $(internals_html) html/index.html devhelp/index.html \ $(dot_php) @@ -274,6 +283,7 @@ $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl) python_generated_files = \ $(srcdir)/html/libvirt-libvirt-lxc.html \ $(srcdir)/html/libvirt-libvirt-qemu.html \ + $(srcdir)/html/libvirt-libvirt-admin.html \ $(srcdir)/html/libvirt-virterror.html \ $(srcdir)/libvirt-api.xml \ $(srcdir)/libvirt-refs.xml \ @@ -281,6 +291,8 @@ python_generated_files = \ $(srcdir)/libvirt-lxc-refs.xml \ $(srcdir)/libvirt-qemu-api.xml \ $(srcdir)/libvirt-qemu-refs.xml \ + $(srcdir)/libvirt-admin-api.xml \ + $(srcdir)/libvirt-admin-refs.xml \ $(NULL) APIBUILD=$(srcdir)/apibuild.py @@ -304,10 +316,12 @@ $(APIBUILD_STAMP): $(srcdir)/apibuild.py \ $(top_srcdir)/include/libvirt/libvirt-stream.h \ $(top_srcdir)/include/libvirt/libvirt-lxc.h \ $(top_srcdir)/include/libvirt/libvirt-qemu.h \ + $(top_srcdir)/include/libvirt/libvirt-admin.h \ $(top_srcdir)/include/libvirt/virterror.h \ $(top_srcdir)/src/libvirt.c \ $(top_srcdir)/src/libvirt-lxc.c \ $(top_srcdir)/src/libvirt-qemu.c \ + $(top_srcdir)/src/libvirt-admin.c \ $(top_srcdir)/src/util/virerror.c \ $(top_srcdir)/src/util/virevent.c \ $(top_srcdir)/src/util/virtypedparam.c @@ -326,9 +340,10 @@ maintainer-clean-local: clean-local todo.html.in rm -rf $(srcdir)/libvirt-qemu-api.xml $(srcdir)/libvirt-qemu-refs.xml rm -rf $(srcdir)/libvirt-lxc-api.xml $(srcdir)/libvirt-lxc-refs.xml + rm -rf $(srcdir)/libvirt-admin-api.xml $(srcdir)/libvirt-admin-refs.xml rm -rf $(APIBUILD_STAMP) -rebuild: api qemu_api lxc_api all +rebuild: api qemu_api lxc_api admin_api all install-data-local: $(mkinstalldirs) $(DESTDIR)$(HTML_DIR) diff --git a/docs/apibuild.py b/docs/apibuild.py index 9fa9361..95e9f27 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -59,6 +59,11 @@ lxc_included_files = { "libvirt-lxc.c": "Implementations for the LXC specific APIs", } +admin_included_files = { + "libvirt-admin.h": "header with admin specific API definitions", + "libvirt-admin.c": "Implementations for the admin specific APIs", +} + ignored_words = { "ATTRIBUTE_UNUSED": (0, "macro keyword"), "ATTRIBUTE_SENTINEL": (0, "macro keyword"), @@ -2018,6 +2023,8 @@ class docBuilder: self.includes = includes + qemu_included_files.keys() elif name == "libvirt-lxc": self.includes = includes + lxc_included_files.keys() + elif name == "libvirt-admin": + self.includes = includes + admin_included_files.keys() self.modules = {} self.headers = {} self.idx = index() @@ -2551,7 +2558,7 @@ class docBuilder: def rebuild(name): - if name not in ["libvirt", "libvirt-qemu", "libvirt-lxc"]: + if name not in ["libvirt", "libvirt-qemu", "libvirt-lxc", "libvirt-admin"]: self.warning("rebuild() failed, unknown module %s") % name return None builder = None @@ -2595,6 +2602,7 @@ if __name__ == "__main__": rebuild("libvirt") rebuild("libvirt-qemu") rebuild("libvirt-lxc") + rebuild("libvirt-admin") if warnings > 0: sys.exit(2) else: diff --git a/libvirt.spec.in b/libvirt.spec.in index afcfe31..b00f99b 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2313,6 +2313,7 @@ exit 0 %{_datadir}/libvirt/api/libvirt-api.xml %{_datadir}/libvirt/api/libvirt-qemu-api.xml %{_datadir}/libvirt/api/libvirt-lxc-api.xml +%{_datadir}/libvirt/api/libvirt-admin-api.xml %doc docs/*.html docs/html docs/*.gif %doc docs/libvirt-api.xml -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
No online docs are build from it since it doesn't really fit into our document structure and new page will need to be created for it, but this is at least a heads-up commit for easier parsing in order to build some documentation (or python bindings) later on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 1 + docs/Makefile.am | 23 +++++++++++++++++++---- docs/apibuild.py | 10 +++++++++- libvirt.spec.in | 1 + 4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/.gitignore b/.gitignore index e5e0788..0b40f4a 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,7 @@ /docs/apibuild.py.stamp /docs/devhelp/libvirt.devhelp /docs/hvsupport.html.in +/docs/libvirt-admin-*.xml /docs/libvirt-api.xml /docs/libvirt-lxc-*.xml /docs/libvirt-qemu-*.xml diff --git a/docs/Makefile.am b/docs/Makefile.am index f43da93..b7b49cb 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -128,8 +128,16 @@ lxc_xml = \ libvirt-lxc-api.xml \ libvirt-lxc-refs.xml
+admin_xml = \ + libvirt-admin-api.xml \ + libvirt-admin-refs.xml +
This indentations looks odd, but you're just following pre-existing code. ACK Michal

This is not going to be very widely used, but for some corner cases and easier (unsafe) debugging, it might be nice. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd-config.c | 5 ++++- daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.conf | 8 ++++++++ daemon/test_libvirtd.aug.in | 1 + tests/confdata/libvirtd.conf | 6 ++++++ tests/confdata/libvirtd.out | 5 +++++ 7 files changed, 26 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index 3694455..bce2d70 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -264,7 +264,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) if (VIR_STRDUP(data->unix_sock_rw_perms, data->auth_unix_rw == REMOTE_AUTH_POLKIT ? "0777" : "0700") < 0 || - VIR_STRDUP(data->unix_sock_ro_perms, "0777") < 0) + VIR_STRDUP(data->unix_sock_ro_perms, "0777") < 0 || + VIR_STRDUP(data->unix_sock_admin_perms, "0700") < 0) goto error; #if WITH_SASL @@ -337,6 +338,7 @@ daemonConfigFree(struct daemonConfig *data) } VIR_FREE(data->access_drivers); + VIR_FREE(data->unix_sock_admin_perms); VIR_FREE(data->unix_sock_ro_perms); VIR_FREE(data->unix_sock_rw_perms); VIR_FREE(data->unix_sock_group); @@ -404,6 +406,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, goto error; GET_CONF_STR(conf, filename, unix_sock_group); + GET_CONF_STR(conf, filename, unix_sock_admin_perms); GET_CONF_STR(conf, filename, unix_sock_ro_perms); GET_CONF_STR(conf, filename, unix_sock_rw_perms); diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h index c996995..b8d2bc0 100644 --- a/daemon/libvirtd-config.h +++ b/daemon/libvirtd-config.h @@ -35,6 +35,7 @@ struct daemonConfig { char *tls_port; char *tcp_port; + char *unix_sock_admin_perms; char *unix_sock_ro_perms; char *unix_sock_rw_perms; char *unix_sock_group; diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug index 5a0807c..b20ceca 100644 --- a/daemon/libvirtd.aug +++ b/daemon/libvirtd.aug @@ -35,6 +35,7 @@ module Libvirtd = let sock_acl_entry = str_entry "unix_sock_group" | str_entry "unix_sock_ro_perms" | str_entry "unix_sock_rw_perms" + | str_entry "unix_sock_admin_perms" | str_entry "unix_sock_dir" let authentication_entry = str_entry "auth_unix_ro" diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 069ef3a..6ef38fa 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -106,9 +106,17 @@ # control, then you may want to relax this too. #unix_sock_rw_perms = "0770" +# Set the UNIX socket permissions for the admin interface socket. +# +# Default allows only owner (root), do not change it unless you are +# sure to whom you are exposing the access to. +#unix_sock_admin_perms = "0700" + # Set the name of the directory in which sockets will be found/created. #unix_sock_dir = "/var/run/libvirt" + + ################################################################# # # Authentication. diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in index 37ff33d..a87df5f 100644 --- a/daemon/test_libvirtd.aug.in +++ b/daemon/test_libvirtd.aug.in @@ -12,6 +12,7 @@ module Test_libvirtd = { "unix_sock_group" = "libvirt" } { "unix_sock_ro_perms" = "0777" } { "unix_sock_rw_perms" = "0770" } + { "unix_sock_admin_perms" = "0700" } { "unix_sock_dir" = "/var/run/libvirt" } { "auth_unix_ro" = "none" } { "auth_unix_rw" = "none" } diff --git a/tests/confdata/libvirtd.conf b/tests/confdata/libvirtd.conf index 2f2ba4b..5029c4c 100644 --- a/tests/confdata/libvirtd.conf +++ b/tests/confdata/libvirtd.conf @@ -89,6 +89,12 @@ unix_sock_ro_perms = "0777" # control then you may want to relax this to: unix_sock_rw_perms = "0770" +# Set the UNIX socket permissions for the admin interface socket. +# +# Default allows only owner (root), do not change it unless you are +# sure to whom you are exposing the access to +unix_sock_admin_perms = "0700" + ################################################################# diff --git a/tests/confdata/libvirtd.out b/tests/confdata/libvirtd.out index 171945d..4d7ed47 100644 --- a/tests/confdata/libvirtd.out +++ b/tests/confdata/libvirtd.out @@ -71,6 +71,11 @@ unix_sock_ro_perms = "0777" # If not using PolicyKit and setting group ownership for access # control then you may want to relax this to: unix_sock_rw_perms = "0770" +# Set the UNIX socket permissions for the admin interface socket. +# +# Default allows only owner (root), do not change it unless you are +# sure to whom you are exposing the access to +unix_sock_admin_perms = "0700" ################################################################# # # Authentication. -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
This is not going to be very widely used, but for some corner cases and easier (unsafe) debugging, it might be nice.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd-config.c | 5 ++++- daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.conf | 8 ++++++++ daemon/test_libvirtd.aug.in | 1 + tests/confdata/libvirtd.conf | 6 ++++++ tests/confdata/libvirtd.out | 5 +++++ 7 files changed, 26 insertions(+), 1 deletion(-)
ACK Michal

For this to pe properly separated from other protocols used by the server, there is second server added which allows access to the whole virNetDaemon to its clients. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 3 ++ daemon/Makefile.am | 34 ++++++++++++++- daemon/admin_server.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 36 ++++++++++++++++ daemon/libvirtd.c | 104 +++++++++++++++++++++++++++++++++++++++----- daemon/libvirtd.h | 14 +++++- po/POTFILES.in | 1 + 7 files changed, 295 insertions(+), 13 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h diff --git a/cfg.mk b/cfg.mk index f1b5024..0d1a03c 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1072,6 +1072,7 @@ sc_po_check: \ $(srcdir)/daemon/remote_dispatch.h \ $(srcdir)/daemon/qemu_dispatch.h \ $(srcdir)/src/remote/remote_client_bodies.h \ + $(srcdir)/daemon/admin_dispatch.h \ $(srcdir)/src/admin/admin_client.h $(srcdir)/daemon/remote_dispatch.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C daemon remote_dispatch.h @@ -1079,6 +1080,8 @@ $(srcdir)/daemon/qemu_dispatch.h: $(srcdir)/src/remote/qemu_protocol.x $(MAKE) -C daemon qemu_dispatch.h $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C src remote/remote_client_bodies.h +$(srcdir)/daemon/admin_dispatch.h: $(srcdir)/src/admin/admin_protocol.x + $(MAKE) -C daemon admin_dispatch.h $(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x $(MAKE) -C src admin/admin_client.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 42dec5d..45c6e9b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -25,6 +25,7 @@ INCLUDES = \ -I$(top_srcdir)/src/conf \ -I$(top_srcdir)/src/rpc \ -I$(top_srcdir)/src/remote \ + -I$(top_srcdir)/src/admin \ -I$(top_srcdir)/src/access \ $(GETTEXT_CPPFLAGS) @@ -34,6 +35,7 @@ DAEMON_GENERATED = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ $(NULL) DAEMON_SOURCES = \ @@ -49,6 +51,7 @@ EXTRA_DIST = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ libvirtd.conf \ libvirtd.init.in \ libvirtd.upstart \ @@ -78,6 +81,9 @@ BUILT_SOURCES = REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x +ADMIN_PROTOCOL = $(top_srcdir)/src/admin/admin_protocol.x + +BUILT_SOURCES += admin_dispatch.h remote_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) @@ -97,6 +103,12 @@ qemu_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ --mode=server qemu QEMU $(QEMU_PROTOCOL) \ > $(srcdir)/qemu_dispatch.h +admin_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) + $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ + --mode=server admin ADMIN $(ADMIN_PROTOCOL) \ + > $(srcdir)/admin_dispatch.h + if WITH_LIBVIRTD # Build a convenience library, for reuse in tests/libvirtdconftest @@ -116,6 +128,25 @@ libvirtd_conf_la_LDFLAGS = \ $(NULL) libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) +noinst_LTLIBRARIES += libvirtd_admin.la +libvirtd_admin_la_SOURCES = \ + admin_server.c \ + ../src/admin/admin_protocol.c + +libvirtd_admin_la_CFLAGS = \ + $(AM_CFLAGS) \ + $(XDR_CFLAGS) \ + $(PIE_CFLAGS) \ + $(WARN_CFLAGS) \ + $(LIBXML_CFLAGS) \ + $(COVERAGE_CFLAGS) + +libvirtd_admin_la_LDFLAGS = \ + $(PIE_LDFLAGS) \ + $(RELRO_LDFLAGS) \ + $(COVERAGE_LDFLAGS) \ + $(NO_INDIRECT_LDFLAGS) + man8_MANS = libvirtd.8 sbin_PROGRAMS = libvirtd @@ -168,6 +199,7 @@ endif WITH_DTRACE_PROBES libvirtd_LDADD += \ libvirtd_conf.la \ + libvirtd_admin.la \ ../src/libvirt-lxc.la \ ../src/libvirt-qemu.la \ ../src/libvirt_driver_remote.la \ diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..810908b --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,116 @@ +/* + * admin_server.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@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "libvirtd.h" +#include "libvirt_internal.h" + +#include "admin_protocol.h" +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin"); + + +void +remoteAdmClientFreeFunc(void *data) +{ + struct daemonAdmClientPrivate *priv = data; + + virObjectUnref(priv->dmn); + VIR_FREE(data); +} + +void * +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + struct daemonAdmClientPrivate *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportSystemError(errno, "%s", _("unable to init mutex")); + return NULL; + } + + /* + * We don't necessarily need to ref this object right now as there + * must be one ref being held throughout the life of the daemon, + * but let's just be safe for future. + */ + priv->dmn = virObjectRef(opaque); + + return priv; +} + +/* Functions */ +static int +adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_connect_open_args *args) +{ + unsigned int flags; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + int ret = -1; + + VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); + virMutexLock(&priv->lock); + + flags = args->flags; + virCheckFlagsGoto(0, cleanup); + + ret = 0; + cleanup: + if (ret < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return ret; +} + +static int +adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) +{ + virNetServerClientDelayedClose(client); + return 0; +} + +#include "admin_dispatch.h" diff --git a/daemon/admin_server.h b/daemon/admin_server.h new file mode 100644 index 0000000..26721a6 --- /dev/null +++ b/daemon/admin_server.h @@ -0,0 +1,36 @@ +/* + * admin_server.h + * + * Copyright (C) 2014 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@redhat.com> + */ + +#ifndef __LIBVIRTD_ADMIN_H__ +# define __LIBVIRTD_ADMIN_H__ + +# include "rpc/virnetserverprogram.h" +# include "rpc/virnetserverclient.h" + + +extern virNetServerProgramProc adminProcs[]; +extern size_t adminNProcs; + +void remoteAdmClientFreeFunc(void *data); +void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); + +#endif /* __ADMIN_REMOTE_H__ */ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 22ba6cb..1205671 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -44,6 +44,7 @@ #include "libvirtd.h" #include "libvirtd-config.h" +#include "admin_server.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" @@ -112,6 +113,7 @@ VIR_LOG_INIT("daemon.libvirtd"); virNetSASLContextPtr saslCtxt = NULL; #endif virNetServerProgramPtr remoteProgram = NULL; +virNetServerProgramPtr adminProgram = NULL; virNetServerProgramPtr qemuProgram = NULL; virNetServerProgramPtr lxcProgram = NULL; @@ -253,18 +255,24 @@ static int daemonUnixSocketPaths(struct daemonConfig *config, bool privileged, char **sockfile, - char **rosockfile) + char **rosockfile, + char **admsockfile) { if (config->unix_sock_dir) { if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0) goto error; - if (privileged && - virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) - goto error; + + if (privileged) { + if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) + goto error; + if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0) + goto error; + } } else { if (privileged) { if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 || - VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0) + VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 || + VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0) goto error; } else { char *rundir = NULL; @@ -280,7 +288,8 @@ daemonUnixSocketPaths(struct daemonConfig *config, } umask(old_umask); - if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0) { + if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 || + virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) { VIR_FREE(rundir); goto error; } @@ -427,13 +436,16 @@ static void daemonInitialize(void) static int ATTRIBUTE_NONNULL(3) daemonSetupNetworking(virNetServerPtr srv, + virNetServerPtr srvAdm, struct daemonConfig *config, const char *sock_path, const char *sock_path_ro, + const char *sock_path_adm, bool ipsock, bool privileged) { virNetServerServicePtr svc = NULL; + virNetServerServicePtr svcAdm = NULL; virNetServerServicePtr svcRO = NULL; virNetServerServicePtr svcTCP = NULL; #if WITH_GNUTLS @@ -442,6 +454,7 @@ daemonSetupNetworking(virNetServerPtr srv, gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; int unix_sock_rw_mask = 0; + int unix_sock_adm_mask = 0; unsigned int cur_fd = STDERR_FILENO + 1; unsigned int nfds = virGetListenFDs(); @@ -461,6 +474,11 @@ daemonSetupNetworking(virNetServerPtr srv, goto error; } + if (virStrToLong_i(config->unix_sock_admin_perms, NULL, 8, &unix_sock_adm_mask) != 0) { + VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_admin_perms); + goto error; + } + if (virStrToLong_i(config->unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) { VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_rw_perms); goto error; @@ -503,6 +521,24 @@ daemonSetupNetworking(virNetServerPtr srv, virNetServerAddService(srv, svcRO, NULL) < 0) goto error; + if (sock_path_adm) { + VIR_DEBUG("Registering unix socket %s", sock_path_adm); + if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, + unix_sock_adm_mask, + unix_sock_gid, + REMOTE_AUTH_NONE, +#if WITH_GNUTLS + NULL, +#endif + true, + config->max_queued_clients, + config->max_client_requests))) + goto error; + } + + if (virNetServerAddService(srvAdm, svcAdm, NULL) < 0) + goto error; + if (ipsock) { if (config->listen_tcp) { VIR_DEBUG("Registering TCP socket %s:%s", @@ -600,6 +636,7 @@ daemonSetupNetworking(virNetServerPtr srv, virObjectUnref(svcTCP); virObjectUnref(svc); virObjectUnref(svcRO); + virObjectUnref(svcAdm); return -1; } @@ -1100,6 +1137,7 @@ daemonUsage(const char *argv0, bool privileged) int main(int argc, char **argv) { virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; + virNetServerPtr srvAdm = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -1107,6 +1145,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; char *sock_file = NULL; char *sock_file_ro = NULL; + char *sock_file_adm = NULL; int timeout = -1; /* -t: Shutdown timeout */ int verbose = 0; int godaemon = 0; @@ -1274,12 +1313,15 @@ int main(int argc, char **argv) { if (daemonUnixSocketPaths(config, privileged, &sock_file, - &sock_file_ro) < 0) { + &sock_file_ro, + &sock_file_adm) < 0) { VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); } - VIR_DEBUG("Decided on socket paths '%s' and '%s'", - sock_file, NULLSTR(sock_file_ro)); + VIR_DEBUG("Decided on socket paths '%s', '%s' and '%s'", + sock_file, + NULLSTR(sock_file_ro), + NULLSTR(sock_file_adm)); if (godaemon) { char ebuf[1024]; @@ -1411,6 +1453,40 @@ int main(int argc, char **argv) { goto cleanup; } + if (!(srvAdm = virNetServerNew(config->min_workers, + config->max_workers, + config->prio_workers, + config->max_clients, + config->max_anonymous_clients, + config->keepalive_interval, + config->keepalive_count, + !!config->keepalive_required, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + dmn))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + + if (virNetDaemonAddServer(dmn, srvAdm) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM, + ADMIN_PROTOCOL_VERSION, + adminProcs, + adminNProcs))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(srvAdm, adminProgram) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); virNetDaemonAutoShutdown(dmn, timeout); @@ -1451,8 +1527,11 @@ int main(int argc, char **argv) { virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START, 0, "start", NULL, NULL); - if (daemonSetupNetworking(srv, config, - sock_file, sock_file_ro, + if (daemonSetupNetworking(srv, srvAdm, + config, + sock_file, + sock_file_ro, + sock_file_adm, ipsock, privileged) < 0) { ret = VIR_DAEMON_ERR_NETWORK; goto cleanup; @@ -1505,9 +1584,11 @@ int main(int argc, char **argv) { virObjectUnref(remoteProgram); virObjectUnref(lxcProgram); virObjectUnref(qemuProgram); + virObjectUnref(adminProgram); virNetDaemonClose(dmn); virObjectUnref(dmn); virObjectUnref(srv); + virObjectUnref(srvAdm); virNetlinkShutdown(); if (statuswrite != -1) { if (ret != 0) { @@ -1524,6 +1605,7 @@ int main(int argc, char **argv) { VIR_FREE(sock_file); VIR_FREE(sock_file_ro); + VIR_FREE(sock_file_adm); VIR_FREE(pid_file); VIR_FREE(remote_config_file); VIR_FREE(run_dir); diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 02d4101..8c1a904 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -1,7 +1,7 @@ /* * libvirtd.h: daemon data structure definitions * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -30,9 +30,11 @@ # include <rpc/types.h> # include <rpc/xdr.h> # include "remote_protocol.h" +# include "admin_protocol.h" # include "lxc_protocol.h" # include "qemu_protocol.h" # include "virthread.h" + # if WITH_SASL # include "virnetsaslcontext.h" # endif @@ -42,6 +44,8 @@ typedef struct daemonClientStream daemonClientStream; typedef daemonClientStream *daemonClientStreamPtr; typedef struct daemonClientPrivate daemonClientPrivate; typedef daemonClientPrivate *daemonClientPrivatePtr; +typedef struct daemonAdmClientPrivate daemonAdmClientPrivate; +typedef daemonAdmClientPrivate *daemonAdmClientPrivatePtr; typedef struct daemonClientEventCallback daemonClientEventCallback; typedef daemonClientEventCallback *daemonClientEventCallbackPtr; @@ -71,6 +75,14 @@ struct daemonClientPrivate { bool keepalive_supported; }; +/* Separate private data for admin connection */ +struct daemonAdmClientPrivate { + /* Just a placeholder, not that there is anything to be locked */ + virMutex lock; + + virNetDaemonPtr dmn; +}; + # if WITH_SASL extern virNetSASLContextPtr saslCtxt; # endif diff --git a/po/POTFILES.in b/po/POTFILES.in index 4afa2f9..189e2cc 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,3 +1,4 @@ +daemon/admin_server.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
For this to pe properly separated from other protocols used by the server, there is second server added which allows access to the whole virNetDaemon to its clients.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 3 ++ daemon/Makefile.am | 34 ++++++++++++++- daemon/admin_server.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 36 ++++++++++++++++ daemon/libvirtd.c | 104 +++++++++++++++++++++++++++++++++++++++----- daemon/libvirtd.h | 14 +++++- po/POTFILES.in | 1 + 7 files changed, 295 insertions(+), 13 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 42dec5d..45c6e9b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -25,6 +25,7 @@ INCLUDES = \ -I$(top_srcdir)/src/conf \ -I$(top_srcdir)/src/rpc \ -I$(top_srcdir)/src/remote \ + -I$(top_srcdir)/src/admin \ -I$(top_srcdir)/src/access \ $(GETTEXT_CPPFLAGS)
@@ -34,6 +35,7 @@ DAEMON_GENERATED = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ $(NULL)
DAEMON_SOURCES = \ @@ -49,6 +51,7 @@ EXTRA_DIST = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ libvirtd.conf \ libvirtd.init.in \ libvirtd.upstart \ @@ -78,6 +81,9 @@ BUILT_SOURCES = REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x +ADMIN_PROTOCOL = $(top_srcdir)/src/admin/admin_protocol.x + +BUILT_SOURCES += admin_dispatch.h
No. This one is not really a built source. Into BUILT_SOURCES should go only those files which are built from the .tar.gz. This one is built from git. I mean, you need whole set of developer tools (e.g. rpcgen in this case) to generate some files which are part of released package. For instance, previously unpacking the package, and running 'make clean' would not clean anything. Now it cleans admin_dispatch.h. Worse, in order to generate it you need rpcgen, which you previously didn't.
remote_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) @@ -97,6 +103,12 @@ qemu_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ --mode=server qemu QEMU $(QEMU_PROTOCOL) \ > $(srcdir)/qemu_dispatch.h
+admin_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) + $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ + --mode=server admin ADMIN $(ADMIN_PROTOCOL) \ + > $(srcdir)/admin_dispatch.h + if WITH_LIBVIRTD
# Build a convenience library, for reuse in tests/libvirtdconftest
diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..810908b --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,116 @@ +/* + * admin_server.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@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "libvirtd.h" +#include "libvirt_internal.h" + +#include "admin_protocol.h" +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin"); + + +void +remoteAdmClientFreeFunc(void *data) +{ + struct daemonAdmClientPrivate *priv = data; + + virObjectUnref(priv->dmn);
virMutexDestroy(&priv->lock);
+ VIR_FREE(data); +} + +void * +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + struct daemonAdmClientPrivate *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportSystemError(errno, "%s", _("unable to init mutex")); + return NULL; + } + + /* + * We don't necessarily need to ref this object right now as there + * must be one ref being held throughout the life of the daemon, + * but let's just be safe for future. + */ + priv->dmn = virObjectRef(opaque); + + return priv; +}
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 22ba6cb..1205671 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -44,6 +44,7 @@ #include "libvirtd.h" #include "libvirtd-config.h"
+#include "admin_server.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" @@ -112,6 +113,7 @@ VIR_LOG_INIT("daemon.libvirtd"); virNetSASLContextPtr saslCtxt = NULL; #endif virNetServerProgramPtr remoteProgram = NULL; +virNetServerProgramPtr adminProgram = NULL; virNetServerProgramPtr qemuProgram = NULL; virNetServerProgramPtr lxcProgram = NULL;
@@ -253,18 +255,24 @@ static int daemonUnixSocketPaths(struct daemonConfig *config, bool privileged, char **sockfile, - char **rosockfile) + char **rosockfile, + char **admsockfile) { if (config->unix_sock_dir) { if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0) goto error; - if (privileged && - virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) - goto error; + + if (privileged) { + if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) + goto error; + if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0) + goto error; + } } else { if (privileged) { if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 || - VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0) + VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 || + VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0) goto error; } else { char *rundir = NULL; @@ -280,7 +288,8 @@ daemonUnixSocketPaths(struct daemonConfig *config, } umask(old_umask);
- if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0) { + if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 || + virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) { VIR_FREE(rundir); goto error; } @@ -427,13 +436,16 @@ static void daemonInitialize(void)
static int ATTRIBUTE_NONNULL(3) daemonSetupNetworking(virNetServerPtr srv, + virNetServerPtr srvAdm, struct daemonConfig *config, const char *sock_path, const char *sock_path_ro, + const char *sock_path_adm, bool ipsock, bool privileged) { virNetServerServicePtr svc = NULL; + virNetServerServicePtr svcAdm = NULL; virNetServerServicePtr svcRO = NULL; virNetServerServicePtr svcTCP = NULL; #if WITH_GNUTLS @@ -442,6 +454,7 @@ daemonSetupNetworking(virNetServerPtr srv, gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; int unix_sock_rw_mask = 0; + int unix_sock_adm_mask = 0;
unsigned int cur_fd = STDERR_FILENO + 1; unsigned int nfds = virGetListenFDs(); @@ -461,6 +474,11 @@ daemonSetupNetworking(virNetServerPtr srv, goto error; }
+ if (virStrToLong_i(config->unix_sock_admin_perms, NULL, 8, &unix_sock_adm_mask) != 0) { + VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_admin_perms); + goto error; + } + if (virStrToLong_i(config->unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) { VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_rw_perms); goto error; @@ -503,6 +521,24 @@ daemonSetupNetworking(virNetServerPtr srv, virNetServerAddService(srv, svcRO, NULL) < 0) goto error;
+ if (sock_path_adm) { + VIR_DEBUG("Registering unix socket %s", sock_path_adm); + if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, + unix_sock_adm_mask, + unix_sock_gid, + REMOTE_AUTH_NONE, +#if WITH_GNUTLS + NULL, +#endif + true, + config->max_queued_clients, + config->max_client_requests))) + goto error; + } + + if (virNetServerAddService(srvAdm, svcAdm, NULL) < 0) + goto error; + if (ipsock) { if (config->listen_tcp) { VIR_DEBUG("Registering TCP socket %s:%s", @@ -600,6 +636,7 @@ daemonSetupNetworking(virNetServerPtr srv, virObjectUnref(svcTCP); virObjectUnref(svc); virObjectUnref(svcRO); + virObjectUnref(svcAdm); return -1; }
@@ -1100,6 +1137,7 @@ daemonUsage(const char *argv0, bool privileged) int main(int argc, char **argv) { virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; + virNetServerPtr srvAdm = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -1107,6 +1145,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; char *sock_file = NULL; char *sock_file_ro = NULL; + char *sock_file_adm = NULL; int timeout = -1; /* -t: Shutdown timeout */ int verbose = 0; int godaemon = 0; @@ -1274,12 +1313,15 @@ int main(int argc, char **argv) { if (daemonUnixSocketPaths(config, privileged, &sock_file, - &sock_file_ro) < 0) { + &sock_file_ro, + &sock_file_adm) < 0) { VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); } - VIR_DEBUG("Decided on socket paths '%s' and '%s'", - sock_file, NULLSTR(sock_file_ro)); + VIR_DEBUG("Decided on socket paths '%s', '%s' and '%s'", + sock_file, + NULLSTR(sock_file_ro), + NULLSTR(sock_file_adm));
if (godaemon) { char ebuf[1024]; @@ -1411,6 +1453,40 @@ int main(int argc, char **argv) { goto cleanup; }
+ if (!(srvAdm = virNetServerNew(config->min_workers, + config->max_workers, + config->prio_workers, + config->max_clients, + config->max_anonymous_clients,
Hm.. This may work for now, but I guess we want different limits here. Users running libvirt in production may have really big worker pool, while admin API will suffice one or two, like 640 kilo of RAM (TM).
+ config->keepalive_interval, + config->keepalive_count,
I guess keepalive on a socket unix is overkill. The eventloop will see EOF immediately as client closes the socket.
+ !!config->keepalive_required, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + dmn))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + + if (virNetDaemonAddServer(dmn, srvAdm) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + }
This is what I was talking about in reply to 02/13. Imagine the limits for domain worker pool and sockets were set really low on daemon startup. No new client can be accepted there. I would expect mgmt application to connect to admin API and size up the limits. However, due to bug in 2/13 all servers are suspended. I'll leave it up to you where you'd like to solve it.
+ + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM, + ADMIN_PROTOCOL_VERSION, + adminProcs, + adminNProcs))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(srvAdm, adminProgram) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } +
Otherwise looking good. Michal

On Thu, May 21, 2015 at 05:46:54PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
For this to pe properly separated from other protocols used by the server, there is second server added which allows access to the whole virNetDaemon to its clients.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 3 ++ daemon/Makefile.am | 34 ++++++++++++++- daemon/admin_server.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 36 ++++++++++++++++ daemon/libvirtd.c | 104 +++++++++++++++++++++++++++++++++++++++----- daemon/libvirtd.h | 14 +++++- po/POTFILES.in | 1 + 7 files changed, 295 insertions(+), 13 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 42dec5d..45c6e9b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -25,6 +25,7 @@ INCLUDES = \ -I$(top_srcdir)/src/conf \ -I$(top_srcdir)/src/rpc \ -I$(top_srcdir)/src/remote \ + -I$(top_srcdir)/src/admin \ -I$(top_srcdir)/src/access \ $(GETTEXT_CPPFLAGS)
@@ -34,6 +35,7 @@ DAEMON_GENERATED = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ $(NULL)
DAEMON_SOURCES = \ @@ -49,6 +51,7 @@ EXTRA_DIST = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h \ libvirtd.conf \ libvirtd.init.in \ libvirtd.upstart \ @@ -78,6 +81,9 @@ BUILT_SOURCES = REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x +ADMIN_PROTOCOL = $(top_srcdir)/src/admin/admin_protocol.x + +BUILT_SOURCES += admin_dispatch.h
No. This one is not really a built source. Into BUILT_SOURCES should go only those files which are built from the .tar.gz. This one is built from git. I mean, you need whole set of developer tools (e.g. rpcgen in this case) to generate some files which are part of released package. For instance, previously unpacking the package, and running 'make clean' would not clean anything. Now it cleans admin_dispatch.h. Worse, in order to generate it you need rpcgen, which you previously didn't.
remote_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) @@ -97,6 +103,12 @@ qemu_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ --mode=server qemu QEMU $(QEMU_PROTOCOL) \ > $(srcdir)/qemu_dispatch.h
+admin_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) + $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ + --mode=server admin ADMIN $(ADMIN_PROTOCOL) \ + > $(srcdir)/admin_dispatch.h + if WITH_LIBVIRTD
# Build a convenience library, for reuse in tests/libvirtdconftest
diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..810908b --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,116 @@ +/* + * admin_server.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@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "libvirtd.h" +#include "libvirt_internal.h" + +#include "admin_protocol.h" +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin"); + + +void +remoteAdmClientFreeFunc(void *data) +{ + struct daemonAdmClientPrivate *priv = data; + + virObjectUnref(priv->dmn);
virMutexDestroy(&priv->lock);
+ VIR_FREE(data); +} + +void * +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + struct daemonAdmClientPrivate *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportSystemError(errno, "%s", _("unable to init mutex")); + return NULL; + } + + /* + * We don't necessarily need to ref this object right now as there + * must be one ref being held throughout the life of the daemon, + * but let's just be safe for future. + */ + priv->dmn = virObjectRef(opaque); + + return priv; +}
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 22ba6cb..1205671 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -44,6 +44,7 @@ #include "libvirtd.h" #include "libvirtd-config.h"
+#include "admin_server.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" @@ -112,6 +113,7 @@ VIR_LOG_INIT("daemon.libvirtd"); virNetSASLContextPtr saslCtxt = NULL; #endif virNetServerProgramPtr remoteProgram = NULL; +virNetServerProgramPtr adminProgram = NULL; virNetServerProgramPtr qemuProgram = NULL; virNetServerProgramPtr lxcProgram = NULL;
@@ -253,18 +255,24 @@ static int daemonUnixSocketPaths(struct daemonConfig *config, bool privileged, char **sockfile, - char **rosockfile) + char **rosockfile, + char **admsockfile) { if (config->unix_sock_dir) { if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0) goto error; - if (privileged && - virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) - goto error; + + if (privileged) { + if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0) + goto error; + if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0) + goto error; + } } else { if (privileged) { if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 || - VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0) + VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 || + VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0) goto error; } else { char *rundir = NULL; @@ -280,7 +288,8 @@ daemonUnixSocketPaths(struct daemonConfig *config, } umask(old_umask);
- if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0) { + if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 || + virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) { VIR_FREE(rundir); goto error; } @@ -427,13 +436,16 @@ static void daemonInitialize(void)
static int ATTRIBUTE_NONNULL(3) daemonSetupNetworking(virNetServerPtr srv, + virNetServerPtr srvAdm, struct daemonConfig *config, const char *sock_path, const char *sock_path_ro, + const char *sock_path_adm, bool ipsock, bool privileged) { virNetServerServicePtr svc = NULL; + virNetServerServicePtr svcAdm = NULL; virNetServerServicePtr svcRO = NULL; virNetServerServicePtr svcTCP = NULL; #if WITH_GNUTLS @@ -442,6 +454,7 @@ daemonSetupNetworking(virNetServerPtr srv, gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; int unix_sock_rw_mask = 0; + int unix_sock_adm_mask = 0;
unsigned int cur_fd = STDERR_FILENO + 1; unsigned int nfds = virGetListenFDs(); @@ -461,6 +474,11 @@ daemonSetupNetworking(virNetServerPtr srv, goto error; }
+ if (virStrToLong_i(config->unix_sock_admin_perms, NULL, 8, &unix_sock_adm_mask) != 0) { + VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_admin_perms); + goto error; + } + if (virStrToLong_i(config->unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) { VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_rw_perms); goto error; @@ -503,6 +521,24 @@ daemonSetupNetworking(virNetServerPtr srv, virNetServerAddService(srv, svcRO, NULL) < 0) goto error;
+ if (sock_path_adm) { + VIR_DEBUG("Registering unix socket %s", sock_path_adm); + if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, + unix_sock_adm_mask, + unix_sock_gid, + REMOTE_AUTH_NONE, +#if WITH_GNUTLS + NULL, +#endif + true, + config->max_queued_clients, + config->max_client_requests))) + goto error; + } + + if (virNetServerAddService(srvAdm, svcAdm, NULL) < 0) + goto error; + if (ipsock) { if (config->listen_tcp) { VIR_DEBUG("Registering TCP socket %s:%s", @@ -600,6 +636,7 @@ daemonSetupNetworking(virNetServerPtr srv, virObjectUnref(svcTCP); virObjectUnref(svc); virObjectUnref(svcRO); + virObjectUnref(svcAdm); return -1; }
@@ -1100,6 +1137,7 @@ daemonUsage(const char *argv0, bool privileged) int main(int argc, char **argv) { virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; + virNetServerPtr srvAdm = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -1107,6 +1145,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; char *sock_file = NULL; char *sock_file_ro = NULL; + char *sock_file_adm = NULL; int timeout = -1; /* -t: Shutdown timeout */ int verbose = 0; int godaemon = 0; @@ -1274,12 +1313,15 @@ int main(int argc, char **argv) { if (daemonUnixSocketPaths(config, privileged, &sock_file, - &sock_file_ro) < 0) { + &sock_file_ro, + &sock_file_adm) < 0) { VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); } - VIR_DEBUG("Decided on socket paths '%s' and '%s'", - sock_file, NULLSTR(sock_file_ro)); + VIR_DEBUG("Decided on socket paths '%s', '%s' and '%s'", + sock_file, + NULLSTR(sock_file_ro), + NULLSTR(sock_file_adm));
if (godaemon) { char ebuf[1024]; @@ -1411,6 +1453,40 @@ int main(int argc, char **argv) { goto cleanup; }
+ if (!(srvAdm = virNetServerNew(config->min_workers, + config->max_workers, + config->prio_workers, + config->max_clients, + config->max_anonymous_clients,
Hm.. This may work for now, but I guess we want different limits here. Users running libvirt in production may have really big worker pool, while admin API will suffice one or two, like 640 kilo of RAM (TM).
Yes, makes sense, I forgot this was a trial part as well. I'll add the configuration and different defaults.
+ config->keepalive_interval, + config->keepalive_count,
I guess keepalive on a socket unix is overkill. The eventloop will see EOF immediately as client closes the socket.
It's not about seeing EOF, but about *not* seeing EOF. We want to disconnect from clients that are caught in an endless loop.
+ !!config->keepalive_required, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + dmn))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + + if (virNetDaemonAddServer(dmn, srvAdm) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + }
This is what I was talking about in reply to 02/13. Imagine the limits for domain worker pool and sockets were set really low on daemon startup. No new client can be accepted there. I would expect mgmt application to connect to admin API and size up the limits. However, due to bug in 2/13 all servers are suspended. I'll leave it up to you where you'd like to solve it.
There's nothing to solve. The client is being added in a particular server and that server will call virNetServerUpdateServicesLocked() only on itself, not virNetDaemonUpdateServices() on the daemon.
+ + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM, + ADMIN_PROTOCOL_VERSION, + adminProcs, + adminNProcs))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(srvAdm, adminProgram) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } +
Otherwise looking good.
Michal

On 22.05.2015 07:22, Martin Kletzander wrote:
On Thu, May 21, 2015 at 05:46:54PM +0200, Michal Privoznik wrote:
On 20.05.2015 07:19, Martin Kletzander wrote:
For this to pe properly separated from other protocols used by the server, there is second server added which allows access to the whole virNetDaemon to its clients.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- cfg.mk | 3 ++ daemon/Makefile.am | 34 ++++++++++++++- daemon/admin_server.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 36 ++++++++++++++++ daemon/libvirtd.c | 104 +++++++++++++++++++++++++++++++++++++++----- daemon/libvirtd.h | 14 +++++- po/POTFILES.in | 1 + 7 files changed, 295 insertions(+), 13 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h
+ !!config->keepalive_required, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + dmn))) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } + + if (virNetDaemonAddServer(dmn, srvAdm) < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + }
This is what I was talking about in reply to 02/13. Imagine the limits for domain worker pool and sockets were set really low on daemon startup. No new client can be accepted there. I would expect mgmt application to connect to admin API and size up the limits. However, due to bug in 2/13 all servers are suspended. I'll leave it up to you where you'd like to solve it.
There's nothing to solve. The client is being added in a particular server and that server will call virNetServerUpdateServicesLocked() only on itself, not virNetDaemonUpdateServices() on the daemon.
Oh, right. I recalled adding updateServices function somewhere so that server stops accepting new clients when a certain limit is reached. I forgot it was updateServicesLocked() which you don't touch. Disregard my comments to this then. Michal

This function accesses the number of connected clients while properly locking the server it returns the data about. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 10 ++++++++++ src/rpc/virnetserver.h | 2 ++ 3 files changed, 13 insertions(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index ac12807..7cd0cf3 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -96,6 +96,7 @@ xdr_virNetMessageError; virNetServerAddProgram; virNetServerAddService; virNetServerClose; +virNetServerGetNClients; virNetServerHasClients; virNetServerKeepAliveRequired; virNetServerNew; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index df844d9..af7f87b 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -882,3 +882,13 @@ virNetServerStart(virNetServerPtr srv) return virNetServerMDNSStart(srv->mdns); } + +size_t +virNetServerGetNClients(virNetServerPtr srv) +{ + size_t ret = 0; + virObjectLock(srv); + ret = srv->nclients; + virObjectUnlock(srv); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 3e312c0..0316610 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -87,4 +87,6 @@ void virNetServerUpdateServices(virNetServerPtr srv, bool enabled); int virNetServerStart(virNetServerPtr srv); +size_t virNetServerGetNClients(virNetServerPtr srv); + #endif /* __VIR_NET_SERVER_H__ */ -- 2.4.0

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: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 23 +++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 1 + po/POTFILES.in | 1 + src/admin/admin_protocol.x | 15 ++++++++++++++- src/admin_protocol-structs | 9 +++++++++ src/libvirt-admin.c | 26 ++++++++++++++++++++++++++ src/libvirt_admin.syms | 1 + 7 files changed, 75 insertions(+), 1 deletion(-) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 810908b..e722757 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -113,4 +113,27 @@ adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, return 0; } + +static char * +admHello(virNetDaemonPtr dmn, + unsigned int flags) +{ + char *ret = NULL; + virNetServerPtr srv = NULL; + size_t nclients; + + virCheckFlags(0, NULL); + + if (!(srv = virNetDaemonGetServer(dmn, 0))) + return NULL; + + nclients = virNetServerGetNClients(srv); + if (virAsprintf(&ret, "Clients: %zu", nclients) < 0) + goto cleanup; + + cleanup: + virObjectUnref(srv); + return ret; +} + #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index b3cfc93..27178b4 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -54,6 +54,7 @@ virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); int virAdmConnectRef(virAdmConnectPtr conn); +char *virAdmHello(virAdmConnectPtr conn, unsigned int flags); # ifdef __cplusplus } diff --git a/po/POTFILES.in b/po/POTFILES.in index 189e2cc..31f03d9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,3 +1,4 @@ +daemon/admin_dispatch.h daemon/admin_server.c daemon/libvirtd-config.c daemon/libvirtd.c diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 63f6a53..03b98ec 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,6 +30,14 @@ struct admin_connect_open_args { unsigned int flags; }; +struct admin_hello_args { + unsigned int flags; +}; + +struct admin_hello_ret { + remote_string greeting; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; @@ -61,5 +69,10 @@ enum admin_procedure { /** * @generate: client */ - ADMIN_PROC_CONNECT_CLOSE = 2 + ADMIN_PROC_CONNECT_CLOSE = 2, + + /** + * @generate: both + */ + ADMIN_PROC_HELLO = 3 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index bc1d489..7af66ce 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -3,7 +3,16 @@ struct admin_connect_open_args { unsigned int flags; }; +struct admin_hello_args { + unsigned int flags; +}; + +struct admin_hello_ret { + remote_string greeting; +}; + enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, + ADMIN_PROC_HELLO = 3, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 11b6fe3..107e27e 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -384,3 +384,29 @@ virAdmConnectRef(virAdmConnectPtr conn) return 0; } + + +/** + * virAdmHello: + * @conn: a connection object + * @flags: unused, 0 for now + * + * Testing function. + * + * Returns the number of connected clients as a string. Yes, as a + * string. Because it's just for testing. + */ +char * +virAdmHello(virAdmConnectPtr conn, + unsigned int flags) +{ + char *ret = NULL; + + virResetLastError(); + + ret = remoteAdminHello(conn, flags); + if (!ret) + virDispatchError(NULL); + + return ret; +} diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms index ea6a8cc..8d5c3f8 100644 --- a/src/libvirt_admin.syms +++ b/src/libvirt_admin.syms @@ -15,4 +15,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmInitialize; virAdmConnectOpen; virAdmConnectClose; + virAdmHello; }; -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
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: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 23 +++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 1 + po/POTFILES.in | 1 + src/admin/admin_protocol.x | 15 ++++++++++++++- src/admin_protocol-structs | 9 +++++++++ src/libvirt-admin.c | 26 ++++++++++++++++++++++++++ src/libvirt_admin.syms | 1 + 7 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 810908b..e722757 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -113,4 +113,27 @@ adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, return 0; }
+ +static char * +admHello(virNetDaemonPtr dmn, + unsigned int flags) +{ + char *ret = NULL; + virNetServerPtr srv = NULL; + size_t nclients; + + virCheckFlags(0, NULL); + + if (!(srv = virNetDaemonGetServer(dmn, 0))) + return NULL; + + nclients = virNetServerGetNClients(srv); + if (virAsprintf(&ret, "Clients: %zu", nclients) < 0) + goto cleanup; + + cleanup: + virObjectUnref(srv); + return ret; +} + #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index b3cfc93..27178b4 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -54,6 +54,7 @@ virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn);
int virAdmConnectRef(virAdmConnectPtr conn); +char *virAdmHello(virAdmConnectPtr conn, unsigned int flags);
# ifdef __cplusplus } diff --git a/po/POTFILES.in b/po/POTFILES.in index 189e2cc..31f03d9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,3 +1,4 @@ +daemon/admin_dispatch.h daemon/admin_server.c daemon/libvirtd-config.c daemon/libvirtd.c diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 63f6a53..03b98ec 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,6 +30,14 @@ struct admin_connect_open_args { unsigned int flags; };
+struct admin_hello_args { + unsigned int flags; +}; + +struct admin_hello_ret { + remote_string greeting;
A-ha! Now I see why you wanted to drag in remote_protocol.c (my comment to 7/13). Well, if you: const REMOTE_STRING_MAX = 4194304; /* A long string, which may NOT be NULL. */ typedef string remote_nonnull_string<REMOTE_STRING_MAX>; /* A long string, which may be NULL. */ typedef remote_nonnull_string *remote_string; then you don't need anymore.
+}; +
Michal

You had only one job. That's what you can say about this example binary. In future, parts of virsh that are usable for this binary should be split into separate shell-utils and virt-admin should gain all the cool features of virsh without too much code addition. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .gitignore | 2 ++ Makefile.am | 4 +-- configure.ac | 7 +++- libvirt.spec.in | 2 ++ tools/virt-admin/Makefile.am | 70 +++++++++++++++++++++++++++++++++++++++ tools/virt-admin/virt-admin.c | 72 +++++++++++++++++++++++++++++++++++++++++ tools/virt-admin/virt-admin.pod | 43 ++++++++++++++++++++++++ 7 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 tools/virt-admin/Makefile.am create mode 100644 tools/virt-admin/virt-admin.c create mode 100644 tools/virt-admin/virt-admin.pod diff --git a/.gitignore b/.gitignore index 0b40f4a..ec1e3e6 100644 --- a/.gitignore +++ b/.gitignore @@ -176,6 +176,8 @@ /tools/virsh-*-edit.c /tools/virt-*-validate /tools/virt-sanlock-cleanup +/tools/virt-admin/virt-admin.1 +/tools/virt-admin/virt-admin /tools/wireshark/src/plugin.c /tools/wireshark/src/libvirt /update.log diff --git a/Makefile.am b/Makefile.am index 4aafe94..cdae769 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2013 Red Hat, Inc. +## Copyright (C) 2005-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 @@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ examples/dominfo examples/domsuspend examples/apparmor \ examples/xml/nwfilter examples/openauth examples/systemtap \ tools/wireshark examples/dommigrate \ - examples/lxcconvert examples/domtop + examples/lxcconvert examples/domtop tools/virt-admin ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 0409f9d..2ac682f 100644 --- a/configure.ac +++ b/configure.ac @@ -1685,6 +1685,10 @@ dnl virsh libraries VIRSH_LIBS="$VIRSH_LIBS $READLINE_LIBS" AC_SUBST([VIRSH_LIBS]) +dnl virt-admin libraries +VIRT_ADMIN_LIBS="$VIRT_ADMIN_LIBS $READLINE_LIBS" +AC_SUBST([VIRT_ADMIN_LIBS]) + dnl check if the network driver should be compiled AC_ARG_WITH([network], @@ -2816,7 +2820,8 @@ AC_CONFIG_FILES([\ examples/xml/nwfilter/Makefile \ examples/lxcconvert/Makefile \ tools/wireshark/Makefile \ - tools/wireshark/src/Makefile]) + tools/wireshark/src/Makefile \ + tools/virt-admin/Makefile]) AC_OUTPUT AC_MSG_NOTICE([]) diff --git a/libvirt.spec.in b/libvirt.spec.in index b00f99b..98887e3 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2209,6 +2209,8 @@ exit 0 %if %{with_admin} %files admin %defattr(-, root, root) +%{_mandir}/man1/virt-admin.1* +%{_bindir}/virt-admin %{_libdir}/%{name}/libvirt_admin.so %endif diff --git a/tools/virt-admin/Makefile.am b/tools/virt-admin/Makefile.am new file mode 100644 index 0000000..47c9646 --- /dev/null +++ b/tools/virt-admin/Makefile.am @@ -0,0 +1,70 @@ +## Copyright (C) 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/>. + +AM_CPPFLAGS = \ + -I$(top_builddir)/include -I$(top_srcdir)/include \ + -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \ + -I$(top_builddir)/src -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/util \ + -I$(top_srcdir) \ + $(GETTEXT_CPPFLAGS) + +AM_LDFLAGS = \ + $(RELRO_LDFLAGS) \ + $(NO_INDIRECT_LDFLAGS) \ + $(NULL) + +POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" + +EXTRA_DIST = virt-admin.pod + +DISTCLEANFILES = + +bin_PROGRAMS = virt-admin + +dist_man1_MANS = virt-admin.1 + +virt_admin_SOURCES = virt-admin.c + +virt_admin_LDFLAGS = \ + $(AM_LDFLAGS) \ + $(COVERAGE_LDFLAGS) \ + $(NULL) + +virt_admin_LDADD = \ + $(STATIC_BINARIES) \ + $(PIE_LDFLAGS) \ + ../../src/libvirt.la \ + ../../src/libvirt-admin.la \ + $(top_srcdir)/gnulib/lib/libgnu.la \ + $(VIRT_ADMIN_LIBS) \ + $(NULL) + +virt_admin_CFLAGS = \ + $(WARN_CFLAGS) \ + $(PIE_CFLAGS) \ + $(COVERAGE_CFLAGS) \ + $(LIBXML_CFLAGS) \ + $(READLINE_CFLAGS) \ + $(NULL) + +virt-admin.1: virt-admin.pod $(top_srcdir)/configure.ac + $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@ \ + && if grep 'POD ERROR' $(srcdir)/$@ ; then \ + rm $(srcdir)/$@; exit 1; fi + +CLEANFILES = *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda +MAINTAINERCLEANFILES = $(dist_man1_MANS) diff --git a/tools/virt-admin/virt-admin.c b/tools/virt-admin/virt-admin.c new file mode 100644 index 0000000..3197551 --- /dev/null +++ b/tools/virt-admin/virt-admin.c @@ -0,0 +1,72 @@ +/* + * virt-admin.c: a shell to exercise the libvirt admin API + * + * 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/>. + * + * Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <locale.h> + +#include "configmake.h" +#include "internal.h" + +#include <viralloc.h> + +int +main(int argc ATTRIBUTE_UNUSED, + char **argv ATTRIBUTE_UNUSED) +{ + int ret = EXIT_FAILURE; + char *greeting = NULL; + const char *uri = NULL; + virAdmConnectPtr conn = NULL; + + if (!setlocale(LC_ALL, "")) { + perror("setlocale"); + /* failure to setup locale is not fatal */ + } + if (!bindtextdomain(PACKAGE, LOCALEDIR)) { + perror("bindtextdomain"); + return EXIT_FAILURE; + } + if (!textdomain(PACKAGE)) { + perror("textdomain"); + return EXIT_FAILURE; + } + + if (argc > 1) + uri = argv[1]; + + if (!(conn = virAdmConnectOpen(uri, 0))) + goto cleanup; + + if (!(greeting = virAdmHello(conn, 0))) + goto cleanup; + + printf("%s\n", greeting); + + ret = EXIT_SUCCESS; + cleanup: + VIR_FREE(greeting); + virAdmConnectClose(conn); + return ret; +} diff --git a/tools/virt-admin/virt-admin.pod b/tools/virt-admin/virt-admin.pod new file mode 100644 index 0000000..b82013b --- /dev/null +++ b/tools/virt-admin/virt-admin.pod @@ -0,0 +1,43 @@ +=head1 NAME + +virt-admin - client for admin interface of libvirt + +=head1 SYNOPSIS + +B<virt-admin> + +=head1 DESCRIPTION + +Libre ip_sum do { lor(); } sit; I'm @. + +=head1 EXIT STATUS + +It should be 0, really. + +=head1 BUGS + +None! + +=head1 AUTHORS + + Please refer to the AUTHORS file distributed with libvirt. + + Martin Kletzander <mkletzan@redhat.com> + +=head1 COPYRIGHT + +Copyright (C) 2014-2015 Red Hat, Inc., and the authors listed in the +libvirt AUTHORS file. + +=head1 LICENSE + +virt-admin is distributed under the terms of the GNU GPL v2+. This is +free software; see the source for copying conditions. There is NO +warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR +PURPOSE + +=head1 SEE ALSO + +L<virsh(1)>, L<http://www.libvirt.org/> + +=cut -- 2.4.0

On 20.05.2015 07:19, Martin Kletzander wrote:
This is a working part of the administration API with some usability helpers. We're still missing documentation and proper client (for which virsh needs to be split), but apart from the last three patches, this series is ready to go in if we decide that the documentation and client work can be done later on.
Martin Kletzander (13): util: add virJSONValueCopy Move daemon-related parts of virNetServer to virNetDaemon Teach gendispatch how to handle admin dispatching files Add admin protocol Build client headers for admin protocol Add admin error domain Add libvirt-admin library Add XML files with admin API specification Add configuration options for permissions on daemon's admin socket Add support for admin API in libvirt daemon rpc: Add virNetServerGetNClients admin: Add virAdmHello function Example virt-admin
Much examples, very good. Wow. I went through the patches. My overall feeling is good. Unfortunately, I could not ACK all the patches as I've found some serious issues. But I think you're on the right track. Michal
participants (3)
-
Daniel P. Berrange
-
Martin Kletzander
-
Michal Privoznik