So far the virInitctlSetRunLevel() is fully automatic. It finds
the correct fifo to use to talk to the init and it will set the
desired runlevel. Well, callers (so far there is just one) will
need to inspect the fifo a bit just before the runlevel is set.
Therefore, expose the internal list of fifos and also allow
caller to explicitly use one.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_driver.c     |  2 +-
 src/util/virinitctl.c    | 66 +++++++++++++++++++++++++---------------
 src/util/virinitctl.h    |  6 +++-
 4 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 89b8ca3b4f..af490be12c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2049,6 +2049,7 @@ virIdentitySetX509DName;
 
 
 # util/virinitctl.h
+virInitctlFifos;
 virInitctlSetRunLevel;
 
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6c0f9b57db..943d199616 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3277,7 +3277,7 @@ lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED,
                          void *opaque)
 {
     int *command = opaque;
-    return virInitctlSetRunLevel(*command);
+    return virInitctlSetRunLevel(NULL, *command);
 }
 
 
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 0b06743151..8f8bbae4bc 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -101,7 +101,20 @@ struct virInitctlRequest {
   verify(sizeof(struct virInitctlRequest) == 384);
 # endif
 
-/*
+
+/* List of fifos that inits are known to listen on */
+const char *virInitctlFifos[] = {
+  "/run/initctl",
+  "/dev/initctl",
+  "/etc/.initctl",
+};
+
+
+/**
+ * virInitctlSetRunLevel:
+ * @fifo: the path to fifo that init listens on (can be NULL for autodetection)
+ * @level: the desired runlevel
+ *
  * Send a message to init to change the runlevel. This function is
  * asynchronous-signal-safe (thus safe to use after fork of a
  * multithreaded parent) - which is good, because it should only be
@@ -110,18 +123,14 @@ struct virInitctlRequest {
  * Returns 1 on success, 0 if initctl does not exist, -1 on error
  */
 int
-virInitctlSetRunLevel(virInitctlRunLevel level)
+virInitctlSetRunLevel(const char *fifo,
+                      virInitctlRunLevel level)
 {
     struct virInitctlRequest req;
     int fd = -1;
     int ret = -1;
-    const char *initctl_fifo = NULL;
+    const int open_flags = O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY;
     size_t i = 0;
-    const char *initctl_fifos[] = {
-        "/run/initctl",
-        "/dev/initctl",
-        "/etc/.initctl",
-    };
 
     memset(&req, 0, sizeof(req));
 
@@ -131,31 +140,39 @@ virInitctlSetRunLevel(virInitctlRunLevel level)
     /* Yes it is an 'int' field, but wants a numeric character. Go figure */
     req.runlevel = '0' + level;
 
-    for (i = 0; i < ARRAY_CARDINALITY(initctl_fifos); i++) {
-        initctl_fifo = initctl_fifos[i];
-
-        if ((fd = open(initctl_fifo,
-                       O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) >= 0)
-            break;
-
-        if (errno != ENOENT) {
+    if (fifo) {
+        if ((fd = open(fifo, open_flags)) < 0) {
             virReportSystemError(errno,
                                  _("Cannot open init control %s"),
-                                 initctl_fifo);
+                                 fifo);
             goto cleanup;
         }
-    }
+    } else {
+        for (i = 0; i < ARRAY_CARDINALITY(virInitctlFifos); i++) {
+            fifo = virInitctlFifos[i];
 
-    /* Ensure we found a valid initctl fifo */
-    if (fd < 0) {
-        ret = 0;
-        goto cleanup;
+            if ((fd = open(fifo, open_flags)) >= 0)
+                break;
+
+            if (errno != ENOENT) {
+                virReportSystemError(errno,
+                                     _("Cannot open init control %s"),
+                                     fifo);
+                goto cleanup;
+            }
+        }
+
+        /* Ensure we found a valid initctl fifo */
+        if (fd < 0) {
+            ret = 0;
+            goto cleanup;
+        }
     }
 
     if (safewrite(fd, &req, sizeof(req)) != sizeof(req)) {
         virReportSystemError(errno,
                              _("Failed to send request to init control %s"),
-                             initctl_fifo);
+                             fifo);
         goto cleanup;
     }
 
@@ -166,7 +183,8 @@ virInitctlSetRunLevel(virInitctlRunLevel level)
     return ret;
 }
 #else
-int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED)
+int virInitctlSetRunLevel(const char *fifo ATTRIBUTE_UNUSED,
+                          virInitctlRunLevel level ATTRIBUTE_UNUSED)
 {
     virReportUnsupportedError();
     return -1;
diff --git a/src/util/virinitctl.h b/src/util/virinitctl.h
index 7ac627883a..f12741c7c1 100644
--- a/src/util/virinitctl.h
+++ b/src/util/virinitctl.h
@@ -33,6 +33,10 @@ typedef enum {
     VIR_INITCTL_RUNLEVEL_LAST
 } virInitctlRunLevel;
 
-int virInitctlSetRunLevel(virInitctlRunLevel level);
+
+extern const char *virInitctlFifos[3];
+
+int virInitctlSetRunLevel(const char *fifo,
+                          virInitctlRunLevel level);
 
 #endif /* LIBVIRT_VIRINITCTL_H */
-- 
2.19.2

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

On Fri, Jan 25, 2019 at 02:31:48PM +0100, Michal Privoznik wrote:
> So far the virInitctlSetRunLevel() is fully automatic. It finds
> the correct fifo to use to talk to the init and it will set the
> desired runlevel. Well, callers (so far there is just one) will
> need to inspect the fifo a bit just before the runlevel is set.
> Therefore, expose the internal list of fifos and also allow
> caller to explicitly use one.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

...

> -int virInitctlSetRunLevel(virInitctlRunLevel level);
> +
> +extern const char *virInitctlFifos[3];

                            I prefer [] to prevent compiler from complaining if
someone adds a new fifo

> +
> +int virInitctlSetRunLevel(const char *fifo,
> +                          virInitctlRunLevel level);
>
>  #endif /* LIBVIRT_VIRINITCTL_H */
> --
Reviewed-by: Erik Skultety <eskultet@redhat.com>

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

On 2/6/19 3:43 PM, Erik Skultety wrote:
> On Fri, Jan 25, 2019 at 02:31:48PM +0100, Michal Privoznik wrote:
>> So far the virInitctlSetRunLevel() is fully automatic. It finds
>> the correct fifo to use to talk to the init and it will set the
>> desired runlevel. Well, callers (so far there is just one) will
>> need to inspect the fifo a bit just before the runlevel is set.
>> Therefore, expose the internal list of fifos and also allow
>> caller to explicitly use one.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
> 
> ...
> 
>> -int virInitctlSetRunLevel(virInitctlRunLevel level);
>> +
>> +extern const char *virInitctlFifos[3];
> 
>                              I prefer [] to prevent compiler from complaining if
> someone adds a new fifo

Problem with this is that then I can't use ARRAY_CARDINALITY() macro in 
5/5. As we discussed in person, I'm turning this into a NULL terminated 
list and adjusting 5/5 correspondingly.

> 
>> +
>> +int virInitctlSetRunLevel(const char *fifo,
>> +                          virInitctlRunLevel level);
>>
>>   #endif /* LIBVIRT_VIRINITCTL_H */
>> --
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 

Michal

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