Re: [libvirt] [PATCH 18/40] Simplify the Xen domain get info/state driver methods

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make the xenUnifiedDomainGetInfo and xenUnifiedDomainGetState drivers call the correct sub-driver APIs directly. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 52 +++++++-------------- src/xen/xen_driver.h | 1 - src/xen/xen_hypervisor.c | 14 +----- src/xen/xen_hypervisor.h | 3 +- src/xen/xend_internal.c | 15 +------ src/xen/xend_internal.h | 3 +- src/xen/xm_internal.c | 15 ++----- src/xen/xm_internal.h | 3 +- src/xen/xs_internal.c | 115 ----------------------------------------------- src/xen/xs_internal.h | 9 ---- 10 files changed, 23 insertions(+), 207 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7d09c6b..988dbce 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -84,7 +84,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { [XEN_UNIFIED_HYPERVISOR_OFFSET] = &xenHypervisorDriver, [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver, - [XEN_UNIFIED_XS_OFFSET] = &xenStoreDriver, [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver, };

@@ -842,15 +841,15 @@ static int xenUnifiedDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && - drivers[i]->xenDomainGetInfo && - drivers[i]->xenDomainGetInfo(dom, info) == 0) - return 0; - return -1; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainGetInfo(dom, info); + else + return xenDaemonDomainGetInfo(dom, info); + } else { + return xenHypervisorGetDomainInfo(dom, info); + } } static int @@ -860,38 +859,17 @@ xenUnifiedDomainGetState(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int ret; virCheckFlags(0, -1); - /* trying drivers in the same order as GetInfo for consistent results: - * hypervisor, xend, xs, and xm */ - - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { - ret = xenHypervisorGetDomainState(dom, state, reason, flags); - if (ret >= 0) - return ret; - } - - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonDomainGetState(dom, state, reason, flags); - if (ret >= 0) - return ret; - } - - if (priv->opened[XEN_UNIFIED_XS_OFFSET]) { - ret = xenStoreDomainGetState(dom, state, reason, flags); - if (ret >= 0) - return ret; - } - - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { - ret = xenXMDomainGetState(dom, state, reason, flags); - if (ret >= 0) - return ret; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainGetState(dom, state, reason); + else + return xenDaemonDomainGetState(dom, state, reason); + } else { + return xenHypervisorGetDomainState(dom, state, reason); } - - return -1; } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 4509161..953da64 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -93,7 +93,6 @@ extern int xenRegister (void); * structure with direct calls in xen_unified.c. */ struct xenUnifiedDriver { - virDrvDomainGetInfo xenDomainGetInfo; virDrvDomainPinVcpu xenDomainPinVcpu; virDrvDomainGetVcpus xenDomainGetVcpus; virDrvConnectListDefinedDomains xenListDefinedDomains; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 7662843..5735b0b 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -871,7 +871,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; #endif struct xenUnifiedDriver xenHypervisorDriver = { - .xenDomainGetInfo = xenHypervisorGetDomainInfo, .xenDomainPinVcpu = xenHypervisorPinVcpu, .xenDomainGetVcpus = xenHypervisorGetVcpus, .xenDomainGetSchedulerType = xenHypervisorGetSchedulerType, @@ -2880,11 +2879,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) int xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) { - if (domain->id < 0) - return -1; - return xenHypervisorGetDomInfo(domain->conn, domain->id, info); - } /** @@ -2892,7 +2887,6 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) * @domain: pointer to the domain block * @state: returned state of the domain * @reason: returned reason for the state - * @flags: additional flags, 0 for now * * Do a hypervisor call to get the related set of domain information. * @@ -2901,16 +2895,10 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) int xenHypervisorGetDomainState(virDomainPtr domain, int *state, - int *reason, - unsigned int flags) + int *reason) { virDomainInfo info; - virCheckFlags(0, -1); - - if (domain->id < 0) - return -1; - if (xenHypervisorGetDomInfo(domain->conn, domain->id, &info) < 0) return -1; diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 9748cf8..b36fcca 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -76,8 +76,7 @@ int xenHypervisorGetDomainInfo (virDomainPtr domain, ATTRIBUTE_NONNULL (1); int xenHypervisorGetDomainState (virDomainPtr domain, int *state, - int *reason, - unsigned int flags) + int *reason) ATTRIBUTE_NONNULL (1); int xenHypervisorGetDomInfo (virConnectPtr conn, int id, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index ce7d3f6..d6a3698 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1648,10 +1648,6 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -1; root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); if (root == NULL) @@ -1668,7 +1664,6 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) * @domain: a domain object * @state: returned domain's state * @reason: returned reason for the state - * @flags: additional flags, 0 for now * * This method looks up domain state and reason. * @@ -1677,17 +1672,10 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) int xenDaemonDomainGetState(virDomainPtr domain, int *state, - int *reason, - unsigned int flags) + int *reason) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; struct sexpr *root; - virCheckFlags(0, -1); - - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -1; - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); if (!root) return -1; @@ -3425,7 +3413,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainGetInfo = xenDaemonDomainGetInfo, .xenDomainPinVcpu = xenDaemonDomainPinVcpu, .xenDomainGetVcpus = xenDaemonDomainGetVcpus, .xenListDefinedDomains = xenDaemonListDefinedDomains, diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 9681068..fd661c9 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -101,8 +101,7 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory); int xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info); int xenDaemonDomainGetState(virDomainPtr domain, int *state, - int *reason, - unsigned int flags); + int *reason); char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus); unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index eddaca0..38f401a 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -81,7 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, #define XM_XML_ERROR "Invalid xml" struct xenUnifiedDriver xenXMDriver = { - .xenDomainGetInfo = xenXMDomainGetInfo, .xenDomainPinVcpu = xenXMDomainPinVcpu, .xenListDefinedDomains = xenXMListDefinedDomains, .xenNumOfDefinedDomains = xenXMNumOfDefinedDomains, @@ -462,15 +461,10 @@ xenXMClose(virConnectPtr conn) * Since these are all offline domains, the state is always SHUTOFF. */ int -xenXMDomainGetState(virDomainPtr domain, - int *state, int *reason, - unsigned int flags) +xenXMDomainGetState(virDomainPtr domain ATTRIBUTE_UNUSED, + int *state, + int *reason) { - virCheckFlags(0, -1); - - if (domain->id != -1) - return -1; - *state = VIR_DOMAIN_SHUTOFF; if (reason) *reason = 0; @@ -490,9 +484,6 @@ xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) const char *filename; xenXMConfCachePtr entry; - if (domain->id != -1) - return -1; - xenUnifiedLock(priv); if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 6424c41..257b663 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -43,8 +43,7 @@ const char *xenXMGetType(virConnectPtr conn); int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info); int xenXMDomainGetState(virDomainPtr domain, int *state, - int *reason, - unsigned int flags); + int *reason); char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags); int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory); int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index dd1f2a0..496c30b 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -56,32 +56,11 @@ static void xenStoreWatchEvent(int watch, int fd, int events, void *data); static void xenStoreWatchListFree(xenStoreWatchListPtr list); -struct xenUnifiedDriver xenStoreDriver = { - .xenDomainGetInfo = xenStoreGetDomainInfo, -}; - /************************************************************************ * * * Helper internal APIs * * * ************************************************************************/ -/** - * virConnectDoStoreList: - * @conn: pointer to the hypervisor connection - * @path: the absolute path of the directory in the store to list - * @nb: OUT pointer to the number of items found - * - * Internal API querying the Xenstore for a list - * - * Returns a string which must be freed by the caller or NULL in case of error - */ -static char ** -virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb) -{ - xenUnifiedPrivatePtr priv = conn->privateData; - - return xs_directory(priv->xshandle, 0, path, nb); -} /** * virDomainDoStoreQuery: @@ -235,100 +214,6 @@ xenStoreClose(virConnectPtr conn) } /** - * xenStoreGetDomainInfo: - * @domain: pointer to the domain block - * @info: the place where information should be stored - * - * Do a hypervisor call to get the related set of domain information. - * - * Returns 0 in case of success, -1 in case of error. - */ -int -xenStoreGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) -{ - char *tmp, **tmp2; - unsigned int nb_vcpus; - char request[200]; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (priv->xshandle == NULL || domain->id == -1) - return -1; - - tmp = virDomainDoStoreQuery(domain->conn, domain->id, "running"); - if (tmp != NULL) { - if (tmp[0] == '1') - info->state = VIR_DOMAIN_RUNNING; - VIR_FREE(tmp); - } else { - info->state = VIR_DOMAIN_NOSTATE; - } - tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target"); - if (tmp != NULL) { - info->memory = atol(tmp); - info->maxMem = atol(tmp); - VIR_FREE(tmp); - } else { - info->memory = 0; - info->maxMem = 0; - } -#if 0 - /* doesn't seems to work */ - tmp = virDomainDoStoreQuery(domain->conn, domain->id, "cpu_time"); - if (tmp != NULL) { - info->cpuTime = atol(tmp); - VIR_FREE(tmp); - } else { - info->cpuTime = 0; - } -#endif - snprintf(request, 199, "/local/domain/%d/cpu", domain->id); - request[199] = 0; - tmp2 = virConnectDoStoreList(domain->conn, request, &nb_vcpus); - if (tmp2 != NULL) { - info->nrVirtCpu = nb_vcpus; - VIR_FREE(tmp2); - } - return 0; -} - -/** - * xenStoreDomainGetState: - * @domain: pointer to the domain block - * @state: returned domain's state - * @reason: returned state reason - * @flags: additional flags, 0 for now - * - * Returns 0 in case of success, -1 in case of error. - */ -int -xenStoreDomainGetState(virDomainPtr domain, - int *state, - int *reason, - unsigned int flags) -{ - char *running; - - virCheckFlags(0, -1); - - if (domain->id == -1) - return -1; - - running = virDomainDoStoreQuery(domain->conn, domain->id, "running"); - - if (running && *running == '1') - *state = VIR_DOMAIN_RUNNING; - else - *state = VIR_DOMAIN_NOSTATE; - if (reason) - *reason = 0; - - VIR_FREE(running); - - return 0; -} - - -/** * xenStoreNumOfDomains: * @conn: pointer to the hypervisor connection * diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index 4390733..bc5bd8c 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -26,19 +26,10 @@ # include "internal.h" # include "driver.h" -extern struct xenUnifiedDriver xenStoreDriver; -int xenStoreInit (void); - int xenStoreOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags); int xenStoreClose (virConnectPtr conn); -int xenStoreGetDomainInfo (virDomainPtr domain, - virDomainInfoPtr info); -int xenStoreDomainGetState (virDomainPtr domain, - int *state, - int *reason, - unsigned int flags); int xenStoreNumOfDomains (virConnectPtr conn); int xenStoreListDomains (virConnectPtr conn, int *ids,