From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The Xen hypervisor driver checks for 'priv->handle < 0' and returns -1, but without raising any error. Fortunately this code will never be executed, since the main Xen driver always checks 'priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]' prior to invoking any hypervisor API. Just the redundant checks for priv->handle Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_hypervisor.c | 98 ++++++++---------------------------------------- 1 file changed, 16 insertions(+), 82 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 9dbbe07..d9941ec 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1165,11 +1165,6 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams) char *schedulertype = NULL; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("priv->handle invalid")); - return NULL; - } if (domain->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -1240,11 +1235,7 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("priv->handle invalid")); - return -1; - } + if (domain->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -1353,11 +1344,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, NULL) < 0) return -1; - if (priv->handle < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("priv->handle invalid")); - return -1; - } if (domain->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2209,7 +2195,7 @@ xenHypervisorOpen(virConnectPtr conn, virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); if (xenHypervisorInitialize() < 0) - return -1; + return VIR_DRV_OPEN_ERROR; priv->handle = -1; @@ -2221,7 +2207,7 @@ xenHypervisorOpen(virConnectPtr conn, priv->handle = ret; - return 0; + return VIR_DRV_OPEN_SUCCESS; } /** @@ -2238,9 +2224,6 @@ xenHypervisorClose(virConnectPtr conn) int ret; xenUnifiedPrivatePtr priv = conn->privateData; - if (priv->handle < 0) - return -1; - ret = VIR_CLOSE(priv->handle); if (ret < 0) return -1; @@ -2259,12 +2242,8 @@ xenHypervisorClose(virConnectPtr conn) * Returns 0 in case of success, -1 in case of error */ int -xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer) +xenHypervisorGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) { - xenUnifiedPrivatePtr priv = conn->privateData; - - if (priv->handle < 0) - return -1; *hvVer = (hv_versions.hv >> 16) * 1000000 + (hv_versions.hv & 0xFFFF) * 1000; return 0; } @@ -2769,9 +2748,6 @@ xenHypervisorNumOfDomains(virConnectPtr conn) int maxids = last_maxids; xenUnifiedPrivatePtr priv = conn->privateData; - if (priv->handle < 0) - return -1; - retry: if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) { virReportOOMError(); @@ -2823,9 +2799,6 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids) int ret, nbids, i; xenUnifiedPrivatePtr priv = conn->privateData; - if (priv->handle < 0) - return -1; - if (maxids == 0) return 0; @@ -2866,12 +2839,6 @@ xenHypervisorDomainGetOSType(virDomainPtr dom) xen_getdomaininfo dominfo; char *ostype = NULL; - if (priv->handle < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain shut off or invalid")); - return NULL; - } - /* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/ if (hv_versions.hypervisor < 2 || hv_versions.dom_interface < 4) { @@ -2911,9 +2878,6 @@ xenHypervisorHasDomain(virConnectPtr conn, int id) xenUnifiedPrivatePtr priv = conn->privateData; xen_getdomaininfo dominfo; - if (priv->handle < 0) - return 0; - XEN_GETDOMAININFO_CLEAR(dominfo); if (virXen_getdomaininfo(priv->handle, id, &dominfo) < 0) @@ -2933,9 +2897,6 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id) virDomainPtr ret; char *name; - if (priv->handle < 0) - return NULL; - XEN_GETDOMAININFO_CLEAR(dominfo); if (virXen_getdomaininfo(priv->handle, id, &dominfo) < 0) @@ -2967,9 +2928,6 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) char *name; int maxids = 100, nids, i, id; - if (priv->handle < 0) - return NULL; - retry: if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) { virReportOOMError(); @@ -3030,13 +2988,9 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) * Returns the maximum of CPU defined by Xen. */ int -xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) +xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *type ATTRIBUTE_UNUSED) { - xenUnifiedPrivatePtr priv = conn->privateData; - - if (priv->handle < 0) - return -1; - return MAX_VIRT_CPUS; } @@ -3057,9 +3011,6 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) xen_getdomaininfo dominfo; int ret; - if (priv->handle < 0) - return 0; - if (kb_per_pages == 0) { kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; if (kb_per_pages <= 0) @@ -3089,9 +3040,7 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) static unsigned long long ATTRIBUTE_NONNULL(1) xenHypervisorGetMaxMemory(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return 0; return xenHypervisorGetDomMaxMemory(domain->conn, domain->id); @@ -3121,9 +3070,6 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) kb_per_pages = 4; } - if (priv->handle < 0) - return -1; - memset(info, 0, sizeof(virDomainInfo)); XEN_GETDOMAININFO_CLEAR(dominfo); @@ -3189,9 +3135,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) int xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; return xenHypervisorGetDomInfo(domain->conn, domain->id, info); @@ -3215,12 +3159,11 @@ xenHypervisorGetDomainState(virDomainPtr domain, int *reason, unsigned int flags) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; virDomainInfo info; virCheckFlags(0, -1); - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; if (xenHypervisorGetDomInfo(domain->conn, domain->id, &info) < 0) @@ -3281,12 +3224,6 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, return -1; } - if (priv->handle < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("priv->handle invalid")); - return -1; - } - memset(&op_sys, 0, sizeof(op_sys)); op_sys.cmd = XEN_V2_OP_GETAVAILHEAP; @@ -3322,7 +3259,7 @@ xenHypervisorPauseDomain(virDomainPtr domain) int ret; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; ret = virXen_pausedomain(priv->handle, domain->id); @@ -3345,7 +3282,7 @@ xenHypervisorResumeDomain(virDomainPtr domain) int ret; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; ret = virXen_unpausedomain(priv->handle, domain->id); @@ -3374,7 +3311,7 @@ xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; ret = virXen_destroydomain(priv->handle, domain->id); @@ -3398,7 +3335,7 @@ xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory) int ret; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; ret = virXen_setmaxmem(priv->handle, domain->id, memory); @@ -3424,7 +3361,7 @@ xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus) int ret; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0 || nvcpus < 1) + if (domain->id < 0 || nvcpus < 1) return -1; ret = virXen_setmaxvcpus(priv->handle, domain->id, nvcpus); @@ -3452,7 +3389,7 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu, int ret; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) + if (domain->id < 0) return -1; ret = virXen_setvcpumap(priv->handle, domain->id, vcpu, @@ -3494,7 +3431,7 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr ipt; int nbinfo, i; - if (priv->handle < 0 || domain->id < 0 || sizeof(cpumap_t) & 7) { + if (domain->id < 0 || sizeof(cpumap_t) & 7) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain shut off or invalid")); return -1; @@ -3556,9 +3493,6 @@ xenHypervisorGetVcpuMax(virDomainPtr domain) int maxcpu; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (priv->handle < 0) - return -1; - /* inactive domain */ if (domain->id < 0) { maxcpu = MAX_VIRT_CPUS; -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Some of the Xen sub-drivers have checks against the VIR_CONNECT_RO flag. This is not required, since such checks are done at the top level before the driver methods are invoked Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xm_internal.c | 24 ++---------------------- src/xen/xs_internal.c | 2 +- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 8ac7cb0..8580793 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -565,7 +565,7 @@ xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) xenXMConfCachePtr entry; int ret = -1; - if (domain->conn->flags & VIR_CONNECT_RO || domain->id != -1 || + if (domain->id != -1 || memory < 1024 * MIN_XEN_GUEST_SIZE) return -1; @@ -604,7 +604,7 @@ xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) xenXMConfCachePtr entry; int ret = -1; - if (domain->conn->flags & VIR_CONNECT_RO || domain->id != -1) + if (domain->id != -1) return -1; xenUnifiedLock(priv); @@ -686,10 +686,6 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain, VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (domain->conn->flags & VIR_CONNECT_RO) { - virReportError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); - return -1; - } if (domain->id != -1) return -2; if (flags & VIR_DOMAIN_VCPU_LIVE) { @@ -814,11 +810,6 @@ xenXMDomainPinVcpu(virDomainPtr domain, virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - if (domain->conn->flags & VIR_CONNECT_RO) { - virReportError(VIR_ERR_INVALID_ARG, - "%s", _("read only connection")); - return -1; - } if (domain->id != -1) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("not inactive domain")); @@ -1003,9 +994,6 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml) xenXMConfCachePtr entry = NULL; xenUnifiedPrivatePtr priv = conn->privateData; - if (conn->flags & VIR_CONNECT_RO) - return NULL; - xenUnifiedLock(priv); if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) { @@ -1140,8 +1128,6 @@ xenXMDomainUndefine(virDomainPtr domain) if (domain->id != -1) return -1; - if (domain->conn->flags & VIR_CONNECT_RO) - return -1; xenUnifiedLock(priv); @@ -1292,9 +1278,6 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (domain->conn->flags & VIR_CONNECT_RO) - return -1; - if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) || (domain->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1386,9 +1369,6 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (domain->conn->flags & VIR_CONNECT_RO) - return -1; - if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) || (domain->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 5f0df63..393e5f9 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -133,7 +133,7 @@ virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) xenUnifiedPrivatePtr priv = domain->conn->privateData; int ret = -1; - if (priv->xshandle == NULL || domain->conn->flags & VIR_CONNECT_RO) + if (priv->xshandle == NULL) return -1; snprintf(s, 255, "/local/domain/%d/%s", domain->id, path); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The Xen driver uses a macro GET_PRIVATE as a supposed shorthand for 'xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) (conn)->privateData'. It does not in fact save any lines of code, and obscures what is happening. Remove it, since it adds no value. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 131 +++++++++++++++++++++++++-------------------------- 1 file changed, 64 insertions(+), 67 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 0342523..2ecb86f 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -470,13 +470,10 @@ clean: return ret; } -#define GET_PRIVATE(conn) \ - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) (conn)->privateData - static int xenUnifiedConnectClose(virConnectPtr conn) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; virObjectUnref(priv->caps); @@ -508,7 +505,7 @@ unsigned long xenUnifiedVersion(void) static const char * xenUnifiedConnectGetType(virConnectPtr conn) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -534,7 +531,7 @@ xenUnifiedConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int featur static int xenUnifiedConnectGetVersion(virConnectPtr conn, unsigned long *hvVer) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -562,7 +559,7 @@ xenUnifiedConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) static int xenUnifiedConnectIsSecure(virConnectPtr conn) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int ret = 1; /* All drivers are secure, except for XenD over TCP */ @@ -583,7 +580,7 @@ xenUnifiedConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) int xenUnifiedConnectGetMaxVcpus(virConnectPtr conn, const char *type) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; if (type && STRCASENEQ(type, "Xen")) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -601,7 +598,7 @@ xenUnifiedConnectGetMaxVcpus(virConnectPtr conn, const char *type) static int xenUnifiedNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) return xenDaemonNodeGetInfo(conn, info); @@ -625,7 +622,7 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn) static int xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int ret; /* Try xenstore. */ @@ -652,7 +649,7 @@ xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids) static int xenUnifiedConnectNumOfDomains(virConnectPtr conn) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int ret; /* Try xenstore. */ @@ -680,7 +677,7 @@ static virDomainPtr xenUnifiedDomainCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) return xenDaemonCreateXML(conn, xmlDesc, flags); @@ -694,7 +691,7 @@ xenUnifiedDomainCreateXML(virConnectPtr conn, static virDomainPtr xenUnifiedDomainLookupByID(virConnectPtr conn, int id) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virDomainPtr ret; /* Reset any connection-level errors in virterror first, in case @@ -725,7 +722,7 @@ static virDomainPtr xenUnifiedDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virDomainPtr ret; /* Reset any connection-level errors in virterror first, in case @@ -763,7 +760,7 @@ static virDomainPtr xenUnifiedDomainLookupByName(virConnectPtr conn, const char *name) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virDomainPtr ret; /* Reset any connection-level errors in virterror first, in case @@ -818,7 +815,7 @@ xenUnifiedDomainIsActive(virDomainPtr dom) static int xenUnifiedDomainIsPersistent(virDomainPtr dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; virDomainPtr currdom = NULL; int ret = -1; @@ -875,7 +872,7 @@ xenUnifiedDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) static int xenUnifiedDomainSuspend(virDomainPtr dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; /* Try non-hypervisor methods first, then hypervisor direct method @@ -898,7 +895,7 @@ xenUnifiedDomainSuspend(virDomainPtr dom) static int xenUnifiedDomainResume(virDomainPtr dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; /* Try non-hypervisor methods first, then hypervisor direct method @@ -922,7 +919,7 @@ static int xenUnifiedDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; virCheckFlags(0, -1); @@ -945,7 +942,7 @@ xenUnifiedDomainShutdown(virDomainPtr dom) static int xenUnifiedDomainReboot(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -961,7 +958,7 @@ static int xenUnifiedDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; virCheckFlags(0, -1); @@ -992,7 +989,7 @@ xenUnifiedDomainDestroy(virDomainPtr dom) static char * xenUnifiedDomainGetOSType(virDomainPtr dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; char *ret; @@ -1008,7 +1005,7 @@ xenUnifiedDomainGetOSType(virDomainPtr dom) static unsigned long long xenUnifiedDomainGetMaxMemory(virDomainPtr dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; unsigned long long ret; @@ -1024,7 +1021,7 @@ xenUnifiedDomainGetMaxMemory(virDomainPtr dom) static int xenUnifiedDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; /* Prefer xend for setting max memory */ @@ -1046,7 +1043,7 @@ xenUnifiedDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) static int xenUnifiedDomainSetMemory(virDomainPtr dom, unsigned long memory) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1061,7 +1058,7 @@ xenUnifiedDomainSetMemory(virDomainPtr dom, unsigned long memory) static int xenUnifiedDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1079,7 +1076,7 @@ xenUnifiedDomainGetState(virDomainPtr dom, int *reason, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int ret; virCheckFlags(0, -1); @@ -1118,7 +1115,7 @@ static int xenUnifiedDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; virCheckFlags(0, -1); if (dxml) { @@ -1155,7 +1152,7 @@ xenUnifiedDomainManagedSavePath(xenUnifiedPrivatePtr priv, virDomainPtr dom) static int xenUnifiedDomainManagedSave(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; char *name; int ret = -1; @@ -1176,7 +1173,7 @@ cleanup: static int xenUnifiedDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; char *name; int ret = -1; @@ -1194,7 +1191,7 @@ xenUnifiedDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) static int xenUnifiedDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; char *name; int ret = -1; @@ -1213,7 +1210,7 @@ static int xenUnifiedDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, unsigned int flags) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virCheckFlags(0, -1); if (dxml) { @@ -1236,7 +1233,7 @@ xenUnifiedDomainRestore(virConnectPtr conn, const char *from) static int xenUnifiedDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) return xenDaemonDomainCoreDump(dom, to, flags); @@ -1247,7 +1244,7 @@ static int xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int ret; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | @@ -1298,7 +1295,7 @@ xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) * affects just _LIVE or _LIVE|_CONFIG; in xen's case, that * depends on xendConfigVersion. */ if (dom) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) flags |= VIR_DOMAIN_VCPU_CONFIG; return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags); @@ -1310,7 +1307,7 @@ static int xenUnifiedDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, int maplen) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1327,7 +1324,7 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i, ret; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1342,7 +1339,7 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, static int xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int ret; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | @@ -1376,7 +1373,7 @@ xenUnifiedDomainGetMaxVcpus(virDomainPtr dom) static char * xenUnifiedDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (dom->id == -1 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { if (priv->opened[XEN_UNIFIED_XM_OFFSET]) @@ -1410,7 +1407,7 @@ xenUnifiedConnectDomainXMLFromNative(virConnectPtr conn, int id; char * tty; int vncport; - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virCheckFlags(0, NULL); @@ -1459,7 +1456,7 @@ xenUnifiedConnectDomainXMLToNative(virConnectPtr conn, virDomainDefPtr def = NULL; char *ret = NULL; virConfPtr conf = NULL; - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; virCheckFlags(0, NULL); @@ -1511,7 +1508,7 @@ xenUnifiedDomainMigratePrepare(virConnectPtr dconn, const char *dname, unsigned long resource) { - GET_PRIVATE(dconn); + xenUnifiedPrivatePtr priv = dconn->privateData; virCheckFlags(XEN_MIGRATION_FLAGS, -1); @@ -1533,7 +1530,7 @@ xenUnifiedDomainMigratePerform(virDomainPtr dom, const char *dname, unsigned long resource) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; virCheckFlags(XEN_MIGRATION_FLAGS, -1); @@ -1600,7 +1597,7 @@ static int xenUnifiedConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; int ret; @@ -1616,7 +1613,7 @@ xenUnifiedConnectListDefinedDomains(virConnectPtr conn, char **const names, static int xenUnifiedConnectNumOfDefinedDomains(virConnectPtr conn) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; int ret; @@ -1632,7 +1629,7 @@ xenUnifiedConnectNumOfDefinedDomains(virConnectPtr conn) static int xenUnifiedDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; int ret = -1; char *name = NULL; @@ -1674,7 +1671,7 @@ xenUnifiedDomainCreate(virDomainPtr dom) static virDomainPtr xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int i; virDomainPtr ret; @@ -1690,7 +1687,7 @@ xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml) static int xenUnifiedDomainUndefineFlags(virDomainPtr dom, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; virCheckFlags(0, -1); @@ -1710,7 +1707,7 @@ xenUnifiedDomainUndefine(virDomainPtr dom) { static int xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; @@ -1735,7 +1732,7 @@ static int xenUnifiedDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1749,7 +1746,7 @@ xenUnifiedDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, static int xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; @@ -1774,7 +1771,7 @@ static int xenUnifiedDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) @@ -1789,7 +1786,7 @@ static int xenUnifiedDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) return xenDaemonUpdateDeviceFlags(dom, xml, flags); @@ -1799,7 +1796,7 @@ xenUnifiedDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, static int xenUnifiedDomainGetAutostart(virDomainPtr dom, int *autostart) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { if (priv->opened[XEN_UNIFIED_XM_OFFSET]) @@ -1816,7 +1813,7 @@ xenUnifiedDomainGetAutostart(virDomainPtr dom, int *autostart) static int xenUnifiedDomainSetAutostart(virDomainPtr dom, int autostart) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { if (priv->opened[XEN_UNIFIED_XM_OFFSET]) @@ -1833,7 +1830,7 @@ xenUnifiedDomainSetAutostart(virDomainPtr dom, int autostart) static char * xenUnifiedDomainGetSchedulerType(virDomainPtr dom, int *nparams) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i; char *schedulertype; @@ -1853,7 +1850,7 @@ xenUnifiedDomainGetSchedulerParametersFlags(virDomainPtr dom, int *nparams, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i, ret; virCheckFlags(0, -1); @@ -1883,7 +1880,7 @@ xenUnifiedDomainSetSchedulerParametersFlags(virDomainPtr dom, int nparams, unsigned int flags) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; int i, ret; virCheckFlags(0, -1); @@ -1913,7 +1910,7 @@ static int xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path, struct _virDomainBlockStats *stats) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) return xenHypervisorDomainBlockStats(dom, path, stats); @@ -1926,7 +1923,7 @@ static int xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) return xenHypervisorDomainInterfaceStats(dom, path, stats); @@ -1941,7 +1938,7 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const char *path, void *buffer, unsigned int flags) { int r; - GET_PRIVATE(dom->conn); + xenUnifiedPrivatePtr priv = dom->conn->privateData; virCheckFlags(0, -1); @@ -1964,7 +1961,7 @@ static int xenUnifiedNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, int startCell, int maxCells) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) return xenHypervisorNodeGetCellsFreeMemory(conn, freeMems, @@ -1979,7 +1976,7 @@ xenUnifiedNodeGetFreeMemory(virConnectPtr conn) { unsigned long long freeMem = 0; int ret; - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { ret = xenHypervisorNodeGetCellsFreeMemory(conn, &freeMem, @@ -2000,7 +1997,7 @@ xenUnifiedConnectDomainEventRegister(virConnectPtr conn, void *opaque, virFreeCallback freefunc) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int ret; xenUnifiedLock(priv); @@ -2024,7 +2021,7 @@ xenUnifiedConnectDomainEventDeregister(virConnectPtr conn, virConnectDomainEventCallback callback) { int ret; - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; xenUnifiedLock(priv); if (priv->xsWatch == -1) { @@ -2050,7 +2047,7 @@ xenUnifiedConnectDomainEventRegisterAny(virConnectPtr conn, void *opaque, virFreeCallback freefunc) { - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; int ret; xenUnifiedLock(priv); @@ -2075,7 +2072,7 @@ xenUnifiedConnectDomainEventDeregisterAny(virConnectPtr conn, int callbackID) { int ret; - GET_PRIVATE(conn); + xenUnifiedPrivatePtr priv = conn->privateData; xenUnifiedLock(priv); if (priv->xsWatch == -1) { -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Since the Xen driver was changed to only execute inside libvirtd, there is no scenario in which it will be opened from a non-privileged context. This all the code dealing with opening the sub-drivers can be simplified to assume that they are always privileged. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 148 +++++++++++++++++++++-------------------------- src/xen/xen_driver.h | 1 - src/xen/xen_hypervisor.c | 9 ++- src/xen/xen_hypervisor.h | 2 +- src/xen/xen_inotify.c | 8 +-- src/xen/xen_inotify.h | 11 ++-- src/xen/xend_internal.c | 9 ++- src/xen/xend_internal.h | 4 +- src/xen/xm_internal.c | 5 +- src/xen/xm_internal.h | 4 +- src/xen/xs_internal.c | 5 +- src/xen/xs_internal.h | 2 +- 12 files changed, 91 insertions(+), 117 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 2ecb86f..b7f1ad4 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -86,14 +86,9 @@ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver, [XEN_UNIFIED_XS_OFFSET] = &xenStoreDriver, [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver, -#if WITH_XEN_INOTIFY - [XEN_UNIFIED_INOTIFY_OFFSET] = &xenInotifyDriver, -#endif }; -#if defined WITH_LIBVIRTD || defined __sun static bool inside_daemon = false; -#endif /** * xenNumaInit: @@ -200,14 +195,14 @@ done: return res; } -#ifdef WITH_LIBVIRTD - static int -xenUnifiedStateInitialize(bool privileged ATTRIBUTE_UNUSED, +xenUnifiedStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - inside_daemon = true; + /* Don't allow driver to work in non-root libvirtd */ + if (privileged) + inside_daemon = true; return 0; } @@ -216,8 +211,6 @@ static virStateDriver state_driver = { .stateInitialize = xenUnifiedStateInitialize, }; -#endif - /*----- Dispatch functions. -----*/ /* These dispatch functions follow the model used historically @@ -298,18 +291,15 @@ xenDomainXMLConfInit(void) static virDrvOpenStatus xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { - int i, ret = VIR_DRV_OPEN_DECLINED; xenUnifiedPrivatePtr priv; char ebuf[1024]; -#ifdef __sun /* * Only the libvirtd instance can open this driver. * Everything else falls back to the remote driver. */ if (!inside_daemon) return VIR_DRV_OPEN_DECLINED; -#endif if (conn->uri == NULL) { if (!xenUnifiedProbe()) @@ -379,110 +369,108 @@ xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int f priv->xshandle = NULL; - /* Hypervisor is only run with privilege & required to succeed */ - if (xenHavePrivilege()) { - VIR_DEBUG("Trying hypervisor sub-driver"); - if (xenHypervisorOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { - VIR_DEBUG("Activated hypervisor sub-driver"); - priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1; - } else { - goto fail; - } - } + /* Hypervisor required to succeed */ + VIR_DEBUG("Trying hypervisor sub-driver"); + if (xenHypervisorOpen(conn, auth, flags) < 0) + goto error; + VIR_DEBUG("Activated hypervisor sub-driver"); + priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1; - /* XenD is required to succeed if privileged */ + /* XenD is required to succeed */ VIR_DEBUG("Trying XenD sub-driver"); - if (xenDaemonOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { - VIR_DEBUG("Activated XenD sub-driver"); - priv->opened[XEN_UNIFIED_XEND_OFFSET] = 1; - - /* XenD is active, so try the xm & xs drivers too, both requird to - * succeed if root, optional otherwise */ - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) { - VIR_DEBUG("Trying XM sub-driver"); - if (xenXMOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { - VIR_DEBUG("Activated XM sub-driver"); - priv->opened[XEN_UNIFIED_XM_OFFSET] = 1; - } - } - VIR_DEBUG("Trying XS sub-driver"); - if (xenStoreOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { - VIR_DEBUG("Activated XS sub-driver"); - priv->opened[XEN_UNIFIED_XS_OFFSET] = 1; - } else { - if (xenHavePrivilege()) - goto fail; /* XS is mandatory when privileged */ - } - } else { - if (xenHavePrivilege()) { - goto fail; /* XenD is mandatory when privileged */ - } else { - VIR_DEBUG("Handing off for remote driver"); - ret = VIR_DRV_OPEN_DECLINED; /* Let remote_driver try instead */ - goto clean; - } - } + if (xenDaemonOpen(conn, auth, flags) < 0) + goto error; + VIR_DEBUG("Activated XenD sub-driver"); + priv->opened[XEN_UNIFIED_XEND_OFFSET] = 1; + + /* For old XenD, the XM driver is required to succeed */ + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) { + VIR_DEBUG("Trying XM sub-driver"); + if (xenXMOpen(conn, auth, flags) < 0) + goto error; + VIR_DEBUG("Activated XM sub-driver"); + priv->opened[XEN_UNIFIED_XM_OFFSET] = 1; + } + + VIR_DEBUG("Trying XS sub-driver"); + if (xenStoreOpen(conn, auth, flags) < 0) + goto error; + VIR_DEBUG("Activated XS sub-driver"); + priv->opened[XEN_UNIFIED_XS_OFFSET] = 1; xenNumaInit(conn); if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) { - VIR_DEBUG("Failed to make capabilities"); - goto fail; + VIR_DEBUG("Errored to make capabilities"); + goto error; } if (!(priv->xmlopt = xenDomainXMLConfInit())) - goto fail; + goto error; #if WITH_XEN_INOTIFY - if (xenHavePrivilege()) { - VIR_DEBUG("Trying Xen inotify sub-driver"); - if (xenInotifyOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { - VIR_DEBUG("Activated Xen inotify sub-driver"); - priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; - } - } + VIR_DEBUG("Trying Xen inotify sub-driver"); + if (xenInotifyOpen(conn, auth, flags) < 0) + goto error; + VIR_DEBUG("Activated Xen inotify sub-driver"); + priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; #endif if (virAsprintf(&priv->saveDir, "%s", XEN_SAVE_DIR) == -1) { virReportOOMError(); - goto fail; + goto error; } if (virFileMakePath(priv->saveDir) < 0) { - VIR_ERROR(_("Failed to create save dir '%s': %s"), priv->saveDir, + VIR_ERROR(_("Errored to create save dir '%s': %s"), priv->saveDir, virStrerror(errno, ebuf, sizeof(ebuf))); - goto fail; + goto error; } return VIR_DRV_OPEN_SUCCESS; -fail: - ret = VIR_DRV_OPEN_ERROR; -clean: +error: VIR_DEBUG("Failed to activate a mandatory sub-driver"); - for (i = 0 ; i < XEN_UNIFIED_NR_DRIVERS ; i++) - if (priv->opened[i]) - drivers[i]->xenClose(conn); +#if WITH_XEN_INOTIFY + if (priv->opened[XEN_UNIFIED_INOTIFY_OFFSET]) + xenInotifyClose(conn); +#endif + if (priv->opened[XEN_UNIFIED_XM_OFFSET]) + xenXMClose(conn); + if (priv->opened[XEN_UNIFIED_XS_OFFSET]) + xenStoreClose(conn); + if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) + xenDaemonClose(conn); + if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) + xenHypervisorClose(conn); virMutexDestroy(&priv->lock); VIR_FREE(priv->saveDir); VIR_FREE(priv); conn->privateData = NULL; - return ret; + return VIR_DRV_OPEN_ERROR; } static int xenUnifiedConnectClose(virConnectPtr conn) { xenUnifiedPrivatePtr priv = conn->privateData; - int i; virObjectUnref(priv->caps); virObjectUnref(priv->xmlopt); virDomainEventStateFree(priv->domainEvents); - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i]) - drivers[i]->xenClose(conn); +#if WITH_XEN_INOTIFY + if (priv->opened[XEN_UNIFIED_INOTIFY_OFFSET]) + xenInotifyClose(conn); +#endif + if (priv->opened[XEN_UNIFIED_XM_OFFSET]) + xenXMClose(conn); + if (priv->opened[XEN_UNIFIED_XS_OFFSET]) + xenStoreClose(conn); + if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) + xenDaemonClose(conn); + if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) + xenHypervisorClose(conn); VIR_FREE(priv->saveDir); virMutexDestroy(&priv->lock); @@ -2485,9 +2473,7 @@ static virDriver xenUnifiedDriver = { int xenRegister(void) { -#ifdef WITH_LIBVIRTD if (virRegisterStateDriver(&state_driver) == -1) return -1; -#endif return virRegisterDriver(&xenUnifiedDriver); } diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index c39e9be..70c1226 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 { - virDrvConnectClose xenClose; /* Only mandatory callback; all others may be NULL */ virDrvConnectGetVersion xenVersion; virDrvConnectGetHostname xenGetHostname; virDrvDomainSuspend xenDomainSuspend; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index d9941ec..6b41898 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -880,7 +880,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain); struct xenUnifiedDriver xenHypervisorDriver = { - .xenClose = xenHypervisorClose, .xenVersion = xenHypervisorGetVersion, .xenDomainSuspend = xenHypervisorPauseDomain, .xenDomainResume = xenHypervisorResumeDomain, @@ -2184,7 +2183,7 @@ VIR_ONCE_GLOBAL_INIT(xenHypervisor) * * Returns 0 or -1 in case of error. */ -virDrvOpenStatus +int xenHypervisorOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -2192,10 +2191,10 @@ xenHypervisorOpen(virConnectPtr conn, int ret; xenUnifiedPrivatePtr priv = conn->privateData; - virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + virCheckFlags(VIR_CONNECT_RO, -1); if (xenHypervisorInitialize() < 0) - return VIR_DRV_OPEN_ERROR; + return -1; priv->handle = -1; @@ -2207,7 +2206,7 @@ xenHypervisorOpen(virConnectPtr conn, priv->handle = ret; - return VIR_DRV_OPEN_SUCCESS; + return 0; } /** diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 786e301..86dca88 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -53,7 +53,7 @@ virDomainPtr char * xenHypervisorDomainGetOSType (virDomainPtr dom); -virDrvOpenStatus +int xenHypervisorOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags); diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index d83708c..b032bba 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -44,10 +44,6 @@ #define VIR_FROM_THIS VIR_FROM_XEN_INOTIFY -struct xenUnifiedDriver xenInotifyDriver = { - .xenClose = xenInotifyClose, -}; - static int xenInotifyXenCacheLookup(virConnectPtr conn, const char *filename, @@ -349,7 +345,7 @@ cleanup: * * Returns 0 or -1 in case of error. */ -virDrvOpenStatus +int xenInotifyOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -359,7 +355,7 @@ xenInotifyOpen(virConnectPtr conn, char *path; xenUnifiedPrivatePtr priv = conn->privateData; - virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + virCheckFlags(VIR_CONNECT_RO, -1); if (priv->configDir) { priv->useXenConfigCache = 1; diff --git a/src/xen/xen_inotify.h b/src/xen/xen_inotify.h index 8b31b50..6055c88 100644 --- a/src/xen/xen_inotify.h +++ b/src/xen/xen_inotify.h @@ -24,13 +24,10 @@ # define __VIR_XEN_INOTIFY_H__ # include "internal.h" -# include "driver.h" -extern struct xenUnifiedDriver xenInotifyDriver; - -virDrvOpenStatus xenInotifyOpen (virConnectPtr conn, - virConnectAuthPtr auth, - unsigned int flags); -int xenInotifyClose (virConnectPtr conn); +int xenInotifyOpen(virConnectPtr conn, + virConnectAuthPtr auth, + unsigned int flags); +int xenInotifyClose(virConnectPtr conn); #endif diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index e1f0708..eb3e63e 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1231,15 +1231,15 @@ error: * * Returns 0 in case of success, -1 in case of error. */ -virDrvOpenStatus +int xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { char *port = NULL; - int ret = VIR_DRV_OPEN_ERROR; + int ret = -1; - virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + virCheckFlags(VIR_CONNECT_RO, -1); /* Switch on the scheme, which we expect to be NULL (file), * "http" or "xen". @@ -1286,7 +1286,7 @@ xenDaemonOpen(virConnectPtr conn, } done: - ret = VIR_DRV_OPEN_SUCCESS; + ret = 0; failed: VIR_FREE(port); @@ -3652,7 +3652,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenClose = xenDaemonClose, .xenVersion = xenDaemonGetVersion, .xenDomainSuspend = xenDaemonDomainSuspend, .xenDomainResume = xenDaemonDomainResume, diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 06c75e1..e5c0896 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -95,8 +95,8 @@ xenDaemonDomainFetch(virConnectPtr xend, /* refactored ones */ -virDrvOpenStatus xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags); +int xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth, + unsigned int flags); int xenDaemonClose(virConnectPtr conn); int xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer); int xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 8580793..1b4d1cf 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 = { - .xenClose = xenXMClose, .xenDomainGetMaxMemory = xenXMDomainGetMaxMemory, .xenDomainSetMaxMemory = xenXMDomainSetMaxMemory, .xenDomainSetMemory = xenXMDomainSetMemory, @@ -419,14 +418,14 @@ xenXMConfigCacheRefresh(virConnectPtr conn) * us watch for changes (see separate driver), otherwise we poll * every few seconds */ -virDrvOpenStatus +int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { xenUnifiedPrivatePtr priv = conn->privateData; - virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + virCheckFlags(VIR_CONNECT_RO, -1); priv->configDir = XM_CONFIG_DIR; diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index df77ac8..6424c41 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -36,8 +36,8 @@ int xenXMConfigCacheRefresh (virConnectPtr conn); int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename); int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename); -virDrvOpenStatus xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags); +int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, + unsigned int flags); int xenXMClose(virConnectPtr conn); const char *xenXMGetType(virConnectPtr conn); int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 393e5f9..eecdcae 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -58,7 +58,6 @@ static void xenStoreWatchEvent(int watch, int fd, int events, void *data); static void xenStoreWatchListFree(xenStoreWatchListPtr list); struct xenUnifiedDriver xenStoreDriver = { - .xenClose = xenStoreClose, .xenDomainShutdown = xenStoreDomainShutdown, .xenDomainReboot = xenStoreDomainReboot, .xenDomainGetOSType = xenStoreDomainGetOSType, @@ -218,14 +217,14 @@ virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name) * * Returns 0 or -1 in case of error. */ -virDrvOpenStatus +int xenStoreOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { xenUnifiedPrivatePtr priv = conn->privateData; - virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + virCheckFlags(VIR_CONNECT_RO, -1); if (flags & VIR_CONNECT_RO) priv->xshandle = xs_daemon_open_readonly(); diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index 84d0d29..29f0165 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -29,7 +29,7 @@ extern struct xenUnifiedDriver xenStoreDriver; int xenStoreInit (void); -virDrvOpenStatus xenStoreOpen (virConnectPtr conn, +int xenStoreOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags); int xenStoreClose (virConnectPtr conn); -- 1.8.1.4

