On 07/20/2011 12:21 AM, Laine Stump wrote:
the domain XML<interface> element is updated in the following ways:
1)<virtualportprofile> can be specified when source type='network'
(previously it was only valid for source type='direct')
(Since virtualPortProfile is going to be used in both the domain and
network RNG, its RNG definition was moved into a separate file that will
be included by both.)
2) A new attribute "portgroup" has been added to the<source>
element. When source type='network' (the only time portgroup is
recognized), extra configuration information will be taken from the
<portgroup> element of the given name.
3) Each virDomainNetDef now also potentially has a virDomainActualNetDef
which is a private object (never exported/imported via the public API,
and not defined in the RNG) that is used to maintain information about
the physical device that was actually used for a NetDef that was of
type VIR_DOMAIN_NET_TYPE_NETWORK.
The virDomainActualNetDef will only be parsed/formatted if the
parse/format function is called with the
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET flag set (which is only
needed when saving/loading a running domain's state info to the
stateDir).
---
The internal flag created a lot of discussion on the list, and what
was finally decided on was to leave the existing
VIR_DOMAIN_XML_INTERNAL_STATUS in the regular flags (and also put my
new VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET there), but put in a
compile-time protection against re-using those bits for the public
API, and a runtime check to make sure nobody calls the public API with
those bits on. Eric took care of this in a separate patch, which was
pushed on Monday.
And in case it isn't obvious:
If we ever add a new public bit in the future such that the compile-time
test fails, we simply move the internal bits to another free location
and recompile - since they are internal, they do not leak over RPC, so
there are no cross-version dependencies and thus can be changed at will.
Meanwhile, if a newer version of libvirt adds new bits and talks to an
older version, the runtime check guarantees that the new bits will be
rejected as unknown rather than accidentally turning on the internal
behavior of the older version.
<p>
- Provides a virtual network using a bridge device in the host.
...
- overridden with the<target> element (see
+
+ Provides a connection whose details are described by the named
Why the blank line between <p> and the start of the paragraph?
[and I really hate it that thunderbird is back to its habits of munging
quoted text in my emails again - for a while there, I was running a
version where the problem went away, but the latest distro upgrade made
it resurface]
+void
+virDomainActualNetDefFree(virDomainActualNetDefPtr def)
Add this to cfg.mk's list of free-like functions.
+{
+ if (!def)
+ return;
+
+ switch (def->type) {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ VIR_FREE(def->data.bridge.brname);
+ break;
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ VIR_FREE(def->data.direct.linkdev);
+ VIR_FREE(def->data.direct.virtPortProfile);
+ break;
+ default:
+ break;
+ }
+}
Memory leak of def itself.
+static int
+virDomainActualNetDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainActualNetDefPtr *def)
+{
+ virDomainActualNetDefPtr actual = NULL;
+ int ret = -1;
+ xmlNodePtr save_ctxt = ctxt->node;
+ char *type = NULL;
+ char *mode = NULL;
+
+ if (VIR_ALLOC(actual)< 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ ctxt->node = node;
+
+ type = virXMLPropString(node, "type");
+ if (!type) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing type attribute in interface's<actual> element"));
+ goto error;
+ }
+ if ((int)(actual->type = virDomainNetTypeFromString(type))< 0) {
This cast is not needed if you tweak domain_conf.h.
+
+ *def = actual;
+ actual = NULL;
+ ret = 0;
+error:
+ VIR_FREE(type);
+ VIR_FREE(mode);
+
+ ctxt->node = save_ctxt;
+ return ret;
Memory leak of actual on error cases.
@@ -6772,7 +6887,8 @@ virDomainDefPtr virDomainDefParseNode(virCapsPtr caps,
}
ctxt->node = root;
- def = virDomainDefParseXML(caps, xml, root, ctxt, expectedVirtTypes, flags);
+ def = virDomainDefParseXML(caps, xml, root, ctxt, expectedVirtTypes,
+ flags);
Your change and then undo made for a spurious hunk.
cleanup:
xmlXPathFreeContext(ctxt);
@@ -6803,7 +6919,8 @@ virDomainObjParseNode(virCapsPtr caps,
}
ctxt->node = root;
- obj = virDomainObjParseXML(caps, xml, ctxt, expectedVirtTypes, flags);
+ obj = virDomainObjParseXML(caps, xml, ctxt, expectedVirtTypes,
+ flags);
and another one.
@@ -10008,7 +10201,10 @@ int virDomainSaveStatus(virCapsPtr caps,
const char *statusDir,
virDomainObjPtr obj)
{
- unsigned int flags = VIR_DOMAIN_XML_SECURE|VIR_DOMAIN_XML_INTERNAL_STATUS;
+ unsigned int flags = (VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_INTERNAL_STATUS |
+ VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET);
+
int ret = -1;
char *xml;
@@ -10099,7 +10295,8 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr caps,
goto error;
if (!(obj = virDomainObjParseFile(caps, statusFile, expectedVirtTypes,
- VIR_DOMAIN_XML_INTERNAL_STATUS)))
+ VIR_DOMAIN_XML_INTERNAL_STATUS |
+ VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET)))
Thinking out loud here - it looks like we never use _INTERNAL_STATUS or
_INTERNAL_ACTUAL_NET in isolation - as of this patch, it is always both
or neither. Maybe we could have let _INTERNAL_STATUS be the key on
whether to also output/parse <actual> elements rather than adding a new
flag. On the other hand, if we ever change our mind and decide that it
makes sense to let the user do 'virsh dumpxml dom --actual' to see which
resources actually got used, then it is easier to change
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET into a public flag, than it is if we
lump all internal actions under a single _INTERNAL_STATUS flags. So no
change necessary for now.
+++ b/src/conf/domain_conf.h
@@ -343,6 +343,27 @@ enum virDomainNetVirtioTxModeType {
VIR_DOMAIN_NET_VIRTIO_TX_MODE_LAST,
};
+/* Config that was actually used to bring up interface, after
+ * resolving network reference. This is private data, only used within
+ * libvirt, but still must maintain backward compatibility, because
+ * different versions of libvirt may read the same data file.
+ */
+typedef struct _virDomainActualNetDef virDomainActualNetDef;
+typedef virDomainActualNetDef *virDomainActualNetDefPtr;
+struct _virDomainActualNetDef {
+ enum virDomainNetType type;
Per the earlier cast comment, use int here, to match most other
_virDomain*Def typed unions.
@@ -743,7 +749,6 @@ virNetworkSaveConfig;
virNetworkSetBridgeMacAddr;
virNetworkSetBridgeName;
-
# node_device_conf.h
A spurious whitespace change.
+++ b/src/libxl/libxl_driver.c
@@ -707,7 +707,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
}
vm->def->id = domid;
- if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL)
+ if ((dom_xml = virDomainDefFormat(vm->def)) == NULL)
Oops, that's a compile failure, caused by over-undoing your
now-abandoned idea of adding a parameter.
ACK with this squashed in:
diff --git i/cfg.mk w/cfg.mk
index 773a3df..f2a951d 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -97,6 +97,7 @@ useless_free_options = \
--name=virCommandFree \
--name=virConfFreeList \
--name=virConfFreeValue \
+ --name=virDomainActualNetDefFree \
--name=virDomainChrDefFree \
--name=virDomainChrSourceDefFree \
--name=virDomainControllerDefFree \
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index e6d07d1..f4a42db 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -752,6 +752,8 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
default:
break;
}
+
+ VIR_FREE(def);
}
void virDomainNetDefFree(virDomainNetDefPtr def)
@@ -2633,7 +2635,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
_("missing type attribute in interface's
<actual> element"));
goto error;
}
- if ((int)(actual->type = virDomainNetTypeFromString(type)) < 0) {
+ if ((actual->type = virDomainNetTypeFromString(type)) < 0) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown type '%s' in interface's
<actual> element"), type);
goto error;
@@ -2679,6 +2681,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
error:
VIR_FREE(type);
VIR_FREE(mode);
+ virDomainActualNetDefFree(actual);
ctxt->node = save_ctxt;
return ret;
@@ -6887,8 +6890,7 @@ virDomainDefPtr virDomainDefParseNode(virCapsPtr caps,
}
ctxt->node = root;
- def = virDomainDefParseXML(caps, xml, root, ctxt, expectedVirtTypes,
- flags);
+ def = virDomainDefParseXML(caps, xml, root, ctxt,
expectedVirtTypes, flags);
cleanup:
xmlXPathFreeContext(ctxt);
@@ -6919,8 +6921,7 @@ virDomainObjParseNode(virCapsPtr caps,
}
ctxt->node = root;
- obj = virDomainObjParseXML(caps, xml, ctxt, expectedVirtTypes,
- flags);
+ obj = virDomainObjParseXML(caps, xml, ctxt, expectedVirtTypes, flags);
cleanup:
xmlXPathFreeContext(ctxt);
diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index 9e9db41..11d27f3 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -351,7 +351,7 @@ enum virDomainNetVirtioTxModeType {
typedef struct _virDomainActualNetDef virDomainActualNetDef;
typedef virDomainActualNetDef *virDomainActualNetDefPtr;
struct _virDomainActualNetDef {
- enum virDomainNetType type;
+ int type; /* enum virDomainNetType */
union {
struct {
char *brname;
diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms
index f03951f..306550b 100644
--- i/src/libvirt_private.syms
+++ w/src/libvirt_private.syms
@@ -743,6 +743,7 @@ virNetworkSaveConfig;
virNetworkSetBridgeMacAddr;
virNetworkSetBridgeName;
+
# node_device_conf.h
virNodeDevCapTypeToString;
virNodeDevCapsDefFree;
diff --git i/src/libxl/libxl_driver.c w/src/libxl/libxl_driver.c
index fa3f6a8..e84fa36 100644
--- i/src/libxl/libxl_driver.c
+++ w/src/libxl/libxl_driver.c
@@ -707,7 +707,7 @@ libxlVmStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
}
vm->def->id = domid;
- if ((dom_xml = virDomainDefFormat(vm->def)) == NULL)
+ if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL)
goto error;
if (libxl_userdata_store(&priv->ctx, domid, "libvirt-xml",
@@ -1852,7 +1852,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
goto cleanup;
}
- if ((xml = virDomainDefFormat(vm->def)) == NULL)
+ if ((xml = virDomainDefFormat(vm->def, 0)) == NULL)
goto cleanup;
xml_len = strlen(xml) + 1;
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list