--- src/conf/interface_conf.c | 11 +++++++---- src/conf/interface_conf.h | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 3c99027..ca82208 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -801,6 +801,7 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) { } VIR_FREE(tmp); } + def->data.bridge.delay = virXMLPropString(bridge, "delay"); ctxt->node = bridge; virInterfaceDefParseBridge(conn, def, ctxt); break; @@ -1038,12 +1039,14 @@ virInterfaceBridgeDefFormat(virConnectPtr conn, virBufferPtr buf, int i; int ret = 0; + virBufferAddLit(buf, " <bridge"); if (def->data.bridge.stp == 1) - virBufferAddLit(buf, " <bridge stp='on'>\n"); + virBufferAddLit(buf, " stp='on'"); else if (def->data.bridge.stp == 0) - virBufferAddLit(buf, " <bridge stp='off'>\n"); - else - virBufferAddLit(buf, " <bridge>\n"); + virBufferAddLit(buf, " stp='off'"); + if (def->data.bridge.delay != NULL) + virBufferVSprintf(buf, " delay='%s'", def->data.bridge.delay); + virBufferAddLit(buf, ">\n"); for (i = 0;i < def->data.bridge.nbItf;i++) { if (virInterfaceBareDevDefFormat(conn, buf, def->data.bridge.itf[i]) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index 2683eee..4216087 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -97,6 +97,7 @@ typedef struct _virInterfaceBridgeDef virInterfaceBridgeDef; typedef virInterfaceBridgeDef *virInterfaceBridgeDefPtr; struct _virInterfaceBridgeDef { int stp; /* 0, 1 or -1 if undefined */ + char *delay; int nbItf; /* number of defined interfaces */ virInterfaceBareDefPtr *itf;/* interfaces */ };

--- src/conf/interface_conf.c | 32 +++++++++++++++++--------------- 1 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index ca82208..d7821e7 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -557,28 +557,30 @@ virInterfaceDefParseBridge(virConnectPtr conn, virInterfaceDefPtr def, bridge = ctxt->node; nbItf = virXPathNodeSet(conn, "./interface", ctxt, &interfaces); - if (nbItf <= 0) { + if (nbItf < 0) { virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("bridge has no interfaces")); + "%s", _("bridge interfaces")); ret = -1; goto error; } - if (VIR_ALLOC_N(def->data.bridge.itf, nbItf) < 0) { - virReportOOMError(conn); - ret = -1; - goto error; - } - def->data.bridge.nbItf = nbItf; - - for (i = 0; i < nbItf;i++) { - ctxt->node = interfaces[i]; - itf = virInterfaceDefParseBareInterface(conn, ctxt, 0); - if (itf == NULL) { + if (nbItf > 0) { + if (VIR_ALLOC_N(def->data.bridge.itf, nbItf) < 0) { + virReportOOMError(conn); ret = -1; - def->data.bridge.nbItf = i; goto error; } - def->data.bridge.itf[i] = itf; + def->data.bridge.nbItf = nbItf; + + for (i = 0; i < nbItf;i++) { + ctxt->node = interfaces[i]; + itf = virInterfaceDefParseBareInterface(conn, ctxt, 0); + if (itf == NULL) { + ret = -1; + def->data.bridge.nbItf = i; + goto error; + } + def->data.bridge.itf[i] = itf; + } } error:

This was accomplished in xml parsing by doing away with the stripped-down virInterfaceBareDef object, and just always using virInterfaceDef, but with restrictions in certain places (eg, the type of subordinate interface allowed in parsing depends on the parent interface). xml formatting was similarly adjusted. In addition, the formatting functions keep track of the level of interface nesting, and insert extra leading spaces on each line accordingly (using %*s).

The only change in formatted xml from previous (aside frmo supporting new combinations of interface types) is that the subordinate ethernet interfaces take up 2 lines rather than one, eg: <interface type='ethernet' name='eth0'> </interface> instead of: <interface type='ethernet' name='eth0'/> --- src/conf/interface_conf.c | 362 +++++++++++++++++++-------------------------- src/conf/interface_conf.h | 13 +-- 2 files changed, 156 insertions(+), 219 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index d7821e7..6efc8aa 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -39,21 +39,18 @@ VIR_ENUM_IMPL(virInterface, VIR_INTERFACE_TYPE_LAST, "ethernet", "bridge", "bond", "vlan" ) +static virInterfaceDefPtr +virInterfaceDefParseXML(virConnectPtr conn, + xmlXPathContextPtr ctxt, int parentIfType); +static int +virInterfaceDefDevFormat(virConnectPtr conn, virBufferPtr buf, + const virInterfaceDefPtr def, int level); + #define virInterfaceReportError(conn, code, fmt...) \ virReportErrorHelper(conn, VIR_FROM_INTERFACE, code, __FILE__, \ __FUNCTION__, __LINE__, fmt) static -void virInterfaceBareDefFree(virInterfaceBareDefPtr def) { - if (def == NULL) - return; - VIR_FREE(def->name); - VIR_FREE(def->mac_or_tag); - VIR_FREE(def->devname); - VIR_FREE(def); -} - -static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { if (def == NULL) return; @@ -90,7 +87,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def) case VIR_INTERFACE_TYPE_BRIDGE: for (i = 0;i < def->data.bridge.nbItf;i++) { if (def->data.bridge.itf[i] != NULL) - virInterfaceBareDefFree(def->data.bridge.itf[i]); + virInterfaceDefFree(def->data.bridge.itf[i]); else break; /* to cope with half parsed data on errors */ } @@ -100,7 +97,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def) VIR_FREE(def->data.bond.target); for (i = 0;i < def->data.bond.nbItf;i++) { if (def->data.bond.itf[i] != NULL) - virInterfaceBareDefFree(def->data.bond.itf[i]); + virInterfaceDefFree(def->data.bond.itf[i]); else break; /* to cope with half parsed data on errors */ } @@ -121,11 +118,9 @@ void virInterfaceDefFree(virInterfaceDefPtr def) } static int -virInterfaceDefParseBasicAttrs(virConnectPtr conn, virInterfaceDefPtr def, - xmlXPathContextPtr ctxt) { +virInterfaceDefParseName(virConnectPtr conn, virInterfaceDefPtr def, + xmlXPathContextPtr ctxt) { char *tmp; - unsigned long mtu; - int ret; tmp = virXPathString(conn, "string(./@name)", ctxt); if (tmp == NULL) { @@ -134,6 +129,14 @@ virInterfaceDefParseBasicAttrs(virConnectPtr conn, virInterfaceDefPtr def, return(-1); } def->name = tmp; + return(0); +} + +static int +virInterfaceDefParseMtu(virConnectPtr conn, virInterfaceDefPtr def, + xmlXPathContextPtr ctxt) { + unsigned long mtu; + int ret; ret = virXPathULong(conn, "string(./mtu/@size)", ctxt, &mtu); if ((ret == -2) || ((ret == 0) && (mtu > 100000))) { @@ -478,80 +481,12 @@ error: } -static virInterfaceBareDefPtr -virInterfaceDefParseBareInterface(virConnectPtr conn, xmlXPathContextPtr ctxt, - int ethernet_only) { - int t; - char *name = NULL; - char *type = NULL; - char *mac_or_tag = NULL; - char *devname = NULL; - virInterfaceBareDefPtr ret = NULL; - - type = virXPathString(conn, "string(./@type)", ctxt); - if (type == NULL) { - virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("interface has no type")); - goto error; - } - if (STREQ(type, "ethernet")) { - t = VIR_INTERFACE_TYPE_ETHERNET; - name = virXPathString(conn, "string(./@name)", ctxt); - mac_or_tag = virXPathString(conn, "string(./mac/@address)", ctxt); - } else if ((STREQ(type, "vlan")) && (ethernet_only == 0)) { - t = VIR_INTERFACE_TYPE_VLAN; - name = virXPathString(conn, "string(./@name)", ctxt); - mac_or_tag = virXPathString(conn, "string(./vlan/@tag)", ctxt); - devname = virXPathString(conn, "string(./vlan/interface/@name)", ctxt); - } else { - virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - _("interface has unsupported type '%s'"), type); - VIR_FREE(type); - goto error; - } - VIR_FREE(type); - if (name == NULL) { - virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("interface has no name")); - goto error; - } - if (t == VIR_INTERFACE_TYPE_VLAN) { - if (mac_or_tag == NULL) { - virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - _("vlan %s has no tag"), name); - goto error; - } - if (devname == NULL) { - virInterfaceReportError(conn, VIR_ERR_XML_ERROR, - _("vlan %s has interface name"), name); - goto error; - } - } - if (VIR_ALLOC(ret) < 0) { - virReportOOMError(conn); - goto error; - } - ret->type = t; - ret->name = name; - ret->mac_or_tag = mac_or_tag; - ret->devname = devname; - return(ret); - -error: - VIR_FREE(name); - VIR_FREE(type); - VIR_FREE(name); - VIR_FREE(name); - VIR_FREE(ret); - return(NULL); -} - static int virInterfaceDefParseBridge(virConnectPtr conn, virInterfaceDefPtr def, xmlXPathContextPtr ctxt) { xmlNodePtr *interfaces = NULL; xmlNodePtr bridge; - virInterfaceBareDefPtr itf; + virInterfaceDefPtr itf; int nbItf, i; int ret = 0; @@ -573,7 +508,7 @@ virInterfaceDefParseBridge(virConnectPtr conn, virInterfaceDefPtr def, for (i = 0; i < nbItf;i++) { ctxt->node = interfaces[i]; - itf = virInterfaceDefParseBareInterface(conn, ctxt, 0); + itf = virInterfaceDefParseXML(conn, ctxt, VIR_INTERFACE_TYPE_BRIDGE); if (itf == NULL) { ret = -1; def->data.bridge.nbItf = i; @@ -594,7 +529,7 @@ virInterfaceDefParseBondItfs(virConnectPtr conn, virInterfaceDefPtr def, xmlXPathContextPtr ctxt) { xmlNodePtr *interfaces = NULL; xmlNodePtr bond = ctxt->node; - virInterfaceBareDefPtr itf; + virInterfaceDefPtr itf; int nbItf, i; int ret = 0; @@ -614,7 +549,7 @@ virInterfaceDefParseBondItfs(virConnectPtr conn, virInterfaceDefPtr def, for (i = 0; i < nbItf;i++) { ctxt->node = interfaces[i]; - itf = virInterfaceDefParseBareInterface(conn, ctxt, 1); + itf = virInterfaceDefParseXML(conn, ctxt, VIR_INTERFACE_TYPE_BOND); if (itf == NULL) { ret = -1; def->data.bond.nbItf = i; @@ -732,7 +667,8 @@ virInterfaceDefParseVlan(virConnectPtr conn, virInterfaceDefPtr def, } static virInterfaceDefPtr -virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) { +virInterfaceDefParseXML(virConnectPtr conn, + xmlXPathContextPtr ctxt, int parentIfType) { virInterfaceDefPtr def; int type; char *tmp; @@ -758,25 +694,46 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) { virReportOOMError(conn); return NULL; } + + if (((parentIfType == VIR_INTERFACE_TYPE_BOND) + && (type != VIR_INTERFACE_TYPE_ETHERNET)) + || ((parentIfType == VIR_INTERFACE_TYPE_BRIDGE) + && (type != VIR_INTERFACE_TYPE_ETHERNET) + && (type != VIR_INTERFACE_TYPE_BOND) + && (type != VIR_INTERFACE_TYPE_VLAN)) + || (parentIfType == VIR_INTERFACE_TYPE_ETHERNET) + || (parentIfType == VIR_INTERFACE_TYPE_VLAN)) + { + virInterfaceReportError(conn, VIR_ERR_XML_ERROR, + _("interface has unsupported type '%s'"), type); + goto error; + } def->type = type; switch (type) { case VIR_INTERFACE_TYPE_ETHERNET: - if (virInterfaceDefParseBasicAttrs(conn, def, ctxt) < 0) + if (virInterfaceDefParseName(conn, def, ctxt) < 0) goto error; tmp = virXPathString(conn, "string(./mac/@address)", ctxt); if (tmp != NULL) def->mac = tmp; - if (virInterfaceDefParseStartMode(conn, def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0) - goto error; + if (parentIfType == VIR_INTERFACE_TYPE_LAST) { + /* only recognize these in toplevel bond interfaces */ + if (virInterfaceDefParseStartMode(conn, def, ctxt) < 0) + goto error; + if (virInterfaceDefParseMtu(conn, def, ctxt) < 0) + goto error; + if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0) + goto error; + } break; case VIR_INTERFACE_TYPE_BRIDGE: { xmlNodePtr bridge; + if (virInterfaceDefParseName(conn, def, ctxt) < 0) + goto error; if (virInterfaceDefParseStartMode(conn, def, ctxt) < 0) goto error; - if (virInterfaceDefParseBasicAttrs(conn, def, ctxt) < 0) + if (virInterfaceDefParseMtu(conn, def, ctxt) < 0) goto error; if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0) goto error; @@ -811,12 +768,18 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) { case VIR_INTERFACE_TYPE_BOND: { xmlNodePtr bond; - if (virInterfaceDefParseBasicAttrs(conn, def, ctxt) < 0) - goto error; - if (virInterfaceDefParseStartMode(conn, def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0) + if (virInterfaceDefParseName(conn, def, ctxt) < 0) goto error; + if (parentIfType == VIR_INTERFACE_TYPE_LAST) { + /* only recognize these in toplevel bond interfaces */ + if (virInterfaceDefParseStartMode(conn, def, ctxt) < 0) + goto error; + if (virInterfaceDefParseMtu(conn, def, ctxt) < 0) + goto error; + if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0) + goto error; + } + bond = virXPathNode(conn, "./bond[1]", ctxt); if (bond == NULL) { virInterfaceReportError(conn, VIR_ERR_XML_ERROR, @@ -881,7 +844,7 @@ virInterfaceDefPtr virInterfaceDefParseNode(virConnectPtr conn, } ctxt->node = root; - def = virInterfaceDefParseXML(conn, ctxt); + def = virInterfaceDefParseXML(conn, ctxt, VIR_INTERFACE_TYPE_LAST); cleanup: xmlXPathFreeContext(ctxt); @@ -990,58 +953,12 @@ cleanup: } static int -virInterfaceBareDevDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED, - virBufferPtr buf, - const virInterfaceBareDefPtr def) { - if (def->type == VIR_INTERFACE_TYPE_ETHERNET) { - if (def->name == NULL) { - virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("bare ethernet has no name")); - return(-1); - } - virBufferVSprintf(buf, " <interface type='ethernet' name='%s'", - def->name); - if (def->mac_or_tag != NULL) { - virBufferVSprintf(buf, ">\n <mac address='%s'/>\n", - def->mac_or_tag); - virBufferAddLit(buf, " </interface>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - } else if (def->type == VIR_INTERFACE_TYPE_VLAN) { - virBufferAddLit(buf, " <interface type='vlan'"); - if (def->name != NULL) - virBufferVSprintf(buf, " name='%s'", def->name); - if (def->mac_or_tag != NULL) { - virBufferAddLit(buf, ">\n"); - virBufferVSprintf(buf, " <vlan tag='%s'", def->mac_or_tag); - if (def->devname != NULL) { - virBufferAddLit(buf, ">\n"); - virBufferVSprintf(buf, " <interface name='%s'/>\n", - def->devname); - virBufferAddLit(buf, " </vlan>\n"); - } else - virBufferAddLit(buf, "/>\n"); - virBufferAddLit(buf, " </interface>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - } else { - virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("bare interface type %d unknown"), - def->type); - return(-1); - } - return(0); -} - -static int virInterfaceBridgeDefFormat(virConnectPtr conn, virBufferPtr buf, - const virInterfaceDefPtr def) { + const virInterfaceDefPtr def, int level) { int i; int ret = 0; - virBufferAddLit(buf, " <bridge"); + virBufferVSprintf(buf, "%*s <bridge", level*2, ""); if (def->data.bridge.stp == 1) virBufferAddLit(buf, " stp='on'"); else if (def->data.bridge.stp == 0) @@ -1051,22 +968,22 @@ virInterfaceBridgeDefFormat(virConnectPtr conn, virBufferPtr buf, virBufferAddLit(buf, ">\n"); for (i = 0;i < def->data.bridge.nbItf;i++) { - if (virInterfaceBareDevDefFormat(conn, buf, def->data.bridge.itf[i]) - < 0) + if (virInterfaceDefDevFormat(conn, buf, + def->data.bridge.itf[i], level+2) < 0) ret = -1; } - virBufferAddLit(buf, " </bridge>\n"); + virBufferVSprintf(buf, "%*s </bridge>\n", level*2, ""); return(ret); } static int virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf, - const virInterfaceDefPtr def) { + const virInterfaceDefPtr def, int level) { int i; int ret = 0; - virBufferAddLit(buf, " <bond"); + virBufferVSprintf(buf, "%*s <bond", level*2, ""); if (def->data.bond.mode == VIR_INTERFACE_BOND_BALRR) virBufferAddLit(buf, " mode='balance-rr'"); else if (def->data.bond.mode == VIR_INTERFACE_BOND_ABACKUP) @@ -1084,8 +1001,8 @@ virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf, virBufferAddLit(buf, ">\n"); if (def->data.bond.monit == VIR_INTERFACE_BOND_MONIT_MII) { - virBufferVSprintf(buf, " <miimon freq='%d'", - def->data.bond.frequency); + virBufferVSprintf(buf, "%*s <miimon freq='%d'", + level*2, "", def->data.bond.frequency); if (def->data.bond.downdelay > 0) virBufferVSprintf(buf, " downdelay='%d'", def->data.bond.downdelay); if (def->data.bond.updelay > 0) @@ -1101,7 +1018,8 @@ virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf, "%s", _("bond arp monitoring has no target")); return(-1); } - virBufferVSprintf(buf, " <arpmon interval='%d' target='%s'", + virBufferVSprintf(buf, "%*s <arpmon interval='%d' target='%s'", + level*2, "", def->data.bond.interval, def->data.bond.target); if (def->data.bond.validate == VIR_INTERFACE_BOND_ARP_ACTIVE) virBufferAddLit(buf, " validate='active'"); @@ -1112,29 +1030,30 @@ virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } for (i = 0;i < def->data.bond.nbItf;i++) { - if (virInterfaceBareDevDefFormat(conn, buf, def->data.bond.itf[i]) < 0) + if (virInterfaceDefDevFormat(conn, buf, def->data.bond.itf[i], level+2) < 0) ret = -1; } - virBufferAddLit(buf, " </bond>\n"); + virBufferVSprintf(buf, "%*s </bond>\n", level*2, ""); return(ret); } static int virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf, - const virInterfaceDefPtr def) { + const virInterfaceDefPtr def, int level) { if (def->data.vlan.tag == NULL) { virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("vlan misses the tag name")); return(-1); } - virBufferVSprintf(buf, " <vlan tag='%s'", def->data.vlan.tag); + virBufferVSprintf(buf, "%*s <vlan tag='%s'", + level*2, "", def->data.vlan.tag); if (def->data.vlan.devname != NULL) { virBufferAddLit(buf, ">\n"); - virBufferVSprintf(buf, " <interface name='%s'/>\n", - def->data.vlan.devname); - virBufferAddLit(buf, " </vlan>\n"); + virBufferVSprintf(buf, "%*s <interface name='%s'/>\n", + level*2, "", def->data.vlan.devname); + virBufferVSprintf(buf, "%*s </vlan>\n", level*2, ""); } else virBufferAddLit(buf, "/>\n"); return(0); @@ -1142,30 +1061,34 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf, static int virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED, - virBufferPtr buf, const virInterfaceDefPtr def) { + virBufferPtr buf, const virInterfaceDefPtr def, + int level) { int pp, ii; for (pp = 0; pp < def->nprotos; pp++) { - virBufferVSprintf(buf, " <protocol family='%s'>\n", def->protos[pp]->family); + virBufferVSprintf(buf, "%*s <protocol family='%s'>\n", + level*2, "", def->protos[pp]->family); if (def->protos[pp]->autoconf) { - virBufferAddLit(buf, " <autoconf/>\n"); + virBufferVSprintf(buf, "%*s <autoconf/>\n", level*2, ""); } if (def->protos[pp]->dhcp) { if (def->protos[pp]->peerdns == 0) - virBufferAddLit(buf, " <dhcp peerdns='no'/>\n"); + virBufferVSprintf(buf, "%*s <dhcp peerdns='no'/>\n", + level*2, ""); else if (def->protos[pp]->peerdns == 1) - virBufferAddLit(buf, " <dhcp peerdns='yes'/>\n"); + virBufferVSprintf(buf, "%*s <dhcp peerdns='yes'/>\n", + level*2, ""); else - virBufferAddLit(buf, " <dhcp/>\n"); + virBufferVSprintf(buf, "%*s <dhcp/>\n", level*2, ""); } for (ii = 0; ii < def->protos[pp]->nips; ii++) { if (def->protos[pp]->ips[ii]->address != NULL) { - virBufferVSprintf(buf, " <ip address='%s'", + virBufferVSprintf(buf, "%*s <ip address='%s'", level*2, "", def->protos[pp]->ips[ii]->address); if (def->protos[pp]->ips[ii]->prefix != 0) { virBufferVSprintf(buf, " prefix='%d'", @@ -1175,18 +1098,19 @@ virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED, } } if (def->protos[pp]->gateway != NULL) { - virBufferVSprintf(buf, " <route gateway='%s'/>\n", - def->protos[pp]->gateway); + virBufferVSprintf(buf, "%*s <route gateway='%s'/>\n", + level*2, "", def->protos[pp]->gateway); } - virBufferAddLit(buf, " </protocol>\n"); + virBufferVSprintf(buf, "%*s </protocol>\n", level*2, ""); } return(0); } static int virInterfaceStartmodeDefFormat(virConnectPtr conn, virBufferPtr buf, - enum virInterfaceStartMode startmode) { + enum virInterfaceStartMode startmode, + int level) { const char *mode; switch (startmode) { case VIR_INTERFACE_START_UNSPECIFIED: @@ -1205,20 +1129,24 @@ virInterfaceStartmodeDefFormat(virConnectPtr conn, virBufferPtr buf, "%s", _("virInterfaceDefFormat unknown startmode")); return -1; } - virBufferVSprintf(buf, " <start mode='%s'/>\n", mode); + virBufferVSprintf(buf, "%*s <start mode='%s'/>\n", level*2, "", mode); return(0); } -char *virInterfaceDefFormat(virConnectPtr conn, - const virInterfaceDefPtr def) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; +static int +virInterfaceDefDevFormat(virConnectPtr conn, virBufferPtr buf, + const virInterfaceDefPtr def, int level) { const char *type = NULL; - if ((def == NULL) || - ((def->name == NULL) && (def->type != VIR_INTERFACE_TYPE_VLAN))) { + if (def == NULL) { virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("virInterfaceDefFormat argument problems")); + "%s", _("virInterfaceDefFormat NULL def")); + goto cleanup; + } + + if ((def->name == NULL) && (def->type != VIR_INTERFACE_TYPE_VLAN)) { + virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("virInterfaceDefFormat missing interface name")); goto cleanup; } @@ -1228,56 +1156,72 @@ char *virInterfaceDefFormat(virConnectPtr conn, goto cleanup; } - virBufferVSprintf(&buf, "<interface type='%s' ", type); + virBufferVSprintf(buf, "%*s<interface type='%s' ", level*2, "", type); if (def->name != NULL) - virBufferEscapeString(&buf, "name='%s'", def->name); - virBufferAddLit(&buf, ">\n"); + virBufferEscapeString(buf, "name='%s'", def->name); + virBufferAddLit(buf, ">\n"); switch (def->type) { case VIR_INTERFACE_TYPE_ETHERNET: - virInterfaceStartmodeDefFormat(conn, &buf, def->startmode); + virInterfaceStartmodeDefFormat(conn, buf, def->startmode, level); if (def->mac != NULL) - virBufferVSprintf(&buf, " <mac address='%s'/>\n", def->mac); + virBufferVSprintf(buf, "%*s <mac address='%s'/>\n", + level*2, "", def->mac); if (def->mtu != 0) - virBufferVSprintf(&buf, " <mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(conn, &buf, def); + virBufferVSprintf(buf, "%*s <mtu size='%d'/>\n", + level*2, "", def->mtu); + virInterfaceProtocolDefFormat(conn, buf, def, level); break; case VIR_INTERFACE_TYPE_BRIDGE: - virInterfaceStartmodeDefFormat(conn, &buf, def->startmode); + virInterfaceStartmodeDefFormat(conn, buf, def->startmode, level); if (def->mtu != 0) - virBufferVSprintf(&buf, " <mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(conn, &buf, def); - virInterfaceBridgeDefFormat(conn, &buf, def); + virBufferVSprintf(buf, "%*s <mtu size='%d'/>\n", + level*2, "", def->mtu); + virInterfaceProtocolDefFormat(conn, buf, def, level); + virInterfaceBridgeDefFormat(conn, buf, def, level); break; case VIR_INTERFACE_TYPE_BOND: - virInterfaceStartmodeDefFormat(conn, &buf, def->startmode); + virInterfaceStartmodeDefFormat(conn, buf, def->startmode, level); if (def->mtu != 0) - virBufferVSprintf(&buf, " <mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(conn, &buf, def); - virInterfaceBondDefFormat(conn, &buf, def); + virBufferVSprintf(buf, "%*s <mtu size='%d'/>\n", + level*2, "", def->mtu); + virInterfaceProtocolDefFormat(conn, buf, def, level); + virInterfaceBondDefFormat(conn, buf, def, level); break; case VIR_INTERFACE_TYPE_VLAN: - virInterfaceStartmodeDefFormat(conn, &buf, def->startmode); + virInterfaceStartmodeDefFormat(conn, buf, def->startmode, level); if (def->mac != NULL) - virBufferVSprintf(&buf, " <mac address='%s'/>\n", def->mac); + virBufferVSprintf(buf, "%*s <mac address='%s'/>\n", + level*2, "", def->mac); if (def->mtu != 0) - virBufferVSprintf(&buf, " <mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(conn, &buf, def); - virInterfaceVlanDefFormat(conn, &buf, def); + virBufferVSprintf(buf, "%*s <mtu size='%d'/>\n", + level*2, "", def->mtu); + virInterfaceProtocolDefFormat(conn, buf, def, level); + virInterfaceVlanDefFormat(conn, buf, def, level); break; } - virBufferAddLit(&buf, "</interface>\n"); + virBufferVSprintf(buf, "%*s</interface>\n", level*2, ""); - if (virBufferError(&buf)) + if (virBufferError(buf)) goto no_memory; - return virBufferContentAndReset(&buf); - + return 0; no_memory: virReportOOMError(conn); cleanup: - virBufferFreeAndReset(&buf); - return NULL; + return -1; +} + +char *virInterfaceDefFormat(virConnectPtr conn, + const virInterfaceDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (virInterfaceDefDevFormat(conn, &buf, def, 0) < 0) { + virBufferFreeAndReset(&buf); + return NULL; + } + return virBufferContentAndReset(&buf); } /* virInterfaceObj manipulation */ diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index 4216087..889ad94 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -84,14 +84,7 @@ enum virInterfaceBondArpValid { VIR_INTERFACE_BOND_ARP_ALL, /* validate all */ }; -typedef struct _virInterfaceBareDef virInterfaceBareDef; -typedef virInterfaceBareDef *virInterfaceBareDefPtr; -struct _virInterfaceBareDef { - int type; /* should be only ethernet or vlan */ - char *name; /* the interface name */ - char *mac_or_tag; /* MAC address for ethernet, TAG for vlan */ - char *devname; /* device name for vlan */ -}; +struct _virInterfaceDef; /* forward declaration required for bridge/bond */ typedef struct _virInterfaceBridgeDef virInterfaceBridgeDef; typedef virInterfaceBridgeDef *virInterfaceBridgeDefPtr; @@ -99,7 +92,7 @@ struct _virInterfaceBridgeDef { int stp; /* 0, 1 or -1 if undefined */ char *delay; int nbItf; /* number of defined interfaces */ - virInterfaceBareDefPtr *itf;/* interfaces */ + struct _virInterfaceDef **itf;/* interfaces */ }; typedef struct _virInterfaceBondDef virInterfaceBondDef; @@ -115,7 +108,7 @@ struct _virInterfaceBondDef { char *target; /* arp monitoring target */ int validate; /* virInterfaceBondArpmValid */ int nbItf; /* number of defined interfaces */ - virInterfaceBareDefPtr *itf; /* interfaces ethernet only */ + struct _virInterfaceDef **itf; /* interfaces ethernet only */ }; typedef struct _virInterfaceVlanDef virInterfaceVlanDef;

On 01/13/2010 12:09 PM, Laine Stump wrote: > > It's been 12 days with no responses to these patches, so I figured I'd > better give them some visibility so they're not forgotten. > > https://www.redhat.com/archives/libvir-list/2010-January/msg00003.html Dan was kind enough to review and ACK these. Now if someone could just push them... ;-)

On 01/19/2010 03:24 PM, Matthias Bolte wrote: > > 2010/1/19 Laine Stump&lt;laine(a)laine.org&gt;: > >> >> On 01/13/2010 12:09 PM, Laine Stump wrote: >> >>> >>> It's been 12 days with no responses to these patches, so I figured I'd >>> better give them some visibility so they're not forgotten. >>> >>> https://www.redhat.com/archives/libvir-list/2010-January/msg00003.html >>> >> >> Dan was kind enough to review and ACK these. Now if someone could just >> push >> them... ;-) >> >> > > Okay. pushed :) > Thanks! > I had to apply this small patch to fix a compile error: > > > diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c > index 6efc8aa..8a17ed6 100644 > --- a/src/conf/interface_conf.c > +++ b/src/conf/interface_conf.c > @@ -705,7 +705,8 @@ virInterfaceDefParseXML(virConnectPtr conn, >          || (parentIfType == VIR_INTERFACE_TYPE_VLAN)) >          { >          virInterfaceReportError(conn, VIR_ERR_XML_ERROR, > -                                _("interface has unsupported type > '%s'"), type); > +                                _("interface has unsupported type '%s'"), > +                                virInterfaceTypeToString(type)); >          goto error; >      } >      def->type = type; > This is troublesome - I'm glad you caught it. The code was so obviously wrong, and yet when I  build with it (just tried it again to be sure), I don't even get a warning (I tried turning on -Werror and the whole process aborts much earlier with warnings in some other file). What CFLAGS are you giving to configure? Here's what I have: CFLAGS="-g -Wformat -Wformat-security -Winit-self -Wall -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Winline -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -Dlint"