Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > > Introduce use of a virDomainDefPtr in the domain hotplug > 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@xxxxxxxxxx> > --- > src/xen/xen_driver.c | 64 +++++++++++++++++++++++++++++++------ > src/xen/xend_internal.c | 85 ++++++++++++++++++++++++++----------------------- > src/xen/xend_internal.h | 10 ++++-- > src/xen/xm_internal.c | 22 +++++++------ > src/xen/xm_internal.h | 6 ++-- > 5 files changed, 122 insertions(+), 65 deletions(-) > > diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c > index 04cb69d..f5f6407 100644 > --- a/src/xen/xen_driver.c > +++ b/src/xen/xen_driver.c > @@ -1695,6 +1695,8 @@ xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml) > { > xenUnifiedPrivatePtr priv = dom->conn->privateData; > unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; > + virDomainDefPtr def = NULL; > + int ret = -1; > > /* > * HACK: xend with xendConfigVersion >= 3 does not support changing live > @@ -1704,12 +1706,17 @@ xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml) > if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) > flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; > > + if (!(def = xenGetDomainDefForDom(dom))) > + goto cleanup; > + > if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) > - return xenXMDomainAttachDeviceFlags(dom, xml, flags); > + ret = xenXMDomainAttachDeviceFlags(dom->conn, def, xml, flags); > else > - return xenDaemonAttachDeviceFlags(dom, xml, flags); > + ret = xenDaemonAttachDeviceFlags(dom->conn, def, xml, flags); > > - return -1; > +cleanup: > + virDomainDefFree(def); > + return ret; > } > > static int > @@ -1717,11 +1724,20 @@ xenUnifiedDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, > unsigned int flags) > { > xenUnifiedPrivatePtr priv = dom->conn->privateData; > + virDomainDefPtr def = NULL; > + int ret = -1; > + > + if (!(def = xenGetDomainDefForDom(dom))) > + goto cleanup; > > if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) > - return xenXMDomainAttachDeviceFlags(dom, xml, flags); > + ret = xenXMDomainAttachDeviceFlags(dom->conn, def, xml, flags); > else > - return xenDaemonAttachDeviceFlags(dom, xml, flags); > + ret = xenDaemonAttachDeviceFlags(dom->conn, def, xml, flags); > + > +cleanup: > + virDomainDefFree(def); > + return ret; > } > > static int > @@ -1729,6 +1745,8 @@ xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml) > { > xenUnifiedPrivatePtr priv = dom->conn->privateData; > unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE; > + virDomainDefPtr def = NULL; > + int ret = -1; > > /* > * HACK: xend with xendConfigVersion >= 3 does not support changing live > @@ -1738,10 +1756,17 @@ xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml) > if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) > flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; > > + if (!(def = xenGetDomainDefForDom(dom))) > + goto cleanup; > + > if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) > - return xenXMDomainDetachDeviceFlags(dom, xml, flags); > + ret = xenXMDomainDetachDeviceFlags(dom->conn, def, xml, flags); > else > - return xenDaemonDetachDeviceFlags(dom, xml, flags); > + ret = xenDaemonDetachDeviceFlags(dom->conn, def, xml, flags); > + > +cleanup: > + virDomainDefFree(def); > + return ret; > } > > static int > @@ -1749,18 +1774,37 @@ xenUnifiedDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, > unsigned int flags) > { > xenUnifiedPrivatePtr priv = dom->conn->privateData; > + virDomainDefPtr def = NULL; > + int ret = -1; > + > + if (!(def = xenGetDomainDefForDom(dom))) > + goto cleanup; > > if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) > - return xenXMDomainDetachDeviceFlags(dom, xml, flags); > + ret = xenXMDomainDetachDeviceFlags(dom->conn, def, xml, flags); > else > - return xenDaemonDetachDeviceFlags(dom, xml, flags); > + ret = xenDaemonDetachDeviceFlags(dom->conn, def, xml, flags); > + > +cleanup: > + virDomainDefFree(def); > + return ret; > } > > static int > xenUnifiedDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, > unsigned int flags) > { > - return xenDaemonUpdateDeviceFlags(dom, xml, flags); > + virDomainDefPtr def = NULL; > + int ret = -1; > + > + if (!(def = xenGetDomainDefForDom(dom))) > + goto cleanup; > + > + ret = xenDaemonUpdateDeviceFlags(dom->conn, def, xml, flags); > + > +cleanup: > + virDomainDefFree(def); > + return ret; > } > > static int > diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c > index 75c980c..2715a3e 100644 > --- a/src/xen/xend_internal.c > +++ b/src/xen/xend_internal.c > @@ -60,7 +60,8 @@ > #define XEND_RCV_BUF_MAX_LEN (256 * 1024) > > static int > -virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, char *class, > +virDomainXMLDevID(virConnectPtr conn, virDomainDefPtr domain, > + virDomainDeviceDefPtr dev, char *class, > char *ref, int ref_len); > > /** > @@ -2202,11 +2203,12 @@ xenDaemonCreateXML(virConnectPtr conn, virDomainDefPtr def) > * Returns 0 in case of success, -1 in case of failure. > */ > int > -xenDaemonAttachDeviceFlags(virDomainPtr domain, > +xenDaemonAttachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr minidef, > My usual comment about the function comments needing updated :). Here and below where the signatures change. ACK. Regards, Jim > const char *xml, > unsigned int flags) > { > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > char *sexpr = NULL; > int ret = -1; > virDomainDeviceDefPtr dev = NULL; > @@ -2217,7 +2219,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, > > virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); > > - if (domain->id < 0) { > + if (minidef->id < 0) { > /* Cannot modify live config if domain is inactive */ > if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > @@ -2247,9 +2249,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, > } > } > > - if (!(def = xenDaemonDomainFetch(domain->conn, > - domain->id, > - domain->name, > + if (!(def = xenDaemonDomainFetch(conn, > + minidef->id, > + minidef->name, > NULL))) > goto cleanup; > > @@ -2275,7 +2277,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, > break; > > case VIR_DOMAIN_DEVICE_NET: > - if (xenFormatSxprNet(domain->conn, > + if (xenFormatSxprNet(conn, > dev->data.net, > &buf, > STREQ(def->os.type, "hvm") ? 1 : 0, > @@ -2320,9 +2322,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, > > sexpr = virBufferContentAndReset(&buf); > > - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) { > + if (virDomainXMLDevID(conn, minidef, dev, class, ref, sizeof(ref))) { > /* device doesn't exist, define it */ > - ret = xend_op(domain->conn, domain->name, "op", "device_create", > + ret = xend_op(conn, def->name, "op", "device_create", > "config", sexpr, NULL); > } else { > if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { > @@ -2330,7 +2332,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, > _("target '%s' already exists"), target); > } else { > /* device exists, attempt to modify it */ > - ret = xend_op(domain->conn, domain->name, "op", "device_configure", > + ret = xend_op(conn, minidef->name, "op", "device_configure", > "config", sexpr, "dev", ref, NULL); > } > } > @@ -2355,11 +2357,12 @@ cleanup: > * Returns 0 in case of success, -1 in case of failure. > */ > int > -xenDaemonUpdateDeviceFlags(virDomainPtr domain, > +xenDaemonUpdateDeviceFlags(virConnectPtr conn, > + virDomainDefPtr minidef, > const char *xml, > unsigned int flags) > { > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > char *sexpr = NULL; > int ret = -1; > virDomainDeviceDefPtr dev = NULL; > @@ -2370,7 +2373,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, > virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | > VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); > > - if (domain->id < 0) { > + if (minidef->id < 0) { > /* Cannot modify live config if domain is inactive */ > if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > @@ -2400,9 +2403,9 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, > } > } > > - if (!(def = xenDaemonDomainFetch(domain->conn, > - domain->id, > - domain->name, > + if (!(def = xenDaemonDomainFetch(conn, > + minidef->id, > + minidef->name, > NULL))) > goto cleanup; > > @@ -2428,13 +2431,13 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, > > sexpr = virBufferContentAndReset(&buf); > > - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) { > + if (virDomainXMLDevID(conn, minidef, dev, class, ref, sizeof(ref))) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > _("requested device does not exist")); > goto cleanup; > } else { > /* device exists, attempt to modify it */ > - ret = xend_op(domain->conn, domain->name, "op", "device_configure", > + ret = xend_op(conn, minidef->name, "op", "device_configure", > "config", sexpr, "dev", ref, NULL); > } > > @@ -2456,11 +2459,12 @@ cleanup: > * Returns 0 in case of success, -1 in case of failure. > */ > int > -xenDaemonDetachDeviceFlags(virDomainPtr domain, > +xenDaemonDetachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr minidef, > const char *xml, > unsigned int flags) > { > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > char class[8], ref[80]; > virDomainDeviceDefPtr dev = NULL; > virDomainDefPtr def = NULL; > @@ -2470,7 +2474,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, > > virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); > > - if (domain->id < 0) { > + if (minidef->id < 0) { > /* Cannot modify live config if domain is inactive */ > if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > @@ -2500,9 +2504,9 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, > } > } > > - if (!(def = xenDaemonDomainFetch(domain->conn, > - domain->id, > - domain->name, > + if (!(def = xenDaemonDomainFetch(conn, > + minidef->id, > + minidef->name, > NULL))) > goto cleanup; > > @@ -2510,7 +2514,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, > VIR_DOMAIN_XML_INACTIVE))) > goto cleanup; > > - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) > + if (virDomainXMLDevID(conn, minidef, dev, class, ref, sizeof(ref))) > goto cleanup; > > if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { > @@ -2524,12 +2528,12 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, > goto cleanup; > } > xendev = virBufferContentAndReset(&buf); > - ret = xend_op(domain->conn, domain->name, "op", "device_configure", > + ret = xend_op(conn, minidef->name, "op", "device_configure", > "config", xendev, "dev", ref, NULL); > VIR_FREE(xendev); > } > else { > - ret = xend_op(domain->conn, domain->name, "op", "device_destroy", > + ret = xend_op(conn, minidef->name, "op", "device_destroy", > "type", class, "dev", ref, "force", "0", "rm_cfg", "1", > NULL); > } > @@ -3334,13 +3338,14 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, > * Returns 0 in case of success, -1 in case of failure. > */ > static int > -virDomainXMLDevID(virDomainPtr domain, > +virDomainXMLDevID(virConnectPtr conn, > + virDomainDefPtr def, > virDomainDeviceDefPtr dev, > char *class, > char *ref, > int ref_len) > { > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > char *xref; > char *tmp; > > @@ -3357,7 +3362,7 @@ virDomainXMLDevID(virDomainPtr domain, > if (dev->data.disk->dst == NULL) > return -1; > xenUnifiedLock(priv); > - xref = xenStoreDomainGetDiskID(domain->conn, domain->id, > + xref = xenStoreDomainGetDiskID(conn, def->id, > dev->data.disk->dst); > xenUnifiedUnlock(priv); > if (xref == NULL) > @@ -3369,13 +3374,13 @@ virDomainXMLDevID(virDomainPtr domain, > return -1; > } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { > char mac[VIR_MAC_STRING_BUFLEN]; > - virDomainNetDefPtr def = dev->data.net; > - virMacAddrFormat(&def->mac, mac); > + virDomainNetDefPtr netdef = dev->data.net; > + virMacAddrFormat(&netdef->mac, mac); > > strcpy(class, "vif"); > > xenUnifiedLock(priv); > - xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, mac); > + xref = xenStoreDomainGetNetworkID(conn, def->id, mac); > xenUnifiedUnlock(priv); > if (xref == NULL) > return -1; > @@ -3388,13 +3393,13 @@ virDomainXMLDevID(virDomainPtr domain, > dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && > dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { > char *bdf; > - virDomainHostdevDefPtr def = dev->data.hostdev; > + virDomainHostdevDefPtr hostdef = dev->data.hostdev; > > if (virAsprintf(&bdf, "%04x:%02x:%02x.%0x", > - def->source.subsys.u.pci.addr.domain, > - def->source.subsys.u.pci.addr.bus, > - def->source.subsys.u.pci.addr.slot, > - def->source.subsys.u.pci.addr.function) < 0) { > + hostdef->source.subsys.u.pci.addr.domain, > + hostdef->source.subsys.u.pci.addr.bus, > + hostdef->source.subsys.u.pci.addr.slot, > + hostdef->source.subsys.u.pci.addr.function) < 0) { > virReportOOMError(); > return -1; > } > @@ -3402,7 +3407,7 @@ virDomainXMLDevID(virDomainPtr domain, > strcpy(class, "pci"); > > xenUnifiedLock(priv); > - xref = xenStoreDomainGetPCIID(domain->conn, domain->id, bdf); > + xref = xenStoreDomainGetPCIID(conn, def->id, bdf); > xenUnifiedUnlock(priv); > VIR_FREE(bdf); > if (xref == NULL) > diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h > index b78145c..62b85ef 100644 > --- a/src/xen/xend_internal.h > +++ b/src/xen/xend_internal.h > @@ -126,10 +126,12 @@ int xenDaemonListDefinedDomains(virConnectPtr conn, > char **const names, > int maxnames); > > -int xenDaemonAttachDeviceFlags(virDomainPtr domain, > +int xenDaemonAttachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr def, > const char *xml, > unsigned int flags); > -int xenDaemonDetachDeviceFlags(virDomainPtr domain, > +int xenDaemonDetachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr def, > const char *xml, > unsigned int flags); > > @@ -161,7 +163,9 @@ int xenDaemonDomainGetVcpus (virConnectPtr conn, > int maxinfo, > unsigned char *cpumaps, > int maplen); > -int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, > +int xenDaemonUpdateDeviceFlags(virConnectPtr conn, > + virDomainDefPtr def, > + const char *xml, > unsigned int flags); > int xenDaemonDomainGetAutostart (virDomainPtr dom, > int *autostart); > diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c > index 76425dd..c2d9915 100644 > --- a/src/xen/xm_internal.c > +++ b/src/xen/xm_internal.c > @@ -1219,7 +1219,8 @@ cleanup: > * Returns 0 in case of success, -1 in case of failure. > */ > int > -xenXMDomainAttachDeviceFlags(virDomainPtr domain, > +xenXMDomainAttachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr minidef, > const char *xml, > unsigned int flags) > { > @@ -1228,12 +1229,12 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, > int ret = -1; > virDomainDeviceDefPtr dev = NULL; > virDomainDefPtr def; > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > > virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); > > if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) || > - (domain->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { > + (minidef->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > _("Xm driver only supports modifying persistent config")); > return -1; > @@ -1241,7 +1242,7 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, > > xenUnifiedLock(priv); > > - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) > + if (!(filename = virHashLookup(priv->nameConfigMap, minidef->name))) > goto cleanup; > if (!(entry = virHashLookup(priv->configCache, filename))) > goto cleanup; > @@ -1284,7 +1285,7 @@ xenXMDomainAttachDeviceFlags(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; > @@ -1309,7 +1310,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, > * Returns 0 in case of success, -1 in case of failure. > */ > int > -xenXMDomainDetachDeviceFlags(virDomainPtr domain, > +xenXMDomainDetachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr minidef, > const char *xml, > unsigned int flags) > { > @@ -1319,12 +1321,12 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, > virDomainDefPtr def; > int ret = -1; > int i; > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > + xenUnifiedPrivatePtr priv = conn->privateData; > > virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); > > if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) || > - (domain->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { > + (minidef->id != -1 && flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > _("Xm driver only supports modifying persistent config")); > return -1; > @@ -1332,7 +1334,7 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, > > xenUnifiedLock(priv); > > - if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) > + if (!(filename = virHashLookup(priv->nameConfigMap, minidef->name))) > goto cleanup; > if (!(entry = virHashLookup(priv->configCache, filename))) > goto cleanup; > @@ -1390,7 +1392,7 @@ xenXMDomainDetachDeviceFlags(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; > diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h > index 28087d3..7d64dc6 100644 > --- a/src/xen/xm_internal.h > +++ b/src/xen/xm_internal.h > @@ -85,11 +85,13 @@ 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, > +int xenXMDomainAttachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr def, > const char *xml, > unsigned int flags); > > -int xenXMDomainDetachDeviceFlags(virDomainPtr domain, > +int xenXMDomainDetachDeviceFlags(virConnectPtr conn, > + virDomainDefPtr def, > const char *xml, > unsigned int flags); > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list