On Fri, May 03, 2013 at 04:12:29PM -0600, Jim Fehlig wrote: The xenInotifyDriver table had one callback in it - the 'close' method. Since we directly call xenInotifyClose, we don't need this in the driver table anymore. Yep, ok. Hah, yes, search + replace gone too far. I've audited the xenInotifyOpen method and the only reasons it would return -1 are all genuine fatal errors which we should having been honouring. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: Probably not likely, but what if xenStoreListDomains() failed, e.g. xshandle somehow became stale? The existing code would try the other drivers if xenstore one failed. Regards, Jim

On Mon, May 06, 2013 at 02:32:13PM -0600, Jim Fehlig wrote: The issue is that trying multiple drivers doesn't actually solve any inconsistencies - the first driver to succeed wins, even if it has stale data. You are right though that we did have some issues in the past - this is why with certain methods we twiddle it so that it would try XenStore before trying XenD, since XenStore was faster & more accurate. This cleanup as maintained this prioritization, so I don't think we're going to regress in this respect. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: Do you think there are any Xen installations running such an old xend toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code. I spent some time testing this one and didn't notice any problems. ACK. Regards, Jim

On Mon, May 06, 2013 at 09:40:43PM -0600, Jim Fehlig wrote: Opps, I made a mistake only checking the hypervisor, which of course will not know about the domain if it is shutoff :-) Adding the following extra hunk fixes it @@ -664,11 +664,21 @@ xenUnifiedDomainIsActive(virDomainPtr dom) int ret = -1; /* ID field in dom may be outdated, so re-lookup */ - currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); + currdom = xenHypervisorLookupDomainByUUID(conn, uuid); + + /* Try XM for inactive domains. */ + if (!currdom) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + currdom = xenXMDomainLookupByUUID(conn, uuid); + else + currdom = xenDaemonLookupByUUID(conn, uuid); + } if (currdom) { ret = currdom->id == -1 ? 0 : 1; virDomainFree(currdom); + } else if (virGetLastError() == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); } return ret; Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Daniel P. Berrange wrote: Yep, this fixes the issue. ACK with it squashed in. Regards, Jim

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: I wonder if the hypervisor driver code was ever used... ACK. Regards, Jim

