[MERGED] osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Holger Freyther]

Holger Freyther has submitted this change and it was merged.

Change subject: mobile: Move starting/stopping a MS into a separate function
..


mobile: Move starting/stopping a MS into a separate function

Move the check if within the mobile app there is no other active
MS using the same L1 socket. This way we can call this function
from the primitive code as well.

Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
---
M src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
M src/host/layer23/src/mobile/app_mobile.c
M src/host/layer23/src/mobile/vty_interface.c
3 files changed, 70 insertions(+), 37 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h 
b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
index 704c972..7abfda1 100644
--- a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
+++ b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
@@ -13,12 +13,15 @@
 int l23_app_work(int *quit);
 int mobile_delete(struct osmocom_ms *ms, int force);
 struct osmocom_ms *mobile_new(char *name);
-int mobile_init(struct osmocom_ms *ms);
-int mobile_exit(struct osmocom_ms *ms, int force);
 int mobile_work(struct osmocom_ms *ms);
+int mobile_start(struct osmocom_ms *ms, char **other_name);
+int mobile_stop(struct osmocom_ms *ms, int force);
 
 void mobile_set_started(struct osmocom_ms *ms, bool state);
 void mobile_set_shutdown(struct osmocom_ms *ms, int state);
 
+
+/* Internal code. Don't call directly */
+int mobile_exit(struct osmocom_ms *ms, int force);
 #endif
 
diff --git a/src/host/layer23/src/mobile/app_mobile.c 
b/src/host/layer23/src/mobile/app_mobile.c
index c5c84e6..b2900ad 100644
--- a/src/host/layer23/src/mobile/app_mobile.c
+++ b/src/host/layer23/src/mobile/app_mobile.c
@@ -181,7 +181,7 @@
 }
 
 /* power-on ms instance */
-int mobile_init(struct osmocom_ms *ms)
+static int mobile_init(struct osmocom_ms *ms)
 {
int rc;
 
@@ -245,6 +245,51 @@
return 0;
 }
 
