In practice, if a virDomainNetDef has a virDomainActualNetDef allocated, the ActualNetDef will *always* contain the bandwidth and vlan data from the NetDef (unless there was also a portgroup involved - see networkAllocateActualDevice()). However, virDomainNetGetActual(Bandwidth|Vlan)() were coded to make it appear as if it might be possible to have a valid bandwidth/vlan in the NetDef, but a NULL in the ActualNetDef. Believing this un-truth could lead to writing unnecessarily defensive code when dealing with the virDomainGetActual*() functions, so this patch makes it more obvious: If there is an ActualNetDef, it will always have a copy of the various appropriate bits from its parent NetDef, and the virDomainGetActual* function will *always* return the data from the ActualNetDef, not from the NetDef. The reason for this effective-NOP patch is that a subsequent patch to change virDomainNetDefFormat will rely on the above rule. --- src/conf/domain_conf.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 064a40e..755066c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18638,8 +18638,11 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface) virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface) { + /* if there is an ActualNetDef, *always* return + * its bandwidth rather than the NetDef's bandwidth. + */ if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && - iface->data.network.actual && iface->data.network.actual->bandwidth) { + iface->data.network.actual) { return iface->data.network.actual->bandwidth; } return iface->bandwidth; @@ -18648,12 +18651,17 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface) virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface) { + virNetDevVlanPtr vlan = &iface->vlan; + + /* if there is an ActualNetDef, *always* return + * its vlan rather than the NetDef's vlan. + */ if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && - iface->data.network.actual && - iface->data.network.actual->vlan.nTags > 0) - return &iface->data.network.actual->vlan; - if (iface->vlan.nTags > 0) - return &iface->vlan; + iface->data.network.actual) + vlan = &iface->data.network.actual->vlan; + + if (vlan->nTags > 0) + return vlan; return 0; } -- 1.8.5.3

