Re: [PATCH] network driver: log error and abort network startup when radvd isn't found

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/18/2011 01:16 PM, Eric Blake wrote:
On 03/18/2011 11:08 AM, Laine Stump wrote:
This is detailed in:

   https://bugzilla.redhat.com/show_bug.cgi?id=688957

Since radvd is executed by daemonizing it, the attempt to exec the
radvd binary doesn't happen until after libvirtd has already received
an exit code from the intermediate forked process, so no error is
detected or logged by __virExec().

We can't require radvd as a prerequisit for the libvirt package (many
s/prerequisit/prerequisite/

installations don't use IPv6, so they don't need it), so instead we
add in a check to verify there is an executable radvd binary prior to
trying to exec it.
---
  src/network/bridge_driver.c |    8 ++++++++
  1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6a02df1..c30620a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -689,6 +689,14 @@ networkStartRadvd(virNetworkObjPtr network)

      network->radvdPid = -1;

+    if (access(RADVD, X_OK)<  0) {
Hmm, should this use

if (!virFileIsExecutable(RADVD)) {

instead?  Then again, I count 10 instances of 'access.*X_OK', but only 5
of FileIsExecutable, so maybe it's worth a separate cleanup (and a
cfg.mk rule to enforce whatever decision we make).

+        virReportSystemError(errno,
+                             _("Cannot find %s - "
+                               "Possibly the package isn't installed"),
+                             RADVD);
+        goto cleanup;
+    }
ACK with the commit typo fixed.

Okay. I pushed it using access(RADVD, X_OK). We can consider converting everything to virFileIsExecutable separately. Thanks!

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]