[libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels

[Martin Kletzander]

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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150826/48d1f054/attachment-0001.sig>