10:18 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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>
---
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