Add typedef for the anonymous enum used for the driver features. This allows the usage of the type in a switch statement and taking advantage of the compilers feature to detect uncovered cases. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/esx/esx_driver.c | 18 ++++++++++++++++-- src/libvirt_internal.h | 4 ++-- src/libxl/libxl_driver.c | 13 ++++++++++++- src/lxc/lxc_driver.c | 24 +++++++++++++++++++----- src/openvz/openvz_driver.c | 15 ++++++++++++++- src/qemu/qemu_driver.c | 8 +++++++- src/remote/remote_daemon_dispatch.c | 19 ++++++++++++++++--- src/vz/vz_driver.c | 15 ++++++++++++++- src/xen/xen_driver.c | 15 ++++++++++++++- 9 files changed, 114 insertions(+), 17 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index b065cdc51323..927267f1cc98 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1122,7 +1122,7 @@ esxConnectSupportsFeature(virConnectPtr conn, int feature) esxPrivate *priv = conn->privateData; esxVI_Boolean supportsVMotion = esxVI_Boolean_Undefined; - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_V1: supportsVMotion = esxSupportsVMotion(priv); @@ -1133,7 +1133,21 @@ esxConnectSupportsFeature(virConnectPtr conn, int feature) return priv->vCenter && supportsVMotion == esxVI_Boolean_True ? 1 : 0; - default: + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_XML_MIGRATABLE: + default: return 0; } } diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 62f490a7dfd5..0e008a65518d 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -45,7 +45,7 @@ int virStateStop(void); * feature. Queries for VIR_DRV_FEATURE_PROGRAM* features are answered * directly by the RPC layer and not by the real driver. */ -enum { +typedef enum { /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ * domainMigratePerform/domainMigrateFinish. */ @@ -123,7 +123,7 @@ enum { * Support for driver close callback rpc */ VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK = 15, -}; +} virDrvFeature; int virConnectSupportsFeature(virConnectPtr conn, int feature); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index c3616a86d7f4..562966cc2062 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5684,12 +5684,23 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature) if (virConnectSupportsFeatureEnsureACL(conn) < 0) return -1; - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_V3: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: case VIR_DRV_FEATURE_MIGRATION_PARAMS: case VIR_DRV_FEATURE_MIGRATION_P2P: return 1; + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_XML_MIGRATABLE: default: return 0; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index fa6fc4643ee2..4f6b93bdbd30 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1793,11 +1793,25 @@ lxcConnectSupportsFeature(virConnectPtr conn, int feature) if (virConnectSupportsFeatureEnsureACL(conn) < 0) return -1; - switch (feature) { - case VIR_DRV_FEATURE_TYPED_PARAM_STRING: - return 1; - default: - return 0; + switch ((virDrvFeature) feature) { + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + return 1; + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_XML_MIGRATABLE: + default: + return 0; } } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 9bd73d85c49f..312cac35a3c3 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2204,10 +2204,23 @@ openvzNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, static int openvzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_PARAMS: case VIR_DRV_FEATURE_MIGRATION_V3: return 1; + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_XML_MIGRATABLE: default: return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9e715e7a00d4..874cb46981a3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1206,7 +1206,7 @@ qemuConnectSupportsFeature(virConnectPtr conn, int feature) if (virConnectSupportsFeatureEnsureACL(conn) < 0) return -1; - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_V2: case VIR_DRV_FEATURE_MIGRATION_V3: case VIR_DRV_FEATURE_MIGRATION_P2P: @@ -1217,6 +1217,12 @@ qemuConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_MIGRATION_OFFLINE: case VIR_DRV_FEATURE_MIGRATION_PARAMS: return 1; + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: default: return 0; } diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index ea67cb7bc018..82f6400ca49d 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -4645,7 +4645,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server ATTRIBUTE remote_connect_supports_feature_ret *ret) { int rv = -1; - int supported; + int supported = -1; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); @@ -4664,17 +4664,30 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server ATTRIBUTE goto cleanup; } - switch (args->feature) { + switch ((virDrvFeature) args->feature) { case VIR_DRV_FEATURE_FD_PASSING: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: supported = 1; break; - + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_XML_MIGRATABLE: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_PARAMS: default: if ((supported = virConnectSupportsFeature(priv->conn, args->feature)) < 0) goto cleanup; break; + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + /* should not be possible! */ + goto cleanup; } done: diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 6d02ef274fec..a425bc85529d 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3079,10 +3079,23 @@ vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) if (virConnectSupportsFeatureEnsureACL(conn) < 0) return -1; - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_PARAMS: case VIR_DRV_FEATURE_MIGRATION_P2P: return 1; + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_XML_MIGRATABLE: default: return 0; } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index f521fd1f2c03..85c3424b11ed 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -619,10 +619,23 @@ xenUnifiedConnectSupportsFeature(virConnectPtr conn, int feature) if (virConnectSupportsFeatureEnsureACL(conn) < 0) return -1; - switch (feature) { + switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_MIGRATION_V1: case VIR_DRV_FEATURE_MIGRATION_DIRECT: return 1; + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_XML_MIGRATABLE: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: default: return 0; } -- 2.13.4

