Changeset 4224ef7 in mainline

Timestamp:

2019-08-06T18:18:37Z (5 years ago)

Author:

Matthieu Riolo <matthieu.riolo@…>

Children:

e55741e

Parents:

dd5c623

git-author:

Michal Koutný <xm.koutny+hos@…> (2015-05-11 16:50:40)

git-committer:

Matthieu Riolo <matthieu.riolo@…> (2019-08-06 18:18:37)

Message:

sysman: Add VFS FS server autostart

• VFS autostart instrumentation removes explicit dependency on FS servers.
• Compositor service properly named, it's now resolved as implicit dependency.

Conflicts:

boot/Makefile.common
uspace/lib/gui/window.c
uspace/srv/locsrv/locsrv.c
uspace/srv/vfs/vfs.c
uspace/srv/vfs/vfs_ops.c

Location:

uspace

Files:

• cfg/sysman/barber.svc (modified) (1 diff)
• cfg/sysman/comp:0__winreg.svc (moved) (moved from uspace/cfg/sysman/compositor.svc )
• cfg/sysman/locfs.mnt (modified) (1 diff)
• cfg/sysman/rootfs.mnt (modified) (1 diff)
• cfg/sysman/vlaunch.svc (modified) (2 diffs)
• lib/gui/window.c (modified) (4 diffs)
• lib/sysman/src/ctl.c (modified) (1 diff)
• srv/locsrv/locsrv.c (modified) (5 diffs)
• srv/sysman/connection_broker.c (modified) (1 diff)
• srv/sysman/sysman.c (modified) (2 diffs)
• srv/sysman/units/unit_mnt.c (modified) (1 diff)
• srv/sysman/units/unit_svc.c (modified) (1 diff)
• srv/vfs/Makefile (modified) (1 diff)
• srv/vfs/vfs.c (modified) (2 diffs)
• srv/vfs/vfs_ops.c (modified) (2 diffs)

uspace/cfg/sysman/barber.svc

@@ -1 +1 @@
 [Unit]
-; explicit dependency on compositor,
-; until exposee to unit name map in locsrv is defined
-After = compositor.svc 
 After = rootfs.mnt
 

uspace/cfg/sysman/locfs.mnt

@@ -1 @@
-[Unit]
-; explicit, VFS not instrumented for autostart
-After = locfs.svc
-
 [Mount]
 What = 

uspace/cfg/sysman/rootfs.mnt

@@ -1 @@
-[Unit]
-; explicit, VFS not instrumented for autostart
-After = fat.svc
-
 [Mount]
 What = devices/\hw\pci0\00:01.0\ata-c1\d0

uspace/cfg/sysman/vlaunch.svc

@@ -1 +1 @@
 [Unit]
-; explicit dependency on compositor,
-; until exposee to unit name map in locsrv is defined
-After = compositor.svc 
 ; explicit because of executable path
 After = rootfs.mnt
@@ -8 +5 @@
 After = locfs.mnt
 
-; unclear status of vlaunch as service (more relevant to user session)
+; unclear status of vlaunch as a service (more relevant to user session)
 [Service]
 ExecStart = /root/app/vlaunch comp:0/winreg

uspace/lib/gui/window.c

@@ -616 +616 @@
         win->surface = NULL;
 
+        unsigned int ipc_flags = IPC_FLAG_AUTOSTART;
         service_id_t reg_dsid;
