[lxc-devel] [PATCH] API shouldn't be calling create for already defined containers or destroy for non defined ones.
Serge Hallyn
serge.hallyn at ubuntu.com
Mon Apr 1 04:12:07 UTC 2013
Quoting S.Çağlar Onur (caglar at 10ur.org):
> Seems like I sent the wrong path, this one works for me.
note your emails are coming through as html and malformed txt, which
will be harder to apply. Would you mind posting a github url?
The lxc-destroy modifications make me a bit uneasy, but they should be
ok.
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 480c4f5..9ed3443 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -523,14 +523,15 @@ static bool lxcapi_create(struct lxc_container *c,
> char *t, char *const argv[])
> goto out;
> }
>
> - if (!create_container_dir(c))
> - goto out;
> -
> if (!c->save_config(c, NULL)) {
> ERROR("failed to save starting configuration for %s\n", c->name);
> goto out;
> }
>
> + /* container is already created if we have a config and rootfs.path is
> accessible */
> + if (c->lxc_conf && c->lxc_conf->rootfs.path &&
> access(c->lxc_conf->rootfs.path, F_OK) == 0)
> + return false;
> +
> /* we're going to fork. but since we'll wait for our child, we
> don't need to lxc_container_get */
>
> @@ -767,6 +768,9 @@ static bool lxcapi_save_config(struct lxc_container *c,
> const char *alt_file)
> return false;
> }
>
> + if (!create_container_dir(c))
> + return false;
> +
> FILE *fout = fopen(alt_file, "w");
> if (!fout)
> return false;
> @@ -788,6 +792,10 @@ static bool lxcapi_destroy(struct lxc_container *c)
> if (!c)
> return false;
>
> + /* container is already destroyed if we don't have a config or
> rootfs.path is not accessible */
> + if (!c->lxc_conf || !c->lxc_conf->rootfs.path ||
> access(c->lxc_conf->rootfs.path, F_OK) != 0)
> + return false;
> +
> pid = fork();
> if (pid < 0)
> return false;
>
>
> On Sun, Mar 31, 2013 at 7:56 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
>
> > What about something like following?
> >
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 480c4f5..eb99e5a 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -508,7 +508,7 @@ static bool lxcapi_create(struct lxc_container *c,
> > char *t, char *const argv[])
> > int len, nargs = 0;
> > char **newargv;
> >
> > - if (!c)
> > + if (!c || access(c->lxc_conf->rootfs.path, F_OK) == 0)
> > return false;
> >
> > len = strlen(LXCTEMPLATEDIR) + strlen(t) + strlen("/lxc-") + 1;
> > @@ -523,9 +523,6 @@ static bool lxcapi_create(struct lxc_container *c,
> > char *t, char *const argv[])
> > goto out;
> > }
> >
> > - if (!create_container_dir(c))
> > - goto out;
> > -
> > if (!c->save_config(c, NULL)) {
> > ERROR("failed to save starting configuration for %s\n", c->name);
> > goto out;
> > @@ -767,6 +764,9 @@ static bool lxcapi_save_config(struct lxc_container
> > *c, const char *alt_file)
> > return false;
> > }
> >
> > + if (!create_container_dir(c))
> > + return false;
> > +
> > FILE *fout = fopen(alt_file, "w");
> > if (!fout)
> > return false;
> > @@ -785,7 +785,7 @@ static bool lxcapi_destroy(struct lxc_container *c)
> > pid_t pid;
> > int ret, status;
> >
> > - if (!c)
> > + if (!c || access(c->lxc_conf->rootfs.path, F_OK) != 0)
> > return false;
> >
> > pid = fork();
> >
> >
> > and this is how it behaves with it;
> >
> > caglar at qgq:~$ cat test.py
> > import lxc
> > c = lxc.Container("abcdef")
> > print (c.set_config_item("lxc.utsname", "abcdef"))
> > print (c.save_config())
> > print (c.create("ubuntu"))
> > print (c.create("ubuntu"))
> > print (c.destroy())
> > print (c.destroy())
> >
> > caglar at qgq:~$ sudo python3 test.py
> > True
> > True
> > True
> > False
> > True
> > False
> >
> >
> > On Sun, Mar 31, 2013 at 7:26 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> >
> >> Hi Stéphane,
> >>
> >> Hmm, then I believe there is another bug somewhere cause here is what
> >> happens with your ordering;
> >>
> >> caglar at qgq:~/Project/lxc$ sudo python3
> >> Python 3.2.3 (default, Oct 19 2012, 19:53:16)
> >> [GCC 4.7.2] on linux2
> >> Type "help", "copyright", "credits" or "license" for more information.
> >> >>> import lxc
> >> __main__:1: Warning: The python-lxc API isn't yet stable and may change
> >> at any point in the future.
> >> >>> c = lxc.Container("abcdef")
> >> >>> c.set_config_item("lxc.utsname", "blah")
> >> True
> >> >>> c.set_config_item("lxc.utsname", "abcdef")
> >> True
> >> >>> c.save_config()
> >> False
> >> >>> c.config_file_name
> >> '/var/lib/lxc/abcdef/config'
> >>
> >> so it looks like save_config don't work if the container directory is not
> >> there and as long as I see only create calls create_container_dir to do
> >> that.
> >>
> >> Maybe correct way to handle that is to call create_container_dir from
> >> save_config as well but checking the rootfs directory's existence from
> >> create/destrory?
> >>
> >> Best,
> >>
> >>
> >> On Sun, Mar 31, 2013 at 5:22 PM, Stéphane Graber <stgraber at ubuntu.com>
> >> wrote:
> >> >
> >> > On 03/31/2013 04:22 PM, S.Çağlar Onur wrote:
> >> > > From: "S.Çağlar Onur" <caglar at 10ur.org>
> >> > >
> >> > > Currently it behaves like following which might be confusing for the
> >> code that checks the return value of those calls to determine whether
> >> operation completed successfully or not.
> >> > >
> >> > >>> c = lxc.Container("r")
> >> > >>>> c.create("ubuntu")
> >> > > True
> >> > >>>> c.create("ubuntu")
> >> > > True
> >> > >>>> c.create("ubuntu")
> >> > > True
> >> > >>>> c.create("ubuntu")
> >> > > True
> >> > >>>> c.create("ubuntu")
> >> > >>>> c.destroy()
> >> > > True
> >> > >>>> c.destroy()
> >> > > lxc-destroy: 'r' does not exist
> >> > > False
> >> > >>>> c.destroy()
> >> > > lxc-destroy: 'r' does not exist
> >> > > False
> >> > >
> >> > > New behaviour
> >> > >
> >> > >>>> c = lxc.Container("r")
> >> > >>>> c.create('ubuntu')
> >> > > True
> >> > >>>> c.create('ubuntu')
> >> > > False
> >> > >>>> c.destroy()
> >> > > True
> >> > >>>> c.destroy()
> >> > > False
> >> > >>>>
> >> >
> >> >
> >> > Won't this break the following?
> >> > c = lxc.Container("abcdef")
> >> > c.set_config_item("lxc.utsname", "blah")
> >> > c.save_config()
> >> > c.create("ubuntu")
> >> >
> >> > I personally always considered ".create()" to mean "generate a new
> >> > rootfs" which doesn't at all mean "generate a new config file".
> >> >
> >> > ".destroy()" on the other hand destroys everything, including the
> >> config.
> >> >
> >> >
> >> >
> >> > > Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
> >> > > ---
> >> > > src/lxc/lxccontainer.c | 4 ++--
> >> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> >> > > index 480c4f5..7a11c85 100644
> >> > > --- a/src/lxc/lxccontainer.c
> >> > > +++ b/src/lxc/lxccontainer.c
> >> > > @@ -508,7 +508,7 @@ static bool lxcapi_create(struct lxc_container
> >> *c, char *t, char *const argv[])
> >> > > int len, nargs = 0;
> >> > > char **newargv;
> >> > >
> >> > > - if (!c)
> >> > > + if (!c || lxcapi_is_defined(c))
> >> > > return false;
> >> > >
> >> > > len = strlen(LXCTEMPLATEDIR) + strlen(t) + strlen("/lxc-") + 1;
> >> > > @@ -785,7 +785,7 @@ static bool lxcapi_destroy(struct lxc_container
> >> *c)
> >> > > pid_t pid;
> >> > > int ret, status;
> >> > >
> >> > > - if (!c)
> >> > > + if (!c || !lxcapi_is_defined(c))
> >> > > return false;
> >> > >
> >> > > pid = fork();
> >> > >
> >> >
> >> >
> >> > --
> >> > Stéphane Graber
> >> > Ubuntu developer
> >> > http://www.ubuntu.com
> >> >
> >> >
> >> >
> >> ------------------------------------------------------------------------------
> >> > Own the Future-Intel(R) Level Up Game Demo Contest 2013
> >> > Rise to greatness in Intel's independent game demo contest. Compete
> >> > for recognition, cash, and the chance to get your game on Steam.
> >> > $5K grand prize plus 10 genre and skill prizes. Submit your demo
> >> > by 6/6/13. http://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2
> >> > _______________________________________________
> >> > Lxc-devel mailing list
> >> > Lxc-devel at lists.sourceforge.net
> >> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
> >> >
> >>
> >>
> >>
> >> --
> >> S.Çağlar Onur <caglar at 10ur.org>
> >>
> >
> >
> >
> > --
> > S.Çağlar Onur <caglar at 10ur.org>
> >
>
>
>
> --
> S.Çağlar Onur <caglar at 10ur.org>
> ------------------------------------------------------------------------------
> Own the Future-Intel® Level Up Game Demo Contest 2013
> Rise to greatness in Intel's independent game demo contest.
> Compete for recognition, cash, and the chance to get your game
> on Steam. $5K grand prize plus 10 genre and skill prizes.
> Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel
More information about the lxc-devel
mailing list