Don't assume that the feature VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK is available for every driver used for the connection. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/remote/remote_daemon_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 82f6400ca49d..bf6c00348a5e 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -4667,7 +4667,6 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server ATTRIBUTE switch ((virDrvFeature) args->feature) { case VIR_DRV_FEATURE_FD_PASSING: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: - case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: supported = 1; break; case VIR_DRV_FEATURE_MIGRATION_V1: @@ -4681,6 +4680,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server ATTRIBUTE case VIR_DRV_FEATURE_XML_MIGRATABLE: case VIR_DRV_FEATURE_MIGRATION_OFFLINE: case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: default: if ((supported = virConnectSupportsFeature(priv->conn, args->feature)) < 0) goto cleanup; -- 2.13.4

Report an error in case the driver does not support connect(Un)registerCloseCallback. The commit 'close callback: move it to driver' (88f09b75eb99) moved the responsibility for the close callback to the driver. But if the driver doesn't support the connectRegisterCloseCallback API this function does nothing, even no unsupported error report. The only case where an error is reported is when the API is supported but the call fails. The same behavior applies to virConnectUnregisterCloseCallback. This behavior is not intended as there are many use cases of this API where the state of for example allocations depends on the result of these functions. To keep the behavior of virsh as before it must silently ignore unsupported error for virConnectRegisterCloseCallback. For the remote driver this change wouldn't be needed, but for the byhve driver, for example. Otherwise the user would see the error message that virsh was unable to register a disconnect callback. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/libvirt-host.c | 24 ++++++++++++++---------- tools/virsh.c | 11 +++++++++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 7ff7407a0874..bfa104b39918 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1221,12 +1221,14 @@ virConnectRegisterCloseCallback(virConnectPtr conn, virCheckConnectReturn(conn, -1); virCheckNonNullArgGoto(cb, error); - if (conn->driver->connectRegisterCloseCallback && - conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb) < 0) - goto error; - - return 0; + if (conn->driver->connectRegisterCloseCallback) { + int ret = conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); error: virDispatchError(conn); return -1; @@ -1256,12 +1258,14 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, virCheckConnectReturn(conn, -1); virCheckNonNullArgGoto(cb, error); - if (conn->driver->connectUnregisterCloseCallback && - conn->driver->connectUnregisterCloseCallback(conn, cb) < 0) - goto error; - - return 0; + if (conn->driver->connectUnregisterCloseCallback) { + int ret = conn->driver->connectUnregisterCloseCallback(conn, cb); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); error: virDispatchError(conn); return -1; diff --git a/tools/virsh.c b/tools/virsh.c index 5f8352e861d3..2df1197252b3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -256,8 +256,15 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force) priv->readonly = readonly; if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect, - ctl, NULL) < 0) - vshError(ctl, "%s", _("Unable to register disconnect callback")); + ctl, NULL) < 0) { + virErrorPtr error = virGetLastError(); + if (error && error->code == VIR_ERR_NO_SUPPORT) { + /* silently ignore the unsupported error */ + virResetLastError(); + } else { + vshError(ctl, "%s", _("Unable to register disconnect callback")); + } + } if (connected && !force) vshError(ctl, "%s", _("Reconnected to the hypervisor")); } -- 2.13.4

