Hi.
On Wed, 18 Dec 2013 18:15:17 +0100
Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:
> While the part above looks okay, I think we should report error if the /dev/initctl is not a pipe. Moreover, with your approach, if it's really not a pipe, we don't call open() and proceed to safewrite(-1, ...) at line 153 which will report EBADF error.
I've adjusted the patch to your suggestion.
> Missing ';' at EOL ^^^ - I guess you missed that during copy-paste.
OOPS. My mistake.
> Looking forward to v2.
Attached. I've tried following Daniel suggestions too.
> BTW: consider using git when sending patches. I couldn't really apply this patch via 'git am'.
I cannot figure out how to force 'git send-email' to reply on a mail.
It seems to insist to send a new one.
Reco
commit be722f5c49a6ceab5af7d4c5e672c0b70f969efd
Author: Reco <recoverym4n@xxxxxxxxx>
Date: Wed Dec 18 23:44:17 2013 +0400
Check whenever init control is a pipe
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 64bc23a..2a2ec27 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -24,7 +24,10 @@
#include <config.h>
#include <sys/param.h>
+#include <sys/types.h>
+#include <dirent.h>
#include <fcntl.h>
+#include <string.h>
#include "internal.h"
#include "virinitctl.h"
@@ -46,11 +49,21 @@
* Copyright (C) 1995-2004 Miquel van Smoorenburg
*/
+#ifdef _DIRENT_HAVE_D_TYPE
+# if defined(__FreeBSD_kernel__)
+# define VIR_INITCTL_DIR "/etc"
+# define VIR_INITCTL_FIFO ".initctl"
+# else
+# define VIR_INITCTL_DIR "/dev"
+# define VIR_INITCTL_FIFO "initctl"
+# endif
+#else
# if defined(__FreeBSD_kernel__)
# define VIR_INITCTL_FIFO "/etc/.initctl"
# else
# define VIR_INITCTL_FIFO "/dev/initctl"
# endif
+#endif
# define VIR_INITCTL_MAGIC 0x03091969
# define VIR_INITCTL_CMD_START 0
@@ -123,6 +136,13 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
char *path = NULL;
int ret = -1;
+#ifdef _DIRENT_HAVE_D_TYPE
+ int dfd = -1;
+ DIR *dev;
+ char *dpath = NULL;
+ struct dirent* dentry;
+#endif
+
memset(&req, 0, sizeof(req));
req.magic = VIR_INITCTL_MAGIC;
@@ -131,6 +151,63 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
/* Yes it is an 'int' field, but wants a numeric character. Go figure */
req.runlevel = '0' + level;
+#ifdef _DIRENT_HAVE_D_TYPE
+ if (vroot) {
+ if (virAsprintf(&dpath, "%s/%s", vroot, VIR_INITCTL_DIR) < 0)
+ return -1;
+ } else {
+ if (VIR_STRDUP(dpath, VIR_INITCTL_DIR) < 0)
+ return -1;
+ }
+
+ if ((dfd = open(dpath,
+ O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY|O_DIRECTORY|O_NOFOLLOW)) < 0) {
+ virReportSystemError(errno,
+ _("%s is not a directory"),
+ dpath);
+ ret = -1;
+ goto cleanup;
+ }
+
+ if ((dev = fdopendir(dfd)) == NULL) {
+ virReportSystemError(errno,
+ _("Failed to open directory %s"),
+ dpath);
+ ret = -1;
+ goto cleanup;
+ }
+
+ while ((dentry = readdir(dev))) {
+ if (strcmp(dentry->d_name, VIR_INITCTL_FIFO) != 0)
+ continue;
+
+ if (dentry->d_type != DT_FIFO)
+ {
+ virReportSystemError(errno,
+ _("Init control %s/%s is not a pipe"),
+ dpath, VIR_INITCTL_FIFO);
+ closedir(dev);
+ ret = -1;
+ goto cleanup;
+ }
+
+ if (virAsprintf(&path, "%s/%s", dpath, VIR_INITCTL_FIFO) < 0)
+ {
+ closedir(dev);
+ ret = -1;
+ goto cleanup;
+ }
+
+ break;
+ }
+
+ closedir(dev);
+
+ if (! path) {
+ ret = 0;
+ goto cleanup;
+ }
+#else
if (vroot) {
if (virAsprintf(&path, "%s/%s", vroot, VIR_INITCTL_FIFO) < 0)
return -1;
@@ -138,8 +215,9 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
if (VIR_STRDUP(path, VIR_INITCTL_FIFO) < 0)
return -1;
}
+#endif
- if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) < 0) {
+ if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW)) < 0) {
if (errno == ENOENT) {
ret = 0;
goto cleanup;
@@ -160,6 +238,10 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
ret = 1;
cleanup:
+#ifdef _DIRENT_HAVE_D_TYPE
+ VIR_FREE(dpath);
+ VIR_FORCE_CLOSE(dfd);
+#endif
VIR_FREE(path);
VIR_FORCE_CLOSE(fd);
return ret;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list