Daniel P. Berrange wrote: ACK. Regards, Jim

Daniel P. Berrange wrote: Lots of nice cleanup in this series. ACK. Regards, Jim

Daniel P. Berrange wrote: Useful to add a note about removing the unused code in the xenstore driver, as you did in the other patches. ACK. Regards, Jim

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The xenGetHostname entry point in the xenUnifiedDriver table was unused. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 3ac2912..16e9743 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 { - virDrvConnectGetHostname xenGetHostname; virDrvDomainGetMaxMemory xenDomainGetMaxMemory; virDrvDomainSetMaxMemory xenDomainSetMaxMemory; virDrvDomainSetMemory xenDomainSetMemory; -- 1.8.1.4

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); -- 1.8.1.4

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, -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Unconditionally call the XenD APIs for save/restore, since that driver will always be open. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 21 ++++----------------- src/xen/xend_internal.c | 2 ++ 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 988dbce..a963e08 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -876,8 +876,6 @@ static int xenUnifiedDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, unsigned int flags) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - virCheckFlags(0, -1); if (dxml) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -885,9 +883,7 @@ xenUnifiedDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, return -1; } - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonDomainSave(dom, to); - return -1; + return xenDaemonDomainSave(dom, to); } static int @@ -923,8 +919,7 @@ xenUnifiedDomainManagedSave(virDomainPtr dom, unsigned int flags) if (!name) goto cleanup; - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - ret = xenDaemonDomainSave(dom, name); + ret = xenDaemonDomainSave(dom, name); cleanup: VIR_FREE(name); @@ -971,8 +966,6 @@ static int xenUnifiedDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, unsigned int flags) { - xenUnifiedPrivatePtr priv = conn->privateData; - virCheckFlags(0, -1); if (dxml) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -980,9 +973,7 @@ xenUnifiedDomainRestoreFlags(virConnectPtr conn, const char *from, return -1; } - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonDomainRestore(conn, from); - return -1; + return xenDaemonDomainRestore(conn, from); } static int @@ -994,11 +985,7 @@ xenUnifiedDomainRestore(virConnectPtr conn, const char *from) static int xenUnifiedDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonDomainCoreDump(dom, to, flags); - return -1; + return xenDaemonDomainCoreDump(dom, to, flags); } static int diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index d6a3698..ccbbf66 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1422,6 +1422,8 @@ xenDaemonDomainSave(virDomainPtr domain, const char *filename) /* We can't save the state of Domain-0, that would mean stopping it too */ if (domain->id == 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Cannot save host domain")); return -1; } -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 91 ++++++++++++++++++++---------------------------- src/xen/xen_driver.h | 2 -- src/xen/xen_hypervisor.c | 90 ----------------------------------------------- src/xen/xen_hypervisor.h | 3 -- src/xen/xend_internal.c | 25 +------------ src/xen/xm_internal.c | 6 ---- 6 files changed, 38 insertions(+), 179 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index a963e08..1db831d 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -993,7 +993,6 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int ret; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -1017,38 +1016,25 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, /* Try non-hypervisor methods first, then hypervisor direct method * as a last resort. */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags); - if (ret != -2) - return ret; - } - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { - ret = xenXMDomainSetVcpusFlags(dom, nvcpus, flags); - if (ret != -2) - return ret; - } - if (flags == VIR_DOMAIN_VCPU_LIVE) - return xenHypervisorSetVcpus(dom, nvcpus); - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainSetVcpusFlags(dom, nvcpus, flags); + else + return xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags); } static int xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { + xenUnifiedPrivatePtr priv = dom->conn->privateData; unsigned int flags = VIR_DOMAIN_VCPU_LIVE; /* Per the documented API, it is hypervisor-dependent whether this * affects just _LIVE or _LIVE|_CONFIG; in xen's case, that * depends on xendConfigVersion. */ - if (dom) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) - flags |= VIR_DOMAIN_VCPU_CONFIG; - return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags); - } - return -1; + if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) + flags |= VIR_DOMAIN_VCPU_CONFIG; + + return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags); } static int @@ -1056,15 +1042,15 @@ xenUnifiedDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, int maplen) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && - drivers[i]->xenDomainPinVcpu && - drivers[i]->xenDomainPinVcpu(dom, vcpu, cpumap, maplen) == 0) - return 0; - return -1; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainPinVcpu(dom, vcpu, cpumap, maplen); + else + return xenDaemonDomainPinVcpu(dom, vcpu, cpumap, maplen); + } else { + return xenHypervisorPinVcpu(dom, vcpu, cpumap, maplen); + } } static int @@ -1073,42 +1059,39 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, unsigned char *cpumaps, int maplen) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i, ret; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainGetVcpus) { - ret = drivers[i]->xenDomainGetVcpus(dom, info, maxinfo, cpumaps, maplen); - if (ret > 0) - return ret; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get VCPUs of inactive domain")); + return -1; + } else { + return xenDaemonDomainGetVcpus(dom, info, maxinfo, cpumaps, maplen); } - return -1; + } else { + return xenHypervisorGetVcpus(dom, info, maxinfo, cpumaps, maplen); + } } static int xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int ret; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonDomainGetVcpusFlags(dom, flags); - if (ret != -2) - return ret; - } - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { - ret = xenXMDomainGetVcpusFlags(dom, flags); - if (ret != -2) - return ret; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainGetVcpusFlags(dom, flags); + else + return xenDaemonDomainGetVcpusFlags(dom, flags); + } else { + if (flags == (VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM)) + return xenHypervisorGetVcpuMax(dom); + else + return xenDaemonDomainGetVcpusFlags(dom, flags); } - if (flags == (VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM)) - return xenHypervisorGetVcpuMax(dom); - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 953da64..4b18b4d 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -93,8 +93,6 @@ extern int xenRegister (void); * structure with direct calls in xen_unified.c. */ struct xenUnifiedDriver { - virDrvDomainPinVcpu xenDomainPinVcpu; - virDrvDomainGetVcpus xenDomainGetVcpus; virDrvConnectListDefinedDomains xenListDefinedDomains; virDrvConnectNumOfDefinedDomains xenNumOfDefinedDomains; virDrvDomainCreate xenDomainCreate; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 5735b0b..df2a93f 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -647,25 +647,6 @@ struct xen_v2d5_setmaxmem { typedef struct xen_v2d5_setmaxmem xen_v2d5_setmaxmem; /* - * The information for a setmaxvcpu system hypercall - */ -#define XEN_V0_OP_SETMAXVCPU 41 -#define XEN_V1_OP_SETMAXVCPU 41 -#define XEN_V2_OP_SETMAXVCPU 15 - -struct xen_v0_setmaxvcpu { - domid_t domain; - uint32_t maxvcpu; -}; -typedef struct xen_v0_setmaxvcpu xen_v0_setmaxvcpu; -typedef struct xen_v0_setmaxvcpu xen_v1_setmaxvcpu; - -struct xen_v2_setmaxvcpu { - uint32_t maxvcpu; -}; -typedef struct xen_v2_setmaxvcpu xen_v2_setmaxvcpu; - -/* * The information for a setvcpumap system hypercall * Note that between 1 and 2 the limitation to 64 physical CPU was lifted * hence the difference in structures @@ -814,7 +795,6 @@ struct xen_op_v0 { xen_v0_getdomaininfolistop getdomaininfolist; xen_v0_domainop domain; xen_v0_setmaxmem setmaxmem; - xen_v0_setmaxvcpu setmaxvcpu; xen_v0_setvcpumap setvcpumap; xen_v0_vcpuinfo getvcpuinfo; uint8_t padding[128]; @@ -846,7 +826,6 @@ struct xen_op_v2_dom { union { xen_v2_setmaxmem setmaxmem; xen_v2d5_setmaxmem setmaxmemd5; - xen_v2_setmaxvcpu setmaxvcpu; xen_v2_setvcpumap setvcpumap; xen_v2d5_setvcpumap setvcpumapd5; xen_v2_vcpuinfo getvcpuinfo; @@ -871,8 +850,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; #endif struct xenUnifiedDriver xenHypervisorDriver = { - .xenDomainPinVcpu = xenHypervisorPinVcpu, - .xenDomainGetVcpus = xenHypervisorGetVcpus, .xenDomainGetSchedulerType = xenHypervisorGetSchedulerType, .xenDomainGetSchedulerParameters = xenHypervisorGetSchedulerParameters, .xenDomainSetSchedulerParameters = xenHypervisorSetSchedulerParameters, @@ -1517,48 +1494,6 @@ virXen_setmaxmem(int handle, int id, unsigned long memory) return ret; } -/** - * virXen_setmaxvcpus: - * @handle: the hypervisor handle - * @id: the domain id - * @vcpus: the numbers of vcpus - * - * Do a low level hypercall to change the max vcpus amount - * - * Returns 0 or -1 in case of failure - */ -static int -virXen_setmaxvcpus(int handle, int id, unsigned int vcpus) -{ - int ret = -1; - - if (hv_versions.hypervisor > 1) { - xen_op_v2_dom op; - - memset(&op, 0, sizeof(op)); - op.cmd = XEN_V2_OP_SETMAXVCPU; - op.domain = (domid_t) id; - op.u.setmaxvcpu.maxvcpu = vcpus; - ret = xenHypervisorDoV2Dom(handle, &op); - } else if (hv_versions.hypervisor == 1) { - xen_op_v1 op; - - memset(&op, 0, sizeof(op)); - op.cmd = XEN_V1_OP_SETMAXVCPU; - op.u.setmaxvcpu.domain = (domid_t) id; - op.u.setmaxvcpu.maxvcpu = vcpus; - ret = xenHypervisorDoV1Op(handle, &op); - } else if (hv_versions.hypervisor == 0) { - xen_op_v0 op; - - memset(&op, 0, sizeof(op)); - op.cmd = XEN_V0_OP_SETMAXVCPU; - op.u.setmaxvcpu.domain = (domid_t) id; - op.u.setmaxvcpu.maxvcpu = vcpus; - ret = xenHypervisorDoV0Op(handle, &op); - } - return ret; -} /** * virXen_setvcpumap: @@ -3004,31 +2939,6 @@ xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory) /** - * xenHypervisorSetVcpus: - * @domain: pointer to domain object - * @nvcpus: the new number of virtual CPUs for this domain - * - * Dynamically change the number of virtual CPUs used by the domain. - * - * Returns 0 in case of success, -1 in case of failure. - */ - -int -xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus) -{ - int ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0 || nvcpus < 1) - return -1; - - ret = virXen_setmaxvcpus(priv->handle, domain->id, nvcpus); - if (ret < 0) - return -1; - return 0; -} - -/** * xenHypervisorPinVcpu: * @domain: pointer to domain object * @vcpu: virtual CPU number diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index b36fcca..bd36bf7 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -86,9 +86,6 @@ int xenHypervisorSetMaxMemory (virDomainPtr domain, ATTRIBUTE_NONNULL (1); int xenHypervisorCheckID (virConnectPtr conn, int id); -int xenHypervisorSetVcpus (virDomainPtr domain, - unsigned int nvcpus) - ATTRIBUTE_NONNULL (1); int xenHypervisorPinVcpu (virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index ccbbf66..dbad83f 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1787,7 +1787,6 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int flags) { char buf[VIR_UUID_BUFLEN]; - xenUnifiedPrivatePtr priv = domain->conn->privateData; int max; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | @@ -1799,21 +1798,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, return -1; } - if ((domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) || - (flags & VIR_DOMAIN_VCPU_MAXIMUM)) - return -2; - - /* With xendConfigVersion 2, only _LIVE is supported. With - * xendConfigVersion 3, only _LIVE|_CONFIG is supported for - * running domains, or _CONFIG for inactive domains. */ - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - if (flags & VIR_DOMAIN_VCPU_CONFIG) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Xend version does not support modifying " - "persistent config")); - return -1; - } - } else if (domain->id < 0) { + if (domain->id < 0) { if (flags & VIR_DOMAIN_VCPU_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain not running")); @@ -1951,17 +1936,11 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - /* If xendConfigVersion is 2, then we can only report _LIVE (and - * xm_internal reports _CONFIG). If it is 3, then _LIVE and - * _CONFIG are always in sync for a running system. */ - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -2; if (domain->id < 0 && (flags & VIR_DOMAIN_VCPU_LIVE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain not active")); @@ -3415,8 +3394,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainPinVcpu = xenDaemonDomainPinVcpu, - .xenDomainGetVcpus = xenDaemonDomainGetVcpus, .xenListDefinedDomains = xenDaemonListDefinedDomains, .xenNumOfDefinedDomains = xenDaemonNumOfDefinedDomains, .xenDomainCreate = xenDaemonDomainCreate, diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 38f401a..fed721a 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 = { - .xenDomainPinVcpu = xenXMDomainPinVcpu, .xenListDefinedDomains = xenXMListDefinedDomains, .xenNumOfDefinedDomains = xenXMNumOfDefinedDomains, .xenDomainCreate = xenXMDomainCreate, @@ -804,11 +803,6 @@ xenXMDomainPinVcpu(virDomainPtr domain, virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - if (domain->id != -1) { - virReportError(VIR_ERR_INVALID_ARG, - "%s", _("not inactive domain")); - return -1; - } xenUnifiedLock(priv); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The xenUnifiedDomainGetXMLDesc driver can assume that the XM and XenD drivers are always present Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 24 +++++++++--------------- src/xen/xend_internal.c | 6 ------ 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 1db831d..f11e5bf 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1106,23 +1106,17 @@ xenUnifiedDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - if (dom->id == -1 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) - return xenXMDomainGetXMLDesc(dom, flags); + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { + return xenXMDomainGetXMLDesc(dom, flags); } else { - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - char *cpus, *res; - xenUnifiedLock(priv); - cpus = xenDomainUsedCpus(dom); - xenUnifiedUnlock(priv); - res = xenDaemonDomainGetXMLDesc(dom, flags, cpus); - VIR_FREE(cpus); - return res; - } + char *cpus, *res; + xenUnifiedLock(priv); + cpus = xenDomainUsedCpus(dom); + xenUnifiedUnlock(priv); + res = xenDaemonDomainGetXMLDesc(dom, flags, cpus); + VIR_FREE(cpus); + return res; } - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return NULL; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index dbad83f..930b882 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1610,17 +1610,11 @@ xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; virDomainDefPtr def; char *xml; /* Flags checked by virDomainDefFormat */ - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - /* fall-through to the next driver to handle */ - return NULL; - } - if (!(def = xenDaemonDomainFetch(domain->conn, domain->id, domain->name, -- 1.8.1.4

On Wed, May 08, 2013 at 03:56:14PM -0600, Jim Fehlig wrote: The MigrateFinish API will always have a virDomainDefPtr which refers to a running domain, so this if() block would never be run when called from migration context. Hence it is safe to remove it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Daniel P. Berrange wrote: Ah, my comment in the previous patch referred to the call to xenDaemonDomainGetXMLDesc here. dom should be inactive at this point right? In which case xenDaemonDomainGetXMLDesc will return NULL if xendConfigVersion < 3_0_4. But again, not sure if the migration functions are meant to be supported in that environment. Patch looks good otherwise and weak ACK assuming the migration functions are for xendConfigVersion >= 3_0_4. Regards, Jim

Daniel P. Berrange wrote: Extra whitespace. Extra whitespace. ACK. Regards, Jim

Daniel P. Berrange wrote: Should this bit about updating the domain id be removed? Will the dom id remain at -1? If the id needs updated, it should probably be retrieved directly from xend. Regards, Jim > - return ret; > + return xend_op(domain->conn, domain->name, "op", "start", NULL); > } > > int > @@ -3378,7 +3361,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, > } > > struct xenUnifiedDriver xenDaemonDriver = { > - .xenDomainCreate = xenDaemonDomainCreate, > .xenDomainDefineXML = xenDaemonDomainDefineXML, > .xenDomainUndefine = xenDaemonDomainUndefine, > .xenDomainAttachDeviceFlags = xenDaemonAttachDeviceFlags, > diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c > index f5348bd..505f959 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 = { > - .xenDomainCreate = xenXMDomainCreate, > .xenDomainDefineXML = xenXMDomainDefineXML, > .xenDomainUndefine = xenXMDomainUndefine, > .xenDomainAttachDeviceFlags = xenXMDomainAttachDeviceFlags, > @@ -923,9 +922,6 @@ xenXMDomainCreate(virDomainPtr domain) > const char *filename; > xenXMConfCachePtr entry; > > - if (domain->id != -1) > - return -1; > - > xenUnifiedLock(priv); > > if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make the domain define/undefine driver methods directly call into either the XenD or XM drivers Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 24 ++++++++---------------- src/xen/xen_driver.h | 2 -- src/xen/xend_internal.c | 10 ---------- src/xen/xm_internal.c | 5 ----- 4 files changed, 8 insertions(+), 33 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index f6c1891..6643a97 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1369,31 +1369,23 @@ static virDomainPtr xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml) { xenUnifiedPrivatePtr priv = conn->privateData; - int i; - virDomainPtr ret; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainDefineXML) { - ret = drivers[i]->xenDomainDefineXML(conn, xml); - if (ret) return ret; - } - return NULL; + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainDefineXML(conn, xml); + else + return xenDaemonDomainDefineXML(conn, xml); } static int xenUnifiedDomainUndefineFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; virCheckFlags(0, -1); - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainUndefine && - drivers[i]->xenDomainUndefine(dom) == 0) - return 0; - - return -1; + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainUndefine(dom); + else + return xenDaemonDomainUndefine(dom); } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index e2c0d68..254c2f5 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -93,8 +93,6 @@ extern int xenRegister (void); * structure with direct calls in xen_unified.c. */ struct xenUnifiedDriver { - virDrvDomainDefineXML xenDomainDefineXML; - virDrvDomainUndefine xenDomainUndefine; virDrvDomainAttachDeviceFlags xenDomainAttachDeviceFlags; virDrvDomainDetachDeviceFlags xenDomainDetachDeviceFlags; virDrvDomainGetSchedulerType xenDomainGetSchedulerType; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index dcd31de..f9b43b8 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2860,9 +2860,6 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) xenUnifiedPrivatePtr priv = conn->privateData; virDomainDefPtr def; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return NULL; - if (!(def = virDomainDefParseString(xmlDesc, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, VIR_DOMAIN_XML_INACTIVE))) { @@ -2905,11 +2902,6 @@ xenDaemonDomainCreate(virDomainPtr domain) int xenDaemonDomainUndefine(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -1; - return xend_op(domain->conn, domain->name, "op", "delete", NULL); } @@ -3361,8 +3353,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainDefineXML = xenDaemonDomainDefineXML, - .xenDomainUndefine = xenDaemonDomainUndefine, .xenDomainAttachDeviceFlags = xenDaemonAttachDeviceFlags, .xenDomainDetachDeviceFlags = xenDaemonDetachDeviceFlags, .xenDomainGetSchedulerType = xenDaemonGetSchedulerType, diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 505f959..bb79c63 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -81,8 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, #define XM_XML_ERROR "Invalid xml" struct xenUnifiedDriver xenXMDriver = { - .xenDomainDefineXML = xenXMDomainDefineXML, - .xenDomainUndefine = xenXMDomainUndefine, .xenDomainAttachDeviceFlags = xenXMDomainAttachDeviceFlags, .xenDomainDetachDeviceFlags = xenXMDomainDetachDeviceFlags, }; @@ -1108,9 +1106,6 @@ xenXMDomainUndefine(virDomainPtr domain) xenXMConfCachePtr entry; int ret = -1; - if (domain->id != -1) - return -1; - xenUnifiedLock(priv); if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make the domain attach/dettach driver methods directly call into either the XenD or XM drivers Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 55 +++++++++++++++++-------------------------------- src/xen/xen_driver.h | 2 -- src/xen/xend_internal.c | 27 ++---------------------- src/xen/xend_internal.h | 6 ++++++ src/xen/xm_internal.c | 13 ++---------- src/xen/xm_internal.h | 10 +++++++-- 6 files changed, 37 insertions(+), 76 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 6643a97..2262713 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_XM_OFFSET] = &xenXMDriver, }; static bool inside_daemon = false; @@ -1397,7 +1396,6 @@ static int xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; /* @@ -1405,14 +1403,13 @@ xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml) * config without touching persistent config, we add the extra flag here * to make this API work */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET] && - priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) + if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainAttachDeviceFlags && - drivers[i]->xenDomainAttachDeviceFlags(dom, xml, flags) == 0) - return 0; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainAttachDeviceFlags(dom, xml, flags); + else + return xenDaemonAttachDeviceFlags(dom, xml, flags); return -1; } @@ -1422,21 +1419,17 @@ xenUnifiedDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainAttachDeviceFlags && - drivers[i]->xenDomainAttachDeviceFlags(dom, xml, flags) == 0) - return 0; - - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainAttachDeviceFlags(dom, xml, flags); + else + return xenDaemonAttachDeviceFlags(dom, xml, flags); } static int xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; /* @@ -1444,16 +1437,13 @@ xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml) * config without touching persistent config, we add the extra flag here * to make this API work */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET] && - priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) + if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainDetachDeviceFlags && - drivers[i]->xenDomainDetachDeviceFlags(dom, xml, flags) == 0) - return 0; - - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainDetachDeviceFlags(dom, xml, flags); + else + return xenDaemonDetachDeviceFlags(dom, xml, flags); } static int @@ -1461,25 +1451,18 @@ xenUnifiedDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->xenDomainDetachDeviceFlags && - drivers[i]->xenDomainDetachDeviceFlags(dom, xml, flags) == 0) - return 0; - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainDetachDeviceFlags(dom, xml, flags); + else + return xenDaemonDetachDeviceFlags(dom, xml, flags); } static int xenUnifiedDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonUpdateDeviceFlags(dom, xml, flags); - return -1; + return xenDaemonUpdateDeviceFlags(dom, xml, flags); } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 254c2f5..e8c2958 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -93,8 +93,6 @@ extern int xenRegister (void); * structure with direct calls in xen_unified.c. */ struct xenUnifiedDriver { - virDrvDomainAttachDeviceFlags xenDomainAttachDeviceFlags; - virDrvDomainDetachDeviceFlags xenDomainDetachDeviceFlags; virDrvDomainGetSchedulerType xenDomainGetSchedulerType; virDrvDomainGetSchedulerParameters xenDomainGetSchedulerParameters; virDrvDomainSetSchedulerParameters xenDomainSetSchedulerParameters; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index f9b43b8..49ef256 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2196,7 +2196,7 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) * * Returns 0 in case of success, -1 in case of failure. */ -static int +int xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) @@ -2219,13 +2219,6 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, _("Cannot modify live config if domain is inactive")); return -1; } - /* If xendConfigVersion < 3 only live config can be changed */ - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Xend version does not support modifying " - "persistent config")); - return -1; - } } else { /* Only live config can be changed if xendConfigVersion < 3 */ if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4 && @@ -2379,13 +2372,6 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, _("Cannot modify live config if domain is inactive")); return -1; } - /* If xendConfigVersion < 3 only live config can be changed */ - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Xend version does not support modifying " - "persistent config")); - return -1; - } } else { /* Only live config can be changed if xendConfigVersion < 3 */ if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4 && @@ -2464,7 +2450,7 @@ cleanup: * * Returns 0 in case of success, -1 in case of failure. */ -static int +int xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) @@ -2486,13 +2472,6 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, _("Cannot modify live config if domain is inactive")); return -1; } - /* If xendConfigVersion < 3 only live config can be changed */ - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Xend version does not support modifying " - "persistent config")); - return -1; - } } else { /* Only live config can be changed if xendConfigVersion < 3 */ if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4 && @@ -3353,8 +3332,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, } struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainAttachDeviceFlags = xenDaemonAttachDeviceFlags, - .xenDomainDetachDeviceFlags = xenDaemonDetachDeviceFlags, .xenDomainGetSchedulerType = xenDaemonGetSchedulerType, .xenDomainGetSchedulerParameters = xenDaemonGetSchedulerParameters, .xenDomainSetSchedulerParameters = xenDaemonSetSchedulerParameters, diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index d773ef9..4a8578b 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -114,6 +114,12 @@ int xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames); +int xenDaemonAttachDeviceFlags(virDomainPtr domain, + const char *xml, + unsigned int flags); +int xenDaemonDetachDeviceFlags(virDomainPtr domain, + const char *xml, + unsigned int flags); virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr); int xenDaemonDomainCreate(virDomainPtr domain); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index bb79c63..fa0392b 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -66,10 +66,6 @@ #define XEN_MAX_PHYSICAL_CPU 1024 char * xenXMAutoAssignMac(void); -static int xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, - unsigned int flags); -static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, - unsigned int flags); #define XM_REFRESH_INTERVAL 10 @@ -80,11 +76,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, #define QEMU_IF_SCRIPT "qemu-ifup" #define XM_XML_ERROR "Invalid xml" -struct xenUnifiedDriver xenXMDriver = { - .xenDomainAttachDeviceFlags = xenXMDomainAttachDeviceFlags, - .xenDomainDetachDeviceFlags = xenXMDomainDetachDeviceFlags, -}; - #ifndef WITH_XEN_INOTIFY static int xenInotifyActive(virConnectPtr conn ATTRIBUTE_UNUSED) { @@ -1241,7 +1232,7 @@ cleanup: * * Returns 0 in case of success, -1 in case of failure. */ -static int +int xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) @@ -1331,7 +1322,7 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, * * Returns 0 in case of success, -1 in case of failure. */ -static int +int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 257b663..731a126 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -30,8 +30,6 @@ # include "virconf.h" # include "domain_conf.h" -extern struct xenUnifiedDriver xenXMDriver; - int xenXMConfigCacheRefresh (virConnectPtr conn); int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename); int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename); @@ -70,4 +68,12 @@ int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long int xenXMDomainGetAutostart(virDomainPtr dom, int *autostart); int xenXMDomainSetAutostart(virDomainPtr dom, int autostart); +int xenXMDomainAttachDeviceFlags(virDomainPtr domain, + const char *xml, + unsigned int flags); + +int xenXMDomainDetachDeviceFlags(virDomainPtr domain, + const char *xml, + unsigned int flags); + #endif -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make the Xen domain scheduler parameter methods directly call into XenD or Xen hypervisor drivers Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 54 +++++++++++++++++++++--------------------------- src/xen/xen_driver.h | 15 -------------- src/xen/xen_hypervisor.c | 25 ---------------------- src/xen/xen_hypervisor.h | 1 - src/xen/xend_internal.c | 12 +++-------- src/xen/xend_internal.h | 12 +++++++---- 6 files changed, 35 insertions(+), 84 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 2262713..ac61677 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -80,12 +80,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, unsigned char *cpumaps, int maplen); -/* The five Xen drivers below us. */ -static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { - [XEN_UNIFIED_HYPERVISOR_OFFSET] = &xenHypervisorDriver, - [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver, -}; - static bool inside_daemon = false; /** @@ -1503,17 +1497,17 @@ static char * xenUnifiedDomainGetSchedulerType(virDomainPtr dom, int *nparams) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i; - char *schedulertype; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; i++) { - if (priv->opened[i] && drivers[i]->xenDomainGetSchedulerType) { - schedulertype = drivers[i]->xenDomainGetSchedulerType(dom, nparams); - if (schedulertype != NULL) - return schedulertype; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot change scheduler parameters")); + return NULL; } + return xenDaemonGetSchedulerType(dom, nparams); + } else { + return xenHypervisorGetSchedulerType(dom, nparams); } - return NULL; } static int @@ -1523,18 +1517,19 @@ xenUnifiedDomainGetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i, ret; virCheckFlags(0, -1); - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) { - if (priv->opened[i] && drivers[i]->xenDomainGetSchedulerParameters) { - ret = drivers[i]->xenDomainGetSchedulerParameters(dom, params, nparams); - if (ret == 0) - return 0; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot change scheduler parameters")); + return -1; } + return xenDaemonGetSchedulerParameters(dom, params, nparams); + } else { + return xenHypervisorGetSchedulerParameters(dom, params, nparams); } - return -1; } static int @@ -1553,20 +1548,19 @@ xenUnifiedDomainSetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - int i, ret; virCheckFlags(0, -1); - /* do the hypervisor call last to get better error */ - for (i = XEN_UNIFIED_NR_DRIVERS - 1; i >= 0; i--) { - if (priv->opened[i] && drivers[i]->xenDomainSetSchedulerParameters) { - ret = drivers[i]->xenDomainSetSchedulerParameters(dom, params, nparams); - if (ret == 0) - return 0; + if (dom->id < 0) { + if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot change scheduler parameters")); + return -1; } + return xenDaemonSetSchedulerParameters(dom, params, nparams); + } else { + return xenHypervisorSetSchedulerParameters(dom, params, nparams); } - - return -1; } static int diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index e8c2958..e33610d 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -82,21 +82,6 @@ extern int xenRegister (void); VIR_MIGRATE_PAUSED | \ VIR_MIGRATE_PERSIST_DEST) -/* _xenUnifiedDriver: - * - * Entry points into the underlying Xen drivers. This structure - * will eventually go away and instead xen unified will make direct - * calls to the underlying Xen drivers. - * - * To reiterate - the goal is to remove elements from this structure - * until it is empty, replacing indirect calls through this - * structure with direct calls in xen_unified.c. - */ -struct xenUnifiedDriver { - virDrvDomainGetSchedulerType xenDomainGetSchedulerType; - virDrvDomainGetSchedulerParameters xenDomainGetSchedulerParameters; - virDrvDomainSetSchedulerParameters xenDomainSetSchedulerParameters; -}; typedef struct xenXMConfCache *xenXMConfCachePtr; typedef struct xenXMConfCache { diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index df2a93f..d7b7cfc 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -849,12 +849,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; # error "unsupported platform" #endif -struct xenUnifiedDriver xenHypervisorDriver = { - .xenDomainGetSchedulerType = xenHypervisorGetSchedulerType, - .xenDomainGetSchedulerParameters = xenHypervisorGetSchedulerParameters, - .xenDomainSetSchedulerParameters = xenHypervisorSetSchedulerParameters, -}; - /** * xenHypervisorDoV0Op: * @handle: the handle to the Xen hypervisor @@ -1124,12 +1118,6 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams) char *schedulertype = NULL; xenUnifiedPrivatePtr priv = domain->conn->privateData; - if (domain->id < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - return NULL; - } - /* * Support only hv_versions.dom_interface >=5 * (Xen3.1.0 or later) @@ -1194,13 +1182,6 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - return -1; - } - /* * Support only hv_versions.dom_interface >=5 * (Xen3.1.0 or later) @@ -1303,12 +1284,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, NULL) < 0) return -1; - if (domain->id < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - return -1; - } - /* * Support only hv_versions.dom_interface >=5 * (Xen3.1.0 or later) diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index bd36bf7..8507bd0 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -36,7 +36,6 @@ struct xenHypervisorVersions { int dom_interface; /* -1,3,4,5,6,7 */ }; -extern struct xenUnifiedDriver xenHypervisorDriver; int xenHypervisorInit(struct xenHypervisorVersions *override_versions); virCapsPtr xenHypervisorMakeCapabilities (virConnectPtr conn); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 49ef256..87b8875 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2975,7 +2975,7 @@ error: * Returns a scheduler name (credit or sedf) which must be freed by the * caller or NULL in case of failure */ -static char * +char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) { xenUnifiedPrivatePtr priv = domain->conn->privateData; @@ -3040,7 +3040,7 @@ error: * * Returns 0 or -1 in case of failure */ -static int +int xenDaemonGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) @@ -3142,7 +3142,7 @@ error: * * Returns 0 or -1 in case of failure */ -static int +int xenDaemonSetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams) @@ -3331,12 +3331,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, return ret; } -struct xenUnifiedDriver xenDaemonDriver = { - .xenDomainGetSchedulerType = xenDaemonGetSchedulerType, - .xenDomainGetSchedulerParameters = xenDaemonGetSchedulerParameters, - .xenDomainSetSchedulerParameters = xenDaemonSetSchedulerParameters, -}; - /** * virDomainXMLDevID: diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 4a8578b..7332303 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -148,10 +148,6 @@ int xenDaemonDomainGetAutostart (virDomainPtr dom, int xenDaemonDomainSetAutostart (virDomainPtr domain, int autostart); -/* xen_unified calls through here. */ -extern struct xenUnifiedDriver xenDaemonDriver; -int xenDaemonInit (void); - virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc); virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); @@ -160,4 +156,12 @@ int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer); +char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams); +int xenDaemonGetSchedulerParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams); +int xenDaemonSetSchedulerParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams); + #endif /* __XEND_INTERNAL_H_ */ -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Unconditionally call into the XenD or XM drivers for autostart handling, since they are guaranteed to be open --- src/xen/xen_driver.c | 18 ++++-------------- src/xen/xend_internal.c | 14 -------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index ac61677..1941dbe 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1465,15 +1465,10 @@ xenUnifiedDomainGetAutostart(virDomainPtr dom, int *autostart) xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) - return xenXMDomainGetAutostart(dom, autostart); + return xenXMDomainGetAutostart(dom, autostart); } else { - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonDomainGetAutostart(dom, autostart); + return xenDaemonDomainGetAutostart(dom, autostart); } - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; } static int @@ -1482,15 +1477,10 @@ xenUnifiedDomainSetAutostart(virDomainPtr dom, int autostart) xenUnifiedPrivatePtr priv = dom->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) - return xenXMDomainSetAutostart(dom, autostart); + return xenXMDomainSetAutostart(dom, autostart); } else { - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) - return xenDaemonDomainSetAutostart(dom, autostart); + return xenDaemonDomainSetAutostart(dom, autostart); } - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; } static char * diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 87b8875..ad69b47 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2541,13 +2541,6 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, int *autostart) { struct sexpr *root; const char *tmp; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - /* xm_internal.c (the support for defined domains from /etc/xen - * config files used by old Xen) will handle this. - */ - if (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) { @@ -2574,13 +2567,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - /* xm_internal.c (the support for defined domains from /etc/xen - * config files used by old Xen) will handle this. - */ - if (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) { -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make the Xen domain stats / peek and node memory driver methods unconditionally call the sub-drivers which are guaranteed to be open. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 59 +++++++++---------------------------------------- src/xen/xend_internal.c | 3 --- 2 files changed, 11 insertions(+), 51 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 1941dbe..d6817eb 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1566,26 +1566,14 @@ static int xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path, struct _virDomainBlockStats *stats) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) - return xenHypervisorDomainBlockStats(dom, path, stats); - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + return xenHypervisorDomainBlockStats(dom, path, stats); } static int xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) - return xenHypervisorDomainInterfaceStats(dom, path, stats); - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + return xenHypervisorDomainInterfaceStats(dom, path, stats); } static int @@ -1593,57 +1581,32 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer, unsigned int flags) { - int r; xenUnifiedPrivatePtr priv = dom->conn->privateData; virCheckFlags(0, -1); - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - r = xenDaemonDomainBlockPeek(dom, path, offset, size, buffer); - if (r != -2) return r; - /* r == -2 means declined, so fall through to XM driver ... */ - } - - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { - if (xenXMDomainBlockPeek(dom, path, offset, size, buffer) == 0) - return 0; - } - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) + return xenXMDomainBlockPeek(dom, path, offset, size, buffer); + else + return xenDaemonDomainBlockPeek(dom, path, offset, size, buffer); } static int xenUnifiedNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, int startCell, int maxCells) { - xenUnifiedPrivatePtr priv = conn->privateData; - - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) - return xenHypervisorNodeGetCellsFreeMemory(conn, freeMems, - startCell, maxCells); - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + return xenHypervisorNodeGetCellsFreeMemory(conn, freeMems, + startCell, maxCells); } static unsigned long long xenUnifiedNodeGetFreeMemory(virConnectPtr conn) { unsigned long long freeMem = 0; - int ret; - xenUnifiedPrivatePtr priv = conn->privateData; - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { - ret = xenHypervisorNodeGetCellsFreeMemory(conn, &freeMem, - -1, 1); - if (ret != 1) - return 0; - return freeMem; - } - - virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return 0; + if (xenHypervisorNodeGetCellsFreeMemory(conn, &freeMem, -1, 1) < 0) + return 0; + return freeMem; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index ad69b47..ba4a018 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3249,9 +3249,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, int vncport; const char *actual; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return -2; /* Decline, allow XM to handle it. */ - /* Security check: The path must correspond to a block device. */ if (domain->id > 0) root = sexpr_get(domain->conn, "/xend/domain/%d?detail=1", -- 1.8.1.4

On Wed, May 08, 2013 at 05:33:13PM -0600, Jim Fehlig wrote: Yep, there were a couple more comments to be removed in the domain vcpu patch, which I've now modified. Thanks for all the reviews so far ! I'm going to push the first 29 patches now. I've tested basic operations and things seem to be working sanely to me. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain lookup APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 23 +++++++++ src/conf/domain_conf.h | 4 ++ src/libvirt_private.syms | 1 + src/xen/xen_driver.c | 132 +++++++++++++++++++++++++++++++++-------------- src/xen/xen_hypervisor.c | 17 +++--- src/xen/xen_hypervisor.h | 8 +-- src/xen/xen_inotify.c | 14 ++--- src/xen/xend_internal.c | 28 ++++------ src/xen/xend_internal.h | 4 +- src/xen/xm_internal.c | 30 ++++------- src/xen/xm_internal.h | 5 +- 11 files changed, 165 insertions(+), 101 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d945b74..1fef6db 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2048,6 +2048,29 @@ error: return NULL; } + +virDomainDefPtr virDomainDefNew(const char *name, + const unsigned char *uuid, + int id) +{ + virDomainDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (!(def->name = strdup(name))) { + VIR_FREE(def); + return NULL; + } + + memcpy(def->uuid, uuid, VIR_UUID_BUFLEN); + def->id = id; + + return def; +} + void virDomainObjAssignDef(virDomainObjPtr domain, const virDomainDefPtr def, bool live, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a0f23a..d5d05f7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2148,6 +2148,10 @@ void virDomainDefFree(virDomainDefPtr vm); virDomainChrDefPtr virDomainChrDefNew(void); +virDomainDefPtr virDomainDefNew(const char *name, + const unsigned char *uuid, + int id); + enum { VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 << 0), VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE = (1 << 1), diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 89b65b5..3b625a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -117,6 +117,7 @@ virDomainDefGenSecurityLabelDef; virDomainDefGetDefaultEmulator; virDomainDefGetSecurityLabelDef; virDomainDefMaybeAddController; +virDomainDefNew; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index d6817eb..86df4b6 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -82,6 +82,58 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, static bool inside_daemon = false; +static virDomainDefPtr xenGetDomainDefForID(virConnectPtr conn, int id) +{ + virDomainDefPtr ret; + + ret = xenHypervisorLookupDomainByID(conn, id); + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + +static virDomainDefPtr xenGetDomainDefForName(virConnectPtr conn, const char *name) +{ + xenUnifiedPrivatePtr priv = conn->privateData; + virDomainDefPtr ret; + + ret = xenDaemonLookupByName(conn, name); + + /* Try XM for inactive domains. */ + if (!ret && + priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + ret = xenXMDomainLookupByName(conn, name); + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + +static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const unsigned char *uuid) +{ + xenUnifiedPrivatePtr priv = conn->privateData; + virDomainDefPtr ret; + + ret = xenHypervisorLookupDomainByUUID(conn, uuid); + + /* Try XM for inactive domains. */ + if (!ret) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + ret = xenXMDomainLookupByUUID(conn, uuid); + else + ret = xenDaemonLookupByUUID(conn, uuid); + } + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + + /** * xenNumaInit: * @conn: pointer to the hypervisor connection @@ -597,12 +649,18 @@ static virDomainPtr xenUnifiedDomainLookupByID(virConnectPtr conn, int id) { virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenHypervisorLookupDomainByID(conn, id); + if (!(def = xenGetDomainDefForID(conn, id))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; + + ret->id = def->id; +cleanup: + virDomainDefFree(def); return ret; } @@ -610,22 +668,19 @@ static virDomainPtr xenUnifiedDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenHypervisorLookupDomainByUUID(conn, uuid); + if (!(def = xenGetDomainDefForUUID(conn, uuid))) + goto cleanup; - /* Try XM for inactive domains. */ - if (!ret) { - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) - ret = xenXMDomainLookupByUUID(conn, uuid); - else - return xenDaemonLookupByUUID(conn, uuid); - } + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + ret->id = def->id; +cleanup: + virDomainDefFree(def); return ret; } @@ -633,18 +688,19 @@ static virDomainPtr xenUnifiedDomainLookupByName(virConnectPtr conn, const char *name) { - xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenDaemonLookupByName(conn, name); + if (!(def = xenGetDomainDefForName(conn, name))) + goto cleanup; - /* Try XM for inactive domains. */ - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) - ret = xenXMDomainLookupByName(conn, name); + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + ret->id = def->id; +cleanup: + virDomainDefFree(def); return ret; } @@ -652,15 +708,14 @@ xenUnifiedDomainLookupByName(virConnectPtr conn, static int xenUnifiedDomainIsActive(virDomainPtr dom) { - virDomainPtr currdom; + virDomainDefPtr def; int ret = -1; - /* ID field in dom may be outdated, so re-lookup */ - currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); + def = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); - if (currdom) { - ret = currdom->id == -1 ? 0 : 1; - virDomainFree(currdom); + if (def) { + ret = def->id == -1 ? 0 : 1; + virDomainDefFree(def); } return ret; @@ -670,22 +725,22 @@ static int xenUnifiedDomainIsPersistent(virDomainPtr dom) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - virDomainPtr currdom = NULL; + virDomainDefPtr def = NULL; int ret = -1; if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { /* Old Xen, pre-inactive domain management. * If the XM driver can see the guest, it is definitely persistent */ - currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid); - if (currdom) + def = xenXMDomainLookupByUUID(dom->conn, dom->uuid); + if (def) ret = 1; else ret = 0; } else { /* New Xen with inactive domain management */ - currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid); - if (currdom) { - if (currdom->id == -1) { + def = xenDaemonLookupByUUID(dom->conn, dom->uuid); + if (def) { + if (def->id == -1) { /* If its inactive, then trivially, it must be persistent */ ret = 1; } else { @@ -697,7 +752,7 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom) virUUIDFormat(dom->uuid, uuidstr); if (virAsprintf(&path, "%s/%s", XEND_DOMAINS_DIR, uuidstr) < 0) { virReportOOMError(); - goto done; + goto cleanup; } if (access(path, R_OK) == 0) ret = 1; @@ -707,9 +762,8 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom) } } -done: - if (currdom) - virDomainFree(currdom); +cleanup: + virDomainDefFree(def); return ret; } diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index d7b7cfc..1643091 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2562,12 +2562,13 @@ xenHypervisorHasDomain(virConnectPtr conn, int id) return 1; } -virDomainPtr + +virDomainDefPtr xenHypervisorLookupDomainByID(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv = conn->privateData; xen_getdomaininfo dominfo; - virDomainPtr ret; + virDomainDefPtr ret; char *name; XEN_GETDOMAININFO_CLEAR(dominfo); @@ -2584,20 +2585,20 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id) if (!name) return NULL; - ret = virGetDomain(conn, name, XEN_GETDOMAININFO_UUID(dominfo)); - if (ret) - ret->id = id; + ret = virDomainDefNew(name, + XEN_GETDOMAININFO_UUID(dominfo), + id); VIR_FREE(name); return ret; } -virDomainPtr +virDomainDefPtr xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { xen_getdomaininfolist dominfos; xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainDefPtr ret; char *name; int maxids = 100, nids, i, id; @@ -2648,7 +2649,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) if (!name) return NULL; - ret = virGetDomain(conn, name, uuid); + ret = virDomainDefNew(name, uuid, id); if (ret) ret->id = id; VIR_FREE(name); diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 8507bd0..1d44a92 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -27,6 +27,7 @@ # include "capabilities.h" # include "driver.h" # include "viruri.h" +# include "domain_conf.h" /* See xenHypervisorInit() for details. */ struct xenHypervisorVersions { @@ -43,10 +44,9 @@ virCapsPtr xenHypervisorMakeCapabilities (virConnectPtr conn); int xenHypervisorHasDomain(virConnectPtr conn, int id); -virDomainPtr - xenHypervisorLookupDomainByID (virConnectPtr conn, - int id); -virDomainPtr +virDomainDefPtr + xenHypervisorLookupDomainByID (virConnectPtr conn, int id); +virDomainDefPtr xenHypervisorLookupDomainByUUID (virConnectPtr conn, const unsigned char *uuid); char * diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index b032bba..576eb10 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -76,7 +76,7 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, unsigned char *uuid) { int i; - virDomainPtr dom; + virDomainDefPtr def; const char *uuid_str; unsigned char rawuuid[VIR_UUID_BUFLEN]; xenUnifiedPrivatePtr priv = conn->privateData; @@ -96,8 +96,8 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, be set during open while we are building our initial list of domains */ VIR_DEBUG("Looking for dom with uuid: %s", uuid_str); - /* XXX Should not have to go via a virDomainPtr obj instance */ - if (!(dom = xenDaemonLookupByUUID(conn, rawuuid))) { + + if (!(def = xenDaemonLookupByUUID(conn, rawuuid))) { /* If we are here, the domain has gone away. search for, and create a domain from the stored list info */ @@ -118,13 +118,13 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, return -1; } - if (!(*name = strdup(dom->name))) { + if (!(*name = strdup(def->name))) { virReportOOMError(); - virDomainFree(dom); + virDomainDefFree(def); return -1; } - memcpy(uuid, dom->uuid, VIR_UUID_BUFLEN); - virDomainFree(dom); + memcpy(uuid, def->uuid, VIR_UUID_BUFLEN); + virDomainDefFree(def); /* succeeded too find domain by uuid */ return 0; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index ba4a018..1d5ebbb 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1113,13 +1113,14 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) * * Returns the domain pointer or NULL in case of error. */ -static virDomainPtr +static virDomainDefPtr sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) { - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; const char *name; const char *tmp; + int id = -1; xenUnifiedPrivatePtr priv = conn->privateData; if (sexpr_uuid(uuid, root, "domain/uuid") < 0) @@ -1128,9 +1129,6 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) if (name == NULL) goto error; - ret = virGetDomain(conn, name, uuid); - if (ret == NULL) return NULL; - tmp = sexpr_node(root, "domain/domid"); /* New 3.0.4 XenD will not report a domid for inactive domains, * so only error out for old XenD @@ -1139,11 +1137,9 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) goto error; if (tmp) - ret->id = sexpr_int(root, "domain/domid"); - else - ret->id = -1; /* An inactive domain */ + id = sexpr_int(root, "domain/domid"); - return ret; + return virDomainDefNew(name, uuid, id); error: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1696,11 +1692,11 @@ xenDaemonDomainGetState(virDomainPtr domain, * * Returns domain info on success; NULL (with errno) on error */ -virDomainPtr +virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname) { struct sexpr *root; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; root = sexpr_get(conn, "/xend/domain/%s?detail=1", domname); if (root == NULL) @@ -2051,10 +2047,10 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, * * Returns a new domain object or NULL in case of failure */ -virDomainPtr +virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - virDomainPtr ret; + virDomainDefPtr ret; char *name = NULL; int id = -1; xenUnifiedPrivatePtr priv = conn->privateData; @@ -2114,12 +2110,8 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (name == NULL) return NULL; - ret = virGetDomain(conn, name, uuid); - if (ret == NULL) goto cleanup; - - ret->id = id; + ret = virDomainDefNew(name, uuid, id); - cleanup: VIR_FREE(name); return ret; } diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 7332303..a2d05f3 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -149,8 +149,8 @@ int xenDaemonDomainSetAutostart (virDomainPtr domain, int autostart); virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc); -virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); -virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); +virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); +virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); int xenDaemonDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookielen, const char *uri_in, char **uri_out, unsigned long flags, const char *dname, unsigned long resource); int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long resource); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fa0392b..880cdeb 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -819,13 +819,13 @@ xenXMDomainPinVcpu(virDomainPtr domain, /* * Find an inactive domain based on its name */ -virDomainPtr +virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) { xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; xenUnifiedLock(priv); @@ -838,12 +838,7 @@ xenXMDomainLookupByName(virConnectPtr conn, const char *domname) if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; - if (!(ret = virGetDomain(conn, domname, entry->def->uuid))) - goto cleanup; - - /* Ensure its marked inactive, because may be cached - handle to a previously active domain */ - ret->id = -1; + ret = virDomainDefNew(domname, entry->def->uuid, -1); cleanup: xenUnifiedUnlock(priv); @@ -871,12 +866,12 @@ xenXMDomainSearchForUUID(const void *payload, /* * Find an inactive domain based on its UUID */ -virDomainPtr +virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; xenUnifiedLock(priv); @@ -886,12 +881,7 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid))) goto cleanup; - if (!(ret = virGetDomain(conn, entry->def->name, uuid))) - goto cleanup; - - /* Ensure its marked inactive, because may be cached - handle to a previously active domain */ - ret->id = -1; + ret = virDomainDefNew(entry->def->name, uuid, -1); cleanup: xenUnifiedUnlock(priv); @@ -1134,7 +1124,7 @@ struct xenXMListIteratorContext { static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { struct xenXMListIteratorContext *ctx = data; - virDomainPtr dom = NULL; + virDomainDefPtr def = NULL; if (ctx->oom) return; @@ -1142,14 +1132,14 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) if (ctx->count == ctx->max) return; - dom = xenDaemonLookupByName(ctx->conn, name); - if (!dom) { + def = xenDaemonLookupByName(ctx->conn, name); + if (!def) { if (!(ctx->names[ctx->count] = strdup(name))) ctx->oom = 1; else ctx->count++; } else { - virDomainFree(dom); + virDomainDefFree(def); } } diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 731a126..95f8283 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -52,9 +52,8 @@ int xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, int xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags); int xenXMDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen); -virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname); -virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid); +virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname); +virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid); int xenXMListDefinedDomains(virConnectPtr conn, char ** const names, int maxnames); int xenXMNumOfDefinedDomains(virConnectPtr conn); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain lifecycle APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 67 +++++++++++++++++++++++++++++++++++++++++++++---- src/xen/xend_internal.c | 60 ++++++++++++++++++++++--------------------- src/xen/xend_internal.h | 10 ++++---- src/xen/xm_internal.c | 8 +++--- 4 files changed, 103 insertions(+), 42 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 86df4b6..c5a4592 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -134,6 +134,13 @@ static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const unsigned } +static virDomainDefPtr xenGetDomainDefForDom(virDomainPtr dom) +{ + /* UUID lookup is more efficient than name lookup */ + return xenGetDomainDefForUUID(dom->conn, dom->uuid); +} + + /** * xenNumaInit: * @conn: pointer to the hypervisor connection @@ -777,22 +784,52 @@ xenUnifiedDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) static int xenUnifiedDomainSuspend(virDomainPtr dom) { - return xenDaemonDomainSuspend(dom); + int ret = -1; + virDomainDefPtr def; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainSuspend(dom->conn, def); + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainResume(virDomainPtr dom) { - return xenDaemonDomainResume(dom); + int ret = -1; + virDomainDefPtr def; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainResume(dom->conn, def); + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { + int ret = -1; + virDomainDefPtr def; + virCheckFlags(0, -1); - return xenDaemonDomainShutdown(dom); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainShutdown(dom->conn, def); + +cleanup: + virDomainDefFree(def); + return ret; } static int @@ -804,18 +841,38 @@ xenUnifiedDomainShutdown(virDomainPtr dom) static int xenUnifiedDomainReboot(virDomainPtr dom, unsigned int flags) { + int ret = -1; + virDomainDefPtr def; + virCheckFlags(0, -1); - return xenDaemonDomainReboot(dom); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainReboot(dom->conn, def); + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { + int ret = -1; + virDomainDefPtr def; + virCheckFlags(0, -1); - return xenDaemonDomainDestroy(dom); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainDestroy(dom->conn, def); + +cleanup: + virDomainDefFree(def); + return ret; } static int diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 1d5ebbb..25841ec 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1251,7 +1251,8 @@ xenDaemonClose(virConnectPtr conn ATTRIBUTE_UNUSED) /** * xenDaemonDomainSuspend: - * @domain: pointer to the Domain block + * @conn: the connection object + * @def: the domain to suspend * * Pause the domain, the domain is not scheduled anymore though its resources * are preserved. Use xenDaemonDomainResume() to resume execution. @@ -1259,41 +1260,42 @@ xenDaemonClose(virConnectPtr conn ATTRIBUTE_UNUSED) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainSuspend(virDomainPtr domain) +xenDaemonDomainSuspend(virConnectPtr conn, virDomainDefPtr def) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } - return xend_op(domain->conn, domain->name, "op", "pause", NULL); + return xend_op(conn, def->name, "op", "pause", NULL); } /** * xenDaemonDomainResume: - * @xend: pointer to the Xen Daemon block - * @name: name for the domain + * @conn: the connection object + * @def: the domain to resume * * Resume the domain after xenDaemonDomainSuspend() has been called * * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainResume(virDomainPtr domain) +xenDaemonDomainResume(virConnectPtr conn, virDomainDefPtr def) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } - return xend_op(domain->conn, domain->name, "op", "unpause", NULL); + return xend_op(conn, def->name, "op", "unpause", NULL); } /** * xenDaemonDomainShutdown: - * @domain: pointer to the Domain block + * @conn: the connection object + * @def: the domain to shutdown * * Shutdown the domain, the OS is requested to properly shutdown * and the domain may ignore it. It will return immediately @@ -1302,20 +1304,21 @@ xenDaemonDomainResume(virDomainPtr domain) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainShutdown(virDomainPtr domain) +xenDaemonDomainShutdown(virConnectPtr conn, virDomainDefPtr def) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } - return xend_op(domain->conn, domain->name, "op", "shutdown", "reason", "poweroff", NULL); + return xend_op(conn, def->name, "op", "shutdown", "reason", "poweroff", NULL); } /** * xenDaemonDomainReboot: - * @domain: pointer to the Domain block + * @conn: the connection object + * @def: the domain to reboot * * Reboot the domain, the OS is requested to properly shutdown * and restart but the domain may ignore it. It will return immediately @@ -1324,20 +1327,21 @@ xenDaemonDomainShutdown(virDomainPtr domain) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainReboot(virDomainPtr domain) +xenDaemonDomainReboot(virConnectPtr conn, virDomainDefPtr def) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } - return xend_op(domain->conn, domain->name, "op", "shutdown", "reason", "reboot", NULL); + return xend_op(conn, def->name, "op", "shutdown", "reason", "reboot", NULL); } /** * xenDaemonDomainDestroy: - * @domain: pointer to the Domain block + * @conn: the connection object + * @def: the domain to destroy * * Abruptly halt the domain, the OS is not properly shutdown and the * resources allocated for the domain are immediately freed, mounted @@ -1349,15 +1353,15 @@ xenDaemonDomainReboot(virDomainPtr domain) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainDestroy(virDomainPtr domain) +xenDaemonDomainDestroy(virConnectPtr conn, virDomainDefPtr def) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } - return xend_op(domain->conn, domain->name, "op", "destroy", NULL); + return xend_op(conn, def->name, "op", "destroy", NULL); } /** @@ -2161,7 +2165,7 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) if (xend_wait_for_devices(conn, def->name) < 0) goto error; - if (xenDaemonDomainResume(dom) < 0) + if (xenDaemonDomainResume(conn, def) < 0) goto error; virDomainDefFree(def); @@ -2170,7 +2174,7 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) error: /* Make sure we don't leave a still-born domain around */ if (dom != NULL) { - xenDaemonDomainDestroy(dom); + xenDaemonDomainDestroy(conn, def); virObjectUnref(dom); } virDomainDefFree(def); diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index a2d05f3..4a6b406 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -87,11 +87,11 @@ int xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth, int xenDaemonClose(virConnectPtr conn); int xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); int xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps); -int xenDaemonDomainSuspend(virDomainPtr domain); -int xenDaemonDomainResume(virDomainPtr domain); -int xenDaemonDomainShutdown(virDomainPtr domain); -int xenDaemonDomainReboot(virDomainPtr domain); -int xenDaemonDomainDestroy(virDomainPtr domain); +int xenDaemonDomainSuspend(virConnectPtr conn, virDomainDefPtr def); +int xenDaemonDomainResume(virConnectPtr conn, virDomainDefPtr def); +int xenDaemonDomainShutdown(virConnectPtr conn, virDomainDefPtr def); +int xenDaemonDomainReboot(virConnectPtr conn, virDomainDefPtr def); +int xenDaemonDomainDestroy(virConnectPtr conn, virDomainDefPtr def); int xenDaemonDomainSave(virDomainPtr domain, const char *filename); int xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, unsigned int flags); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 880cdeb..99df922 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -899,7 +899,7 @@ xenXMDomainCreate(virDomainPtr domain) int ret = -1; xenUnifiedPrivatePtr priv= domain->conn->privateData; const char *filename; - xenXMConfCachePtr entry; + xenXMConfCachePtr entry = NULL; xenUnifiedLock(priv); @@ -925,15 +925,15 @@ xenXMDomainCreate(virDomainPtr domain) if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if (xenDaemonDomainResume(domain) < 0) + if (xenDaemonDomainResume(domain->conn, entry->def) < 0) goto error; xenUnifiedUnlock(priv); return 0; error: - if (domain->id != -1) { - xenDaemonDomainDestroy(domain); + if (domain->id != -1 && entry) { + xenDaemonDomainDestroy(domain->conn, entry->def); domain->id = -1; } xenUnifiedUnlock(priv); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain save APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 72 ++++++++++++++++++++++++++++++++++++------------- src/xen/xend_internal.c | 17 +++++++----- src/xen/xend_internal.h | 7 +++-- src/xen/xm_internal.c | 25 ++++++++--------- src/xen/xm_internal.h | 3 ++- 5 files changed, 83 insertions(+), 41 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 4f95aeb..595c5c9 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1034,14 +1034,25 @@ static int xenUnifiedDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, unsigned int flags) { + int ret = -1; + virDomainDefPtr def; + virCheckFlags(0, -1); + if (dxml) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("xml modification unsupported")); return -1; } - return xenDaemonDomainSave(dom, to); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenDaemonDomainSave(dom->conn, def, to); + +cleanup: + virDomainDefFree(def); + return ret; } static int @@ -1051,11 +1062,12 @@ xenUnifiedDomainSave(virDomainPtr dom, const char *to) } static char * -xenUnifiedDomainManagedSavePath(xenUnifiedPrivatePtr priv, virDomainPtr dom) +xenUnifiedDomainManagedSavePath(xenUnifiedPrivatePtr priv, + virDomainDefPtr def) { char *ret; - if (virAsprintf(&ret, "%s/%s.save", priv->saveDir, dom->name) < 0) { + if (virAsprintf(&ret, "%s/%s.save", priv->saveDir, def->name) < 0) { virReportOOMError(); return NULL; } @@ -1068,19 +1080,23 @@ static int xenUnifiedDomainManagedSave(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - char *name; + char *name = NULL; + virDomainDefPtr def = NULL; int ret = -1; virCheckFlags(0, -1); - name = xenUnifiedDomainManagedSavePath(priv, dom); - if (!name) + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + if (!(name = xenUnifiedDomainManagedSavePath(priv, def))) goto cleanup; - ret = xenDaemonDomainSave(dom, name); + ret = xenDaemonDomainSave(dom->conn, def, name); cleanup: VIR_FREE(name); + virDomainDefFree(def); return ret; } @@ -1088,17 +1104,23 @@ static int xenUnifiedDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - char *name; + char *name = NULL; + virDomainDefPtr def = NULL; int ret = -1; virCheckFlags(0, -1); - name = xenUnifiedDomainManagedSavePath(priv, dom); - if (!name) - return ret; + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + if (!(name = xenUnifiedDomainManagedSavePath(priv, def))) + goto cleanup; ret = virFileExists(name); + +cleanup: VIR_FREE(name); + virDomainDefFree(def); return ret; } @@ -1106,16 +1128,21 @@ static int xenUnifiedDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - char *name; + char *name = NULL; + virDomainDefPtr def = NULL; int ret = -1; virCheckFlags(0, -1); - name = xenUnifiedDomainManagedSavePath(priv, dom); - if (!name) - return ret; + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + if (!(name = xenUnifiedDomainManagedSavePath(priv, def))) + goto cleanup; ret = unlink(name); + +cleanup: VIR_FREE(name); return ret; } @@ -1492,12 +1519,15 @@ xenUnifiedDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; int ret = -1; + virDomainDefPtr def = NULL; char *name = NULL; virCheckFlags(0, -1); - name = xenUnifiedDomainManagedSavePath(priv, dom); - if (!name) + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + if (!(name = xenUnifiedDomainManagedSavePath(priv, def))) goto cleanup; if (virFileExists(name)) { @@ -1508,11 +1538,15 @@ xenUnifiedDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) } if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - ret = xenXMDomainCreate(dom); + ret = xenXMDomainCreate(dom->conn, def); else - ret = xenDaemonDomainCreate(dom); + ret = xenDaemonDomainCreate(dom->conn, def); + + if (ret >= 0) + dom->id = def->id; cleanup: + virDomainDefFree(def); VIR_FREE(name); return ret; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 1f4718f..c5f5cc5 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1413,22 +1413,24 @@ xenDaemonDomainGetOSType(virConnectPtr conn, * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainSave(virDomainPtr domain, const char *filename) +xenDaemonDomainSave(virConnectPtr conn, + virDomainDefPtr def, + const char *filename) { - if (domain->id < 0) { + if (def->id < 0) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Domain %s isn't running."), domain->name); + _("Domain %s isn't running."), def->name); return -1; } /* We can't save the state of Domain-0, that would mean stopping it too */ - if (domain->id == 0) { + if (def->id == 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Cannot save host domain")); return -1; } - return xend_op(domain->conn, domain->name, "op", "save", "file", filename, NULL); + return xend_op(conn, def->name, "op", "save", "file", filename, NULL); } /** @@ -2863,9 +2865,10 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) return NULL; } int -xenDaemonDomainCreate(virDomainPtr domain) +xenDaemonDomainCreate(virConnectPtr conn, + virDomainDefPtr def) { - return xend_op(domain->conn, domain->name, "op", "start", NULL); + return xend_op(conn, def->name, "op", "start", NULL); } int diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 87e0a0f..01d321f 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -92,7 +92,9 @@ int xenDaemonDomainResume(virConnectPtr conn, virDomainDefPtr def); int xenDaemonDomainShutdown(virConnectPtr conn, virDomainDefPtr def); int xenDaemonDomainReboot(virConnectPtr conn, virDomainDefPtr def); int xenDaemonDomainDestroy(virConnectPtr conn, virDomainDefPtr def); -int xenDaemonDomainSave(virDomainPtr domain, const char *filename); +int xenDaemonDomainSave(virConnectPtr conn, + virDomainDefPtr def, + const char *filename); int xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, unsigned int flags); int xenDaemonDomainRestore(virConnectPtr conn, const char *filename); @@ -131,7 +133,8 @@ int xenDaemonDetachDeviceFlags(virDomainPtr domain, unsigned int flags); virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr); -int xenDaemonDomainCreate(virDomainPtr domain); +int xenDaemonDomainCreate(virConnectPtr conn, + virDomainDefPtr def); int xenDaemonDomainUndefine(virDomainPtr domain); int xenDaemonDomainSetVcpus (virDomainPtr domain, diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index c942ab6..5e9e5d1 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -898,48 +898,49 @@ cleanup: * Start a domain from an existing defined config file */ int -xenXMDomainCreate(virDomainPtr domain) +xenXMDomainCreate(virConnectPtr conn, + virDomainDefPtr def) { char *sexpr; int ret = -1; - xenUnifiedPrivatePtr priv= domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry = NULL; xenUnifiedLock(priv); - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) + if (!(filename = virHashLookup(priv->nameConfigMap, def->name))) goto error; if (!(entry = virHashLookup(priv->configCache, filename))) goto error; - if (!(sexpr = xenFormatSxpr(domain->conn, entry->def, priv->xendConfigVersion))) + if (!(sexpr = xenFormatSxpr(conn, entry->def, priv->xendConfigVersion))) goto error; - ret = xenDaemonDomainCreateXML(domain->conn, sexpr); + ret = xenDaemonDomainCreateXML(conn, sexpr); VIR_FREE(sexpr); if (ret != 0) goto error; - if ((ret = xenDaemonDomainLookupByName_ids(domain->conn, domain->name, + if ((ret = xenDaemonDomainLookupByName_ids(conn, def->name, entry->def->uuid)) < 0) goto error; - domain->id = ret; + def->id = ret; - if (xend_wait_for_devices(domain->conn, domain->name) < 0) + if (xend_wait_for_devices(conn, def->name) < 0) goto error; - if (xenDaemonDomainResume(domain->conn, entry->def) < 0) + if (xenDaemonDomainResume(conn, entry->def) < 0) goto error; xenUnifiedUnlock(priv); return 0; error: - if (domain->id != -1 && entry) { - xenDaemonDomainDestroy(domain->conn, entry->def); - domain->id = -1; + if (def->id != -1 && entry) { + xenDaemonDomainDestroy(conn, entry->def); + def->id = -1; } xenUnifiedUnlock(priv); return -1; diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 7ced287..62bdecd 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -66,7 +66,8 @@ virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char int xenXMListDefinedDomains(virConnectPtr conn, char ** const names, int maxnames); int xenXMNumOfDefinedDomains(virConnectPtr conn); -int xenXMDomainCreate(virDomainPtr domain); +int xenXMDomainCreate(virConnectPtr conn, + virDomainDefPtr def); virDomainPtr xenXMDomainDefineXML(virConnectPtr con, const char *xml); int xenXMDomainUndefine(virDomainPtr domain); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain VCPU APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 84 ++++++++++++++++++++++++++++++++++++++++-------- src/xen/xen_hypervisor.c | 42 ++++++++++++------------ src/xen/xen_hypervisor.h | 9 ++++-- src/xen/xend_internal.c | 81 ++++++++++++++++++++++++++-------------------- src/xen/xend_internal.h | 17 ++++++---- src/xen/xm_internal.c | 34 ++++++++++---------- src/xen/xm_internal.h | 19 ++++++++--- 7 files changed, 187 insertions(+), 99 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 1566077..e68ed6c 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -645,11 +645,30 @@ xenUnifiedConnectNumOfDomains(virConnectPtr conn) static virDomainPtr xenUnifiedDomainCreateXML(virConnectPtr conn, - const char *xmlDesc, unsigned int flags) + const char *xml, + unsigned int flags) { + xenUnifiedPrivatePtr priv = conn->privateData; + virDomainDefPtr def = NULL; + virDomainPtr ret = NULL; + virCheckFlags(0, NULL); - return xenDaemonCreateXML(conn, xmlDesc); + if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, + 1 << VIR_DOMAIN_VIRT_XEN, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + if (xenDaemonCreateXML(conn, def) < 0) + goto cleanup; + + ret = virGetDomain(conn, def->name, def->uuid); + if (ret) + ret->id = def->id; + +cleanup: + virDomainDefFree(def); + return ret; } static virDomainPtr @@ -1178,6 +1197,8 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -1198,13 +1219,20 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, return -1; } + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + /* Try non-hypervisor methods first, then hypervisor direct method * as a last resort. */ if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return xenXMDomainSetVcpusFlags(dom, nvcpus, flags); + ret = xenXMDomainSetVcpusFlags(dom->conn, def, nvcpus, flags); else - return xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags); + ret = xenDaemonDomainSetVcpusFlags(dom->conn, def, nvcpus, flags); + +cleanup: + virDomainDefFree(def); + return ret; } static int @@ -1227,15 +1255,24 @@ xenUnifiedDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, int maplen) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; if (dom->id < 0) { if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return xenXMDomainPinVcpu(dom, vcpu, cpumap, maplen); + ret = xenXMDomainPinVcpu(dom->conn, def, vcpu, cpumap, maplen); else - return xenDaemonDomainPinVcpu(dom, vcpu, cpumap, maplen); + ret = xenDaemonDomainPinVcpu(dom->conn, def, vcpu, cpumap, maplen); } else { - return xenHypervisorPinVcpu(dom, vcpu, cpumap, maplen); + ret = xenHypervisorPinVcpu(dom->conn, def, vcpu, cpumap, maplen); } + +cleanup: + virDomainDefFree(def); + return ret; } static int @@ -1244,39 +1281,58 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, unsigned char *cpumaps, int maplen) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + if (dom->id < 0) { if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get VCPUs of inactive domain")); - return -1; + goto cleanup; } else { - return xenDaemonDomainGetVcpus(dom, info, maxinfo, cpumaps, maplen); + ret = xenDaemonDomainGetVcpus(dom->conn, def, info, maxinfo, cpumaps, maplen); } } else { - return xenHypervisorGetVcpus(dom, info, maxinfo, cpumaps, maplen); + ret = xenHypervisorGetVcpus(dom->conn, def, info, maxinfo, cpumaps, maplen); } + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + if (dom->id < 0) { if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return xenXMDomainGetVcpusFlags(dom, flags); + ret = xenXMDomainGetVcpusFlags(dom->conn, def, flags); else - return xenDaemonDomainGetVcpusFlags(dom, flags); + ret = xenDaemonDomainGetVcpusFlags(dom->conn, def, flags); } else { if (flags == (VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM)) - return xenHypervisorGetVcpuMax(dom); + ret = xenHypervisorGetVcpuMax(dom->conn, def); else - return xenDaemonDomainGetVcpusFlags(dom, flags); + ret = xenDaemonDomainGetVcpusFlags(dom->conn, def, flags); } + +cleanup: + virDomainDefFree(def); + return ret; } static int diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index f37f48a..3cdc0fc 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2931,16 +2931,16 @@ xenHypervisorSetMaxMemory(virConnectPtr conn, */ int -xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu, - unsigned char *cpumap, int maplen) +xenHypervisorPinVcpu(virConnectPtr conn, + virDomainDefPtr def, + unsigned int vcpu, + unsigned char *cpumap, + int maplen) { int ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; - - if (domain->id < 0) - return -1; + xenUnifiedPrivatePtr priv = conn->privateData; - ret = virXen_setvcpumap(priv->handle, domain->id, vcpu, + ret = virXen_setvcpumap(priv->handle, def->id, vcpu, cpumap, maplen); if (ret < 0) return -1; @@ -2967,7 +2967,8 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu, * Returns the number of info filled in case of success, -1 in case of failure. */ int -xenHypervisorGetVcpus(virDomainPtr domain, +xenHypervisorGetVcpus(virConnectPtr conn, + virDomainDefPtr def, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, @@ -2975,22 +2976,22 @@ xenHypervisorGetVcpus(virDomainPtr domain, { xen_getdomaininfo dominfo; int ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; virVcpuInfoPtr ipt; int nbinfo, i; - if (domain->id < 0 || sizeof(cpumap_t) & 7) { + if (sizeof(cpumap_t) & 7) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain shut off or invalid")); + _("invalid cpumap_t size")); return -1; } /* first get the number of virtual CPUs in this domain */ XEN_GETDOMAININFO_CLEAR(dominfo); - ret = virXen_getdomaininfo(priv->handle, domain->id, + ret = virXen_getdomaininfo(priv->handle, def->id, &dominfo); - if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != domain->id)) { + if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != def->id)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot get domain details")); return -1; @@ -3003,7 +3004,7 @@ xenHypervisorGetVcpus(virDomainPtr domain, for (i = 0, ipt = info; i < nbinfo; i++, ipt++) { if ((cpumaps != NULL) && (i < maxinfo)) { - ret = virXen_getvcpusinfo(priv->handle, domain->id, i, + ret = virXen_getvcpusinfo(priv->handle, def->id, i, ipt, (unsigned char *)VIR_GET_CPUMAP(cpumaps, maplen, i), maplen); @@ -3013,7 +3014,7 @@ xenHypervisorGetVcpus(virDomainPtr domain, return -1; } } else { - ret = virXen_getvcpusinfo(priv->handle, domain->id, i, + ret = virXen_getvcpusinfo(priv->handle, def->id, i, ipt, NULL, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3034,22 +3035,23 @@ xenHypervisorGetVcpus(virDomainPtr domain, * the maximum number of virtual CPUs the guest was booted with. */ int -xenHypervisorGetVcpuMax(virDomainPtr domain) +xenHypervisorGetVcpuMax(virConnectPtr conn, + virDomainDefPtr def) { xen_getdomaininfo dominfo; int ret; int maxcpu; - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; /* inactive domain */ - if (domain->id < 0) { + if (def->id < 0) { maxcpu = MAX_VIRT_CPUS; } else { XEN_GETDOMAININFO_CLEAR(dominfo); - ret = virXen_getdomaininfo(priv->handle, domain->id, + ret = virXen_getdomaininfo(priv->handle, def->id, &dominfo); - if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != domain->id)) + if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != def->id)) return -1; maxcpu = XEN_GETDOMAININFO_MAXCPUID(dominfo) + 1; } diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 9ee1f13..1cf1e14 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -89,18 +89,21 @@ int xenHypervisorSetMaxMemory (virConnectPtr conn, ATTRIBUTE_NONNULL (1); int xenHypervisorCheckID (virConnectPtr conn, int id); -int xenHypervisorPinVcpu (virDomainPtr domain, +int xenHypervisorPinVcpu (virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpu, unsigned char *cpumap, int maplen) ATTRIBUTE_NONNULL (1); -int xenHypervisorGetVcpus (virDomainPtr domain, +int xenHypervisorGetVcpus (virConnectPtr conn, + virDomainDefPtr def, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen) ATTRIBUTE_NONNULL (1); -int xenHypervisorGetVcpuMax (virDomainPtr domain) +int xenHypervisorGetVcpuMax (virConnectPtr conn, + virDomainDefPtr def) ATTRIBUTE_NONNULL (1); char * xenHypervisorGetSchedulerType (virDomainPtr domain, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 527f717..ebafa56 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1773,7 +1773,8 @@ xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps) * the unified driver should keep trying. */ int -xenDaemonDomainSetVcpusFlags(virDomainPtr domain, +xenDaemonDomainSetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpus, unsigned int flags) { @@ -1789,7 +1790,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, return -1; } - if (domain->id < 0) { + if (def->id < 0) { if (flags & VIR_DOMAIN_VCPU_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain not running")); @@ -1807,7 +1808,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, /* Unfortunately, xend_op does not validate whether this exceeds * the maximum. */ flags |= VIR_DOMAIN_VCPU_MAXIMUM; - if ((max = xenDaemonDomainGetVcpusFlags(domain, flags)) < 0) { + if ((max = xenDaemonDomainGetVcpusFlags(conn, def, flags)) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("could not determine max vcpus for the domain")); return -1; @@ -1820,7 +1821,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, } snprintf(buf, sizeof(buf), "%d", vcpus); - return xend_op(domain->conn, domain->name, "op", "set_vcpus", "vcpus", + return xend_op(conn, def->name, "op", "set_vcpus", "vcpus", buf, NULL); } @@ -1841,14 +1842,15 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, * Returns 0 for success; -1 (with errno) on error */ int -xenDaemonDomainPinVcpu(virDomainPtr domain, +xenDaemonDomainPinVcpu(virConnectPtr conn, + virDomainDefPtr minidef, unsigned int vcpu, unsigned char *cpumap, int maplen) { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; int i, j, ret; - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; virDomainDefPtr def = NULL; if (maplen > (int)sizeof(cpumap_t)) { @@ -1876,12 +1878,12 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, snprintf(buf, sizeof(buf), "%d", vcpu); - ret = xend_op(domain->conn, domain->name, "op", "pincpu", "vcpu", buf, + ret = xend_op(conn, minidef->name, "op", "pincpu", "vcpu", buf, "cpumap", mapstr, NULL); - if (!(def = xenDaemonDomainFetch(domain->conn, - domain->id, - domain->name, + if (!(def = xenDaemonDomainFetch(conn, + minidef->id, + minidef->name, NULL))) goto cleanup; @@ -1923,7 +1925,9 @@ cleanup: */ int -xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) +xenDaemonDomainGetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, + unsigned int flags) { struct sexpr *root; int ret; @@ -1932,13 +1936,13 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (domain->id < 0 && (flags & VIR_DOMAIN_VCPU_LIVE)) { + if (def->id < 0 && (flags & VIR_DOMAIN_VCPU_LIVE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain not active")); return -1; } - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name); if (root == NULL) return -1; @@ -1974,7 +1978,8 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) * Returns the number of info filled in case of success, -1 in case of failure. */ int -xenDaemonDomainGetVcpus(virDomainPtr domain, +xenDaemonDomainGetVcpus(virConnectPtr conn, + virDomainDefPtr def, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, @@ -1986,7 +1991,7 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, unsigned char *cpumap; int vcpu, cpu; - root = sexpr_get(domain->conn, "/xend/domain/%s?op=vcpuinfo", domain->name); + root = sexpr_get(conn, "/xend/domain/%s?op=vcpuinfo", def->name); if (root == NULL) return -1; @@ -2129,25 +2134,25 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) * * Returns a new domain object or NULL in case of failure */ -virDomainPtr -xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) +int +xenDaemonCreateXML(virConnectPtr conn, virDomainDefPtr def) { int ret; char *sexpr; - virDomainPtr dom = NULL; + const char *tmp; + struct sexpr *root; xenUnifiedPrivatePtr priv = conn->privateData; - virDomainDefPtr def; - if (!(def = virDomainDefParseString(xmlDesc, priv->caps, priv->xmlopt, - 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_XML_INACTIVE))) - return NULL; - - if (!(sexpr = xenFormatSxpr(conn, def, priv->xendConfigVersion))) { - virDomainDefFree(def); - return NULL; + if (def->id != -1) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Domain %s is already running"), + def->name); + return -1; } + if (!(sexpr = xenFormatSxpr(conn, def, priv->xendConfigVersion))) + return -1; + ret = xenDaemonDomainCreateXML(conn, sexpr); VIR_FREE(sexpr); if (ret != 0) { @@ -2156,8 +2161,19 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) /* This comes before wait_for_devices, to ensure that latter cleanup will destroy the domain upon failure */ - if (!(dom = virDomainLookupByName(conn, def->name))) + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name); + if (root == NULL) + goto error; + + tmp = sexpr_node(root, "domain/domid"); + if (!tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Domain %s did not start"), + def->name); goto error; + } + if (tmp) + def->id = sexpr_int(root, "domain/domid"); if (xend_wait_for_devices(conn, def->name) < 0) goto error; @@ -2166,16 +2182,13 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc) goto error; virDomainDefFree(def); - return dom; + return 0; error: /* Make sure we don't leave a still-born domain around */ - if (dom != NULL) { + if (def->id != -1) xenDaemonDomainDestroy(conn, def); - virObjectUnref(dom); - } - virDomainDefFree(def); - return NULL; + return -1; } /** diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 1284db3..b78145c 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -140,18 +140,23 @@ int xenDaemonDomainCreate(virConnectPtr conn, int xenDaemonDomainUndefine(virConnectPtr conn, virDomainDefPtr def); -int xenDaemonDomainSetVcpus (virDomainPtr domain, +int xenDaemonDomainSetVcpus (virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpus); -int xenDaemonDomainSetVcpusFlags (virDomainPtr domain, +int xenDaemonDomainSetVcpusFlags (virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpus, unsigned int flags); -int xenDaemonDomainPinVcpu (virDomainPtr domain, +int xenDaemonDomainPinVcpu (virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpu, unsigned char *cpumap, int maplen); -int xenDaemonDomainGetVcpusFlags (virDomainPtr domain, +int xenDaemonDomainGetVcpusFlags (virConnectPtr conn, + virDomainDefPtr def, unsigned int flags); -int xenDaemonDomainGetVcpus (virDomainPtr domain, +int xenDaemonDomainGetVcpus (virConnectPtr conn, + virDomainDefPtr def, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, @@ -163,7 +168,7 @@ int xenDaemonDomainGetAutostart (virDomainPtr dom, int xenDaemonDomainSetAutostart (virDomainPtr domain, int autostart); -virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc); +int xenDaemonCreateXML(virConnectPtr conn, virDomainDefPtr def); virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); int xenDaemonDomainMigratePrepare (virConnectPtr dconn, diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index ee3557f..9bb204a 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -657,11 +657,12 @@ cleanup: * the unified driver should keep trying. */ int -xenXMDomainSetVcpusFlags(virDomainPtr domain, +xenXMDomainSetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpus, unsigned int flags) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; int ret = -1; @@ -671,8 +672,6 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain, VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (domain->id != -1) - return -2; if (flags & VIR_DOMAIN_VCPU_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -681,14 +680,14 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain, xenUnifiedLock(priv); - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) + if (!(filename = virHashLookup(priv->nameConfigMap, def->name))) goto cleanup; if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; /* Hypervisor maximum. */ - if ((max = xenUnifiedConnectGetMaxVcpus(domain->conn, NULL)) < 0) { + if ((max = xenUnifiedConnectGetMaxVcpus(conn, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine max vcpus for the domain")); goto cleanup; @@ -715,7 +714,7 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain, /* If this fails, should we try to undo our changes to the * in-memory representation of the config file. I say not! */ - if (xenXMConfigSaveFile(domain->conn, entry->filename, entry->def) < 0) + if (xenXMConfigSaveFile(conn, entry->filename, entry->def) < 0) goto cleanup; ret = 0; @@ -735,19 +734,19 @@ cleanup: * issued, and -2 if the unified driver should keep trying. */ int -xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) +xenXMDomainGetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, + unsigned int flags) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; - int ret = -2; + int ret = -1; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (domain->id != -1) - return -2; if (flags & VIR_DOMAIN_VCPU_LIVE) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("domain not active")); return -1; @@ -755,7 +754,7 @@ xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) xenUnifiedLock(priv); - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) + if (!(filename = virHashLookup(priv->nameConfigMap, def->name))) goto cleanup; if (!(entry = virHashLookup(priv->configCache, filename))) @@ -781,12 +780,13 @@ cleanup: * Returns 0 for success; -1 (with errno) on error */ int -xenXMDomainPinVcpu(virDomainPtr domain, +xenXMDomainPinVcpu(virConnectPtr conn, + virDomainDefPtr def, unsigned int vcpu ATTRIBUTE_UNUSED, unsigned char *cpumap, int maplen) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; int ret = -1; @@ -798,7 +798,7 @@ xenXMDomainPinVcpu(virDomainPtr domain, xenUnifiedLock(priv); - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) { + if (!(filename = virHashLookup(priv->nameConfigMap, def->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("virHashLookup")); goto cleanup; } @@ -812,7 +812,7 @@ xenXMDomainPinVcpu(virDomainPtr domain, entry->def->cpumask = virBitmapNewData(cpumap, maplen); if (!entry->def->cpumask) goto cleanup; - if (xenXMConfigSaveFile(domain->conn, entry->filename, entry->def) < 0) + if (xenXMConfigSaveFile(conn, entry->filename, entry->def) < 0) goto cleanup; ret = 0; diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index fbe06b3..8b74ee6 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -55,12 +55,21 @@ int xenXMDomainSetMaxMemory(virConnectPtr conn, unsigned long memory); unsigned long long xenXMDomainGetMaxMemory(virConnectPtr conn, virDomainDefPtr def); -int xenXMDomainSetVcpus(virDomainPtr domain, unsigned int vcpus); -int xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, +int xenXMDomainSetVcpus(virConnectPtr conn, + virDomainDefPtr def, + unsigned int vcpus); +int xenXMDomainSetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, + unsigned int vcpus, + unsigned int flags); +int xenXMDomainGetVcpusFlags(virConnectPtr conn, + virDomainDefPtr def, unsigned int flags); -int xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags); -int xenXMDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, - unsigned char *cpumap, int maplen); +int xenXMDomainPinVcpu(virConnectPtr conn, + virDomainDefPtr def, + unsigned int vcpu, + unsigned char *cpumap, + int maplen); virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname); virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain autostart APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 26 ++++++++++++++++++++++---- src/xen/xend_internal.c | 14 +++++++++----- src/xen/xend_internal.h | 10 ++++++---- src/xen/xm_internal.c | 22 ++++++++++++---------- src/xen/xm_internal.h | 6 ++++-- 5 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 1019957..fde72ab 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1807,24 +1807,42 @@ static int xenUnifiedDomainGetAutostart(virDomainPtr dom, int *autostart) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - return xenXMDomainGetAutostart(dom, autostart); + ret = xenXMDomainGetAutostart(def, autostart); } else { - return xenDaemonDomainGetAutostart(dom, autostart); + ret = xenDaemonDomainGetAutostart(dom->conn, def, autostart); } + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainSetAutostart(virDomainPtr dom, int autostart) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { - return xenXMDomainSetAutostart(dom, autostart); + ret = xenXMDomainSetAutostart(def, autostart); } else { - return xenDaemonDomainSetAutostart(dom, autostart); + ret = xenDaemonDomainSetAutostart(dom->conn, def, autostart); } + +cleanup: + virDomainDefFree(def); + return ret; } static char * diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 669e7db..639d9d6 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2547,12 +2547,14 @@ cleanup: } int -xenDaemonDomainGetAutostart(virDomainPtr domain, int *autostart) +xenDaemonDomainGetAutostart(virConnectPtr conn, + virDomainDefPtr def, + int *autostart) { struct sexpr *root; const char *tmp; - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name); if (root == NULL) { virReportError(VIR_ERR_XEN_CALL, "%s", _("xenDaemonGetAutostart failed to find this domain")); @@ -2571,14 +2573,16 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, int *autostart) } int -xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) +xenDaemonDomainSetAutostart(virConnectPtr conn, + virDomainDefPtr def, + int autostart) { struct sexpr *root, *autonode; virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name); if (root == NULL) { virReportError(VIR_ERR_XEN_CALL, "%s", _("xenDaemonSetAutostart failed to find this domain")); @@ -2617,7 +2621,7 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) content = virBufferContentAndReset(&buffer); - if (xend_op(domain->conn, "", "op", "new", "config", content, NULL) != 0) { + if (xend_op(conn, "", "op", "new", "config", content, NULL) != 0) { virReportError(VIR_ERR_XEN_CALL, "%s", _("Failed to redefine sexpr")); goto error; diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 62b85ef..3a7c0ac 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -167,10 +167,12 @@ int xenDaemonUpdateDeviceFlags(virConnectPtr conn, virDomainDefPtr def, const char *xml, unsigned int flags); -int xenDaemonDomainGetAutostart (virDomainPtr dom, - int *autostart); -int xenDaemonDomainSetAutostart (virDomainPtr domain, - int autostart); +int xenDaemonDomainGetAutostart(virConnectPtr conn, + virDomainDefPtr def, + int *autostart); +int xenDaemonDomainSetAutostart(virConnectPtr conn, + virDomainDefPtr def, + int autostart); int xenDaemonCreateXML(virConnectPtr conn, virDomainDefPtr def); virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 94dc500..09e0794 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1418,28 +1418,29 @@ xenXMDomainBlockPeek(virDomainPtr dom ATTRIBUTE_UNUSED, static char * -xenXMAutostartLinkName(virDomainPtr dom) +xenXMAutostartLinkName(virDomainDefPtr def) { char *ret; - if (virAsprintf(&ret, "/etc/xen/auto/%s", dom->name) < 0) + if (virAsprintf(&ret, "/etc/xen/auto/%s", def->name) < 0) return NULL; return ret; } static char * -xenXMDomainConfigName(virDomainPtr dom) +xenXMDomainConfigName(virDomainDefPtr def) { char *ret; - if (virAsprintf(&ret, "/etc/xen/%s", dom->name) < 0) + if (virAsprintf(&ret, "/etc/xen/%s", def->name) < 0) return NULL; return ret; } int -xenXMDomainGetAutostart(virDomainPtr dom, int *autostart) +xenXMDomainGetAutostart(virDomainDefPtr def, + int *autostart) { - char *linkname = xenXMAutostartLinkName(dom); - char *config = xenXMDomainConfigName(dom); + char *linkname = xenXMAutostartLinkName(def); + char *config = xenXMDomainConfigName(def); int ret = -1; if (!linkname || !config) { @@ -1465,10 +1466,11 @@ cleanup: int -xenXMDomainSetAutostart(virDomainPtr dom, int autostart) +xenXMDomainSetAutostart(virDomainDefPtr def, + int autostart) { - char *linkname = xenXMAutostartLinkName(dom); - char *config = xenXMDomainConfigName(dom); + char *linkname = xenXMAutostartLinkName(def); + char *config = xenXMDomainConfigName(def); int ret = -1; if (!linkname || !config) { diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index ba38806..5be59b1 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -83,8 +83,10 @@ int xenXMDomainUndefine(virConnectPtr conn, virDomainDefPtr def); int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer); -int xenXMDomainGetAutostart(virDomainPtr dom, int *autostart); -int xenXMDomainSetAutostart(virDomainPtr dom, int autostart); +int xenXMDomainGetAutostart(virDomainDefPtr def, + int *autostart); +int xenXMDomainSetAutostart(virDomainDefPtr def, + int autostart); int xenXMDomainAttachDeviceFlags(virConnectPtr conn, virDomainDefPtr def, -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Introduce use of a virDomainDefPtr in the domain stats & peek APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/block_stats.c | 6 +++--- src/xen/block_stats.h | 2 +- src/xen/xen_driver.c | 37 +++++++++++++++++++++++++++++++++---- src/xen/xen_hypervisor.c | 11 ++++++----- src/xen/xen_hypervisor.h | 9 +++++---- src/xen/xend_internal.c | 21 +++++++++++---------- src/xen/xend_internal.h | 7 ++++++- src/xen/xm_internal.c | 3 ++- src/xen/xm_internal.h | 7 ++++++- 9 files changed, 73 insertions(+), 30 deletions(-) diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c index 9f5823c..5adbf6c 100644 --- a/src/xen/block_stats.c +++ b/src/xen/block_stats.c @@ -359,16 +359,16 @@ xenLinuxDomainDeviceID(int domid, const char *path) int xenLinuxDomainBlockStats(xenUnifiedPrivatePtr priv, - virDomainPtr dom, + virDomainDefPtr def, const char *path, struct _virDomainBlockStats *stats) { - int device = xenLinuxDomainDeviceID(dom->id, path); + int device = xenLinuxDomainDeviceID(def->id, path); if (device < 0) return -1; - return read_bd_stats(priv, device, dom->id, stats); + return read_bd_stats(priv, device, def->id, stats); } #endif /* __linux__ */ diff --git a/src/xen/block_stats.h b/src/xen/block_stats.h index 0a3c40a..6633d97 100644 --- a/src/xen/block_stats.h +++ b/src/xen/block_stats.h @@ -28,7 +28,7 @@ # include "xen_driver.h" extern int xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv, - virDomainPtr dom, const char *path, + virDomainDefPtr def, const char *path, struct _virDomainBlockStats *stats); extern int xenLinuxDomainDeviceID(int domid, const char *dev); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index c2127d6..246076d 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1955,14 +1955,34 @@ static int xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path, struct _virDomainBlockStats *stats) { - return xenHypervisorDomainBlockStats(dom, path, stats); + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenHypervisorDomainBlockStats(dom->conn, def, path, stats); + +cleanup: + virDomainDefFree(def); + return ret; } static int xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { - return xenHypervisorDomainInterfaceStats(dom, path, stats); + virDomainDefPtr def = NULL; + int ret = -1; + + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + + ret = xenHypervisorDomainInterfaceStats(def, path, stats); + +cleanup: + virDomainDefFree(def); + return ret; } static int @@ -1971,13 +1991,22 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const char *path, void *buffer, unsigned int flags) { xenUnifiedPrivatePtr priv = dom->conn->privateData; + virDomainDefPtr def = NULL; + int ret = -1; virCheckFlags(0, -1); + if (!(def = xenGetDomainDefForDom(dom))) + goto cleanup; + if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return xenXMDomainBlockPeek(dom, path, offset, size, buffer); + ret = xenXMDomainBlockPeek(dom->conn, def, path, offset, size, buffer); else - return xenDaemonDomainBlockPeek(dom, path, offset, size, buffer); + ret = xenDaemonDomainBlockPeek(dom->conn, def, path, offset, size, buffer); + +cleanup: + virDomainDefFree(def); + return ret; } static int diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index dc40a92..cefcda4 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1368,17 +1368,18 @@ xenHypervisorSetSchedulerParameters(virConnectPtr conn, int -xenHypervisorDomainBlockStats(virDomainPtr dom, +xenHypervisorDomainBlockStats(virConnectPtr conn, + virDomainDefPtr def, const char *path, struct _virDomainBlockStats *stats) { #ifdef __linux__ - xenUnifiedPrivatePtr priv = dom->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; int ret; xenUnifiedLock(priv); /* Need to lock because it hits the xenstore handle :-( */ - ret = xenLinuxDomainBlockStats(priv, dom, path, stats); + ret = xenLinuxDomainBlockStats(priv, def, path, stats); xenUnifiedUnlock(priv); return ret; #else @@ -1396,7 +1397,7 @@ xenHypervisorDomainBlockStats(virDomainPtr dom, * virNetwork interface, as yet not decided. */ int -xenHypervisorDomainInterfaceStats(virDomainPtr dom, +xenHypervisorDomainInterfaceStats(virDomainDefPtr def, const char *path, struct _virDomainInterfaceStats *stats) { @@ -1411,7 +1412,7 @@ xenHypervisorDomainInterfaceStats(virDomainPtr dom, _("invalid path, should be vif<domid>.<n>.")); return -1; } - if (rqdomid != dom->id) { + if (rqdomid != def->id) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid path, vif<domid> should match this domain ID")); return -1; diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 1e5bb67..6aeab79 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -122,13 +122,14 @@ int xenHypervisorSetSchedulerParameters(virConnectPtr conn, int nparams) ATTRIBUTE_NONNULL (1); -int xenHypervisorDomainBlockStats (virDomainPtr domain, +int xenHypervisorDomainBlockStats (virConnectPtr conn, + virDomainDefPtr def, const char *path, struct _virDomainBlockStats *stats) ATTRIBUTE_NONNULL (1); -int xenHypervisorDomainInterfaceStats (virDomainPtr domain, - const char *path, - struct _virDomainInterfaceStats *stats) +int xenHypervisorDomainInterfaceStats (virDomainDefPtr def, + const char *path, + struct _virDomainInterfaceStats *stats) ATTRIBUTE_NONNULL (1); int xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index a78de96..244940f 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3238,13 +3238,14 @@ error: * Returns 0 if successful, -1 if error, -2 if declined. */ int -xenDaemonDomainBlockPeek(virDomainPtr domain, +xenDaemonDomainBlockPeek(virConnectPtr conn, + virDomainDefPtr minidef, const char *path, unsigned long long offset, size_t size, void *buffer) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; struct sexpr *root = NULL; int fd = -1, ret = -1; virDomainDefPtr def; @@ -3254,12 +3255,12 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *actual; /* Security check: The path must correspond to a block device. */ - if (domain->id > 0) - root = sexpr_get(domain->conn, "/xend/domain/%d?detail=1", - domain->id); - else if (domain->id < 0) - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", - domain->name); + if (minidef->id > 0) + root = sexpr_get(conn, "/xend/domain/%d?detail=1", + minidef->id); + else if (minidef->id < 0) + root = sexpr_get(conn, "/xend/domain/%s?detail=1", + minidef->name); else { /* This call always fails for dom0. */ virReportError(VIR_ERR_OPERATION_INVALID, @@ -3274,8 +3275,8 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, id = xenGetDomIdFromSxpr(root, priv->xendConfigVersion); xenUnifiedLock(priv); - tty = xenStoreDomainGetConsolePath(domain->conn, id); - vncport = xenStoreDomainGetVNCPort(domain->conn, id); + tty = xenStoreDomainGetConsolePath(conn, id); + vncport = xenStoreDomainGetVNCPort(conn, id); xenUnifiedUnlock(priv); if (!(def = xenParseSxpr(root, priv->xendConfigVersion, NULL, tty, diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index cef7da4..aa05130 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -187,7 +187,12 @@ int xenDaemonDomainMigratePerform (virConnectPtr conn, const char *uri, unsigned long flags, const char *dname, unsigned long resource); -int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer); +int xenDaemonDomainBlockPeek(virConnectPtr conn, + virDomainDefPtr def, + const char *path, + unsigned long long offset, + size_t size, + void *buffer); char * xenDaemonGetSchedulerType(virConnectPtr conn, int *nparams); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 09e0794..28afa0b 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1405,7 +1405,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn, } int -xenXMDomainBlockPeek(virDomainPtr dom ATTRIBUTE_UNUSED, +xenXMDomainBlockPeek(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, unsigned long long offset ATTRIBUTE_UNUSED, size_t size ATTRIBUTE_UNUSED, diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 5be59b1..5dbc0bf 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -81,7 +81,12 @@ int xenXMDomainCreate(virConnectPtr conn, int xenXMDomainDefineXML(virConnectPtr con, virDomainDefPtr def); int xenXMDomainUndefine(virConnectPtr conn, virDomainDefPtr def); -int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer); +int xenXMDomainBlockPeek(virConnectPtr conn, + virDomainDefPtr def, + const char *path, + unsigned long long offset, + size_t size, + void *buffer); int xenXMDomainGetAutostart(virDomainDefPtr def, int *autostart); -- 1.8.1.4