Implement the hypervisor APIs virConnectRegisterCloseCallback and virConnectUnregisterCloseCallbacks and mark this feature as supported in testConnectSupportsFeature. This increases test test coverage of the test suite as then the testConnectRegisterCloseCallback and testConnectUnregisterCloseCallback (which are almost identical to the implementations of the remote and vz driver) will be called by the virsh tests. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/test/test_driver.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 203358c7017f..2773f5c758c8 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -121,6 +121,7 @@ struct _testDriver { virDomainObjListPtr domains; virNetworkObjListPtr networks; virObjectEventStatePtr eventState; + virConnectCloseCallbackDataPtr closeCallback; }; typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; @@ -157,6 +158,7 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->ifaces); virObjectUnref(driver->pools); virObjectUnref(driver->eventState); + virObjectUnref(driver->closeCallback); virMutexUnlock(&driver->lock); virMutexDestroy(&driver->lock); @@ -404,6 +406,7 @@ testDriverNew(void) .free = testDomainDefNamespaceFree, }; testDriverPtr ret; + virConnectCloseCallbackDataPtr closeCallback; if (VIR_ALLOC(ret) < 0) return NULL; @@ -423,6 +426,12 @@ testDriverNew(void) !(ret->pools = virStoragePoolObjListNew())) goto error; + closeCallback = virNewConnectCloseCallbackData(); + if (!closeCallback) + goto error; + + ret->closeCallback = closeCallback; + virAtomicIntSet(&ret->nextDomID, 1); return ret; @@ -6834,9 +6843,9 @@ testConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { switch ((virDrvFeature) feature) { + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: return 1; - case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_FD_PASSING: case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: case VIR_DRV_FEATURE_MIGRATION_DIRECT: @@ -6855,6 +6864,56 @@ testConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, } } +static int +testConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + testDriverPtr priv = conn->privateData; + int ret = -1; + + testDriverLock(priv); + + if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A close callback is already registered")); + goto cleanup; + } + + virConnectCloseCallbackDataRegister(priv->closeCallback, conn, cb, + opaque, freecb); + ret = 0; + + cleanup: + testDriverUnlock(priv); + return ret; +} + + +static int +testConnectUnregisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb) +{ + testDriverPtr priv = conn->privateData; + int ret = -1; + + testDriverLock(priv); + + if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != cb) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A different callback was requested")); + goto cleanup; + } + + virConnectCloseCallbackDataUnregister(priv->closeCallback, cb); + ret = 0; + + cleanup: + testDriverUnlock(priv); + return ret; +} + static virHypervisorDriver testHypervisorDriver = { .name = "Test", @@ -6863,7 +6922,9 @@ static virHypervisorDriver testHypervisorDriver = { .connectGetVersion = testConnectGetVersion, /* 0.1.1 */ .connectGetHostname = testConnectGetHostname, /* 0.6.3 */ .connectGetMaxVcpus = testConnectGetMaxVcpus, /* 0.3.2 */ + .connectRegisterCloseCallback = testConnectRegisterCloseCallback, /* 4.2.0 */ .connectSupportsFeature = testConnectSupportsFeature, /* 4.2.0 */ + .connectUnregisterCloseCallback = testConnectUnregisterCloseCallback, /* 4.2.0 */ .nodeGetInfo = testNodeGetInfo, /* 0.1.1 */ .nodeGetCPUStats = testNodeGetCPUStats, /* 2.3.0 */ .nodeGetFreeMemory = testNodeGetFreeMemory, /* 2.3.0 */ -- 2.13.4

Refactor testConnectClose as it's then obvious that conn->privateData is set to NULL in all cases. In addition, 'testConnectCloseInternal' can be better reused. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/test/test_driver.c | 61 +++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d450be21704e..fca607f57d51 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1429,10 +1429,36 @@ testConnectAuthenticate(virConnectPtr conn, return ret; } -static virDrvOpenStatus testConnectOpen(virConnectPtr conn, - virConnectAuthPtr auth, - virConfPtr conf ATTRIBUTE_UNUSED, - unsigned int flags) + +static void +testDriverCloseInternal(testDriverPtr driver) +{ + bool dflt = false; + + if (driver == defaultPrivconn) { + dflt = true; + virMutexLock(&defaultLock); + if (--defaultConnections) { + virMutexUnlock(&defaultLock); + return; + } + } + + testDriverLock(driver); + testDriverFree(driver); + + if (dflt) { + defaultPrivconn = NULL; + virMutexUnlock(&defaultLock); + } +} + + +static virDrvOpenStatus +testConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) { int ret; @@ -1473,33 +1499,16 @@ static virDrvOpenStatus testConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int testConnectClose(virConnectPtr conn) + +static int +testConnectClose(virConnectPtr conn) { - testDriverPtr privconn = conn->privateData; - bool dflt = false; - - if (privconn == defaultPrivconn) { - dflt = true; - virMutexLock(&defaultLock); - if (--defaultConnections) { - conn->privateData = NULL; - virMutexUnlock(&defaultLock); - return 0; - } - } - - testDriverLock(privconn); - testDriverFree(privconn); - - if (dflt) { - defaultPrivconn = NULL; - virMutexUnlock(&defaultLock); - } - + testDriverCloseInternal(conn->privateData); conn->privateData = NULL; return 0; } + static int testConnectGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) { -- 2.13.4

In case of an error do the cleanup of the private data of the connection. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/test/test_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fca607f57d51..3b55453efe00 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1493,8 +1493,11 @@ testConnectOpen(virConnectPtr conn, return ret; /* Fake authentication. */ - if (testConnectAuthenticate(conn, auth) < 0) + if (testConnectAuthenticate(conn, auth) < 0) { + testDriverCloseInternal(conn->privateData); + conn->privateData = NULL; return VIR_DRV_OPEN_ERROR; + } return VIR_DRV_OPEN_SUCCESS; } -- 2.13.4

The test driver state (@testDriver) uses it's own reference counting and locking implementation. Instead of doing that, convert @testDriver into a virObjectLockable and use the provided functionalities. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/test/test_driver.c | 207 ++++++++++++++++++++++--------------------------- 1 file changed, 94 insertions(+), 113 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 3b55453efe00..f1dd11867143 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -94,7 +94,7 @@ typedef struct _testAuth testAuth; typedef struct _testAuth *testAuthPtr; struct _testDriver { - virMutex lock; + virObjectLockable parent; virNodeInfo nodeInfo; virInterfaceObjListPtr ifaces; @@ -127,9 +127,22 @@ typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; static testDriverPtr defaultPrivconn; -static int defaultConnections; static virMutex defaultLock = VIR_MUTEX_INITIALIZER; +static virClassPtr testDriverClass; +static void testDriverDispose(void *obj); +static int testDriverOnceInit(void) +{ + if (!(testDriverClass = virClassNew(virClassForObjectLockable(), + "testDriver", + sizeof(testDriver), + testDriverDispose))) + return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(testDriver) + #define TEST_MODEL "i686" #define TEST_EMULATOR "/usr/bin/test-hv" @@ -145,10 +158,9 @@ static const virNodeInfo defaultNodeInfo = { }; static void -testDriverFree(testDriverPtr driver) +testDriverDispose(void *obj) { - if (!driver) - return; + testDriverPtr driver = obj; virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); @@ -159,23 +171,9 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->pools); virObjectUnref(driver->eventState); virObjectUnref(driver->closeCallback); - virMutexUnlock(&driver->lock); - virMutexDestroy(&driver->lock); - - VIR_FREE(driver); } -static void testDriverLock(testDriverPtr driver) -{ - virMutexLock(&driver->lock); -} - -static void testDriverUnlock(testDriverPtr driver) -{ - virMutexUnlock(&driver->lock); -} - static void testObjectEventQueue(testDriverPtr driver, virObjectEventPtr event) { @@ -408,14 +406,11 @@ testDriverNew(void) testDriverPtr ret; virConnectCloseCallbackDataPtr closeCallback; - if (VIR_ALLOC(ret) < 0) + if (testDriverInitialize() < 0) return NULL; - if (virMutexInit(&ret->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - goto error; - } + if (!(ret = virObjectLockableNew(testDriverClass))) + return NULL; if (!(ret->xmlopt = virDomainXMLOptionNew(NULL, NULL, &ns, NULL, NULL)) || !(ret->eventState = virObjectEventStateNew()) || @@ -437,7 +432,7 @@ testDriverNew(void) return ret; error: - testDriverFree(ret); + virObjectUnref(ret); return NULL; } @@ -1271,7 +1266,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) if (!(privconn = testDriverNew())) return VIR_DRV_OPEN_ERROR; - testDriverLock(privconn); + virObjectLock(privconn); conn->privateData = privconn; if (!(privconn->caps = testBuildCapabilities(conn))) @@ -1288,14 +1283,14 @@ testOpenFromFile(virConnectPtr conn, const char *file) xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return VIR_DRV_OPEN_SUCCESS; error: xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); - testDriverFree(privconn); + virObjectUnref(privconn); conn->privateData = NULL; return VIR_DRV_OPEN_ERROR; } @@ -1313,8 +1308,8 @@ testOpenDefault(virConnectPtr conn) size_t i; virMutexLock(&defaultLock); - if (defaultConnections++) { - conn->privateData = defaultPrivconn; + if (defaultPrivconn) { + conn->privateData = virObjectRef(defaultPrivconn); virMutexUnlock(&defaultLock); return VIR_DRV_OPEN_SUCCESS; } @@ -1363,9 +1358,8 @@ testOpenDefault(virConnectPtr conn) return ret; error: - testDriverFree(privconn); + virObjectUnref(privconn); conn->privateData = NULL; - defaultConnections--; ret = VIR_DRV_OPEN_ERROR; goto cleanup; } @@ -1379,9 +1373,9 @@ testConnectAuthenticate(virConnectPtr conn, ssize_t i; char *username = NULL, *password = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if (privconn->numAuths == 0) { - testDriverUnlock(privconn); + virObjectUnlock(privconn); return 0; } @@ -1423,7 +1417,7 @@ testConnectAuthenticate(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); VIR_FREE(username); VIR_FREE(password); return ret; @@ -1433,24 +1427,11 @@ testConnectAuthenticate(virConnectPtr conn, static void testDriverCloseInternal(testDriverPtr driver) { - bool dflt = false; - - if (driver == defaultPrivconn) { - dflt = true; - virMutexLock(&defaultLock); - if (--defaultConnections) { - virMutexUnlock(&defaultLock); - return; - } - } - - testDriverLock(driver); - testDriverFree(driver); - - if (dflt) { + virMutexLock(&defaultLock); + bool disposed = !virObjectUnref(driver); + if (disposed && driver == defaultPrivconn) defaultPrivconn = NULL; - virMutexUnlock(&defaultLock); - } + virMutexUnlock(&defaultLock); } @@ -1581,9 +1562,9 @@ static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { testDriverPtr privconn = conn->privateData; - testDriverLock(privconn); + virObjectLock(privconn); memcpy(info, &privconn->nodeInfo, sizeof(virNodeInfo)); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return 0; } @@ -1591,9 +1572,9 @@ static char *testConnectGetCapabilities(virConnectPtr conn) { testDriverPtr privconn = conn->privateData; char *xml; - testDriverLock(privconn); + virObjectLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return xml; } @@ -1628,9 +1609,9 @@ static int testConnectNumOfDomains(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int count; - testDriverLock(privconn); + virObjectLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return count; } @@ -1683,7 +1664,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, if (flags & VIR_DOMAIN_START_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; - testDriverLock(privconn); + virObjectLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, NULL, parse_flags)) == NULL) goto cleanup; @@ -1718,7 +1699,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, virObjectUnlock(dom); testObjectEventQueue(privconn, event); virDomainDefFree(def); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -2887,7 +2868,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn, size_t i; int ret = -1; - testDriverLock(privconn); + virObjectLock(privconn); if (startCell >= privconn->numCells) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Range exceeds available cells")); @@ -2902,7 +2883,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn, ret = i; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -2960,12 +2941,12 @@ testNodeGetFreeMemory(virConnectPtr conn) unsigned int freeMem = 0; size_t i; - testDriverLock(privconn); + virObjectLock(privconn); for (i = 0; i < privconn->numCells; i++) freeMem += privconn->cells[i].freeMem; - testDriverUnlock(privconn); + virObjectUnlock(privconn); return freeMem; } @@ -3002,7 +2983,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; @@ -3026,7 +3007,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) cleanup: virDomainObjEndAPI(&privdom); testObjectEventQueue(privconn, event); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -3797,9 +3778,9 @@ testInterfaceObjFindByName(testDriverPtr privconn, { virInterfaceObjPtr obj; - testDriverLock(privconn); + virObjectLock(privconn); obj = virInterfaceObjListFindByName(privconn->ifaces, name); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) virReportError(VIR_ERR_NO_INTERFACE, @@ -3816,9 +3797,9 @@ testConnectNumOfInterfaces(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int ninterfaces; - testDriverLock(privconn); + virObjectLock(privconn); ninterfaces = virInterfaceObjListNumOfInterfaces(privconn->ifaces, true); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ninterfaces; } @@ -3831,10 +3812,10 @@ testConnectListInterfaces(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int nnames; - testDriverLock(privconn); + virObjectLock(privconn); nnames = virInterfaceObjListGetNames(privconn->ifaces, true, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return nnames; } @@ -3846,9 +3827,9 @@ testConnectNumOfDefinedInterfaces(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int ninterfaces; - testDriverLock(privconn); + virObjectLock(privconn); ninterfaces = virInterfaceObjListNumOfInterfaces(privconn->ifaces, false); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ninterfaces; } @@ -3861,10 +3842,10 @@ testConnectListDefinedInterfaces(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int nnames; - testDriverLock(privconn); + virObjectLock(privconn); nnames = virInterfaceObjListGetNames(privconn->ifaces, false, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return nnames; } @@ -3899,10 +3880,10 @@ testInterfaceLookupByMACString(virConnectPtr conn, char *ifacenames[] = { NULL, NULL }; virInterfacePtr ret = NULL; - testDriverLock(privconn); + virObjectLock(privconn); ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, ifacenames, 2); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (ifacect == 0) { virReportError(VIR_ERR_NO_INTERFACE, @@ -3950,7 +3931,7 @@ testInterfaceChangeBegin(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("there is another transaction running.")); @@ -3964,7 +3945,7 @@ testInterfaceChangeBegin(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -3978,7 +3959,7 @@ testInterfaceChangeCommit(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3993,7 +3974,7 @@ testInterfaceChangeCommit(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4008,7 +3989,7 @@ testInterfaceChangeRollback(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -4026,7 +4007,7 @@ testInterfaceChangeRollback(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4066,7 +4047,7 @@ testInterfaceDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if ((def = virInterfaceDefParseString(xmlStr)) == NULL) goto cleanup; @@ -4080,7 +4061,7 @@ testInterfaceDefineXML(virConnectPtr conn, cleanup: virInterfaceDefFree(def); virInterfaceObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4184,9 +4165,9 @@ testStoragePoolObjFindByName(testDriverPtr privconn, { virStoragePoolObjPtr obj; - testDriverLock(privconn); + virObjectLock(privconn); obj = virStoragePoolObjFindByName(privconn->pools, name); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -4244,9 +4225,9 @@ testStoragePoolObjFindByUUID(testDriverPtr privconn, virStoragePoolObjPtr obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; - testDriverLock(privconn); + virObjectLock(privconn); obj = virStoragePoolObjFindByUUID(privconn->pools, uuid); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) { virUUIDFormat(uuid, uuidstr); @@ -4312,10 +4293,10 @@ testConnectNumOfStoragePools(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int numActive = 0; - testDriverLock(privconn); + virObjectLock(privconn); numActive = virStoragePoolObjNumOfStoragePools(privconn->pools, conn, true, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return numActive; } @@ -4329,10 +4310,10 @@ testConnectListStoragePools(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int n = 0; - testDriverLock(privconn); + virObjectLock(privconn); n = virStoragePoolObjGetNames(privconn->pools, conn, true, NULL, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return n; } @@ -4344,10 +4325,10 @@ testConnectNumOfDefinedStoragePools(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int numInactive = 0; - testDriverLock(privconn); + virObjectLock(privconn); numInactive = virStoragePoolObjNumOfStoragePools(privconn->pools, conn, false, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return numInactive; } @@ -4361,10 +4342,10 @@ testConnectListDefinedStoragePools(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int n = 0; - testDriverLock(privconn); + virObjectLock(privconn); n = virStoragePoolObjGetNames(privconn->pools, conn, false, NULL, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return n; } @@ -4380,10 +4361,10 @@ testConnectListAllStoragePools(virConnectPtr conn, virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1); - testDriverLock(privconn); + virObjectLock(privconn); ret = virStoragePoolObjListExport(conn, privconn->pools, pools, NULL, flags); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4545,7 +4526,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; @@ -4596,7 +4577,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virStoragePoolDefFree(newDef); testObjectEventQueue(privconn, event); virStoragePoolObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return pool; } @@ -4615,7 +4596,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; @@ -4648,7 +4629,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virStoragePoolDefFree(newDef); testObjectEventQueue(privconn, event); virStoragePoolObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return pool; } @@ -5050,7 +5031,7 @@ testStorageVolLookupByKey(virConnectPtr conn, .conn = conn, .key = key, .voldef = NULL }; virStorageVolPtr vol = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if ((obj = virStoragePoolObjListSearch(privconn->pools, testStorageVolLookupByKeyCallback, &data)) && data.voldef) { @@ -5060,7 +5041,7 @@ testStorageVolLookupByKey(virConnectPtr conn, NULL, NULL); virStoragePoolObjEndAPI(&obj); } - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!vol) virReportError(VIR_ERR_NO_STORAGE_VOL, @@ -5094,7 +5075,7 @@ testStorageVolLookupByPath(virConnectPtr conn, .conn = conn, .path = path, .voldef = NULL }; virStorageVolPtr vol = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if ((obj = virStoragePoolObjListSearch(privconn->pools, testStorageVolLookupByPathCallback, &data)) && data.voldef) { @@ -5104,7 +5085,7 @@ testStorageVolLookupByPath(virConnectPtr conn, NULL, NULL); virStoragePoolObjEndAPI(&obj); } - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!vol) virReportError(VIR_ERR_NO_STORAGE_VOL, @@ -6889,7 +6870,7 @@ testConnectRegisterCloseCallback(virConnectPtr conn, testDriverPtr priv = conn->privateData; int ret = -1; - testDriverLock(priv); + virObjectLock(priv); if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -6902,7 +6883,7 @@ testConnectRegisterCloseCallback(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(priv); + virObjectUnlock(priv); return ret; } @@ -6914,7 +6895,7 @@ testConnectUnregisterCloseCallback(virConnectPtr conn, testDriverPtr priv = conn->privateData; int ret = -1; - testDriverLock(priv); + virObjectLock(priv); if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != cb) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -6926,7 +6907,7 @@ testConnectUnregisterCloseCallback(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(priv); + virObjectUnlock(priv); return ret; } -- 2.13.4

Set the eventID for remoteRelayDomainQemuMonitorEvent explicitly to an invalid value. Although the value is not used by remoteRelayDomainQemuMonitorEvent, but it might be less prone to errors for further refactorings. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/remote/remote_daemon_dispatch.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index bf6c00348a5e..ff26574c9f6b 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -6215,6 +6215,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); + callback->eventID = -1; callback->callbackID = -1; ref = callback; if (VIR_APPEND_ELEMENT(priv->qemuEventCallbacks, -- 2.13.4

This patch introduces virNetServerGetProgramLocked. It's a function to determine which program has to be used for a given @msg. This function will be reused in the next patch. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/rpc/virnetserver.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 204f425264fa..4cd42ad7fd40 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -182,6 +182,29 @@ static void virNetServerHandleJob(void *jobOpaque, void *opaque) VIR_FREE(job); } + +/** + * virNetServerGetProgramLocked: + * @srv: server (must be locked by the caller) + * @msg: message + * + * Searches @srv for the right program for a given message @msg. + * + * Returns a pointer to the server program or NULL if not found. + */ +static virNetServerProgramPtr +virNetServerGetProgramLocked(virNetServerPtr srv, + virNetMessagePtr msg) +{ + size_t i; + for (i = 0; i < srv->nprograms; i++) { + if (virNetServerProgramMatches(srv->programs[i], msg)) + return srv->programs[i]; + } + return NULL; +} + + static int virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetMessagePtr msg, void *opaque) @@ -189,19 +212,13 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetServerPtr srv = opaque; virNetServerProgramPtr prog = NULL; unsigned int priority = 0; - size_t i; int ret = -1; VIR_DEBUG("server=%p client=%p message=%p", srv, client, msg); virObjectLock(srv); - for (i = 0; i < srv->nprograms; i++) { - if (virNetServerProgramMatches(srv->programs[i], msg)) { - prog = srv->programs[i]; - break; - } - } + prog = virNetServerGetProgramLocked(srv, msg); if (srv->workers) { virNetServerJobPtr job; -- 2.13.4

Use virNetServerGetProgram() to determine the virNetServerProgram instead of using hard coded global variables. This allows us to remove the global variables @remoteProgram and @qemuProgram as they're now no longer necessary. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- Note: I'm not 100% sure that there is no case where the lock for @client is already held by the thread which is calling virNetServerGetProgram and thus the lock order would be violated (the lock order has to be @server -> @client in the violating case it would be @client -> @server and therefore a deadlock might occur). --- src/libvirt_remote.syms | 1 + src/remote/remote_daemon.c | 4 +-- src/remote/remote_daemon.h | 3 --- src/remote/remote_daemon_dispatch.c | 52 ++++++++++++++++++------------------- src/rpc/gendispatch.pl | 2 ++ src/rpc/virnetserver.c | 23 ++++++++++++++++ src/rpc/virnetserver.h | 2 ++ 7 files changed, 56 insertions(+), 31 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 97e22275b980..c31b16cd5909 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -120,6 +120,7 @@ virNetServerGetCurrentUnauthClients; virNetServerGetMaxClients; virNetServerGetMaxUnauthClients; virNetServerGetName; +virNetServerGetProgram; virNetServerGetThreadPoolParameters; virNetServerHasClients; virNetServerNew; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 078430f91c97..64f67ef2fefb 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -71,8 +71,6 @@ VIR_LOG_INIT("daemon.libvirtd"); #if WITH_SASL virNetSASLContextPtr saslCtxt = NULL; #endif -virNetServerProgramPtr remoteProgram = NULL; -virNetServerProgramPtr qemuProgram = NULL; volatile bool driversInitialized = false; @@ -1061,6 +1059,8 @@ int main(int argc, char **argv) { virNetServerPtr srv = NULL; virNetServerPtr srvAdm = NULL; virNetServerProgramPtr adminProgram = NULL; + virNetServerProgramPtr qemuProgram = NULL; + virNetServerProgramPtr remoteProgram = NULL; virNetServerProgramPtr lxcProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h index 4467f71da910..93f2e1f531b0 100644 --- a/src/remote/remote_daemon.h +++ b/src/remote/remote_daemon.h @@ -82,7 +82,4 @@ struct daemonClientPrivate { # if WITH_SASL extern virNetSASLContextPtr saslCtxt; # endif -extern virNetServerProgramPtr remoteProgram; -extern virNetServerProgramPtr qemuProgram; - #endif /* __REMOTE_DAEMON_H__ */ diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 95940ffdeefe..3dce6195230e 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3808,9 +3808,9 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, } static int -remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr) { int rv = -1; @@ -3828,7 +3828,7 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUS if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); /* eventID, callbackID, and legacy are not used */ callback->eventID = -1; callback->callbackID = -1; @@ -3912,7 +3912,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE; callback->callbackID = -1; callback->legacy = true; @@ -4110,9 +4110,9 @@ remoteDispatchDomainGetState(virNetServerPtr server ATTRIBUTE_UNUSED, * VIR_DRV_SUPPORTS_FEATURE(VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK), * and must not mix the two styles. */ static int -remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_domain_event_register_any_args *args) { @@ -4149,7 +4149,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; callback->legacy = true; @@ -4185,9 +4185,9 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU static int -remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_domain_event_callback_register_any_args *args, remote_connect_domain_event_callback_register_any_ret *ret) @@ -4226,7 +4226,7 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5352,7 +5352,7 @@ remoteDispatchDomainMigratePrepareTunnel3Params(virNetServerPtr server ATTRIBUTE goto cleanup; if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)) || - !(stream = daemonCreateClientStream(client, st, remoteProgram, + !(stream = daemonCreateClientStream(client, st, virNetServerGetProgram(server, msg), &msg->header, false))) goto cleanup; @@ -5709,9 +5709,9 @@ static int remoteDispatchDomainCreateWithFiles(virNetServerPtr server ATTRIBUTE_ static int -remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_network_event_register_any_args *args, remote_connect_network_event_register_any_ret *ret) @@ -5750,7 +5750,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5832,9 +5832,9 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ } static int -remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_storage_pool_event_register_any_args *args, remote_connect_storage_pool_event_register_any_ret *ret) @@ -5873,7 +5873,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5954,9 +5954,9 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB } static int -remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_node_device_event_register_any_args *args, remote_connect_node_device_event_register_any_ret *ret) @@ -5995,7 +5995,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -6076,9 +6076,9 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr server ATTRIBU } static int -remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_secret_event_register_any_args *args, remote_connect_secret_event_register_any_ret *ret) @@ -6117,7 +6117,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -6198,9 +6198,9 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U } static int -qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED, +qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, qemu_connect_domain_monitor_event_register_args *args, qemu_connect_domain_monitor_event_register_ret *ret) @@ -6234,7 +6234,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(qemuProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = -1; callback->callbackID = -1; ref = callback; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index fb15cc4849bc..725319bf8da4 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -990,6 +990,7 @@ elsif ($mode eq "server") { if ($call->{streamflag} ne "none") { print " virStreamPtr st = NULL;\n"; print " daemonClientStreamPtr stream = NULL;\n"; + print " virNetServerProgramPtr remoteProgram = NULL;\n"; if ($call->{sparseflag} ne "none") { print " const bool sparse = args->flags & $call->{sparseflag};\n" } else { @@ -1037,6 +1038,7 @@ elsif ($mode eq "server") { print " if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)))\n"; print " goto cleanup;\n"; print "\n"; + print " remoteProgram = virNetServerGetProgram(server, msg);\n"; print " if (!(stream = daemonCreateClientStream(client, st, remoteProgram, &msg->header, sparse)))\n"; print " goto cleanup;\n"; print "\n"; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 4cd42ad7fd40..b1ff4a0cd751 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -205,6 +205,29 @@ virNetServerGetProgramLocked(virNetServerPtr srv, } +/** + * virNetServerGetProgram: + * @srv: server (must NOT be locked by the caller) + * @msg: message + * + * Searches @srv for the right program for a given message @msg. + * + * Returns a pointer to the server program or NULL if not found. + */ +virNetServerProgramPtr +virNetServerGetProgram(virNetServerPtr srv, + virNetMessagePtr msg) +{ + virNetServerProgramPtr ret = NULL; + + virObjectLock(srv); + ret = virNetServerGetProgramLocked(srv, msg); + virObjectUnlock(srv); + + return ret; +} + + static int virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetMessagePtr msg, void *opaque) diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index a79c39fdb2e7..1867e46664ba 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -76,6 +76,8 @@ int virNetServerSetTLSContext(virNetServerPtr srv, virNetTLSContextPtr tls); # endif +virNetServerProgramPtr virNetServerGetProgram(virNetServerPtr srv, + virNetMessagePtr msg); int virNetServerAddClient(virNetServerPtr srv, virNetServerClientPtr client); -- 2.13.4