[lxc-devel] [PATCH] API shouldn't be calling create for already defined containers or destroy for non defined ones.
S.Çağlar Onur
caglar at 10ur.org
Mon Apr 1 04:24:39 UTC 2013
OK here it is
https://github.com/caglar10ur/lxc-upstream
sorry about the malformed text...
On Mon, Apr 1, 2013 at 12:13 AM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> Yeah I realized that, I'll send the github url in a moment
>
>
> On Mon, Apr 1, 2013 at 12:12 AM, Serge Hallyn <serge.hallyn at ubuntu.com>wrote:
>
>> 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
>>
>>
>
>
> --
> S.Çağlar Onur <caglar at 10ur.org>
>
--
S.Çağlar Onur <caglar at 10ur.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130401/b77f22a6/attachment.html>
More information about the lxc-devel
mailing list