Mon, Sep 14, 2015 at 05:59:19PM CEST, pwieczorkiewicz(a)suse.de wrote:
On Mon, 14 Sep 2015 15:07:32 +0200
Jiri Pirko <jiri(a)resnulli.us> wrote:
> >--- a/teamd/teamd.c
> >+++ b/teamd/teamd.c
> >@@ -1621,10 +1621,6 @@ static int teamd_set_default_pid_file(struct
> >teamd_context *ctx)
> > if (ctx->pid_file)
> > return 0;
> >
> >- err = teamd_make_rundir();
> >- if (err)
> >- return err;
> >-
> > err = asprintf(&ctx->pid_file, TEAMD_RUN_DIR"%s.pid",
> > ctx->team_devname); if (err == -1) {
> > teamd_log_err("Failed allocate memory for PID file
> > string.");
> >@@ -1676,6 +1672,10 @@ int main(int argc, char **argv)
> > int err;
> > struct teamd_context *ctx;
> >
> >+ err = teamd_make_rundir();
> >+ if (err)
> >+ return ret;
> >+
>
>
> What's up with the teamd_make_rundir call move? Any comments to that
> at least? I think is should be rather in a separate patch.
The move was needed in order to assign proper owner to that dir before
dropping privileges. Since /var/run or /run is root owned dir such
chown operation is not possible afterward.
Okay. Then the squash with this patch does not seem correct to me.
There were basically two calls to teamd_make_rundir() following each
other quite shortly. As far as I checked that calls could not happen
independently, thus I concluded that it would be more consistent if
there was a single call at the process start beginning since we might
need to adjust dir properties anyway.
True. Could you please reduce that into 1 call? That would be great.
Thanks!