Re: [libvirt] [PATCH 3/8] parallels: implement functions for domain life cycle management

On 07/27/2012 01:32 PM, Daniel P. Berrange wrote: >On Thu, Jul 26, 2012 at 10:32:02PM +0400, Dmitry Guryanov wrote: >>Add functions for create/shutdown/destroy and suspend/resume domain. >> >>Signed-off-by: Dmitry Guryanov <dguryanov(a)parallels.com&gt; >>--- >> src/parallels/parallels_driver.c | 143 ++++++++++++++++++++++++++++++++++++++ >> src/parallels/parallels_utils.c | 18 +++++ >> src/parallels/parallels_utils.h | 2 + >> 3 files changed, 163 insertions(+), 0 deletions(-) >> >>diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c >>index c716b25..8c20d27 100644 >>--- a/src/parallels/parallels_driver.c >>+++ b/src/parallels/parallels_driver.c >>@@ -128,6 +128,12 @@ parallelsDomObjFreePrivate(void *p) >> VIR_FREE(p); >> }; >>+static void >>+parallelsDomainEventQueue(parallelsConnPtr driver, virDomainEventPtr event) >>+{ >>+ virDomainEventStateQueue(driver->domainEventState, event); >>+} >I don't see any code which ever deals with dispatching of the domain >event state. Is this something you're just preparing now, so that you >can wire up events in a future patch ? > >>+ >> static virCapsPtr >> parallelsBuildCapabilities(void) >> { >>@@ -805,6 +811,138 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) >> return ret; >> } >>+typedef int (*parallelsChangeStateFunc) (virDomainObjPtr privdom); >>+#define PARALLELS_UUID(x) (((parallelsDomObjPtr)(x->privateData))->uuid) >>+ >>+static int >>+parallelsDomainChangeState(virDomainPtr domain, >>+ virDomainState req_state, const char *req_state_name, >>+ parallelsChangeStateFunc chstate, >>+ virDomainState new_state, int reason, >>+ int event_type, int event_detail) >>+{ >>+ parallelsConnPtr privconn = domain->conn->privateData; >>+ virDomainObjPtr privdom; >>+ virDomainEventPtr event = NULL; >>+ int state; >>+ int ret = -1; >>+ >>+ parallelsDriverLock(privconn); >>+ privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); >>+ parallelsDriverUnlock(privconn); >>+ >>+ if (privdom == NULL) { >>+ parallelsDomNotFoundError(domain); >>+ goto cleanup; >>+ } >>+ >>+ state = virDomainObjGetState(privdom, NULL); >>+ if (state != req_state) { >>+ virReportError(VIR_ERR_INTERNAL_ERROR, _("domain '%s' not %s"), >>+ privdom->def->name, req_state_name); >>+ goto cleanup; >>+ } >>+ >>+ if (chstate(privdom)) >>+ goto cleanup; >>+ >>+ virDomainObjSetState(privdom, new_state, reason); >>+ >>+ event = virDomainEventNewFromObj(privdom, event_type, event_detail); >>+ ret = 0; >>+ >>+ cleanup: >>+ if (privdom) >>+ virDomainObjUnlock(privdom); >>+ >>+ if (event) { >>+ parallelsDriverLock(privconn); >>+ parallelsDomainEventQueue(privconn, event); >>+ parallelsDriverUnlock(privconn); >>+ } >>+ return ret; >>+} >One comment I'd have here is that the event dispatch only works >within the context of this single virConnectPtr instance. If you >have 2 active connections to libvirt and one starts a guest, >the other connection won't see any event. Likewise if someone >starts/stops something using the parallels tools directly no >events will get queued. > >Is there any way we can obtain some kind of notification from >the core parallels software stack ? Even if it is indirect eg >with UserModeLinux we detect stop/start events only, using an >inotify watch on the directory containing the UML monitor >socket :-) I thought that events, issued with virDomainEventStateQueue affect all connections ... I think it's better to remove all code, which deals with evens then.