Re: [libvirt] [PATCH 17/40] Simplify the Xen domain get/set (max) memory driver methods

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Simplify the Xen memory limit driver methods to directly call the most appropriate sub-driver Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 50 ++++++++++----------------- src/xen/xen_driver.h | 3 -- src/xen/xen_hypervisor.c | 35 ++++--------------- src/xen/xen_hypervisor.h | 3 +- src/xen/xend_internal.c | 15 -------- src/xen/xm_internal.c | 16 +++++---- src/xen/xs_internal.c | 90 ------------------------------------------------ src/xen/xs_internal.h | 4 --- 8 files changed, 36 insertions(+), 180 deletions(-)

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 8ee3c4c..7d09c6b 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -801,53 +801,41 @@ static unsigned long long xenUnifiedDomainGetMaxMemory(virDomainPtr dom) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - unsigned long long ret; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainGetMaxMemory) { - ret = drivers[i]->xenDomainGetMaxMemory(dom); - if (ret != 0) return ret; - } - return 0; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainGetMaxMemory(dom); + else + return xenDaemonDomainGetMaxMemory(dom); + } else { + return xenHypervisorGetMaxMemory(dom); + } } static int xenUnifiedDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - /* Prefer xend for setting max memory */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - if (xenDaemonDomainSetMaxMemory(dom, memory) == 0) - return 0; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainSetMaxMemory(dom, memory); + else + return xenDaemonDomainSetMaxMemory(dom, memory); + } else { + return xenHypervisorSetMaxMemory(dom, memory); } - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (i != XEN_UNIFIED_XEND_OFFSET && - priv->opened[i] && - drivers[i]->xenDomainSetMaxMemory && - drivers[i]->xenDomainSetMaxMemory(dom, memory) == 0) - return 0; - - return -1; } static int xenUnifiedDomainSetMemory(virDomainPtr dom, unsigned long memory) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && - drivers[i]->xenDomainSetMemory && - drivers[i]->xenDomainSetMemory(dom, memory) == 0) - return 0; - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainSetMemory(dom, memory); + else + return xenDaemonDomainSetMemory(dom, memory); } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 16e9743..4509161 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -93,9 +93,6 @@ extern int xenRegister (void); * structure with direct calls in xen_unified.c. */ struct xenUnifiedDriver { - virDrvDomainGetMaxMemory xenDomainGetMaxMemory; - virDrvDomainSetMaxMemory xenDomainSetMaxMemory; - virDrvDomainSetMemory xenDomainSetMemory; virDrvDomainGetInfo xenDomainGetInfo; virDrvDomainPinVcpu xenDomainPinVcpu; virDrvDomainGetVcpus xenDomainGetVcpus; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 8636d52..7662843 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -870,11 +870,7 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; # error "unsupported platform" #endif -static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain); - struct xenUnifiedDriver xenHypervisorDriver = { - .xenDomainGetMaxMemory = xenHypervisorGetMaxMemory, - .xenDomainSetMaxMemory = xenHypervisorSetMaxMemory, .xenDomainGetInfo = xenHypervisorGetDomainInfo, .xenDomainPinVcpu = xenHypervisorPinVcpu, .xenDomainGetVcpus = xenHypervisorGetVcpus, @@ -2763,9 +2759,8 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, } /** - * xenHypervisorGetDomMaxMemory: - * @conn: connection data - * @id: domain id + * xenHypervisorDomMaxMemory: + * @dom: domain * * Retrieve the maximum amount of physical memory allocated to a * domain. @@ -2773,9 +2768,9 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, * Returns the memory size in kilobytes or 0 in case of error. */ unsigned long -xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) +xenHypervisorGetMaxMemory(virDomainPtr dom) { - xenUnifiedPrivatePtr priv = conn->privateData; + xenUnifiedPrivatePtr priv = dom->conn->privateData; xen_getdomaininfo dominfo; int ret; @@ -2787,32 +2782,14 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) XEN_GETDOMAININFO_CLEAR(dominfo); - ret = virXen_getdomaininfo(priv->handle, id, &dominfo); + ret = virXen_getdomaininfo(priv->handle, dom->id, &dominfo); - if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != id)) + if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != dom->id)) return 0; return (unsigned long) XEN_GETDOMAININFO_MAX_PAGES(dominfo) * kb_per_pages; } -/** - * xenHypervisorGetMaxMemory: - * @domain: a domain object or NULL - * - * Retrieve the maximum amount of physical memory allocated to a - * domain. If domain is NULL, then this get the amount of memory reserved - * to Domain0 i.e. the domain where the application runs. - * - * Returns the memory size in kilobytes or 0 in case of error. - */ -static unsigned long long ATTRIBUTE_NONNULL(1) -xenHypervisorGetMaxMemory(virDomainPtr domain) -{ - if (domain->id < 0) - return 0; - - return xenHypervisorGetDomMaxMemory(domain->conn, domain->id); -} /** * xenHypervisorGetDomInfo: diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 450b4f1..9748cf8 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -68,8 +68,7 @@ virCapsPtr char * xenHypervisorGetCapabilities (virConnectPtr conn); unsigned long - xenHypervisorGetDomMaxMemory (virConnectPtr conn, - int id); + xenHypervisorGetMaxMemory(virDomainPtr dom); int xenHypervisorGetMaxVcpus (virConnectPtr conn, const char *type); int xenHypervisorGetDomainInfo (virDomainPtr domain, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 75c1514..ce7d3f6 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1491,10 +1491,6 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain) { unsigned long long ret = 0; struct sexpr *root; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return 0; /* can we ask for a subset ? worth it ? */ root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); @@ -1523,10 +1519,6 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -1; snprintf(buf, sizeof(buf), "%lu", VIR_DIV_UP(memory, 1024)); return xend_op(domain->conn, domain->name, "op", "maxmem_set", "memory", @@ -1553,10 +1545,6 @@ int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -1; snprintf(buf, sizeof(buf), "%lu", VIR_DIV_UP(memory, 1024)); return xend_op(domain->conn, domain->name, "op", "mem_target_set", @@ -3437,9 +3425,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory, - .xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory, - .xenDomainSetMemory = xenDaemonDomainSetMemory, .xenDomainGetInfo = xenDaemonDomainGetInfo, .xenDomainPinVcpu = xenDaemonDomainPinVcpu, .xenDomainGetVcpus = xenDaemonDomainGetVcpus, diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 34339c5..eddaca0 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -81,9 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, #define XM_XML_ERROR "Invalid xml" struct xenUnifiedDriver xenXMDriver = { - .xenDomainGetMaxMemory = xenXMDomainGetMaxMemory, - .xenDomainSetMaxMemory = xenXMDomainSetMaxMemory, - .xenDomainSetMemory = xenXMDomainSetMemory, .xenDomainGetInfo = xenXMDomainGetInfo, .xenDomainPinVcpu = xenXMDomainPinVcpu, .xenListDefinedDomains = xenXMListDefinedDomains, @@ -564,9 +561,12 @@ xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) xenXMConfCachePtr entry; int ret = -1; - if (domain->id != -1 || - memory < 1024 * MIN_XEN_GUEST_SIZE) + if (memory < 1024 * MIN_XEN_GUEST_SIZE) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory %lu too small, min %lu"), + memory, (unsigned long)1024 * MIN_XEN_GUEST_SIZE); return -1; + } xenUnifiedLock(priv); @@ -603,8 +603,12 @@ xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) xenXMConfCachePtr entry; int ret = -1; - if (domain->id != -1) + if (memory < 1024 * MIN_XEN_GUEST_SIZE) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory %lu too small, min %lu"), + memory, (unsigned long)1024 * MIN_XEN_GUEST_SIZE); return -1; + } xenUnifiedLock(priv); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 40d0be2..dd1f2a0 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -57,8 +57,6 @@ static void xenStoreWatchEvent(int watch, int fd, int events, void *data); static void xenStoreWatchListFree(xenStoreWatchListPtr list); struct xenUnifiedDriver xenStoreDriver = { - .xenDomainGetMaxMemory = xenStoreDomainGetMaxMemory, - .xenDomainSetMemory = xenStoreDomainSetMemory, .xenDomainGetInfo = xenStoreGetDomainInfo, }; @@ -111,36 +109,6 @@ virDomainDoStoreQuery(virConnectPtr conn, int domid, const char *path) return xs_read(priv->xshandle, 0, &s[0], &len); } -/** - * virDomainDoStoreWrite: - * @domain: a domain object - * @path: the relative path of the data in the store to retrieve - * - * Internal API setting up a string value in the Xenstore - * Requires write access to the XenStore - * - * Returns 0 in case of success, -1 in case of failure - */ -static int -virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) -{ - char s[256]; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - int ret = -1; - - if (priv->xshandle == NULL) - return -1; - - snprintf(s, 255, "/local/domain/%d/%s", domain->id, path); - s[255] = 0; - - if (xs_write(priv->xshandle, 0, &s[0], value, strlen(value))) - ret = 0; - - return ret; -} - - /************************************************************************ * * * Canonical internal APIs * @@ -359,64 +327,6 @@ xenStoreDomainGetState(virDomainPtr domain, return 0; } -/** - * xenStoreDomainSetMemory: - * @domain: pointer to the domain block - * @memory: the max memory size in kilobytes. - * - * Change the maximum amount of memory allowed in the xen store - * - * Returns 0 in case of success, -1 in case of error. - */ -int -xenStoreDomainSetMemory(virDomainPtr domain, unsigned long memory) -{ - int ret; - char value[20]; - - if (memory < 1024 * MIN_XEN_GUEST_SIZE) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - if (domain->id == -1) - return -1; - if ((domain->id == 0) && (memory < (2 * MIN_XEN_GUEST_SIZE * 1024))) - return -1; - snprintf(value, 19, "%lu", memory); - value[19] = 0; - ret = virDomainDoStoreWrite(domain, "memory/target", &value[0]); - if (ret < 0) - return -1; - return 0; -} - -/** - * xenStoreDomainGetMaxMemory: - * @domain: pointer to the domain block - * - * Ask the xenstore for the maximum memory allowed for a domain - * - * Returns the memory size in kilobytes or 0 in case of error. - */ -unsigned long long -xenStoreDomainGetMaxMemory(virDomainPtr domain) -{ - char *tmp; - unsigned long long ret = 0; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id == -1) - return 0; - - xenUnifiedLock(priv); - tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target"); - if (tmp != NULL) { - ret = atol(tmp); - VIR_FREE(tmp); - } - xenUnifiedUnlock(priv); - return ret; -} /** * xenStoreNumOfDomains: diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index da98eea..4390733 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -43,10 +43,6 @@ int xenStoreNumOfDomains (virConnectPtr conn); int xenStoreListDomains (virConnectPtr conn, int *ids, int maxids); -unsigned long xenStoreGetMaxMemory (virDomainPtr domain); -int xenStoreDomainSetMemory (virDomainPtr domain, - unsigned long memory); -unsigned long long xenStoreDomainGetMaxMemory(virDomainPtr domain); int xenStoreDomainGetVNCPort(virConnectPtr conn, int domid);