[libvirt] [PATCH] domain: include portgroup in interface status xml

[Michal Privoznik]

On 11.02.2015 02:51, Laine Stump wrote:
> Prior to commit 7d5bf484747 (first appearing in libvirt 1.2.2), the
> status XML of a domain's interface was missing a lot of important
> information; mainly it just output the config of the interface, plus
> the name of the tap device and qemu device alias. Commit 7d5bf484747
> changed the status XML to include many important bits of information
> that were required to make network "hook" scripts useful - bandwidth
> information, vlan tag, the name of the bridge (or physical device in
> the case of macvtap) that the tap/macvtap device was attached to - the
> commit log for 7d5bf484747 has a very detailed explanation of the
> change. For quick reference - in the example given there, prior to the
> change, status XML looked like figure [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>
> 
> and after the change, it looked like figure [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>
> 
> You'll notice that bandwidth info, physdev, and macvtap mode have been
> added, but the network and portgroup names are now missing - I didn't
> think that this information was of any use once the needed
> bandwidth/vlan/etc config had been pulled from the network/portgroup.
> 
> I was wrong.
> 
> A few months after that change a user on IRC asked what happened to
> portgroup in the status XML and described how he used it (more or less
> as a tag to decide what external information to use in a hook script
> that was run at startup/migration time - see
> http://wiki.libvirt.org/page/OVS_and_PVLANS ). At that time I planned
> to make a patch to re-add portgroup, but life intervened as that was
> just prior to a transatlantic move involving several weeks of
> "vacation". During this time I somehow forgot to make the patch, and
> also mistakenly remembered that I *had* made it.
> 
> Subsequent to this, as a part of mprivozn's work to add support for
> network-specific hooks, I did re-add the output of the network name in
> status XML, but once again completely forgot about portgroup. This was
> in commit a3609121 (first appearing in libvirt 1.2.11). This made the
> status XML from the above example look like this:
> 
>       <interface type='direct'>
>         <source network='testnet' 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>
> 
> *This* patch just adds the portgroup back to the status XML, so the
>  same example interface will look like this:
> 
>       <interface type='direct'>
>         <source network='testnet' portgroup='admin'
>                 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>
> 
> The result is that the status XML now contains all information about
> how the interface is setup (bandwidth, physical device, tap device,
> etc), in addition to pointers to its origin (the network and
> portgroup).
> ---
> I had previously asked about this topic here:
> 
>   https://www.redhat.com/archives/libvir-list/2014-June/msg00416.html
> 
> but then forgot to follow up on it...
> 
>  src/conf/domain_conf.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8600eff..d870bb6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -17591,14 +17591,18 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
>          if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK && !inSubelement) {
>              /* When we're putting our output into the <actual>
>               * subelement rather than the main <interface>, the
> -             * network name isn't included in the <source> because the
> -             * main interface element's <source> has the same info
> -             * already. If we've been called to output directly into
> -             * the main element's <source> though (the case here -
> -             * "!inSubElement"), we *do* need to output the network
> -             * name, because the caller won't have done it).
> +             * network name and portgroup don't need to be included in
> +             * the <source> here because the main interface element's
> +             * <source> has the same info already. If we've been
> +             * called to output directly into the main element's
> +             * <source> though (the case here - "!inSubElement"), we
> +             * *do* need to output network/portgroup, because the
> +             * caller won't have done it).
>               */
> -            virBufferEscapeString(buf, " network='%s'", def->data.network.name);
> +            virBufferEscapeString(buf, " network='%s'",
> +                                  def->data.network.name);
> +            virBufferEscapeString(buf, " portgroup='%s'",
> +                                  def->data.network.portgroup);
>          }
>          if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
>              actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
> 

ACK

Michal