[libvirt] inconsistency in attribute usage between hv drivers

[Laine Stump]

I just came across this problem while looking for the proper place to 
put <ip> info for the host side of <interface type='ethernet'>. I don't 
think there's really anything that can be done about it now, since 
everything below has been in place for so long, but maybe it can serve 
as a cautionary tale.


Prologue: A week or two ago, I made patches to support <interface 
type='ethernet'> in the LXC driver. I did this not because I have a need 
for that, but because it seemed like the perfect avenue for 
investigating the best way to add IP address and route config to 
<interface>, as it is one environment where we have a need *and* the 
ability to setup IP info for both the host and guest sides of the 
interface. I accomplished this by simply generalizing the existing 
<interface type='bridge'> code to not attach the host-side veth device 
to a bridge.


This makes all common attributes between type='ethernet' and 
type='bridge' consistent, including the configuration of guest-side and 
host-side device names, and here we come to the first inconsistency - 
the device name for the veth/tap on the host side is  in the <target> 
element (which I've always been told is supposed to be used for 
device-specific info about how the device will be presented on the 
*guest* side, but I suppose this is because <interface type='direct'> 
uses <source dev='blah'> to provide the name of the physical device that 
the macvtap device (named in <target dev='macvblah'/> is attached to).  
And I guess because the dev attribute was already in-use in <target> the 
much more recently added guest-side name is given in the <guest> 
subelement (in LXC at least):


     <interface type='ethernet'>

       <target dev='hostblah'/>  <-- name of veth on host side

       <guest dev='guestblah'/> <-- name of veth on guest side

       ...


Now, this "is what it is". There is no changing it. In particular, the 
use of <target dev> for the host-side device name has been in use for so 
long and is so deeply ingrained that it's just part of the scenery.

Moving for a minute to  the IP address/route info for the guest side is 
already configured in the toplevel <ip> and <route> subelements of 
<interface>,


    <interface type='ethernet'>

      <ip address='1.2.3.4' .../>

    </interface>


My first thought was to put the <ip> for the host side inside <source>:


    <interface type='ethernet'>

      <source>

        <ip address='1.2.3.5' .../> <-- host side

      </source>

      <ip address='1.2.3.4' .../> <-- guest side

    </interface>


That makes sense to me because it parallels what we do with PCI 
addresses for hostdev - the PCI address in <source> tells what the 
address is on the host, and the one at the toplevel of <hostdev> tells 
what the PCI address will be in the guest. It *doesn't* fit well with 
the fact that the device we're setting up is named in <target> though, 
so we're already inconsistent, but putting the host-side IP config in 
<target> seems like an invitation to mix things up even more in the 
future, since <target> is used everywhere else as a place to put stuff 
related to what the device looks like in the guest (although... for 
<interface>, <target> *is* currently used only for the host-side 
tap/veth device name; so maybe we just can define the purpose of 
<target> differently in this one case... But I digress).


So I went to the code to add parsing of <ip> inside <source>, and found 
that the type-specific data for ethernet has a dev, and that field in 
the object is set during parsing from <source dev='blah'/>, *and* that 
the openvz driver uses that dev name as the name of the network device 
in the *guest*! (even though my understanding of <source>  is that it is 
supposed to be used for information about the *host* side of any device. 
Sigh.) So on openvz, we have this:


     <interface type='ethernet'>

       <target dev='hostblah'/>  <-- name of interface on host side

       <source dev='guestblah'/> <-- name of interface on guest side

       ...


which seems exactly the opposite of what it should be. Note that this 
doesn't seem to be documented anywhere except in the few lines in 
openvz_driver.c that implement it. (Note that, prior to commit 9a4b705f7 
back in 2010, which added support for type='ethernet' in openvz, the 
"dev" field was already in the ethernet-specific sub-object of 
virDomainNetDef, but it was only used to hold the name of the bridge 
that a tap device was attached to when converting from a qemu 
commandline to XML (also, as previously stated, for type='direct' it is 
used to name the physical device on the host that the macvtap interface 
is connected to, i.e. *exactly* the opposite end of the chain). It looks 
like the poster of the openvz patch just saw an opportune 
existing-but-unused-by-type=ethernet attribute and took advantage, and 
it wasn't noticed in review).


I'm actually skeptical that anyone even uses this, since openvz is a 
less-used driver, type='ethernet' is less used among interface types, 
and even after that it's valid to not provide a name, in which case one 
is automatically generated. But there is no way to verify that, so we're 
stuck with it. It is true that the LXC version of type='ethernet' was 
added only a few days ago, and there hasn't yet been a release, so I 
*could* modify it to be consistent with the attribute usage in the 
openvz driver. But that would make <interface type='ethernet'> in LXC 
inconsistent with <interface type='bridge'> in LXC, which is a much 
worse situation. IT seems there is nothing to change. As I said above 
"it is what it is".


So why am I writing this message? Partly to just vent frustration (aka 
"whine") I suppose :-). Also partly as public testimony that my 
seemingly excessive worrying over attribute naming and placement is 
justified, and not just OCD. Also, as I said above, as a cautionary tale 
that any patch adding *any* XML config knob, or making use of an 
existing XML config knob needs extra scrutiny to help minimize such 
messes in the future.