Other *Format() functions (e.g. virNetDevBandwidthFormat()) return with no action when called with a NULL *Def pointer. This makes virNetDevVlanFormat() consistent with that behavior. --- src/conf/netdev_vlan_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index f58b4b8..dbe203e 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -144,7 +144,7 @@ virNetDevVlanFormat(const virNetDevVlan *def, virBufferPtr buf) { size_t i; - if (def->nTags == 0) + if (!(def && def->nTags)) return 0; if (!def->tag) { -- 1.8.5.3

We will need to call virDomainNetDefFormat() from the network hook (in the network driver). --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 755066c..b5b70cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15504,7 +15504,7 @@ virDomainActualNetDefFormat(virBufferPtr buf, return 0; } -static int +int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 97d6337..2467f65 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2402,6 +2402,10 @@ int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virDomainDiskSourcePoolDefPtr srcpool, unsigned int flags); +int virDomainNetDefFormat(virBufferPtr buf, + virDomainNetDefPtr def, + unsigned int flags); + int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0896287..bb7b6da 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -300,6 +300,7 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemDumpTypeFromString; virDomainMemDumpTypeToString; +virDomainNetDefFormat; virDomainNetDefFree; virDomainNetFind; virDomainNetFindIdx; -- 1.8.5.3

This moves the call to virNetDevBandwidthFormat() in virDomainNetDefFormat() to be called right after the call to virNetDevVPortProfileFormat(), so that a single chunk of that function can be placed inside an if that conditionally calls virDomainActualNetDefContentsFormat() instead (next patch). The re-ordering necessitates modifying a couple of test data files. --- src/conf/domain_conf.c | 6 +++--- .../qemuhotplug-console-compat-2+console-virtio.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5b70cd..67fc372 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15599,6 +15599,9 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) return -1; + if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) + return -1; + virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); if (def->ifname && @@ -15651,9 +15654,6 @@ virDomainNetDefFormat(virBufferPtr buf, virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); } - if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) - return -1; - virBufferAdjustIndent(buf, -6); if (virDomainDeviceInfoFormat(buf, &def->info, diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml index a484e82..25fc120 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml @@ -59,12 +59,12 @@ <interface type='network'> <mac address='52:54:00:ea:35:6f'/> <source network='default'/> - <target dev='vnet0'/> - <model type='virtio'/> <bandwidth> <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> + <target dev='vnet0'/> + <model type='virtio'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml index 065ef2d..e5c45a3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml @@ -59,12 +59,12 @@ <interface type='network'> <mac address='52:54:00:ea:35:6f'/> <source network='default'/> - <target dev='vnet0'/> - <model type='virtio'/> <bandwidth> <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> + <target dev='vnet0'/> + <model type='virtio'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml index 064a05f..f70e20a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml @@ -45,11 +45,11 @@ <interface type='network'> <mac address='52:54:00:24:a5:9f'/> <source network='default'/> - <model type='rtl8139'/> <bandwidth> <inbound average='1000' peak='4000' burst='1024'/> <outbound average='128' peak='256' burst='32768'/> </bandwidth> + <model type='rtl8139'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'> -- 1.8.5.3

This function is currently only called from one place, but in a subsequent patch will be called from a 2nd place. The new function exactly replicates the original behavior of the part of virDomainActualNetDefFormat() that it replaces, but takes a virDomainNetDefPtr instead of virDomainActualNetDefPtr, and uses the virDomainNetGetActual*() functions whenever possible, rather than reaching into def->data.network.actual - this is to be sure that we are reporting exactly what is being used internally, just in case there are any discrepancies (there shouldn't be). --- src/conf/domain_conf.c | 101 +++++++++++++++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 67fc372..5c7e6ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15427,78 +15427,100 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf, return 0; } +/* virDomainActualNetDefContentsFormat() - format just the subelements + * of <interface> that may be overridden by what is in the + * virDomainActualNetDef, but inside the current element, rather + * than enclosed in an <actual> subelement. + */ static int -virDomainActualNetDefFormat(virBufferPtr buf, - virDomainActualNetDefPtr def, - unsigned int flags) +virDomainActualNetDefContentsFormat(virBufferPtr buf, + virDomainNetDefPtr def, + const char *typeStr, + unsigned int flags) { - const char *type; const char *mode; - if (!def) - return 0; - - type = virDomainNetTypeToString(def->type); - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected net type %d"), def->type); - return -1; - } - - virBufferAsprintf(buf, "<actual type='%s'", type); - if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV && - def->data.hostdev.def.managed) { - virBufferAddLit(buf, " managed='yes'"); - } - virBufferAddLit(buf, ">\n"); - - virBufferAdjustIndent(buf, 2); - switch (def->type) { + switch (virDomainNetGetActualType(def)) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, "<source bridge='%s'/>\n", - def->data.bridge.brname); + virDomainNetGetActualBridgeName(def)); break; case VIR_DOMAIN_NET_TYPE_DIRECT: virBufferAddLit(buf, "<source"); - if (def->data.direct.linkdev) - virBufferEscapeString(buf, " dev='%s'", - def->data.direct.linkdev); + virBufferEscapeString(buf, " dev='%s'", + virDomainNetGetActualDirectDev(def)); - mode = virNetDevMacVLanModeTypeToString(def->data.direct.mode); + mode = virNetDevMacVLanModeTypeToString(virDomainNetGetActualDirectMode(def)); if (!mode) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected source mode %d"), - def->data.direct.mode); + virDomainNetGetActualDirectMode(def)); return -1; } virBufferAsprintf(buf, " mode='%s'/>\n", mode); break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: - if (virDomainHostdevDefFormatSubsys(buf, &def->data.hostdev.def, + if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def), flags, true) < 0) { return -1; } break; case VIR_DOMAIN_NET_TYPE_NETWORK: - if (def->class_id) - virBufferAsprintf(buf, "<class id='%u'/>", def->class_id); + if (def->data.network.actual && def->data.network.actual->class_id) + virBufferAsprintf(buf, "<class id='%u'/>", + def->data.network.actual->class_id); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected net type %s"), type); + _("unexpected actual net type %s"), typeStr); return -1; } - if (virNetDevVlanFormat(&def->vlan, buf) < 0) + if (virNetDevVlanFormat(virDomainNetGetActualVlan(def), buf) < 0) return -1; - if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) + if (virNetDevVPortProfileFormat(virDomainNetGetActualVirtPortProfile(def), buf) < 0) return -1; - if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) + if (virNetDevBandwidthFormat(virDomainNetGetActualBandwidth(def), buf) < 0) return -1; + return 0; +} + + +/* virDomainActualNetDefFormat() - format the ActualNetDef + * info inside an <actual> element, as required for internal storage + * of domain status + */ +static int +virDomainActualNetDefFormat(virBufferPtr buf, + virDomainNetDefPtr def, + unsigned int flags) +{ + unsigned int type = virDomainNetGetActualType(def); + const char *typeStr = virDomainNetTypeToString(type); + + if (!def) + return 0; + if (!typeStr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->type); + return -1; + } + + virBufferAsprintf(buf, "<actual type='%s'", typeStr); + if (type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virDomainHostdevDefPtr hostdef = virDomainNetGetActualHostdev(def); + if (hostdef && hostdef->managed) + virBufferAddLit(buf, " managed='yes'"); + } + virBufferAddLit(buf, ">\n"); + + virBufferAdjustIndent(buf, 2); + if (virDomainActualNetDefContentsFormat(buf, def, typeStr, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</actual>\n"); return 0; @@ -15536,8 +15558,13 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " portgroup='%s'", def->data.network.portgroup); virBufferAddLit(buf, "/>\n"); + + /* ONLY for internal status storage - format the ActualNetDef + * as a subelement of <interface> so that no persistent config + * data is overwritten. + */ if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && - (virDomainActualNetDefFormat(buf, def->data.network.actual, flags) < 0)) + (virDomainActualNetDefFormat(buf, def, flags) < 0)) return -1; break; -- 1.8.5.3

On 21.02.2014 17:37, Laine Stump wrote: ACK Michal

Until now, the "live" XML status of an <interface type='network'> device would always show the network information, rather than the exact hardware device that was used. It would also show the name any portgroup the interface belonged to, rather than providing the configuration that was derived from the portgroup. As an example, given the following network definition: [A] <network> <name>testnet</name> <forward type='bridge' dev='p4p1_0'> <interface dev='p4p1_0'/> <interface dev='p4p1_1'/> <interface dev='p4p1_2'/> <interface dev='p4p1_3'/> </forward> <portgroup name='admin'> <bandwidth> <inbound average='1000' peak='5000' burst='1024'/> <outbound average='128' peak='256' burst='256'/> </bandwidth> </portgroup> </network> and the following domain <interface>: [B] <interface type='network'> <source network='testnet' portgroup='admin'/> </interface> the output of "virsh dumpxml $domain" while the domain was running would yield something like this: [C] <interface type='network'> <source network='testnet' portgroup='admin'/> <target dev='macvtap0'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> In order to learn the exact bandwidth information of the interface, a management application would need to retrieve the XML for testnet, then search for the portgroup named "admin". Even worse, there was no simple and standard way to learn which host physdev the macvtap0 device is attached to. Internally, libvirt has always kept this information in the virDomainDef that's held in memory, as well as storing it in the (libvirt-internal-only) domain status XML (in /etc/libvirt/qemu/$domain.xml). In order to not confuse the runtime "actual state" with the config of the device, it's internally stored like this: [D] <interface type='network'> <source network='testnet' portgroup='admin'/> <actual type='direct'> <source dev='p4p1_0' mode='bridge'/> <bandwidth> <inbound average='1000' peak='5000' burst='1024'/> <outbound average='128' peak='256' burst='256'/> </bandwidth> </actual> <target dev='macvtap0'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> This was never exposed outside of libvirt though, because I thought it would be too awkward for a management application to need to look in two places for the same information, but I also wasn't sure that it would be okay to overwrite the config info (in this case "<source network='testnet' portgroup='admin'/>") with the actual runtime info (everything inside <actual> above). Now the time has come that this information must be made available to management applications (in particular, so that a network "plugged" hook will have full information about the device that is being plugged in), so it's time to take the leap and decide that it is acceptable for the config info to be replaced with actual runtime state (but *only* when reporting domain live status, *not* when saving state in /etc/libvirt/qemu/$domain.xml - that remains the same so that there is no loss of information). That is what this patch does. With this patch applied, the output of "virsh dumpxml $domain" when the domain is running will contain something like this: [E] <interface type='direct'> <source dev='p4p1_0' mode='bridge'/> <bandwidth> <inbound average='1000' peak='5000' burst='1024'/> <outbound average='128' peak='256' burst='256'/> </bandwidth> <target dev='macvtap0'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> In effect, everything that is internally stored within <actual> is moved up a level to where a management application will expect it. This means that the management application will only look in a single place to learn - the type of interface in use, the name of the physdev (if relevant), the <bandwidth>, <vlan>, and <virtualport> settings in use. The potential downside is that a management app looking at this output will not see that the physdev 'p4p1_0' was actually allocated from the network named 'testnet', or that the bandwidth numbers were taken from the portgroup 'admin'. However, if they are interested in that info, they can always get the "inactive" XML for the domain. An example of where this could cause problems is in virt-manager's network device display, which shows the status of the device, but allows you to edit that status info and save it as the new config. Previously virt-manager would always display the information in example [C] above, and allow editing that. With this patch, it will instead display what is in [E] and allow editing it directly, which could lead to some confusion. I would suggest that virt-manager have an "edit" button which would change the display from the "live" xml to the "inactive" xml, so that editing would be done on that; such a change would both handle the new situation, and also be compatible with older releases. --- Change from V1: Updated arglist for virDomainActualNetDefContentsFormat() to match change in Patch 5/7. src/conf/domain_conf.c | 189 +++++++++++++++++++++++++++++-------------------- 1 file changed, 114 insertions(+), 75 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8291b73..695a8e7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15543,104 +15543,143 @@ virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { - const char *type = virDomainNetTypeToString(def->type); + /* publicActual is true if we should report the current state in + * def->data.network.actual *instead of* the config (*not* in + * addition to) + */ + unsigned int actualType = virDomainNetGetActualType(def); + bool publicActual + = (def->type == VIR_DOMAIN_NET_TYPE_NETWORK && def->data.network.actual && + !(flags & (VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET))); + const char *typeStr; + virDomainHostdevDefPtr hostdef = NULL; char macstr[VIR_MAC_STRING_BUFLEN]; - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected net type %d"), def->type); - return -1; + + if (publicActual) { + if (!(typeStr = virDomainNetTypeToString(actualType))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected actual net type %d"), actualType); + return -1; + } + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) + hostdef = virDomainNetGetActualHostdev(def); + } else { + if (!(typeStr = virDomainNetTypeToString(def->type))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->type); + return -1; + } + if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + hostdef = &def->data.hostdev.def; } - virBufferAsprintf(buf, " <interface type='%s'", type); - if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV && - def->data.hostdev.def.managed) { + virBufferAsprintf(buf, " <interface type='%s'", typeStr); + if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); - } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 6); virBufferAsprintf(buf, "<mac address='%s'/>\n", virMacAddrFormat(&def->mac, macstr)); - switch (def->type) { - case VIR_DOMAIN_NET_TYPE_NETWORK: - virBufferEscapeString(buf, "<source network='%s'", - def->data.network.name); - virBufferEscapeString(buf, " portgroup='%s'", - def->data.network.portgroup); - virBufferAddLit(buf, "/>\n"); - - /* ONLY for internal status storage - format the ActualNetDef - * as a subelement of <interface> so that no persistent config - * data is overwritten. + if (publicActual) { + /* when there is a virDomainActualNetDef, and we haven't been + * asked to 1) report the domain's inactive XML, or 2) give + * the internal version of the ActualNetDef separately in an + * <actual> subelement, we can just put the ActualDef data in + * the standard place... (this is for public reporting of + * interface status) */ - if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && - (virDomainActualNetDefFormat(buf, def, flags) < 0)) + if (virDomainActualNetDefContentsFormat(buf, def, typeStr, false, flags) < 0) return -1; - break; + } else { + /* ...but if we've asked for the inactive XML (rather than + * status), or to report the ActualDef as a separate <actual> + * subelement (this is how we privately store interface + * status), or there simply *isn't* any ActualNetDef, then + * format the NetDef's data here, and optionally format the + * ActualNetDef as an <actual> subelement of this element. + */ + switch (def->type) { + case VIR_DOMAIN_NET_TYPE_NETWORK: + virBufferEscapeString(buf, "<source network='%s'", + def->data.network.name); + virBufferEscapeString(buf, " portgroup='%s'", + def->data.network.portgroup); + virBufferAddLit(buf, "/>\n"); - case VIR_DOMAIN_NET_TYPE_ETHERNET: - virBufferEscapeString(buf, "<source dev='%s'/>\n", - def->data.ethernet.dev); - if (def->data.ethernet.ipaddr) - virBufferAsprintf(buf, "<ip address='%s'/>\n", - def->data.ethernet.ipaddr); - break; + /* ONLY for internal status storage - format the ActualNetDef + * as a subelement of <interface> so that no persistent config + * data is overwritten. + */ + if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && + (virDomainActualNetDefFormat(buf, def, flags) < 0)) + return -1; + break; - case VIR_DOMAIN_NET_TYPE_BRIDGE: - virBufferEscapeString(buf, "<source bridge='%s'/>\n", - def->data.bridge.brname); - if (def->data.bridge.ipaddr) { - virBufferAsprintf(buf, "<ip address='%s'/>\n", - def->data.bridge.ipaddr); - } - break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: + virBufferEscapeString(buf, "<source dev='%s'/>\n", + def->data.ethernet.dev); + if (def->data.ethernet.ipaddr) + virBufferAsprintf(buf, "<ip address='%s'/>\n", + def->data.ethernet.ipaddr); + break; - case VIR_DOMAIN_NET_TYPE_SERVER: - case VIR_DOMAIN_NET_TYPE_CLIENT: - case VIR_DOMAIN_NET_TYPE_MCAST: - if (def->data.socket.address) { - virBufferAsprintf(buf, "<source address='%s' port='%d'/>\n", - def->data.socket.address, def->data.socket.port); - } else { - virBufferAsprintf(buf, "<source port='%d'/>\n", - def->data.socket.port); - } - break; + case VIR_DOMAIN_NET_TYPE_BRIDGE: + virBufferEscapeString(buf, "<source bridge='%s'/>\n", + def->data.bridge.brname); + if (def->data.bridge.ipaddr) { + virBufferAsprintf(buf, "<ip address='%s'/>\n", + def->data.bridge.ipaddr); + } + break; - case VIR_DOMAIN_NET_TYPE_INTERNAL: - virBufferEscapeString(buf, "<source name='%s'/>\n", - def->data.internal.name); - break; + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + if (def->data.socket.address) { + virBufferAsprintf(buf, "<source address='%s' port='%d'/>\n", + def->data.socket.address, def->data.socket.port); + } else { + virBufferAsprintf(buf, "<source port='%d'/>\n", + def->data.socket.port); + } + break; - case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferEscapeString(buf, "<source dev='%s'", - def->data.direct.linkdev); - virBufferAsprintf(buf, " mode='%s'", - virNetDevMacVLanModeTypeToString(def->data.direct.mode)); - virBufferAddLit(buf, "/>\n"); - break; + case VIR_DOMAIN_NET_TYPE_INTERNAL: + virBufferEscapeString(buf, "<source name='%s'/>\n", + def->data.internal.name); + break; - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - if (virDomainHostdevDefFormatSubsys(buf, &def->data.hostdev.def, - flags, true) < 0) { - return -1; + case VIR_DOMAIN_NET_TYPE_DIRECT: + virBufferEscapeString(buf, "<source dev='%s'", + def->data.direct.linkdev); + virBufferAsprintf(buf, " mode='%s'", + virNetDevMacVLanModeTypeToString(def->data.direct.mode)); + virBufferAddLit(buf, "/>\n"); + break; + + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + if (virDomainHostdevDefFormatSubsys(buf, &def->data.hostdev.def, + flags, true) < 0) { + return -1; + } + break; + + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_LAST: + break; } - break; - case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_LAST: - break; + if (virNetDevVlanFormat(&def->vlan, buf) < 0) + return -1; + if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) + return -1; + if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) + return -1; } - if (virNetDevVlanFormat(&def->vlan, buf) < 0) - return -1; - if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) - return -1; - if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) - return -1; - virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); if (def->ifname && -- 1.8.5.3

On 21.02.2014 14:58, Laine Stump wrote: ACK although the indentation of XML we're passing to the hook script seems off: <hookData> <interface type='network'> ... </interface> <network> ... </network> <domain type='kvm'> </domain> </hookData> This is not a show stopper for me. I wonder if we should push these patches now, even during the freeze as this is very closely related to the network hooks - one of the main features in this release. Michal