[lxc-devel] [PATCH] lxc api: fix some config_path oddities
Stéphane Graber
stgraber at ubuntu.com
Mon Feb 11 04:22:37 UTC 2013
On 02/08/2013 05:06 PM, Serge Hallyn wrote:
> 1. When calling c->set_config_path(), update configfile. I.e. if we
> are setting the config_path to /var/lib/lxc, then the configfile should
> be changed to /var/lib/lxc/$container/config
>
> 2. Add an optional configpath argument to lxc_container_new. If NULL,
> then the default will be used (as before). If set, then the passed-in
> path will be used. This way you can do
>
> c1 = lxc.Container("r1", "/var/lib/lxc");
> c2 = lxc.Container("r2", "/home/user/lxcbase");
>
> (Note I did *not* implement the python or lua binding to pass that
> argument along)
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
I'll update the python binding tomorrow to export the new option of the
constructor.
> ---
> src/lua-lxc/core.c | 2 +-
> src/lxc/lxc.h | 2 +-
> src/lxc/lxccontainer.c | 72 ++++++++++++++++++++++++++++++++-------
> src/lxc/lxccontainer.h | 2 +-
> src/python-lxc/lxc.c | 2 +-
> src/tests/Makefile.am | 3 +-
> src/tests/containertests.c | 4 +--
> src/tests/createtest.c | 2 +-
> src/tests/destroytest.c | 2 +-
> src/tests/get_item.c | 8 ++---
> src/tests/getkeys.c | 2 +-
> src/tests/lxcpath.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++
> src/tests/saveconfig.c | 2 +-
> src/tests/shutdowntest.c | 2 +-
> src/tests/startone.c | 2 +-
> 15 files changed, 162 insertions(+), 30 deletions(-)
> create mode 100644 src/tests/lxcpath.c
>
> diff --git a/src/lua-lxc/core.c b/src/lua-lxc/core.c
> index ae4d9b2..9225158 100644
> --- a/src/lua-lxc/core.c
> +++ b/src/lua-lxc/core.c
> @@ -44,7 +44,7 @@
> static int container_new(lua_State *L)
> {
> const char *name = luaL_checkstring(L, 1);
> - struct lxc_container *c = lxc_container_new(name);
> + struct lxc_container *c = lxc_container_new(name, NULL);
>
> if (c) {
> lua_boxpointer(L, c);
> diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
> index 349bbbc..a651d04 100644
> --- a/src/lxc/lxc.h
> +++ b/src/lxc/lxc.h
> @@ -182,7 +182,7 @@ extern const char const *lxc_version(void);
> /*
> * Create and return a new lxccontainer struct.
> */
> -extern struct lxc_container *lxc_container_new(const char *name);
> +extern struct lxc_container *lxc_container_new(const char *name, const char *configpath);
>
> /*
> * Returns 1 on success, 0 on failure.
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 6f01645..733cbb6 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -871,10 +871,45 @@ static const char *lxcapi_get_config_path(struct lxc_container *c)
> return (const char *)(c->config_path);
> }
>
> +/*
> + * not for export
> + * Just recalculate the c->configfile based on the
> + * c->config_path, which must be set.
> + * The lxc_container must be locked or not yet public.
> + */
> +static bool set_config_filename(struct lxc_container *c)
> +{
> + char *newpath;
> + int len, ret;
> +
> + if (!c->config_path)
> + return false;
> +
> + /* $lxc_path + "/" + c->name + "/" + "config" + '\0' */
> + len = strlen(c->config_path) + strlen(c->name) + strlen("config") + 3;
> + newpath = malloc(len);
> + if (!newpath)
> + return false;
> +
> + ret = snprintf(newpath, len, "%s/%s/config", c->config_path, c->name);
> + if (ret < 0 || ret >= len) {
> + fprintf(stderr, "Error printing out config file name\n");
> + free(newpath);
> + return false;
> + }
> +
> + if (c->configfile)
> + free(c->configfile);
> + c->configfile = newpath;
> +
> + return true;
> +}
> +
> static bool lxcapi_set_config_path(struct lxc_container *c, const char *path)
> {
> char *p;
> bool b = false;
> + char *oldpath = NULL;
>
> if (!c)
> return b;
> @@ -883,13 +918,28 @@ static bool lxcapi_set_config_path(struct lxc_container *c, const char *path)
> return b;
>
> p = strdup(path);
> - if (!p)
> + if (!p) {
> + ERROR("Out of memory setting new lxc path");
> goto err;
> + }
> +
> b = true;
> if (c->config_path)
> - free(c->config_path);
> + oldpath = c->config_path;
> c->config_path = p;
> +
> + /* Since we've changed the config path, we have to change the
> + * config file name too */
> + if (!set_config_filename(c)) {
> + ERROR("Out of memory setting new config filename");
> + b = false;
> + free(c->config_path);
> + c->config_path = oldpath;
> + oldpath = NULL;
> + }
> err:
> + if (oldpath)
> + free(oldpath);
> lxcunlock(c->privlock);
> return b;
> }
> @@ -938,10 +988,9 @@ out:
> }
>
>
> -struct lxc_container *lxc_container_new(const char *name)
> +struct lxc_container *lxc_container_new(const char *name, const char *configpath)
> {
> struct lxc_container *c;
> - int ret, len;
>
> c = malloc(sizeof(*c));
> if (!c) {
> @@ -950,7 +999,11 @@ struct lxc_container *lxc_container_new(const char *name)
> }
> memset(c, 0, sizeof(*c));
>
> - c->config_path = default_lxc_path();
> + if (configpath)
> + c->config_path = strdup(configpath);
> + else
> + c->config_path = default_lxc_path();
> +
> if (!c->config_path) {
> fprintf(stderr, "Out of memory");
> goto err;
> @@ -976,17 +1029,10 @@ struct lxc_container *lxc_container_new(const char *name)
> goto err;
> }
>
> - len = strlen(c->config_path)+strlen(c->name)+strlen("/config")+2;
> - c->configfile = malloc(len);
> - if (!c->configfile) {
> + if (!set_config_filename(c)) {
> fprintf(stderr, "Error allocating config file pathname\n");
> goto err;
> }
> - ret = snprintf(c->configfile, len, "%s/%s/config", c->config_path, c->name);
> - if (ret < 0 || ret >= len) {
> - fprintf(stderr, "Error printing out config file name\n");
> - goto err;
> - }
>
> if (file_exists(c->configfile))
> lxcapi_load_config(c, NULL);
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 32c501e..de802a8 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -82,7 +82,7 @@ struct lxc_container {
> #endif
> };
>
> -struct lxc_container *lxc_container_new(const char *name);
> +struct lxc_container *lxc_container_new(const char *name, const char *configpath);
> int lxc_container_get(struct lxc_container *c);
> int lxc_container_put(struct lxc_container *c);
> int lxc_get_wait_states(const char **states);
> diff --git a/src/python-lxc/lxc.c b/src/python-lxc/lxc.c
> index 83e2659..16356f9 100644
> --- a/src/python-lxc/lxc.c
> +++ b/src/python-lxc/lxc.c
> @@ -85,7 +85,7 @@ Container_init(Container *self, PyObject *args, PyObject *kwds)
> &name))
> return -1;
>
> - self->container = lxc_container_new(name);
> + self->container = lxc_container_new(name, NULL);
> if (!self->container) {
> fprintf(stderr, "%d: error creating lxc_container %s\n", __LINE__, name);
> return -1;
> diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
> index 5850727..a883d36 100644
> --- a/src/tests/Makefile.am
> +++ b/src/tests/Makefile.am
> @@ -15,6 +15,7 @@ lxc_test_createtest_SOURCES = createtest.c
> lxc_test_shutdowntest_SOURCES = shutdowntest.c
> lxc_test_get_item_SOURCES = get_item.c
> lxc_test_getkeys_SOURCES = getkeys.c
> +lxc_test_lxcpath_SOURCES = lxcpath.c
>
> AM_CFLAGS=-I$(top_srcdir)/src \
> -DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
> @@ -24,6 +25,6 @@ AM_CFLAGS=-I$(top_srcdir)/src \
>
> bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
> lxc-test-destroytest lxc-test-saveconfig lxc-test-createtest \
> - lxc-test-shutdowntest lxc-test-get_item lxc-test-getkeys
> + lxc-test-shutdowntest lxc-test-get_item lxc-test-getkeys lxc-test-lxcpath
>
> endif
> diff --git a/src/tests/containertests.c b/src/tests/containertests.c
> index d68f17c..8868faa 100644
> --- a/src/tests/containertests.c
> +++ b/src/tests/containertests.c
> @@ -103,7 +103,7 @@ int main(int argc, char *argv[])
>
> ret = 1;
> /* test refcounting */
> - c = lxc_container_new(MYNAME);
> + c = lxc_container_new(MYNAME, NULL);
> if (!c) {
> fprintf(stderr, "%d: error creating lxc_container %s\n", __LINE__, MYNAME);
> goto out;
> @@ -149,7 +149,7 @@ int main(int argc, char *argv[])
> }
>
> /* test a real container */
> - c = lxc_container_new(MYNAME);
> + c = lxc_container_new(MYNAME, NULL);
> if (!c) {
> fprintf(stderr, "%d: error creating lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> diff --git a/src/tests/createtest.c b/src/tests/createtest.c
> index 48ce922..c2abee2 100644
> --- a/src/tests/createtest.c
> +++ b/src/tests/createtest.c
> @@ -33,7 +33,7 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
> int ret = 1;
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/destroytest.c b/src/tests/destroytest.c
> index 28f0577..0552b4c 100644
> --- a/src/tests/destroytest.c
> +++ b/src/tests/destroytest.c
> @@ -65,7 +65,7 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
> int ret = 1;
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/get_item.c b/src/tests/get_item.c
> index 0d32eca..d3e6d29 100644
> --- a/src/tests/get_item.c
> +++ b/src/tests/get_item.c
> @@ -35,7 +35,7 @@ int main(int argc, char *argv[])
> int ret;
> char v1[2], v2[256], v3[2048];
>
> - if ((c = lxc_container_new("testxyz")) == NULL) {
> + if ((c = lxc_container_new("testxyz", NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> @@ -157,7 +157,7 @@ int main(int argc, char *argv[])
> lxc_container_put(c);
>
> // new test with real container
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> @@ -165,7 +165,7 @@ int main(int argc, char *argv[])
> c->destroy(c);
> lxc_container_put(c);
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> @@ -179,7 +179,7 @@ int main(int argc, char *argv[])
> lxc_container_put(c);
>
> /* XXX TODO load_config needs to clear out any old config first */
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/getkeys.c b/src/tests/getkeys.c
> index 9bb5593..bab6883 100644
> --- a/src/tests/getkeys.c
> +++ b/src/tests/getkeys.c
> @@ -35,7 +35,7 @@ int main(int argc, char *argv[])
> int len, ret;
> char v3[2048];
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/lxcpath.c b/src/tests/lxcpath.c
> new file mode 100644
> index 0000000..8d8ad9c
> --- /dev/null
> +++ b/src/tests/lxcpath.c
> @@ -0,0 +1,85 @@
> +/* liblxcapi
> + *
> + * Copyright © 2012 Serge Hallyn <serge.hallyn at ubuntu.com>.
> + * Copyright © 2012 Canonical Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +#include "../lxc/lxccontainer.h"
> +
> +#include <unistd.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +
> +#define MYNAME "lxctest1"
> +
> +#define TSTERR(x) do { \
> + fprintf(stderr, "%d: %s", __LINE__, x); \
> +} while (0)
> +
> +int main()
> +{
> + struct lxc_container *c;
> + const char *p1, *p2;
> + int retval = -1;
> +
> + c = lxc_container_new(MYNAME, NULL);
> + if (!c) {
> + TSTERR("create using default path");
> + goto err;
> + }
> + p1 = c->get_config_path(c);
> + p2 = c->config_file_name(c);
> + if (!p1 || !p2 || strncmp(p1, p2, strlen(p1))) {
> + TSTERR("Bad result for path names");
> + goto err;
> + }
> +
> +#define CPATH "/boo"
> +#define FPATH "/boo/lxctest1/config"
> + if (!c->set_config_path(c, "/boo")) {
> + TSTERR("Error setting custom path");
> + goto err;
> + }
> + p1 = c->get_config_path(c);
> + p2 = c->config_file_name(c);
> + if (strcmp(p1, CPATH) || strcmp(p2, FPATH)) {
> + TSTERR("Bad result for path names after set_config_path()");
> + goto err;
> + }
> + lxc_container_put(c);
> +
> + c = lxc_container_new(MYNAME, CPATH);
> + if (!c) {
> + TSTERR("create using custom path");
> + goto err;
> + }
> +
> + p1 = c->get_config_path(c);
> + p2 = c->config_file_name(c);
> + if (strcmp(p1, CPATH) || strcmp(p2, FPATH)) {
> + TSTERR("Bad result for path names after create with custom path");
> + goto err;
> + }
> +
> + retval = 0;
> +
> +err:
> + lxc_container_put(c);
> + return retval;
> +}
> diff --git a/src/tests/saveconfig.c b/src/tests/saveconfig.c
> index bbaea19..fa84e32 100644
> --- a/src/tests/saveconfig.c
> +++ b/src/tests/saveconfig.c
> @@ -65,7 +65,7 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
> int ret = 1;
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/shutdowntest.c b/src/tests/shutdowntest.c
> index 9ad8a65..a1a84e8 100644
> --- a/src/tests/shutdowntest.c
> +++ b/src/tests/shutdowntest.c
> @@ -34,7 +34,7 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
> int ret = 1;
>
> - if ((c = lxc_container_new(MYNAME)) == NULL) {
> + if ((c = lxc_container_new(MYNAME, NULL)) == NULL) {
> fprintf(stderr, "%d: error opening lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
> goto out;
> diff --git a/src/tests/startone.c b/src/tests/startone.c
> index f76ed1e..1eb3e99 100644
> --- a/src/tests/startone.c
> +++ b/src/tests/startone.c
> @@ -103,7 +103,7 @@ int main(int argc, char *argv[])
>
> ret = 1;
> /* test a real container */
> - c = lxc_container_new(MYNAME);
> + c = lxc_container_new(MYNAME, NULL);
> if (!c) {
> fprintf(stderr, "%d: error creating lxc_container %s\n", __LINE__, MYNAME);
> ret = 1;
>
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130210/9fbe3c2d/attachment.pgp>
More information about the lxc-devel
mailing list