On 09/20/2011 05:17 PM, Tyler Coumbes wrote:
Add a generic utility library for working with the TUN driver (/dev/net/tun).
Thanks for posting these patches. Next time, can you convince your
mailer to send the series as a single thread (that is, make both 1/2 and
2/2 have In-Reply-To set to point to 0/2)? 'git send-email' makes this
relatively easy. Also, sending patches with a diffstat makes it easier
to see at a glance how much the patch involves.
---
diff --git a/src/Makefile.am b/src/Makefile.am
index 738ee91..ddd1b77 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -88,7 +88,8 @@ UTIL_SOURCES = \
util/xml.c util/xml.h \
util/virterror.c util/virterror_internal.h \
util/virkeycode.c util/virkeycode.h \
- util/virkeymaps.h
+ util/virkeymaps.h \
+ util/tunctl.c util/tunctl.h
Being a new file, it would probably be better to go with a name starting
with 'vir', as in virtunctl (that is, this file represents libvirt's
wrappers around TUN, and not a generic TUN manipulation library).
EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
@@ -1182,6 +1183,8 @@ if WITH_NETWORK
USED_SYM_FILES += libvirt_network.syms
endif
+USED_SYM_FILES += libvirt_tunctl.syms
If [vir]tunctl is compiled unconditionally, then we don't need a new
.syms file - just stick the new symbols in libvirt_private.syms. On the
other hand, since TUN tends to be a Linux-specific concept, I think you
need to compile this code conditionally, at which point maybe the
existing libvirt_linux.syms is a better fit.
+
EXTRA_DIST += \
libvirt_public.syms \
libvirt_private.syms \
And if all else fails and you really do need to create a new .syms file,
then it needs to be listed in EXTRA_DIST.
diff --git a/src/libvirt_tunctl.syms b/src/libvirt_tunctl.syms
new file mode 100644
index 0000000..d1e00bb
--- /dev/null
+++ b/src/libvirt_tunctl.syms
@@ -0,0 +1,5 @@
+#tunctl.h
+createTap;
+delTap;
+tapSetInterfaceUp;
+tapSetInterfaceMac;
This naming isn't namespace clean. It might be better to go with:
virTunCreateTap
virTunDeleteTap
virTunSetInterfaceUp
virTunSetInterfaceMac
Oh, and if all of the functions are named tap instead of tun, then maybe
naming the file virtap instead of virtun would make more sense. Which
is it, tap or tun?
diff --git a/src/util/tunctl.c b/src/util/tunctl.c
new file mode 100644
index 0000000..e758e6d
--- /dev/null
+++ b/src/util/tunctl.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright (C) 2007, 2009, 2011 Red Hat, Inc.
It's okay to list earlier years if your file was copying substantial
layout from another file with those years, but if the file really is
from scratch, it may be okay to list just 2011.
What file were you copying from,
+ *
+ * Authors:
+ */
Feel free to list yourself, if you'd like. This line looks awkward with
no author listing.
+
+# include<config.h>
+# include "tunctl.h"
+# include "virfile.h"
+
+# include<stdlib.h>
+# include<stdio.h>
+# include<string.h>
+# include<unistd.h>
+# include<fcntl.h>
+# include<errno.h>
+# include<arpa/inet.h>
+# include<sys/types.h>
+# include<sys/socket.h>
+# include<sys/ioctl.h>
+# include<paths.h>
+# include<sys/wait.h>
+
+# include<linux/param.h> /* HZ */
Yep, definitely Linux-specific, so the Makefile.am will need to be
written so that this file is only conditionally compiled - basically,
your new file should be in the same conditionals as the existing
src/util/bridge.c.
+# ifdef IFF_VNET_HDR
+static int tapProbeVnetHdr(int tapfd)
+{
+# if defined(IFF_VNET_HDR)&& defined(TUNGETFEATURES)&& defined(TUNGETIFF)
This looks very similar to brProbeVnetHdr in src/util/bridge.c. If you
are going to refactor code into a new file for larger reuse later, then
it may be better to do the full code motion in a single commit, rather
than to add the new function now and delete the original code later.
That's not a hard-and-fast rule, but whichever way you go, it would be
nice to mention in the commit message that you are adding the new code
here in order to refactor common code out of bridge.c later. It took me
a while to realize that you were reusing code rather than writing it
from scratch.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list