[libvirt] [PATCHv4 1/2] conf: make 'vnet' prefix a macro

Using a macro ensures that all the code is looking for the same prefix. * src/conf/domain_conf.h (VIR_NET_GENERATED_PREFIX): New macro. * src/conf/domain_conf.c (virDomainNetDefParseXML): Use it. * src/uml/uml_conf.c (umlConnectTapDevice): Likewise. * src/qemu/qemu_command.c (qemuNetworkIfaceConnect): Likewise. Suggested by Laine Stump. --- v4: new patch src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 4 ++++ src/qemu/qemu_command.c | 8 ++++---- src/uml/uml_conf.c | 8 ++++---- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 257a1ea..72eccde 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2819,7 +2819,7 @@ virDomainNetDefParseXML(virCapsPtr caps, ifname = virXMLPropString(cur, "dev"); if ((ifname != NULL) && ((flags & VIR_DOMAIN_XML_INACTIVE) && - (STRPREFIX((const char*)ifname, "vnet")))) { + (STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX)))) { /* An auto-generated target name, blank it out */ VIR_FREE(ifname); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c748f52..dd33eb0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -429,6 +429,10 @@ struct _virDomainNetDef { virBandwidthPtr bandwidth; }; +/* Used for prefix of ifname of any network name generated dynamically + * by libvirt, and cannot be used for a persistent network name. */ +# define VIR_NET_GENERATED_PREFIX "vnet" + enum virDomainChrDeviceType { VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL = 0, VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee42f1d..6a2e2ae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -188,7 +188,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, int err; int tapfd = -1; int vnet_hdr = 0; - int template_ifname = 0; + bool template_ifname = false; unsigned char tapmac[VIR_MAC_BUFLEN]; int actualType = virDomainNetGetActualType(net); @@ -244,15 +244,15 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } if (!net->ifname || - STRPREFIX(net->ifname, "vnet") || + STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - if (!(net->ifname = strdup("vnet%d"))) { + if (!(net->ifname = strdup(VIR_NET_GENERATED_PREFIX "%d"))) { virReportOOMError(); goto cleanup; } /* avoid exposing vnet%d in getXMLDesc or error outputs */ - template_ifname = 1; + template_ifname = true; } if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) && diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 417271e..7b5e094 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -115,7 +115,7 @@ umlConnectTapDevice(virConnectPtr conn, const char *bridge) { brControl *brctl = NULL; - int template_ifname = 0; + bool template_ifname = false; int err; unsigned char tapmac[VIR_MAC_BUFLEN]; @@ -126,13 +126,13 @@ umlConnectTapDevice(virConnectPtr conn, } if (!net->ifname || - STRPREFIX(net->ifname, "vnet") || + STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - if (!(net->ifname = strdup("vnet%d"))) + if (!(net->ifname = strdup(VIR_NET_GENERATED_PREFIX "%d"))) goto no_memory; /* avoid exposing vnet%d in getXMLDesc or error outputs */ - template_ifname = 1; + template_ifname = true; } memcpy(tapmac, net->mac, VIR_MAC_BUFLEN); -- 1.7.4.4

Originally noticed by comparing the xml generated by virDomainSave with the xml produced by reparsing and redumping that xml, but I also did an audit of every last use of VIR_DOMAIN_XML_INACTIVE in domain_conf.c to ensure that no other discrepancies exist. * src/conf/domain_conf.c (virDomainDeviceInfoIsSet): Add parameter, and update all callers. Make static. (virDomainNetDefFormat): Skip generated ifname. (virDomainDefFormatInternal): Skip default <seclabel>. (virDomainChrSourceDefParseXML): Skip generated pty path, and add parameter. Update callers. * src/conf/domain_conf.h (virDomainDeviceInfoIsSet): Delete. * src/libvirt_private.syms (domain_conf.h): Update. --- v4: also tweak virDomainDefFormatInternal, virDomainChrSourceDefParseXML, patch is now fully tested and complete v3: new patch in RFC state src/conf/domain_conf.c | 41 ++++++++++++++++++++++++----------------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 72eccde..e182cd6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1411,11 +1411,12 @@ int virDomainDeviceVirtioSerialAddressIsValid( } -int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info) +static int +virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) { if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) return 1; - if (info->alias) + if (info->alias && !(flags & VIR_DOMAIN_XML_INACTIVE)) return 1; return 0; } @@ -3297,7 +3298,7 @@ error: * <target>, which is used by <serial> but not <smartcard>). */ static int virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, - xmlNodePtr cur) + xmlNodePtr cur, unsigned int flags) { char *bindHost = NULL; char *bindService = NULL; @@ -3320,7 +3321,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PIPE: case VIR_DOMAIN_CHR_TYPE_UNIX: - if (path == NULL) + /* PTY path is only parsed from live xml. */ + if (path == NULL && + (def->type != VIR_DOMAIN_CHR_TYPE_PTY || + !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path"); break; @@ -3571,7 +3575,7 @@ virDomainChrDefParseXML(virCapsPtr caps, } cur = node->children; - remaining = virDomainChrSourceDefParseXML(&def->source, cur); + remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags); if (remaining < 0) goto error; if (remaining) { @@ -3703,7 +3707,7 @@ virDomainSmartcardDefParseXML(xmlNodePtr node, } cur = node->children; - if (virDomainChrSourceDefParseXML(&def->data.passthru, cur) < 0) + if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags) < 0) goto error; if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { @@ -8736,7 +8740,7 @@ virDomainControllerDefFormat(virBufferPtr buf, break; } - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -8966,9 +8970,14 @@ virDomainNetDefFormat(virBufferPtr buf, break; } - if (def->ifname) + + if (def->ifname && + !((flags & VIR_DOMAIN_XML_INACTIVE) && + (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) { + /* Skip auto-generated target names for inactive config. */ virBufferEscapeString(buf, " <target dev='%s'/>\n", def->ifname); + } if (def->model) { virBufferEscapeString(buf, " <model type='%s'/>\n", def->model); @@ -9204,7 +9213,7 @@ virDomainChrDefFormat(virBufferPtr buf, break; } - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; } @@ -9232,7 +9241,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <smartcard mode='%s'", mode); switch (def->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virDomainDeviceInfoIsSet(&def->info)) { + if (!virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, "/>\n"); return 0; } @@ -9282,7 +9291,7 @@ virDomainSoundDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <sound model='%s'", model); - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -9311,7 +9320,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <memballoon model='%s'", model); - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -9360,7 +9369,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <watchdog model='%s' action='%s'", model, action); - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -9443,7 +9452,7 @@ virDomainInputDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <input type='%s' bus='%s'", type, bus); - if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -10265,9 +10274,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC && !def->seclabel.baselabel && (flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferAsprintf(&buf, " <seclabel type='%s' model='%s' relabel='%s'/>\n", - sectype, def->seclabel.model, - def->seclabel.norelabel ? "no" : "yes"); + /* This is the default for inactive xml, so nothing to output. */ } else { virBufferAsprintf(&buf, " <seclabel type='%s' model='%s' relabel='%s'>\n", sectype, def->seclabel.model, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index dd33eb0..abf9cbd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1412,7 +1412,6 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr); int virDomainDeviceVirtioSerialAddressIsValid(virDomainDeviceVirtioSerialAddressPtr addr); -int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info); void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8b4d582..830222b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -267,7 +267,6 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefFree; virDomainDeviceDefParse; -virDomainDeviceInfoIsSet; virDomainDeviceInfoIterate; virDomainDevicePCIAddressIsValid; virDomainDeviceTypeToString; -- 1.7.4.4

