[libvirt] [PATCHv2 0/2] interface driver build cleanup and udev backend addition

Refactored the cross use of netcf and interface for the interface driver into only interface driver references. Made netcf the primary backend provider for the interface driver. Added a udev based read-only interface driver backend to be used when netcf support is not built in. Change from v1: * rebased against master * Fixed copyright header * Added libvirt.spec.in modification * Changed virsh -V output Untested: * udev: Devices that are part of a bond. * libvirt.spec: Need some feedback from DV and the RH guys on how they want it packaged Unknown: * Should virInterfaceGetXMLDesc() be implemented for the udev backend? It will as a result contain a lot less info than the netcf based backend would for the same device (e.g. does the device start on boot). Doug Goldstein (2): build: define WITH_INTERFACE for the driver interface: add udev based backend for virInterface .gnulib | 2 +- configure.ac | 37 ++- daemon/Makefile.am | 2 +- daemon/libvirtd.c | 8 +- libvirt.spec.in | 13 +- src/Makefile.am | 34 ++- .../{netcf_driver.c => interface_backend_netcf.c} | 2 +- src/interface/interface_backend_udev.c | 395 ++++++++++++++++++++ .../{netcf_driver.h => interface_driver.h} | 0 tests/virdrivermoduletest.c | 2 +- tools/virsh.c | 8 +- 11 files changed, 480 insertions(+), 23 deletions(-) rename src/interface/{netcf_driver.c => interface_backend_netcf.c} (99%) create mode 100644 src/interface/interface_backend_udev.c rename src/interface/{netcf_driver.h => interface_driver.h} (100%) -- 1.7.8.6

