Menu

#132 Make minidlna to be container friendly.

open
nobody
None
5
2017-10-23
2017-10-04
huszty
No

Hi!
I am running minidlna in docker container and I have an improvement idea for the project. My problem is that containers are more often got restarted, even forcefully comparing to native applications. In cases when the container filesystem is not recreated (especially the rw layer on the top) before the application process is restarted, minidlna stuck and keeps exiting with the error:
minidlna.c:943: error: MiniDLNA is already running. EXITING.
This is because the run lock file is left there from the previous run.
I think minidlna could deal with this better if it would not use lock file conditionally. I have a few proposals what could help here:
Processes in containers are supposed to write logs and error to stdout in order to let them consumed by the container runtime, such as docker. In case of minidlna this is achieved by running with -d flag. So the idea here is to not use the lock file if running with this flag. Anyway this is I think not needed for debugging cases, which is the original use-case for -d.
Introduce a new command line flag for avoiding lock file usage. Moreover this flag could trigger further changes that is optimal if running in container so e.g. extensive logging is also not needed (which is the case for -d).
Introduce an environment variable for the same. Services in containers are usually configured by environment variables.
Make the lock usage automatic. This can be achieved by testing if minidlna is running with pid 1. This is the case when it is running in a container without any process manager. In this case, minidlna can be absolutely sure that there are no other instances are running in it's namespace.

Hope this request makes sense.

Discussion

  • Shrimpkin

    Shrimpkin - 2017-10-06

    First off, I don't use containers much.

    If you are not using root to run minidlna, would this work?

    -P /dev/null
    

    IMO, the -d option should only control daemon mode (using it would disable daemon). The -v option would enable full debug logging. If -d is used without -v, then the log level from the config file would be used. A patch like this:

    diff --git a/minidlna.c b/minidlna.c
    index 48e56ff..aa85a92 100644
    --- a/minidlna.c
    +++ b/minidlna.c
    @@ -820,6 +820,7 @@ init(int argc, char **argv)
                break;
            case 'd':
                debug_flag = 1;
    +           break;
            case 'v':
                verbose_flag = 1;
                break;
    @@ -941,7 +942,7 @@ init(int argc, char **argv)
    
        if (verbose_flag)
        {
    -       strcpy(log_str+65, "debug");
    +       strcpy(log_str+65, "maxdebug");
            log_level = log_str;
        }
        else if (!log_level)
    @@ -949,13 +950,7 @@ init(int argc, char **argv)
    
        /* Set the default log file path to NULL (stdout) */
        path = NULL;
    -   if (debug_flag)
    -   {
    -       pid = getpid();
    -       strcpy(log_str+65, "maxdebug");
    -       log_level = log_str;
    -   }
    -   else if (GETFLAG(SYSTEMD_MASK))
    +   if (debug_flag || GETFLAG(SYSTEMD_MASK))
        {
            pid = getpid();
        }
    
     
    • huszty

      huszty - 2017-10-21

      Hi!
      Thanks. Seems the pidfile trick works with -P /dev/null.
      I like the patch above. Maybe I would rename the debug_flag variable to no_daemon or something.

       
  • Shrimpkin

    Shrimpkin - 2017-10-23

    I try to limit code changes to make patches easier to apply downstream.

     

Log in to post a comment.