Re: [libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels
On Tue, Aug 25, 2015 at 11:47:53PM -0400, Laine Stump wrote:
These two patches fix this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1252780
which was also separate reported in libvirt-users a couple days ago.
Most people running CentOS6 or RHEL6 also run the downstream version
of libvirt-0.10.2 that has many patches backported from later upstream
releases, but not the patch described in the commit logs of these
proposed patches. And the problem being fixed here only occurs on a
2.6.x kernel, while most people running upstream libvirt are running
something much more modern. That's why not many people have seen the
bug that these patches fix. Still, we claim to fully support upstream
on RHEL6/CentOS6, so we'd better do it right!
Two notes:
1) I tried timing 20 iterations of net-start...net-delete with these
patches vs. a libvirt build that simply calls the ioctls in the
beginnng (i.e. it doesn't try netlink then fall back to ioctl if that
fails), and the time to completion was within 0.25%, so the extra
overhead of trying one then the other isn't significant even on the
older machines that will always fall back to ioctl.
2) There are 2 different versions of Patch 2/2. I did the first one as
an exercise to see how ugly it would be to *not* duplicate the entire
virNetlinkDelLink() function inside the new
virNetDevBridgeDelete(). It was pretty ugly, so I made a cleaner
version that duplicates the code instead. I would be okay pushing
either version, but I think I slightly prefer the one that duplicates
code and remains uncomplicated.
To be honest, I dislike both approaches. Sure, the second one is not
that messy, but looking at those patches I have another idea. At
first I thought that since there are not that many callers of
virNetlinkDelLink(), what if we did not report the error inside that
function at all and just return the errno and callers would report
errors themselves. But that's not good either, virNetlinkDelLink()
has very nice error reporting. So what if we took the second version,
but the code that's duplicated (e.g. preparing netlink messages) that
can fail by itself could be moved into its own functions that would be
called in both virNetlinkDelLink() and virNetDevBridgeDelete() so that
the duplication would be minimal and more prone to differentiation in
the future.
Laine Stump (2):
util: fallback to ioctl(SIOCBRADDBR) if netlink RTM_NEWLINK fails
util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails
src/util/virnetdevbridge.c | 112 ++++++++++++++++++++++++++++++++------------
src/util/virnetdevmacvlan.c | 2 +-
src/util/virnetlink.c | 17 +++++--
src/util/virnetlink.h | 2 +-
4 files changed, 98 insertions(+), 35 deletions(-)
--
2.1.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)