+int mobile_start(struct osmocom_ms *ms, char **other_name)
+{
+   struct osmocom_ms *tmp;
+   int rc;
+
+   if (ms->shutdown != MS_SHUTDOWN_COMPL)
+   return 0;
+
+   llist_for_each_entry(tmp, _list, entity) {
+   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
+   continue;
+   if (!strcmp(ms->settings.layer2_socket_path,
+   tmp->settings.layer2_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same layer2-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -1;
+   }
+   if (!strcmp(ms->settings.sap_socket_path,
+   tmp->settings.sap_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same sap-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -2;
+   }
+   }
+
+   rc = mobile_init(ms);
+   if (rc < 0)
+   return -3;
+   return 0;
+}
+
+int mobile_stop(struct osmocom_ms *ms, int force)
+{
+   if (force && ms->shutdown <= MS_SHUTDOWN_IMSI_DETACH)
+   return mobile_exit(ms, 1);
+   if (!force && ms->shutdown == MS_SHUTDOWN_NONE)
+   return mobile_exit(ms, 0);
+   return 0;
+}
+
+
 /* create ms instance */
 struct osmocom_ms *mobile_new(char *name)
 {
diff --git a/src/host/layer23/src/mobile/vty_interface.c 
b/src/host/layer23/src/mobile/vty_interface.c
index eafed6e..81c20fe 100644
--- a/src/host/layer23/src/mobile/vty_interface.c
+++ b/src/host/layer23/src/mobile/vty_interface.c
@@ -2696,35 +2696,25 @@
 DEFUN(cfg_no_shutdown, cfg_ms_no_shutdown_cmd, "no shutdown",
NO_STR "Activate and run MS")
 {
-   struct osmocom_ms *ms = vty->index, *tmp;
+   struct osmocom_ms *ms = vty->index;
+   char *other_name = NULL;
int rc;
 
-   if (ms->shutdown != MS_SHUTDOWN_COMPL)
-   return CMD_SUCCESS;
-
-   llist_for_each_entry(tmp, _list, entity) {
-   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
-   continue;
-   if (!strcmp(ms->settings.layer2_socket_path,
-   tmp->settings.layer2_socket_path)) {
-   vty_out(vty, "Cannot start MS '%s', because MS '%s' "
-   "use the same layer2-socket.%sPlease shutdown "
-   "MS '%s' first.%s", ms->name, tmp->name,
- 

osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Harald Welte]

Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/5100
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
Gerrit-PatchSet: 3
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No

[PATCH] osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Holger Freyther]

Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5100

to look at the new patch set (#3).

mobile: Move starting/stopping a MS into a separate function

Move the check if within the mobile app there is no other active
MS using the same L1 socket. This way we can call this function
from the primitive code as well.

Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
---
M src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
M src/host/layer23/src/mobile/app_mobile.c
M src/host/layer23/src/mobile/vty_interface.c
3 files changed, 70 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/00/5100/3

diff --git a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h 
b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
index 704c972..7abfda1 100644
--- a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
+++ b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
@@ -13,12 +13,15 @@
 int l23_app_work(int *quit);
 int mobile_delete(struct osmocom_ms *ms, int force);
 struct osmocom_ms *mobile_new(char *name);
-int mobile_init(struct osmocom_ms *ms);
-int mobile_exit(struct osmocom_ms *ms, int force);
 int mobile_work(struct osmocom_ms *ms);
+int mobile_start(struct osmocom_ms *ms, char **other_name);
+int mobile_stop(struct osmocom_ms *ms, int force);
 
 void mobile_set_started(struct osmocom_ms *ms, bool state);
 void mobile_set_shutdown(struct osmocom_ms *ms, int state);
 
+
+/* Internal code. Don't call directly */
+int mobile_exit(struct osmocom_ms *ms, int force);
 #endif
 
diff --git a/src/host/layer23/src/mobile/app_mobile.c 
b/src/host/layer23/src/mobile/app_mobile.c
index c5c84e6..b2900ad 100644
--- a/src/host/layer23/src/mobile/app_mobile.c
+++ b/src/host/layer23/src/mobile/app_mobile.c
@@ -181,7 +181,7 @@
 }
 
 /* power-on ms instance */
-int mobile_init(struct osmocom_ms *ms)
+static int mobile_init(struct osmocom_ms *ms)
 {
int rc;
 
@@ -245,6 +245,51 @@
return 0;
 }
 
+int mobile_start(struct osmocom_ms *ms, char **other_name)
+{
+   struct osmocom_ms *tmp;
+   int rc;
+
+   if (ms->shutdown != MS_SHUTDOWN_COMPL)
+   return 0;
+
+   llist_for_each_entry(tmp, _list, entity) {
+   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
+   continue;
+   if (!strcmp(ms->settings.layer2_socket_path,
+   tmp->settings.layer2_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same layer2-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -1;
+   }
+   if (!strcmp(ms->settings.sap_socket_path,
+   tmp->settings.sap_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same sap-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -2;
+   }
+   }
+
+   rc = mobile_init(ms);
+   if (rc < 0)
+   return -3;
+   return 0;
+}
+
+int mobile_stop(struct osmocom_ms *ms, int force)
+{
+   if (force && ms->shutdown <= MS_SHUTDOWN_IMSI_DETACH)
+   return mobile_exit(ms, 1);
+   if (!force && ms->shutdown == MS_SHUTDOWN_NONE)
+   return mobile_exit(ms, 0);
+   return 0;
+}
+
+
 /* create ms instance */
 struct osmocom_ms *mobile_new(char *name)
 {
diff --git a/src/host/layer23/src/mobile/vty_interface.c 
b/src/host/layer23/src/mobile/vty_interface.c
index eafed6e..81c20fe 100644
--- a/src/host/layer23/src/mobile/vty_interface.c
+++ b/src/host/layer23/src/mobile/vty_interface.c
@@ -2696,35 +2696,25 @@
 DEFUN(cfg_no_shutdown, cfg_ms_no_shutdown_cmd, "no shutdown",
NO_STR "Activate and run MS")
 {
-   struct osmocom_ms *ms = vty->index, *tmp;
+   struct osmocom_ms *ms = vty->index;
+   char *other_name = NULL;
int rc;
 
-   if (ms->shutdown != MS_SHUTDOWN_COMPL)
-   return CMD_SUCCESS;
-
-   llist_for_each_entry(tmp, _list, entity) {
-   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
-   continue;
-   if (!strcmp(ms->settings.layer2_socket_path,
-   tmp->settings.layer2_socket_path)) {
-   vty_out(vty, "Cannot start MS '%s', because MS '%s' "
-   "use the same layer2-socket.%sPlease shutdown "
-   "MS '%s' first.%s", ms->name, tmp->name,
-   VTY_NEWLINE, tmp->name, VTY_NEWLINE);
-   

osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Harald Welte]

Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/5100/2/src/host/layer23/src/mobile/app_mobile.c
File src/host/layer23/src/mobile/app_mobile.c:

Line 262:   "use the same layer2-socket.\nPlease 
shutdown "
> sure about \n in the middle of a LOGP? Makes sense in VTY output but "corru
the old code used VTY_NEWLINE instead of \n, which isn't any better either.  I 
think OsmocomBB needs a thorough clean up of such invalid use of libosmocore 
logging, but that shouldn't be mixed with this completely unrelated patch which 
just moves code around in this case.


-- 
To view, visit https://gerrit.osmocom.org/5100
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
Gerrit-PatchSet: 2
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: Yes

osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Neels Hofmeyr]

Patch Set 2: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/5100/2/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
File src/host/layer23/include/osmocom/bb/mobile/app_mobile.h:

Line 20: int mobile_stop(struct osmocom_ms *ms, int force);
as n00b I can't understand the difference between mobile_stop() and 
mobile_exit(), maybe the functions could use an api comment?


https://gerrit.osmocom.org/#/c/5100/2/src/host/layer23/src/mobile/app_mobile.c
File src/host/layer23/src/mobile/app_mobile.c:

Line 253:   if (ms->shutdown != MS_SHUTDOWN_COMPL)
(log?)


Line 262:   "use the same layer2-socket.\nPlease 
shutdown "
sure about \n in the middle of a LOGP? Makes sense in VTY output but "corrupts" 
the log output


Line 278:   if (rc < 0)
(log?)


Line 288:   return mobile_exit(ms, 0);
(log why nothing is done?)


-- 
To view, visit https://gerrit.osmocom.org/5100
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
Gerrit-PatchSet: 2
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: Yes

[PATCH] osmocom-bb[master]: mobile: Move starting/stopping a MS into a separate function

[Holger Freyther]

Review at  https://gerrit.osmocom.org/5100

mobile: Move starting/stopping a MS into a separate function

Move the check if within the mobile app there is no other active
MS using the same L1 socket. This way we can call this function
from the primitive code as well.

Change-Id: Ib4aa5ff212fa6bead8f620abaecc6a0b51a99fec
---
M src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
M src/host/layer23/src/mobile/app_mobile.c
M src/host/layer23/src/mobile/vty_interface.c
3 files changed, 67 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/00/5100/1

diff --git a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h 
b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
index 23d0ac8..eb54e2c 100644
--- a/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
+++ b/src/host/layer23/include/osmocom/bb/mobile/app_mobile.h
@@ -14,9 +14,10 @@
 int l23_app_work(int *quit);
 int mobile_delete(struct osmocom_ms *ms, int force);
 struct osmocom_ms *mobile_new(char *name);
-int mobile_init(struct osmocom_ms *ms);
 int mobile_exit(struct osmocom_ms *ms, int force);
 int mobile_work(struct osmocom_ms *ms);
+int mobile_start(struct osmocom_ms *ms, char **other_name);
+int mobile_stop(struct osmocom_ms *ms, int force);
 
 void mobile_set_started(struct osmocom_ms *ms, bool state);
 void mobile_set_shutdown(struct osmocom_ms *ms, int state);
diff --git a/src/host/layer23/src/mobile/app_mobile.c 
b/src/host/layer23/src/mobile/app_mobile.c
index c5c84e6..b2900ad 100644
--- a/src/host/layer23/src/mobile/app_mobile.c
+++ b/src/host/layer23/src/mobile/app_mobile.c
@@ -181,7 +181,7 @@
 }
 
 /* power-on ms instance */
-int mobile_init(struct osmocom_ms *ms)
+static int mobile_init(struct osmocom_ms *ms)
 {
int rc;
 
@@ -245,6 +245,51 @@
return 0;
 }
 
+int mobile_start(struct osmocom_ms *ms, char **other_name)
+{
+   struct osmocom_ms *tmp;
+   int rc;
+
+   if (ms->shutdown != MS_SHUTDOWN_COMPL)
+   return 0;
+
+   llist_for_each_entry(tmp, _list, entity) {
+   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
+   continue;
+   if (!strcmp(ms->settings.layer2_socket_path,
+   tmp->settings.layer2_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same layer2-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -1;
+   }
+   if (!strcmp(ms->settings.sap_socket_path,
+   tmp->settings.sap_socket_path)) {
+   LOGP(DMOB, LOGL_ERROR, "Cannot start MS '%s', because 
MS '%s' "
+   "use the same sap-socket.\nPlease shutdown "
+   "MS '%s' first.\n", ms->name, tmp->name, 
tmp->name);
+   *other_name = tmp->name;
+   return -2;
+   }
+   }
+
+   rc = mobile_init(ms);
+   if (rc < 0)
+   return -3;
+   return 0;
+}
+
+int mobile_stop(struct osmocom_ms *ms, int force)
+{
+   if (force && ms->shutdown <= MS_SHUTDOWN_IMSI_DETACH)
+   return mobile_exit(ms, 1);
+   if (!force && ms->shutdown == MS_SHUTDOWN_NONE)
+   return mobile_exit(ms, 0);
+   return 0;
+}
+
+
 /* create ms instance */
 struct osmocom_ms *mobile_new(char *name)
 {
diff --git a/src/host/layer23/src/mobile/vty_interface.c 
b/src/host/layer23/src/mobile/vty_interface.c
index eafed6e..81c20fe 100644
--- a/src/host/layer23/src/mobile/vty_interface.c
+++ b/src/host/layer23/src/mobile/vty_interface.c
@@ -2696,35 +2696,25 @@
 DEFUN(cfg_no_shutdown, cfg_ms_no_shutdown_cmd, "no shutdown",
NO_STR "Activate and run MS")
 {
-   struct osmocom_ms *ms = vty->index, *tmp;
+   struct osmocom_ms *ms = vty->index;
+   char *other_name = NULL;
int rc;
 
-   if (ms->shutdown != MS_SHUTDOWN_COMPL)
-   return CMD_SUCCESS;
-
-   llist_for_each_entry(tmp, _list, entity) {
-   if (tmp->shutdown == MS_SHUTDOWN_COMPL)
-   continue;
-   if (!strcmp(ms->settings.layer2_socket_path,
-   tmp->settings.layer2_socket_path)) {
-   vty_out(vty, "Cannot start MS '%s', because MS '%s' "
-   "use the same layer2-socket.%sPlease shutdown "
-   "MS '%s' first.%s", ms->name, tmp->name,
-   VTY_NEWLINE, tmp->name, VTY_NEWLINE);
-   return CMD_WARNING;
-   }
-   if (!strcmp(ms->settings.sap_socket_path,
-   tmp->settings.sap_socket_path)) {
-   vty_out(vty, "Cannot start MS