Based exclusively on work by Eric Blake in a patch posted with the same subject. However some modifications related to comments and my plans to add another backend. Added WITH_INTERFACE as the only automake variable deciding whether to build the driver and using WITH_NETCF to identify that we're wanting to use the netcf library as the backend. * configure.ac: Added with_interface * src/interface/netcf_driver.c: Renamed.. * src/interface/interface_backend_netcf.c: ..to this to match storage. * src/interface/netcf_driver.h: Renamed.. * src/interface/interface_driver.h: ..to this. * daemon/Makefile.am: Respect WITH_INTERFACE and WITH_NETCF. * libvirt.spec.in: Add RPM support for --with-interface --- configure.ac | 33 ++++++++++++++++++- daemon/Makefile.am | 2 +- daemon/libvirtd.c | 8 ++-- libvirt.spec.in | 13 ++++--- src/Makefile.am | 24 +++++++++++--- .../{netcf_driver.c => interface_backend_netcf.c} | 2 +- .../{netcf_driver.h => interface_driver.h} | 0 tests/virdrivermoduletest.c | 2 +- tools/virsh.c | 5 ++- 9 files changed, 67 insertions(+), 22 deletions(-) rename src/interface/{netcf_driver.c => interface_backend_netcf.c} (99%) rename src/interface/{netcf_driver.h => interface_driver.h} (100%) diff --git a/configure.ac b/configure.ac index 47a72b9..3e2fd0a 100644 --- a/configure.ac +++ b/configure.ac @@ -1948,7 +1948,6 @@ AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"]) AC_SUBST([NETCF_CFLAGS]) AC_SUBST([NETCF_LIBS]) - AC_ARG_WITH([secrets], AC_HELP_STRING([--with-secrets], [with local secrets management driver @<:@default=yes@:>@]),[],[with_secrets=yes]) @@ -2787,6 +2786,36 @@ if test "$with_nwfilter" = "yes" ; then fi AM_CONDITIONAL([WITH_NWFILTER], [test "$with_nwfilter" = "yes"]) +dnl check if the interface driver should be compiled +AC_ARG_WITH([interface], + AC_HELP_STRING([--with-interface], + [with host interface driver @<:@default=check@:>@]), [], + [with_interface=check]) + +dnl Don't compile the interface driver without libvirtd +if test "$with_libvirtd" = "no" ; then + with_interface=no +fi + +dnl The interface driver depends on the netcf library +if test "$with_interface:$with_netcf" = "check:yes" ; then + with_interface=yes +fi + +if test "$with_interface:$with_netcf" = "check:no" ; then + with_interface=no +fi + +if test "$with_interface:$with_netcf" = "yes:no" ; then + AC_MSG_ERROR([Requested the Interface driver without netcf support]) +fi + +if test "$with_interface" = "yes" ; then + AC_DEFINE_UNQUOTED([WITH_INTERFACE], [1], + [whether the interface driver is enabled]) +fi +AM_CONDITIONAL([WITH_INTERFACE], [test "$with_interface" = "yes"]) + dnl libblkid is used by several storage drivers; therefore we probe dnl for it unconditionally. AC_ARG_WITH([libblkid], @@ -2998,7 +3027,7 @@ AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([ Libvirtd: $with_libvirtd]) -AC_MSG_NOTICE([ netcf: $with_netcf]) +AC_MSG_NOTICE([Interface: $with_interface]) AC_MSG_NOTICE([ macvtap: $with_macvtap]) AC_MSG_NOTICE([ virtport: $with_virtualport]) AC_MSG_NOTICE([]) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b45349c..3405c67 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -151,7 +151,7 @@ if WITH_NETWORK libvirtd_LDADD += ../src/libvirt_driver_network.la endif -if WITH_NETCF +if WITH_INTERFACE libvirtd_LDADD += ../src/libvirt_driver_interface.la endif diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 6973df6..1156bd6 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -78,8 +78,8 @@ # ifdef WITH_NETWORK # include "network/bridge_driver.h" # endif -# ifdef WITH_NETCF -# include "interface/netcf_driver.h" +# ifdef WITH_INTERFACE +# include "interface/interface_driver.h" # endif # ifdef WITH_STORAGE # include "storage/storage_driver.h" @@ -382,7 +382,7 @@ static void daemonInitialize(void) # ifdef WITH_NWFILTER virDriverLoadModule("nwfilter"); # endif -# ifdef WITH_NETCF +# ifdef WITH_INTERFACE virDriverLoadModule("interface"); # endif # ifdef WITH_XEN @@ -404,7 +404,7 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK networkRegister(); # endif -# ifdef WITH_NETCF +# ifdef WITH_INTERFACE interfaceRegister(); # endif # ifdef WITH_STORAGE diff --git a/libvirt.spec.in b/libvirt.spec.in index 8c4c08d..853cef7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -92,6 +92,7 @@ # A few optional bits off by default, we enable later %define with_polkit 0%{!?_without_polkit:0} %define with_capng 0%{!?_without_capng:0} +%define with_interface 0%{!?_without_interface:0} %define with_netcf 0%{!?_without_netcf:0} %define with_udev 0%{!?_without_udev:0} %define with_hal 0%{!?_without_hal:0} @@ -200,6 +201,12 @@ %define with_netcf 0%{!?_without_netcf:%{server_drivers}} %endif +# interface is the driver that wraps netcf or udev interface management +# backends in Fedora 18 / RHEL-7 or newer +%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7 +%define with_interface 0%{!?_without_interface:%{server_drivers}} +%endif + # udev is used to manage host devices in Fedora 12 / RHEL-6 or newer %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 %define with_udev 0%{!?_without_udev:%{server_drivers}} @@ -281,12 +288,6 @@ %define with_nodedev 0 %endif -%if %{with_netcf} -%define with_interface 1 -%else -%define with_interface 0 -%endif - %if %{with_storage_fs} || %{with_storage_mpath} || %{with_storage_iscsi} || %{with_storage_lvm} || %{with_storage_disk} %define with_storage 1 %else diff --git a/src/Makefile.am b/src/Makefile.am index 9f27fcf..4ae741b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -552,8 +552,17 @@ PARALLELS_DRIVER_SOURCES = \ NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c -INTERFACE_DRIVER_SOURCES = \ - interface/netcf_driver.h interface/netcf_driver.c +INTERFACE_DRIVER_SOURCES = + +if WITH_INTERFACE +INTERFACE_DRIVER_SOURCES += \ + interface/interface_driver.h +endif + +if WITH_NETCF +INTERFACE_DRIVER_SOURCES += \ + interface/interface_backend_netcf.c +endif SECRET_DRIVER_SOURCES = \ secret/secret_driver.h secret/secret_driver.c @@ -1021,7 +1030,7 @@ endif EXTRA_DIST += network/default.xml -if WITH_NETCF +if WITH_INTERFACE if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_interface.la else @@ -1029,10 +1038,13 @@ noinst_LTLIBRARIES += libvirt_driver_interface.la # Stateful, so linked to daemon instead #libvirt_la_BUILT_LIBADD += libvirt_driver_interface.la endif -libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS) \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_driver_interface_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_interface_la_LIBADD = $(NETCF_LIBS) +libvirt_driver_interface_la_LIBADD = +if WITH_NETCF +libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) +libvirt_driver_interface_la_LIBADD += $(NETCF_LIBS) +endif if WITH_DRIVER_MODULES libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_interface_la_LDFLAGS += -module -avoid-version diff --git a/src/interface/netcf_driver.c b/src/interface/interface_backend_netcf.c similarity index 99% rename from src/interface/netcf_driver.c rename to src/interface/interface_backend_netcf.c index 935be66..9285b08 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/interface_backend_netcf.c @@ -27,7 +27,7 @@ #include "virterror_internal.h" #include "datatypes.h" -#include "netcf_driver.h" +#include "interface_driver.h" #include "interface_conf.h" #include "memory.h" diff --git a/src/interface/netcf_driver.h b/src/interface/interface_driver.h similarity index 100% rename from src/interface/netcf_driver.h rename to src/interface/interface_driver.h diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 4d6e91e..8762de4 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -79,7 +79,7 @@ mymain(void) #ifdef WITH_NWFILTER TEST("nwfilter", NULL); #endif -#ifdef WITH_NETCF +#ifdef WITH_INTERFACE TEST("interface", NULL); #endif #ifdef WITH_QEMU diff --git a/tools/virsh.c b/tools/virsh.c index 242f789..7a175ed 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2705,8 +2705,11 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_BRIDGE vshPrint(ctl, " Bridging"); #endif -#ifdef WITH_NETCF +#if defined(WITH_INTERFACE) vshPrint(ctl, " Interface"); +#if defined(WITH_NETCF) + vshPrint(ctl, " netcf"); +#endif #endif #ifdef WITH_NWFILTER vshPrint(ctl, " Nwfilter"); -- 1.7.8.6

On 09/09/2012 06:02 PM, Doug Goldstein wrote:
Based exclusively on work by Eric Blake in a patch posted with the same subject. However some modifications related to comments and my plans to add another backend.
Added WITH_INTERFACE as the only automake variable deciding whether to build the driver and using WITH_NETCF to identify that we're wanting to use the netcf library as the backend.
In general I think it's good to separate the concept of the interface driver from its netcf backend (regardless of what other backends we may end up putting in. I did notice that if I specify --with-netcf --without-interface, there was no error, but instead the "netcf: " line at the end of configure said "-lnetcf", implying that it might end up being added to the link lines somewhere even though it also said "Interface: no". It think if with-interface == no, then with-netcf should also be set to no (or an error generated if it's being forced to yes, since there's no other use for netcf). Beyond that, I will have to defer to Eric's superior knowledge of autoconf for any more detailed comments about the changes to configure.ac.
* configure.ac: Added with_interface * src/interface/netcf_driver.c: Renamed.. * src/interface/interface_backend_netcf.c: ..to this to match storage. * src/interface/netcf_driver.h: Renamed.. * src/interface/interface_driver.h: ..to this. * daemon/Makefile.am: Respect WITH_INTERFACE and WITH_NETCF. * libvirt.spec.in: Add RPM support for --with-interface --- configure.ac | 33 ++++++++++++++++++- daemon/Makefile.am | 2 +- daemon/libvirtd.c | 8 ++-- libvirt.spec.in | 13 ++++--- src/Makefile.am | 24 +++++++++++--- .../{netcf_driver.c => interface_backend_netcf.c} | 2 +- .../{netcf_driver.h => interface_driver.h} | 0 tests/virdrivermoduletest.c | 2 +- tools/virsh.c | 5 ++- 9 files changed, 67 insertions(+), 22 deletions(-) rename src/interface/{netcf_driver.c => interface_backend_netcf.c} (99%) rename src/interface/{netcf_driver.h => interface_driver.h} (100%)
diff --git a/configure.ac b/configure.ac index 47a72b9..3e2fd0a 100644 --- a/configure.ac +++ b/configure.ac @@ -1948,7 +1948,6 @@ AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"]) AC_SUBST([NETCF_CFLAGS]) AC_SUBST([NETCF_LIBS])
- AC_ARG_WITH([secrets], AC_HELP_STRING([--with-secrets], [with local secrets management driver @<:@default=yes@:>@]),[],[with_secrets=yes])
@@ -2787,6 +2786,36 @@ if test "$with_nwfilter" = "yes" ; then fi AM_CONDITIONAL([WITH_NWFILTER], [test "$with_nwfilter" = "yes"])
+dnl check if the interface driver should be compiled +AC_ARG_WITH([interface], + AC_HELP_STRING([--with-interface], + [with host interface driver @<:@default=check@:>@]), [], + [with_interface=check]) + +dnl Don't compile the interface driver without libvirtd +if test "$with_libvirtd" = "no" ; then + with_interface=no +fi + +dnl The interface driver depends on the netcf library +if test "$with_interface:$with_netcf" = "check:yes" ; then + with_interface=yes +fi + +if test "$with_interface:$with_netcf" = "check:no" ; then + with_interface=no +fi + +if test "$with_interface:$with_netcf" = "yes:no" ; then + AC_MSG_ERROR([Requested the Interface driver without netcf support]) +fi + +if test "$with_interface" = "yes" ; then + AC_DEFINE_UNQUOTED([WITH_INTERFACE], [1], + [whether the interface driver is enabled]) +fi +AM_CONDITIONAL([WITH_INTERFACE], [test "$with_interface" = "yes"]) + dnl libblkid is used by several storage drivers; therefore we probe dnl for it unconditionally. AC_ARG_WITH([libblkid], @@ -2998,7 +3027,7 @@ AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([ Libvirtd: $with_libvirtd]) -AC_MSG_NOTICE([ netcf: $with_netcf]) +AC_MSG_NOTICE([Interface: $with_interface]) AC_MSG_NOTICE([ macvtap: $with_macvtap]) AC_MSG_NOTICE([ virtport: $with_virtualport]) AC_MSG_NOTICE([]) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b45349c..3405c67 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -151,7 +151,7 @@ if WITH_NETWORK libvirtd_LDADD += ../src/libvirt_driver_network.la endif
-if WITH_NETCF +if WITH_INTERFACE libvirtd_LDADD += ../src/libvirt_driver_interface.la endif
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 6973df6..1156bd6 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -78,8 +78,8 @@ # ifdef WITH_NETWORK # include "network/bridge_driver.h" # endif -# ifdef WITH_NETCF -# include "interface/netcf_driver.h" +# ifdef WITH_INTERFACE +# include "interface/interface_driver.h" # endif # ifdef WITH_STORAGE # include "storage/storage_driver.h" @@ -382,7 +382,7 @@ static void daemonInitialize(void) # ifdef WITH_NWFILTER virDriverLoadModule("nwfilter"); # endif -# ifdef WITH_NETCF +# ifdef WITH_INTERFACE virDriverLoadModule("interface"); # endif # ifdef WITH_XEN @@ -404,7 +404,7 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK networkRegister(); # endif -# ifdef WITH_NETCF +# ifdef WITH_INTERFACE interfaceRegister(); # endif # ifdef WITH_STORAGE diff --git a/libvirt.spec.in b/libvirt.spec.in index 8c4c08d..853cef7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -92,6 +92,7 @@ # A few optional bits off by default, we enable later %define with_polkit 0%{!?_without_polkit:0} %define with_capng 0%{!?_without_capng:0} +%define with_interface 0%{!?_without_interface:0} %define with_netcf 0%{!?_without_netcf:0} %define with_udev 0%{!?_without_udev:0} %define with_hal 0%{!?_without_hal:0} @@ -200,6 +201,12 @@ %define with_netcf 0%{!?_without_netcf:%{server_drivers}} %endif
+# interface is the driver that wraps netcf or udev interface management +# backends in Fedora 18 / RHEL-7 or newer +%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7 +%define with_interface 0%{!?_without_interface:%{server_drivers}} +%endif + # udev is used to manage host devices in Fedora 12 / RHEL-6 or newer %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 %define with_udev 0%{!?_without_udev:%{server_drivers}} @@ -281,12 +288,6 @@ %define with_nodedev 0 %endif
-%if %{with_netcf} -%define with_interface 1 -%else -%define with_interface 0 -%endif - %if %{with_storage_fs} || %{with_storage_mpath} || %{with_storage_iscsi} || %{with_storage_lvm} || %{with_storage_disk} %define with_storage 1 %else diff --git a/src/Makefile.am b/src/Makefile.am index 9f27fcf..4ae741b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -552,8 +552,17 @@ PARALLELS_DRIVER_SOURCES = \ NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c
-INTERFACE_DRIVER_SOURCES = \ - interface/netcf_driver.h interface/netcf_driver.c +INTERFACE_DRIVER_SOURCES = + +if WITH_INTERFACE +INTERFACE_DRIVER_SOURCES += \ + interface/interface_driver.h +endif + +if WITH_NETCF +INTERFACE_DRIVER_SOURCES += \ + interface/interface_backend_netcf.c +endif
SECRET_DRIVER_SOURCES = \ secret/secret_driver.h secret/secret_driver.c @@ -1021,7 +1030,7 @@ endif EXTRA_DIST += network/default.xml
-if WITH_NETCF +if WITH_INTERFACE if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_interface.la else @@ -1029,10 +1038,13 @@ noinst_LTLIBRARIES += libvirt_driver_interface.la # Stateful, so linked to daemon instead #libvirt_la_BUILT_LIBADD += libvirt_driver_interface.la endif -libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS) \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_driver_interface_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_interface_la_LIBADD = $(NETCF_LIBS) +libvirt_driver_interface_la_LIBADD = +if WITH_NETCF +libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) +libvirt_driver_interface_la_LIBADD += $(NETCF_LIBS) +endif if WITH_DRIVER_MODULES libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_interface_la_LDFLAGS += -module -avoid-version diff --git a/src/interface/netcf_driver.c b/src/interface/interface_backend_netcf.c similarity index 99% rename from src/interface/netcf_driver.c rename to src/interface/interface_backend_netcf.c index 935be66..9285b08 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/interface_backend_netcf.c @@ -27,7 +27,7 @@
#include "virterror_internal.h" #include "datatypes.h" -#include "netcf_driver.h" +#include "interface_driver.h" #include "interface_conf.h" #include "memory.h"
diff --git a/src/interface/netcf_driver.h b/src/interface/interface_driver.h similarity index 100% rename from src/interface/netcf_driver.h rename to src/interface/interface_driver.h diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 4d6e91e..8762de4 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -79,7 +79,7 @@ mymain(void) #ifdef WITH_NWFILTER TEST("nwfilter", NULL); #endif -#ifdef WITH_NETCF +#ifdef WITH_INTERFACE TEST("interface", NULL); #endif #ifdef WITH_QEMU diff --git a/tools/virsh.c b/tools/virsh.c index 242f789..7a175ed 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2705,8 +2705,11 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_BRIDGE vshPrint(ctl, " Bridging"); #endif -#ifdef WITH_NETCF +#if defined(WITH_INTERFACE) vshPrint(ctl, " Interface"); +#if defined(WITH_NETCF) + vshPrint(ctl, " netcf"); +#endif #endif #ifdef WITH_NWFILTER vshPrint(ctl, " Nwfilter");

Add a read-only udev based backend for virInterface. Useful for distros that do not have netcf support yet. Multiple libvirt based utilities use a HAL based fallback when virInterface is not available which is less than ideal. This implements: * virConnectNumOfInterfaces() * virConnectListInterfaces() * virConnectNumOfDefinedInterfaces() * virConnectListDefinedInterfaces() * virConnectInterfaceLookupByName() * virConnectInterfaceLookupByMACString() --- .gnulib | 2 +- configure.ac | 10 +- src/Makefile.am | 12 +- src/interface/interface_backend_udev.c | 395 ++++++++++++++++++++++++++++++++ tools/virsh.c | 3 + 5 files changed, 417 insertions(+), 5 deletions(-) create mode 100644 src/interface/interface_backend_udev.c diff --git a/.gnulib b/.gnulib index 440a1db..271dd74 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 +Subproject commit 271dd74fdf54ec2a03e73a5173b0b5697f6088f1 diff --git a/configure.ac b/configure.ac index 3e2fd0a..d7f9b51 100644 --- a/configure.ac +++ b/configure.ac @@ -2803,11 +2803,15 @@ if test "$with_interface:$with_netcf" = "check:yes" ; then fi if test "$with_interface:$with_netcf" = "check:no" ; then - with_interface=no + if test "$with_udev" = "yes" ; then + with_interface=yes + else + with_interface=no + fi fi -if test "$with_interface:$with_netcf" = "yes:no" ; then - AC_MSG_ERROR([Requested the Interface driver without netcf support]) +if test "$with_interface:$with_netcf:$with_udev" = "yes:no:no" ; then + AC_MSG_ERROR([Requested the Interface driver without netcf or udev support]) fi if test "$with_interface" = "yes" ; then diff --git a/src/Makefile.am b/src/Makefile.am index 4ae741b..53f1fbf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -557,11 +557,16 @@ INTERFACE_DRIVER_SOURCES = if WITH_INTERFACE INTERFACE_DRIVER_SOURCES += \ interface/interface_driver.h -endif if WITH_NETCF INTERFACE_DRIVER_SOURCES += \ interface/interface_backend_netcf.c +else +if HAVE_UDEV +INTERFACE_DRIVER_SOURCES += \ + interface/interface_backend_udev.c +endif +endif endif SECRET_DRIVER_SOURCES = \ @@ -1044,6 +1049,11 @@ libvirt_driver_interface_la_LIBADD = if WITH_NETCF libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) libvirt_driver_interface_la_LIBADD += $(NETCF_LIBS) +else +if HAVE_UDEV +libvirt_driver_interface_la_CFLAGS += $(UDEV_CFLAGS) +libvirt_driver_interface_la_LIBADD += $(UDEV_LIBS) +endif endif if WITH_DRIVER_MODULES libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c new file mode 100644 index 0000000..981a045 --- /dev/null +++ b/src/interface/interface_backend_udev.c @@ -0,0 +1,395 @@ +/* + * interface_backend_udev.c: udev backend for virInterface + * + * Copyright (C) 2012 Doug Goldstein <cardoe@cardoe.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + */ +#include <config.h> + +#include <libudev.h> + +#include "virterror_internal.h" +#include "datatypes.h" +#include "interface_driver.h" +#include "interface_conf.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_INTERFACE + +struct udev_iface_driver { + struct udev *udev; +}; + +enum udev_status { UDEV_IFACE_ACTIVE, UDEV_IFACE_INACTIVE, UDEV_IFACE_ALL }; + +static struct udev_enumerate * +udevIfaceGetDevices(struct udev *udev, enum udev_status status) +{ + struct udev_enumerate *enumerate; + + /* Check input params for sanity */ + if (!udev) + return NULL; + + /* Create a new enumeration to create a list */ + enumerate = udev_enumerate_new(udev); + + if (!enumerate) + return NULL; + + /* Enumerate all network subsystem devices */ + udev_enumerate_add_match_subsystem(enumerate, "net"); + + /* Ignore devices that are part of a bridge */ + udev_enumerate_add_nomatch_sysattr(enumerate, "brport/state", NULL); + + /* State of the device */ + switch (status) { + case UDEV_IFACE_ACTIVE: + udev_enumerate_add_match_sysattr(enumerate, "operstate", "up"); + break; + + case UDEV_IFACE_INACTIVE: + udev_enumerate_add_match_sysattr(enumerate, "operstate", "down"); + break; + + case UDEV_IFACE_ALL: + break; + } + + /* We don't want to see the TUN devices that QEMU creates for other gets + * running on this machine. By saying nomatch NULL, we just are getting + * devices without the tun_flags sysattr. + */ + udev_enumerate_add_nomatch_sysattr(enumerate, "tun_flags", NULL); + + return enumerate; +} + +static virDrvOpenStatus +udevIfaceOpenInterface(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +{ + struct udev_iface_driver *driverState = NULL; + + if (VIR_ALLOC(driverState) < 0) { + virReportOOMError(); + goto err; + } + + driverState->udev = udev_new(); + if (!driverState->udev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create udev context")); + goto err; + } + + conn->interfacePrivateData = driverState; + + return VIR_DRV_OPEN_SUCCESS; + +err: + VIR_FREE(driverState); + + return VIR_DRV_OPEN_ERROR; +} + +static int +udevIfaceCloseInterface(virConnectPtr conn) +{ + struct udev_iface_driver *driverState; + + if (conn->interfacePrivateData != NULL) { + driverState = conn->interfacePrivateData; + + udev_unref(driverState->udev); + + VIR_FREE(driverState); + } + + conn->interfacePrivateData = NULL; + return 0; +} + +static int +udevIfaceNumOfInterfaces(virConnectPtr conn) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get number of active interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + count++; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static int +udevIfaceListInterfaces(virConnectPtr conn, char **const names, int names_len) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get list of active interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + struct udev_device *dev; + const char *path; + + path = udev_list_entry_get_name(dev_entry); + dev = udev_device_new_from_syspath(udev, path); + names[count] = strdup(udev_device_get_sysname(dev)); + udev_device_unref(dev); + + count++; + if (count > names_len) + break; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static int +udevIfaceNumOfDefinedInterfaces(virConnectPtr conn) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_INACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get number of defined interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + count++; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static int +udevIfaceListDefinedInterfaces(virConnectPtr conn, + char **const names, + int names_len) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_INACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get list of defined interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + struct udev_device *dev; + const char *path; + + path = udev_list_entry_get_name(dev_entry); + dev = udev_device_new_from_syspath(udev, path); + names[count] = strdup(udev_device_get_sysname(dev)); + udev_device_unref(dev); + + count++; + if (count > names_len) + break; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static virInterfacePtr +udevIfaceLookupByName(virConnectPtr conn, const char *name) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_device *dev; + const char *macaddr; + virInterfacePtr ret; + + /* get a device reference based on the device name */ + dev = udev_device_new_from_subsystem_sysname(udev, "net", name); + if (!dev) { + virReportError(VIR_ERR_NO_INTERFACE, + _("couldn't find interface named '%s'"), + name); + return NULL; + } + + macaddr = udev_device_get_sysattr_value(dev, "address"); + ret = virGetInterface(conn, name, macaddr); + udev_device_unref(dev); + + udev_unref(udev); + + return ret; +} + +static virInterfacePtr +udevIfaceLookupByMACString(virConnectPtr conn, const char *macstr) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *dev_entry; + struct udev_device *dev; + const char *name; + virInterfacePtr ret; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ALL); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to lookup interface with MAC address '%s'"), + macstr); + return NULL; + } + + /* Match on MAC */ + udev_enumerate_add_match_sysattr(enumerate, "address", macstr); + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + dev_entry = udev_enumerate_get_list_entry(enumerate); + + /* Check that we got something back */ + if (!dev_entry) { + virReportError(VIR_ERR_NO_INTERFACE, + _("couldn't find interface with MAC address '%s'"), + macstr); + return NULL; + } + + /* Check that we didn't get multiple items back */ + if (udev_list_entry_get_next(dev_entry)) { + virReportError(VIR_ERR_MULTIPLE_INTERFACES, + _("the MAC address '%s' matches multiple interfaces"), + macstr); + return NULL; + } + + dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(dev_entry)); + name = udev_device_get_sysname(dev); + ret = virGetInterface(conn, name, macstr); + udev_device_unref(dev); + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return ret; +} +static virInterfaceDriver udevIfaceDriver = { + "Interface", + .open = udevIfaceOpenInterface, + .close = udevIfaceCloseInterface, + .numOfInterfaces = udevIfaceNumOfInterfaces, + .listInterfaces = udevIfaceListInterfaces, + .numOfDefinedInterfaces = udevIfaceNumOfDefinedInterfaces, + .listDefinedInterfaces = udevIfaceListDefinedInterfaces, + .interfaceLookupByName = udevIfaceLookupByName, + .interfaceLookupByMACString = udevIfaceLookupByMACString, +}; + +int +interfaceRegister(void) { + if (virRegisterInterfaceDriver(&udevIfaceDriver) < 0) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to register udev interface driver")); + return 0; +} diff --git a/tools/virsh.c b/tools/virsh.c index 7a175ed..8f976a3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2710,6 +2710,9 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #if defined(WITH_NETCF) vshPrint(ctl, " netcf"); #endif +#if !defined(WITH_NETCF) && defined(HAVE_UDEV) + vshPrint(ctl, " udev"); +#endif #endif #ifdef WITH_NWFILTER vshPrint(ctl, " Nwfilter"); -- 1.7.8.6

