[lxc-devel] [PATCH] lxc-checkpoint -r should actually wait for the restore to happen
Tycho Andersen
tycho.andersen at canonical.com
Tue Mar 24 20:08:28 UTC 2015
On Tue, Mar 24, 2015 at 08:05:17PM +0000, Serge Hallyn wrote:
> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > On Tue, Mar 24, 2015 at 07:40:48PM +0000, Serge Hallyn wrote:
> > > Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > > > + if (pid != 0)
> > > > + wait_for_pid(pid);
> > >
> > > Sorry - I suspect some package builds will fail on the ignore of
> > > return value here. Could this be
> > >
> > > if (pid != 0)
> > > return wait_for_pid(pid) == 0;
> >
> > Sure,
> >
> >
> > From ca37de3207433bc0d2c55b466c99d931334389b3 Mon Sep 17 00:00:00 2001
> > From: Tycho Andersen <tycho.andersen at canonical.com>
> > Date: Fri, 20 Mar 2015 10:17:31 -0600
> > Subject: [PATCH] lxc-checkpoint -r should actually wait for the restore to
> > happen
> >
> > v2: use wait_for_pid
> > v2: check wait_for_pid's return value
> >
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> > src/lxc/lxc_checkpoint.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/src/lxc/lxc_checkpoint.c b/src/lxc/lxc_checkpoint.c
> > index cfa08fc..86f1f4b 100644
> > --- a/src/lxc/lxc_checkpoint.c
> > +++ b/src/lxc/lxc_checkpoint.c
> > @@ -27,6 +27,7 @@
> > #include "config.h"
> > #include "lxc.h"
> > #include "arguments.h"
> > +#include "utils.h"
> >
> > static char *checkpoint_dir = NULL;
> > static bool stop = false;
> > @@ -168,6 +169,9 @@ bool restore(struct lxc_container *c)
> >
> > lxc_container_put(c);
>
> I'll say though that in general I prefer for forked paths
> to exit. what you have will work bc nothing else is done
> after pid 0 returns here, but it's far more reassuring if
> the "if (pid == 0)" path right above this in restore()
> does a exit() at the end of the if statement. Then you
> can drop the if (pid != 0) check below.
>
> This sounds nit-picky, but imo makes the code much easier
> to validate.
>
> Would you mind sending either a new or separate patch to
> do that (assuming i made sense and you agree)
Sure, I can send a new patch.
Tycho
> > + if (pid != 0)
> > + return wait_for_pid(pid) == 0;
> > +
> > return ret;
> > }
> >
> > --
> > 2.1.0
> >
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list