[PATCH] Only list devices that have an ifcfg file in ncf_lookup_by_mac_string.
by Laine Stump
drv_lookup_by_mac_string would previously find all interfaces that had
a directory in /sys/class/net with an address file containing the
correct MAC address. This would include transient interfaces like
libvirt's virbr*, tun*, etc. These transient interfaces can't be
configured with netcf anyway, so it doesn't make sense to list them
here. This patch checks each interface name before adding it to the
list, to make sure an ifcfg file exists for that interface. If not,
it's not added to the list.
This came up because there was a BZ filed against libvirt's iface-name
command (which calls ncf_lookup_by_mac_string()) - it gave an error
when trying to lookup an interface with a mac address of
00:00:00:00:00:00:
https://bugzilla.redhat.com/show_bug.cgi?id=580348
iface-name will log an error if there is more than one interface
matching the given mac address; in this case, lo (which is configured
with an ifcfg file) shows up in the list of all interfaces with a mac
of 00:00:00:00:00:00. However, libvirt's virbr0 (which *isn't* in the
list of all interfaces, because it has no ifcfg file) also matches
this MAC. That led libvirt to give an error when it apparently
shouldn't. Although that doesn't seem to be an important bug to fix,
it did reveal this inconsistency in listing of interfaces (if an
interface shows up when looking in one way, it should show up when
looking in other ways as well.)
An alternate fix to this would be to enhance libvirt to report
multiple interfaces in the iface-name command, and then add support
for transient interfaces to all the rest of the netcf API for
consistency's sake. That seems a bit drastic, though.
---
src/drv_initscripts.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 6cfbbd4..ceba52c 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -121,6 +121,18 @@ static int is_slave(struct netcf *ncf, const char *intf) {
return 0;
}
+static bool has_ifcfg_file(struct netcf *ncf, const char *name) {
+ int nmatches;
+
+ nmatches = aug_fmt_match(ncf, NULL,
+ "%s[ DEVICE = '%s'"
+ " or BRIDGE = '%s'"
+ " or MASTER = '%s'"
+ " or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",
+ ifcfg_path, name, name, name, name);
+ return nmatches > 0;
+}
+
static int cmpstrp(const void *p1, const void *p2) {
const char *s1 = * (const char **)p1;
const char *s2 = * (const char **)p2;
@@ -1087,6 +1099,8 @@ int drv_lookup_by_mac_string(struct netcf *ncf, const char *mac,
int cnt = 0;
for (int i = 0; i < nmatches; i++) {
+ if (!has_ifcfg_file(ncf, matches[i]))
+ continue;
r = xasprintf(&ifcfg, "%s[DEVICE = '%s']", ifcfg_path, matches[i]);
ERR_NOMEM(r < 0, ncf);
--
1.6.6.1
14 years
- 2
- 1
seems netcf lost something of ifcfg
by Osier Yang
hello there.
following is the result I got with 'cat', 'augtool', 'ncftool' on
/etc/sysconfig/network-scripts/ifcfg-eth0:
# cat /etc/sysconfig/network-scripts/ifcfg-eth0
DEVICE=eth0
BOOTPROTO=dhcp
HWADDR=00:1E:4F:D8:CB:7E
IPV6INIT=yes
IPV6_AUTOCONF=yes
ONBOOT=yes
OPTIONS=layer2=1
# augtool
augtool> print /files//etc/sysconfig/network-scripts/ifcfg-eth0
/files/etc/sysconfig/network-scripts/ifcfg-eth0
/files/etc/sysconfig/network-scripts/ifcfg-eth0/DEVICE = "eth0"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/BOOTPROTO = "dhcp"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/HWADDR =
"00:1E:4F:D8:CB:7E"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/IPV6INIT = "yes"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/IPV6_AUTOCONF = "yes"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/ONBOOT = "yes"
/files/etc/sysconfig/network-scripts/ifcfg-eth0/OPTIONS = "layer2=1"
augtool>
# ncftool
ncftool> dumpxml eth0
<?xml version="1.0"?>
<interface type="ethernet" name="eth0">
<start mode="onboot"/>
<mac address="00:1E:4F:D8:CB:7E"/>
<protocol family="ipv4">
<dhcp/>
</protocol>
<protocol family="ipv6">
<autoconf/>
</protocol>
</interface>
As we can see above, netcf lost "OPTIONS=layer2=1". Is it expected so?
Regards
osier
14 years
- 1
- 0
[PATCH] Improved run_program
by Laine Stump
The original run_program simply called system(3), which was expedient,
but failed to do things like close open file descriptors prior to
exec'ing the child binary. This particular omission caused complaints
when SELinux audited for leaked file descriptors during exec.
The new run program does the fork/exec manually, and in between it
closes all open file descriptors in the child process, as well as
clearing and restoring the signal mask to prevent a potential race
condition when killing off the parent's signal handlers in the child
process.
The code was heavily inspired by virRun() in libvirt, but is greatly
simplified, since netcf doesn't (currently) need all the facilities of
virRun().
---
configure.ac | 7 +++
src/netcf.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 133 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 321cd22..9f002d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -38,6 +38,13 @@ NETCF_CHECK_READLINE
NETCF_LIBDEPS=$(echo $LIBAUGEAS_LIBS $LIBEXSLT_LIBS $LIBXSLT_LIBS $LIBXML_LIBS $LIBNL_LIBS)
AC_SUBST([NETCF_LIBDEPS])
+AC_CHECK_HEADER([pthread.h],
+ [AC_CHECK_LIB([pthread],[pthread_join],[
+ AC_DEFINE([HAVE_LIBPTHREAD],[],[Define if pthread (-lpthread)])
+ AC_DEFINE([HAVE_PTHREAD_H],[],[Define if <pthread.h>])
+ LIBS="-lpthread $LIBS"
+ ])])
+
AC_OUTPUT(Makefile \
gnulib/lib/Makefile \
gnulib/tests/Makefile \
diff --git a/src/netcf.c b/src/netcf.c
index 7cfcc6a..7c642a3 100644
--- a/src/netcf.c
+++ b/src/netcf.c
@@ -27,6 +27,10 @@
#include <string.h>
#include <stdarg.h>
#include <stdio.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <errno.h>
#include "safe-alloc.h"
#include "internal.h"
@@ -262,25 +266,134 @@ int ncf_put_aug(struct netcf *ncf, const char *aug_xml, char **ncf_xml) {
* Internal helpers
*/
+static int
+exec_program(struct netcf *ncf,
+ const char *const*argv,
+ const char *commandline,
+ pid_t *pid)
+{
+ sigset_t oldmask, newmask;
+ struct sigaction sig_action;
+ char errbuf[128];
+
+ /* commandline is only used for error reporting */
+ if (commandline == NULL)
+ commandline = argv[0];
+ /*
+ * Need to block signals now, so that child process can safely
+ * kill off caller's signal handlers without a race.
+ */
+ sigfillset(&newmask);
+ if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) {
+ report_error(ncf, NETCF_EEXEC,
+ "failed to set signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ goto error;
+ }
+
+ *pid = fork();
+
+ ERR_THROW(*pid < 0, ncf, EEXEC,
+ "failed to set signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+
+ if (*pid) { /* parent */
+ /* Restore our original signal mask now that the child is
+ safely running */
+ ERR_THROW(pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0,
+ ncf, EEXEC,
+ "failed to restore signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ return 0;
+ }
+
+ /* child */
+
+ /* Clear out all signal handlers from parent so nothing unexpected
+ can happen in our child once we unblock signals */
+
+ sig_action.sa_handler = SIG_DFL;
+ sig_action.sa_flags = 0;
+ sigemptyset(&sig_action.sa_mask);
+
+ int i;
+ for (i = 1; i < NSIG; i++) {
+ /* Only possible errors are EFAULT or EINVAL
+ The former wont happen, the latter we
+ expect, so no need to check return value */
+
+ sigaction(i, &sig_action, NULL);
+ }
+
+ /* Unmask all signals in child, since we've no idea what the
+ caller's done with their signal mask and don't want to
+ propagate that to children */
+ sigemptyset(&newmask);
+ if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
+ report_error(ncf, NETCF_EEXEC,
+ "failure while unmasking signals in child for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ _exit(1);
+ }
+
+ /* close all open file descriptors */
+ int openmax = sysconf (_SC_OPEN_MAX);
+ for (i = 3; i < openmax; i++)
+ close(i);
+
+ execvp(argv[0], (char **) argv);
+
+ /* if execvp() returns, it has failed */
+ report_error(ncf, NETCF_EEXEC, "Failed to execute '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ _exit(1);
+
+error:
+ /* This is cleanup of parent process only - child
+ should never jump here on error */
+ return -1;
+}
+
+/**
+ * Run a command without using the shell.
+ *
+ * return 0 if the command run and exited with 0 status; Otherwise
+ * return -1
+ *
+ */
int run_program(struct netcf *ncf, const char *const *argv) {
- int status, success;
- char *argv_str = argv_to_string(argv);
+ pid_t childpid;
+ int exitstatus, waitret;
+ char *argv_str;
int ret = -1;
+ char errbuf[128];
- /* BIG FIXME!!! Before any general release, this *must* be
- * replaced with a call to a function similar to libVirt's
- * virRun(), and if there is an error returned, anything the
- * program produced on stderr or stdout should be placed in
- * ncf->errdetails.
- */
- status = system(argv_str);
- success = WIFEXITED(status) && WEXITSTATUS(status) == 0;
- ERR_THROW(!success, ncf, EEXEC,
- "Running '%s' failed with error code %d",
- argv_str == NULL ? argv[0] : argv_str, WEXITSTATUS(status));
+ argv_str = argv_to_string(argv);
+ ERR_NOMEM(argv_str == NULL, ncf);
+ exec_program(ncf, argv, argv_str, &childpid);
+ ERR_BAIL(ncf);
+
+ while ((waitret = waitpid(childpid, &exitstatus, 0) == -1) &&
+ errno == EINTR) {
+ /* empty loop */
+ }
+
+ ERR_THROW(waitret == -1, ncf, EEXEC,
+ "Failed waiting for completion of '%s': %s",
+ argv_str, strerror_r(errno, errbuf, sizeof(errbuf)));
+ ERR_THROW(!WIFEXITED(exitstatus) && WIFSIGNALED(exitstatus), ncf, EEXEC,
+ "'%s' exited abnormally with signal: %d",
+ argv_str, WTERMSIG(exitstatus));
+ ERR_THROW(!WIFEXITED(exitstatus), ncf, EEXEC,
+ "'%s' exited improperly: %d",
+ argv_str, WEXITSTATUS(exitstatus));
+ ERR_THROW(WEXITSTATUS(exitstatus) != 0, ncf, EEXEC,
+ "Running '%s' failed with exit code %d",
+ argv_str, WEXITSTATUS(exitstatus));
ret = 0;
+
error:
FREE(argv_str);
return ret;
--
1.6.6.1
14 years
- 2
- 3
- ← Newer
- 1
- Older →