On Sun, Sep 09, 2012 at 05:02:09PM -0500, Doug Goldstein wrote:
Add a read-only udev based backend for virInterface. Useful for distros that do not have netcf support yet. Multiple libvirt based utilities use a HAL based fallback when virInterface is not available which is less than ideal. This implements: * virConnectNumOfInterfaces() * virConnectListInterfaces() * virConnectNumOfDefinedInterfaces() * virConnectListDefinedInterfaces() * virConnectInterfaceLookupByName() * virConnectInterfaceLookupByMACString() --- .gnulib | 2 +- configure.ac | 10 +- src/Makefile.am | 12 +- src/interface/interface_backend_udev.c | 395 ++++++++++++++++++++++++++++++++ tools/virsh.c | 3 + 5 files changed, 417 insertions(+), 5 deletions(-) create mode 100644 src/interface/interface_backend_udev.c
I'm somewhat loathe to include this impl. The functionality provided is so limited as compared to netcf, that I don't think it'd really be a viable alternative. Also the semantics of this driver are somewhat different to what we intend these APIs to report. The intent was not that we report all physical interfaces, but rather that we have an idea of "logical interfaces" which comprise potentially many physical interfaces. I know you are filtering out TUN devices and anything that's part of a bridge, so you get kind of close to what netcf would do, but it still feels a little wrong. If we did, however, want to go down the route of having a generic udev based impl, then I think you'd probably want to actually do this as an alternative netcf driver. That way you can re-use the existing code for reverse-engineering an interface XML config from the live config. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Sep 10, 2012 at 5:08 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Sun, Sep 09, 2012 at 05:02:09PM -0500, Doug Goldstein wrote:
Add a read-only udev based backend for virInterface. Useful for distros that do not have netcf support yet. Multiple libvirt based utilities use a HAL based fallback when virInterface is not available which is less than ideal. This implements: * virConnectNumOfInterfaces() * virConnectListInterfaces() * virConnectNumOfDefinedInterfaces() * virConnectListDefinedInterfaces() * virConnectInterfaceLookupByName() * virConnectInterfaceLookupByMACString() --- .gnulib | 2 +- configure.ac | 10 +- src/Makefile.am | 12 +- src/interface/interface_backend_udev.c | 395 ++++++++++++++++++++++++++++++++ tools/virsh.c | 3 + 5 files changed, 417 insertions(+), 5 deletions(-) create mode 100644 src/interface/interface_backend_udev.c
I'm somewhat loathe to include this impl.
The functionality provided is so limited as compared to netcf, that I don't think it'd really be a viable alternative. Also the semantics of this driver are somewhat different to what we intend these APIs to report. The intent was not that we report all physical interfaces, but rather that we have an idea of "logical interfaces" which comprise potentially many physical interfaces. I know you are filtering out TUN devices and anything that's part of a bridge, so you get kind of close to what netcf would do, but it still feels a little wrong.
If we did, however, want to go down the route of having a generic udev based impl, then I think you'd probably want to actually do this as an alternative netcf driver. That way you can re-use the existing code for reverse-engineering an interface XML config from the live config.
Daniel --
The goal here is to provide virInterface APIs for everyone out there that doesn't have netcf, which is basically every Linux distro except RedHat/Fedora and Debian. Things like virt-install / virt-manger fall back to using HAL, which no distro carries anymore, when virInterface is unavailable. Other programs out there fall back to their own udev based implementation. The idea with these patches is to do the same thing that's being done with libvirt-designer, rather than every user of libvirt implementing virInterface calls with a fall back implementation that's different, we combine it into libvirt and just tell people to use virInterface. The reason why the creation is left out is that it's really the task for netcf. Also having libvirt bring up and teardown physical interfaces outside of the normal distro's network configs feels wrong. I'm not opposed to change that behavior and implement the full API. Lastly, I've run this on a RHEL6 machine and the output is identical to the netcf based backend. The machine doesn't have a bond device hence why I haven't compared that, but I noted that in the patchset. -- Doug Goldstein
participants (3)
-
Daniel P. Berrange
-
Doug Goldstein
-
Laine Stump