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@xxxxxxxxxx> > --- > 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;
--
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list