On 07/29/2011 01:15 PM, Eric Blake wrote:
Originally noticed by comparing the xml generated by virDomainSave with the xml produced by reparsing and redumping that xml, but I also did an audit of every last use of VIR_DOMAIN_XML_INACTIVE in domain_conf.c to ensure that no other discrepancies exist.
Picky Picky Picky :-) ACK.

On 07/29/2011 02:56 PM, Laine Stump wrote:
On 07/29/2011 01:15 PM, Eric Blake wrote:
Originally noticed by comparing the xml generated by virDomainSave with the xml produced by reparsing and redumping that xml, but I also did an audit of every last use of VIR_DOMAIN_XML_INACTIVE in domain_conf.c to ensure that no other discrepancies exist.
Picky Picky Picky :-)
ACK.
Thanks; series pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/29/2011 01:15 PM, Eric Blake wrote:
Using a macro ensures that all the code is looking for the same prefix.
* src/conf/domain_conf.h (VIR_NET_GENERATED_PREFIX): New macro. * src/conf/domain_conf.c (virDomainNetDefParseXML): Use it. * src/uml/uml_conf.c (umlConnectTapDevice): Likewise. * src/qemu/qemu_command.c (qemuNetworkIfaceConnect): Likewise. Suggested by Laine Stump. ---
ACK
participants (2)
-
Eric Blake
-
Laine Stump