Re: [libvirt] [PATCHv2 4/9] conf: support abstracted interface info in domain interface XML

On 07/20/2011 12:21 AM, Laine Stump wrote: 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. ... 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] Add this to cfg.mk's list of free-like functions. Memory leak of def itself. This cast is not needed if you tweak domain_conf.h. Memory leak of actual on error cases. Your change and then undo made for a spurious hunk. and another one. 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. Per the earlier cast comment, use int here, to match most other _virDomain*Def typed unions. A spurious whitespace change. 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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org