From 0b027b131c97ec901c3b3dc9211ee434d21291dc Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Fri, 3 Dec 2021 15:57:33 +0100 Subject: [PATCH 1/3] ModemManager: refactoring procd init script The way the init script is written now, we get a bad output when calling the ubus service backend. ubus call service list "{'verbose':true,'name':'modemmanager'}" >{ > "modemmanager": { > "instances": { > "instance1": { > "running": true, > "pid": 20511, > "command": [ > "sh", > "-c", > ". >/usr/share/ModemManager/modemmanager.common; \t >mkdir -m 0755 -p /var/run/modemmanager; \t >mm_cleanup_interfaces; \t >( mm_report_events_from_cache ) >/dev/null 2>&1 & \t >/usr/sbin/ModemManager" > ], > "term_timeout": 5, > "respawn": { > "threshold": 3600, > "timeout": 5, > "retry": 5 > }, > "pidfile":"/var/run/modemmanager/modemmanager.pid" > } > } > } >}" I also get the output in the log that the PID file cannot be created. > daemon.err procd: Failed to remove pidfile: :No such file or directory The changes in this commit fixes this issues, by moving startup into a wrapper script. Signed-off-by: Florian Eckert --- net/modemmanager/Makefile | 1 + net/modemmanager/files/modemmanager.init | 10 +++--- .../files/usr/sbin/ModemManager-wrapper | 33 +++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 net/modemmanager/files/usr/sbin/ModemManager-wrapper diff --git a/net/modemmanager/Makefile b/net/modemmanager/Makefile index ce87b0a36..db59989b2 100644 --- a/net/modemmanager/Makefile +++ b/net/modemmanager/Makefile @@ -98,6 +98,7 @@ define Package/modemmanager/install $(INSTALL_DIR) $(1)/usr/sbin $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/sbin/ModemManager $(1)/usr/sbin + $(INSTALL_BIN) ./files/usr/sbin/ModemManager-wrapper $(1)/usr/sbin $(INSTALL_DIR) $(1)/usr/bin $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/bin/mmcli $(1)/usr/bin diff --git a/net/modemmanager/files/modemmanager.init b/net/modemmanager/files/modemmanager.init index b3f9f9290..a3f6c1b12 100755 --- a/net/modemmanager/files/modemmanager.init +++ b/net/modemmanager/files/modemmanager.init @@ -22,14 +22,12 @@ start_service() { # it starts. # # All these commands need to be executed on every MM start, even after - # procd-triggered respawns, which is why they're all included as instance command + # procd-triggered respawns, which is why this is wrapped in a startup + # wrapper script called '/usr/sbin/ModemManager-wrapper'. # + . /usr/share/ModemManager/modemmanager.common procd_open_instance - procd_set_param command sh -c ". /usr/share/ModemManager/modemmanager.common; \ - mkdir -m 0755 -p ${MODEMMANAGER_RUNDIR}; \ - mm_cleanup_interfaces; \ - ( mm_report_events_from_cache ) >/dev/null 2>&1 & \ - /usr/sbin/ModemManager" + procd_set_param command /usr/sbin/ModemManager-wrapper procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" procd_set_param pidfile "${MODEMMANAGER_PID_FILE}" procd_close_instance diff --git a/net/modemmanager/files/usr/sbin/ModemManager-wrapper b/net/modemmanager/files/usr/sbin/ModemManager-wrapper new file mode 100644 index 000000000..f5fb6d1f2 --- /dev/null +++ b/net/modemmanager/files/usr/sbin/ModemManager-wrapper @@ -0,0 +1,33 @@ +#!/bin/sh + +trap_with_arg() { + func="$1" ; shift + for sig ; do + # shellcheck disable=SC2064 + trap "$func $sig" "$sig" + done +} + +func_trap() { + logger "ModemManager-wrapper[$$]" "Sending signal ${1}..." + kill "-${1}" "$CHILD" 2>/dev/null +} + +main() { + . /usr/share/ModemManager/modemmanager.common + + trap_with_arg func_trap INT TERM KILL + + mkdir -p "${MODEMMANAGER_RUNDIR}" + chmod 0755 "${MODEMMANAGER_RUNDIR}" + mm_cleanup_interfaces + + /usr/sbin/ModemManager "$@" 1>/dev/null 2>/dev/null & + CHILD="$!" + sleep 2 + mm_report_events_from_cache + + wait "$CHILD" +} + +main "$@" From dc7095baff0172760090a4c9f88bca11f6f38838 Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Mon, 6 Dec 2021 10:33:05 +0100 Subject: [PATCH 2/3] ModemManager: add service options Signed-off-by: Florian Eckert --- net/modemmanager/files/modemmanager.init | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/modemmanager/files/modemmanager.init b/net/modemmanager/files/modemmanager.init index a3f6c1b12..7f014dc56 100755 --- a/net/modemmanager/files/modemmanager.init +++ b/net/modemmanager/files/modemmanager.init @@ -4,6 +4,8 @@ USE_PROCD=1 START=70 +LOG_LEVEL="INFO" + stop_service() { # Load common utils . /usr/share/ModemManager/modemmanager.common @@ -28,6 +30,8 @@ start_service() { . /usr/share/ModemManager/modemmanager.common procd_open_instance procd_set_param command /usr/sbin/ModemManager-wrapper + procd_append_param command --log-level="$LOG_LEVEL" + [ "$LOG_LEVEL" = "DEBUG" ] && procd_append_param command --debug procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" procd_set_param pidfile "${MODEMMANAGER_PID_FILE}" procd_close_instance From 45a56a889943b437f78fa2bfca3d5d8ac555c77e Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Mon, 13 Dec 2021 13:06:47 +0100 Subject: [PATCH 3/3] modemmanager: refactoring hotplug debug logging The output of the hotplug is very chatty and floods the log with messages that are not necessary in functioning operation. So that the log can be filtered. A log level was added to each message as the first opiton on mm_log function call. In addition, the facility of the hotplug script has been set to daemon, which in my view fits better than user. Signed-off-by: Florian Eckert --- net/modemmanager/files/modemmanager.common | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/net/modemmanager/files/modemmanager.common b/net/modemmanager/files/modemmanager.common index b4139abad..6367eb32b 100644 --- a/net/modemmanager/files/modemmanager.common +++ b/net/modemmanager/files/modemmanager.common @@ -20,7 +20,8 @@ MODEMMANAGER_EVENTS_CACHE="${MODEMMANAGER_RUNDIR}/events.cache" # Common logging mm_log() { - logger -t "ModemManager" "hotplug: $*" + local level="$1"; shift + logger -p "daemon.${level}" -t "ModemManager[$$]" "hotplug: $*" } ################################################################################ @@ -169,7 +170,7 @@ mm_wait_for_modem() { while [ $n -ge 0 ]; do [ -d "${sysfspath}" ] || { - mm_log "error: ignoring modem detection request: no device at ${sysfspath}" + mm_log "error" "ignoring modem detection request: no device at ${sysfspath}" proto_set_available "${cfg}" 0 return 1 } @@ -177,10 +178,10 @@ mm_wait_for_modem() { # Check if the modem exists at the given sysfs path if ! mmcli -m "${sysfspath}" > /dev/null 2>&1 then - mm_log "error: modem not detected at sysfs path" + mm_log "error" "modem not detected at sysfs path" else - mm_log "modem exported successfully at ${sysfspath}" - mm_log "setting interface '${cfg}' as available" + mm_log "info" "modem exported successfully at ${sysfspath}" + mm_log "info" "setting interface '${cfg}' as available" proto_set_available "${cfg}" 1 return 0 fi @@ -189,7 +190,7 @@ mm_wait_for_modem() { n=$((n-step)) done - mm_log "error: timed out waiting for the modem to get exported at ${sysfspath}" + mm_log "error" "timed out waiting for the modem to get exported at ${sysfspath}" proto_set_available "${cfg}" 0 return 2 } @@ -201,7 +202,7 @@ mm_report_modem_wait() { parent_sysfspath=$(mm_find_physdev_sysfs_path "$sysfspath") [ -n "${parent_sysfspath}" ] || { - mm_log "error: parent device sysfspath not found" + mm_log "error" "parent device sysfspath not found" return } @@ -212,23 +213,23 @@ mm_report_modem_wait() { cfg=$(mm_get_modem_config "${parent_sysfspath}") if [ -n "${cfg}" ]; then - mm_log "interface '${cfg}' is set to configure device '${parent_sysfspath}'" - mm_log "now waiting for modem at sysfs path ${parent_sysfspath}" + mm_log "info" "interface '${cfg}' is set to configure device '${parent_sysfspath}'" + mm_log "info" "now waiting for modem at sysfs path ${parent_sysfspath}" mm_set_modem_wait_status "${parent_sysfspath}" "processed" # Launch subshell for the explicit wait ( mm_wait_for_modem "${cfg}" "${parent_sysfspath}" ) > /dev/null 2>&1 & else - mm_log "no need to wait for modem at sysfs path ${parent_sysfspath}" + mm_log "info" "no need to wait for modem at sysfs path ${parent_sysfspath}" mm_set_modem_wait_status "${parent_sysfspath}" "ignored" fi ;; "processed") - mm_log "already waiting for modem at sysfs path ${parent_sysfspath}" + mm_log "info" "already waiting for modem at sysfs path ${parent_sysfspath}" ;; "ignored") ;; *) - mm_log "error: unknown status read for device at sysfs path ${parent_sysfspath}" + mm_log "error" "unknown status read for device at sysfs path ${parent_sysfspath}" ;; esac } @@ -258,7 +259,7 @@ mm_cleanup_interface_by_sysfspath() { cfg=$(mm_get_modem_config "$dev") [ -n "${cfg}" ] || return - mm_log "setting interface '$cfg' as unavailable" + mm_log "info" "setting interface '$cfg' as unavailable" proto_set_available "${cfg}" 0 } @@ -286,7 +287,7 @@ mm_report_event() { esac # Report the event - mm_log "event reported: action=${action}, name=${name}, subsystem=${subsystem}" + mm_log "debug" "event reported: action=${action}, name=${name}, subsystem=${subsystem}" mmcli --report-kernel-event="action=${action},name=${name},subsystem=${subsystem}" 1>/dev/null 2>&1 & # Wait for added modem if a sysfspath is given @@ -302,7 +303,7 @@ mm_report_event_from_cache_line() { subsystem=$(echo "${event_line}" | awk -F ',' '{ print $3 }') sysfspath=$(echo "${event_line}" | awk -F ',' '{ print $4 }') - mm_log "cached event found: action=${action}, name=${name}, subsystem=${subsystem}, sysfspath=${sysfspath}" + mm_log "debug" "cached event found: action=${action}, name=${name}, subsystem=${subsystem}, sysfspath=${sysfspath}" mm_report_event "${action}" "${name}" "${subsystem}" "${sysfspath}" } @@ -317,11 +318,11 @@ mm_report_events_from_cache() { # Wait for ModemManager to be available in the bus while [ $n -ge 0 ]; do sleep $step - mm_log "checking if ModemManager is available..." + mm_log "info" "checking if ModemManager is available..." if ! mmcli -L >/dev/null 2>&1 then - mm_log "ModemManager not yet available" + mm_log "info" "ModemManager not yet available" else mmrunning=1 break @@ -330,7 +331,7 @@ mm_report_events_from_cache() { done [ ${mmrunning} -eq 1 ] || { - mm_log "error: couldn't report initial kernel events: ModemManager not running" + mm_log "error" "couldn't report initial kernel events: ModemManager not running" return }