On 07/20/2011 05:54 PM, Eric Blake wrote:
On 07/20/2011 12:21 AM, Laine Stump wrote:
<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?
I usually put a few blank lines in between what I'm changing and the surrounding text to minimize the amount of changed lines due to re-flowing. I then close up the blank lines before I'm done. OR I don't - this time I forgot.
[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]
I've been very bothered lately by Thunderbird's belief that it's okay to mess around with my text however it sees fit. For example, I can't get it to honor extra indenting whitespace I put at the beginning of lines when I'm putting XML examples in my message.
+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.
Derp!
+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.
Double derp! (This previously didn't leak because I allocated directly into the parent object, and let the caller's destructor cleanup on failure).
@@ -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.
Okay. But the original struct this was copied from (virDomainNetDef) uses the enum directly.
@@ -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:
Squashed and pushed. Thanks!