-        errno_t rc = loc_service_get_id(winreg, &reg_dsid, 0);
+        errno_t rc = loc_service_get_id(winreg, &reg_dsid, ipc_flags);
         if (rc != EOK) {
                 free(win);
@@ -624 +625 @@
 
         async_sess_t *reg_sess =
-            loc_service_connect(reg_dsid, INTERFACE_COMPOSITOR, 0);
+            loc_service_connect(reg_dsid, INTERFACE_COMPOSITOR, ipc_flags);
+
         if (reg_sess == NULL) {
                 free(win);
@@ -639 +641 @@
         }
 
-        win->osess = loc_service_connect(out_dsid, INTERFACE_COMPOSITOR, 0);
+
+        win->osess = loc_service_connect(out_dsid, INTERFACE_COMPOSITOR, ipc_flags);
+
         if (win->osess == NULL) {
                 free(win);
@@ -645 +649 @@
         }
 
-        win->isess = loc_service_connect(in_dsid, INTERFACE_COMPOSITOR, 0);
+        win->isess = loc_service_connect(in_dsid, INTERFACE_COMPOSITOR, ipc_flags);
+
         if (win->isess == NULL) {
                 async_hangup(win->osess);

uspace/lib/sysman/src/ctl.c

@@ -33 +33 @@
 #include <sysman/sysman.h>
 
+/*
+ * TODO
+ * Non-blocking favor of this API is effectively incomplete as it doesn't
+ * provide means how to obtain result of the start operation.
+ * Probably devise individual API for brokers that could exploit the fact that
+ * broker knows when appropriate exposee is created and the request succeeded.
+ * Still though, it's necessary to centralize timeout into sysman.
+ */
 int sysman_unit_start(const char *unit_name, int flags)
 {

uspace/srv/locsrv/locsrv.c

@@ -339 +339 @@
         char *service_name = NULL;
 
+        /*
+         * All services in 'device' namespace are considered to be drivers and
+         * devman is thus requested to start. Otherwise name of unit is made
+         * from fully qualified name of service (namespace separator is changed
+         * for usage in unit name.
+         */
         if (str_cmp(ns_name, LOC_DEVICE_NAMESPACE) == 0) {
                 asprintf(&service_name, "%s", SERVICE_NAME_DEVMAN);
@@ -359 +365 @@
         }
 
-        //printf("%s(%s) before\n", __func__, unit_name);//DEBUG
         int rc = sysman_unit_start(unit_name, IPC_FLAG_BLOCKING);
-        //printf("%s(%s) after %i\n", __func__, unit_name, rc);//DEBUG
         free(unit_name);
         free(service_name);
@@ -811 +815 @@
          */
         if (svc == NULL) {
-                //printf("%s: service '%s/%s' not found\n", NAME, ns_name, name);//DEBUG
-                if (flags & (IPC_FLAG_AUTOSTART | IPC_FLAG_BLOCKING)) {
-                        /* TODO:
-                         * consider non-blocking service start, return
-                         * some dummy id and block only after connection
-                         * request (actually makes more sense as those who asks
-                         * for ID might be someone else than those connecting)
-                         */
-                        if (!start_requested && (flags & IPC_FLAG_AUTOSTART)) {
-                                rc = loc_service_request_start(ns_name, name);
-                                if (rc != EOK) {
-                                        goto finish;
-                                }
-                                start_requested = true;
+                /* TODO:
+                 * consider non-blocking service start, return
+                 * some dummy id and block only after connection
+                 * request (actually makes more sense as those who asks
+                 * for ID might be someone else than those connecting)
+                 * Note:
+                 * service_list_mutex is released as we don't need to keep it
+                 * while waiting for start request to finish.
+                 */
+                if ((flags & IPC_FLAG_AUTOSTART) && !start_requested) {
+                        fibril_mutex_unlock(&services_list_mutex);
+                        rc = loc_service_request_start(ns_name, name);
+                        fibril_mutex_lock(&services_list_mutex);
+                        start_requested = true;
+
+                        if (rc != EOK) {
+                                goto finish;
+                        } else {
+                                goto recheck;
                         }
-
+                }
+
+                if (flags & IPC_FLAG_BLOCKING) {
                         fibril_condvar_wait(&services_list_cv,
                             &services_list_mutex);
@@ -833 +844 @@
                 rc = ENOENT;
         } else {
-                //printf("%s: service '%s/%s' FOUND\n", NAME, ns_name, name);//DEBUG
                 rc = EOK;
         }
@@ -1621 +1631 @@
 
         /* Let sysman know we are broker */
-        printf("%s: sysman_broker_register : pre\n", NAME);
         rc = sysman_broker_register();
-        printf("%s: sysman_broker_register : post\n", NAME);
         if (rc != EOK) {
                 printf("%s: Error registering at sysman (%i)\n", NAME, rc);

uspace/srv/sysman/connection_broker.c

@@ -37 +37 @@
         sysman_log(LVL_DEBUG2, "%s", __func__);
         async_answer_0(iid, EOK);
-        // TODO implement
+        /* TODO implement
+         *  What exactly? Similar behavior that has locsrv with servers,
+         *  so that subsequent calls can be assigned to broker. Still that
+         *  makes sense only when brokers will somehow scope unit/exposee
+         *  names. Why I wanted this registration?
+         */
 }
 

uspace/srv/sysman/sysman.c

@@ -160 +160 @@
 /** Create and queue job for unit
  *
- * @param[in]  callback  callback must explicitly delete reference to job
+ * @param[in]  callback  (optional) callback must explicitly delete reference
+ *                       to job
  */
 int sysman_queue_job(unit_t *unit, unit_state_t target_state,
@@ -170 +171 @@
         }
 
-        job_add_ref(job);
-        sysman_object_observer(job, callback, callback_arg);
+        if (callback != NULL) {
+                job_add_ref(job);
+                sysman_object_observer(job, callback, callback_arg);
+        }
 
         job_add_ref(job);

uspace/srv/sysman/units/unit_mnt.c

@@ -151 +151 @@
 {
         mount_data_t *mnt_data = arg;
-        /*sysman_log(LVL_DEBUG2, "%s(%p, %p, %p, %p, %x, %u)",
+        sysman_log(LVL_DEBUG2, "%s(%p, %p, %p, %p, %x, %u)",
             __func__,
             mnt_data->type, mnt_data->mountpoint, mnt_data->device, mnt_data->options,
-            mnt_data->flags, mnt_data->instance);*/
+            mnt_data->flags, mnt_data->instance);
+
         int rc = mount(mnt_data->type, mnt_data->mountpoint, mnt_data->device,
             mnt_data->options ? mnt_data->options : "",

uspace/srv/sysman/units/unit_svc.c

@@ -87 +87 @@
         int rc = task_spawnv(NULL, NULL, u_svc->exec_start.path,
             u_svc->exec_start.argv);
-        sysman_log(LVL_DEBUG2, "task_spawn(%s, %s)", u_svc->exec_start.path, u_svc->exec_start.argv[0]);
+
         if (rc != EOK) {
                 unit->state = STATE_FAILED;

uspace/srv/vfs/Makefile

@@ -29 +29 @@
 
 USPACE_PREFIX = ../..
+LIBS = $(LIBSYSMAN_PREFIX)/libsysman.a
+EXTRA_CFLAGS += -I$(LIBSYSMAN_PREFIX)/include
 BINARY = vfs
 STATIC_NEEDED = y

uspace/srv/vfs/vfs.c

@@ -40 +40 @@
 #include <ipc/services.h>
 #include <abi/ipc/methods.h>
-#include <libarch/config.h>
 #include <ns.h>
+#include <as.h>
 #include <async.h>
+#include <atomic.h>
 #include <errno.h>
 #include <str_error.h>
 #include <stdio.h>
+#include <ipc/services.h>
+#include <macros.h>
 #include <stdbool.h>
+#include <stdio.h>
 #include <str.h>
-#include <as.h>
-#include <macros.h>
+#include <sysman/broker.h>
 #include "vfs.h"
 
@@ -145 +148 @@
 
         /*
+         * Let sysman know we are broker
+         */
+        rc = sysman_broker_register();
+        if (rc != EOK) {
+                printf("%s: Error registering at sysman (%i)\n", NAME, rc);
+                return rc;
+        }
+        
+        /*
          * Start accepting connections.
          */

uspace/srv/vfs/vfs_ops.c

@@ -137 +137 @@
         while (true) {
                 fs_handle = fs_name_to_handle(instance, fsname, false);
+                if (!fs_handle) {
+                        if ((flags & IPC_FLAG_AUTOSTART)) {
+                                /*
+                                 * Temporarily release the lock, we don't need it while
+                                 * waiting for start request (which may lead to deadlock).
+                                 */
+                                fibril_mutex_unlock(&fs_list_lock);
+                                rc = vfs_fs_request_start(fs_name, instance);
+                                fibril_mutex_lock(&fs_list_lock);
+
+                                if (rc != EOK) {
+                                        fibril_mutex_unlock(&fs_list_lock);
+                                        async_answer_0(callid, rc);
+                                        async_answer_0(rid, rc);
+                                        free(mp);
+                                        free(fs_name);
+                                        free(opts);
+                                        return;
+                                }
+                                /*
+                                 * Succesful start request, new server should be
+                                 * registered.
+                                 */
+                                continue;
+                        }
+                }
 
                 if (fs_handle != 0 || !(flags & VFS_MOUNT_BLOCKING))
@@ -188 +214 @@
 
         return EOK;
+}
+
+static int vfs_fs_request_start(const char *fs_name, unsigned int instance)
+{
+        char *unit_name = NULL;
+
+        assert(instance == 0);
+        /*
+         * Unit name is made simply by considering service of the same name as
+         * given FS name.
+         * TODO instance identifier is not implemented.
+         */
+        asprintf(&unit_name, "%s%c%s", fs_name, UNIT_NAME_SEPARATOR,
+            UNIT_SVC_TYPE_NAME);
+        if (unit_name == NULL) {
+                return ENOMEM;
+        }
+
+        int rc = sysman_unit_start(unit_name, IPC_FLAG_BLOCKING);
+
+        free(unit_name);
+        return rc;
 }