This patch series fixes this feature enoough that it works:

1) emits "peer" attribute in formatted XML when present in the NetDef
   object, so that the config will "stick"

2) swaps "address" and "peer" for qemu, so that "address" consistently
   refers to the IP address used by the guest, and "peer" to the address
   used by the host.

3) ... and the rest

*BUT* it doesn't address the sub-optimal naming of the new attribute,
nor does it fix the documentation which is incorrect not only in its
description, but also in the starting version number for QEMU
support. Also, I'm skeptical that this new feature is useful for the
types of lxc interfaces that are supported (macvtap i.e. "direct", and
a veth connected to a bridge) - from my understanding, it would only
be useful for a type='ethernet' interface (a tap/veth pair not
connected to any bridge), and that isn't supported by lxc yet; for
type='bridge' and type='network' (which is also connecting to a
bridge) I don't see the use case.

So I'm torn about whether these patches should be put in for this
release in order to made the already-pushed code work, or if we should
just hold off until we:

1) find/agree on a better name for the new attribute (see my earlier
   mail titled 'interface "peer address" patches are broken for details
   on my opinion)

2) decide if it's actually useful to support the "peer" address for
   type='network|bridge" in lxc (it isn't in qemu).

3) fix the documentation (I started into that when I realized the 

By not pushing the fixes, we guarantee that nobody can use the
feature, and thus will technically still be able to change the name of
the attribute even after arelease has passed (because we won't break
anyone's usable config).

Opinions on what to do?

(I would consider reverting the original patches temporarily until
it's all sorted out, but I don't know what kind of conflicts that
would cause; I know that there has been at least one bugfix patch)


Laine Stump (8):
  conf: clean up virDomainNetIpParseXML()
  tests: mock virNetDevSetIPAddress
  conf: emit an IP address "peer" attribute in XML when present
  util: allow calling virSocketAddrGetIpPrefix with NULL netmask or
    address
  qemu/lxc: use correct prefix when setting tap/veth IP address
  qemu/lxc: log peer address when setting a domain interface's IP
  qemu: swap "address" and "peer" when setting IP address for tap
    interfaces
  qemu: require "peer" if ip address is supplied for an interface

 src/conf/domain_conf.c                             | 46 +++++++++++++---------
 src/lxc/lxc_container.c                            | 36 +++++++++++++----
 src/qemu/qemu_domain.c                             | 44 ++++++++++++++++++---
 src/qemu/qemu_interface.c                          | 44 +++++++++++++++------
 src/util/virsocketaddr.c                           |  8 ++--
 src/util/virsocketaddr.h                           |  3 +-
 .../qemuxml2argv-net-eth-peer.args                 | 20 ++++++++++
 .../qemuxml2argvdata/qemuxml2argv-net-eth-peer.xml | 30 ++++++++++++++
 tests/qemuxml2argvmock.c                           | 10 ++++-
 tests/qemuxml2argvtest.c                           |  1 +
 .../qemuxml2xmlout-net-eth-peer.xml                | 33 ++++++++++++++++
 tests/qemuxml2xmltest.c                            |  1 +
 12 files changed, 226 insertions(+), 50 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-peer.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-peer.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-peer.xml

-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to