[lxc-devel] [PATCH] define list container api

Stéphane Graber stgraber at ubuntu.com
Thu Oct 10 14:02:41 UTC 2013


On Thu, Oct 10, 2013 at 08:33:28AM -0500, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > On Wed, Oct 09, 2013 at 07:17:43AM -0500, Serge Hallyn wrote:
> > > Two new commands are defined: list_defined_containers() and
> > > list_active_containers().  Both take an lxcpath (NULL means
> > > use the default lxcpath) and return the number of containers
> > > found.  If a lxc_container ** is passed in, then an array of
> > > lxc_container's is returned, one for each container found.
> > > The caller must then lxc_container_put() each container and
> > > free the array, as shown in the new list testcase.
> > > 
> > > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> > 
> > Overall looks good, thanks!
> > 
> > I just have one question/comment below that may impact memory/cpu usage
> > and performance quite a bit on machines with a lot of containers.
> > 
> > > ---
> > >  src/lxc/lxccontainer.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/lxc/lxccontainer.h |  19 ++++++
> > >  src/tests/Makefile.am  |   6 +-
> > >  src/tests/list.c       |  57 ++++++++++++++++++
> > >  4 files changed, 235 insertions(+), 2 deletions(-)
> > >  create mode 100644 src/tests/list.c
> > > 
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 13ed4d2..5a0edce 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -2744,3 +2744,158 @@ int lxc_get_wait_states(const char **states)
> > >  			states[i] = lxc_state2str(i);
> > >  	return MAX_STATE;
> > >  }
> > > +
> > > +
> > > +bool add_to_clist(struct lxc_container ***list, struct lxc_container *c, int pos)
> > > +{
> > > +	struct lxc_container **newlist = realloc(*list, pos * sizeof(struct lxc_container *));
> > > +	if (!newlist) {
> > > +		free(*list);
> > > +		*list = NULL;
> > > +		ERROR("Out of memory");
> > > +		return false;
> > > +	}
> > > +
> > > +	*list = newlist;
> > > +	newlist[pos-1] = c;
> > > +	return true;
> > > +}
> > > +
> > > +int list_defined_containers(const char *lxcpath, struct lxc_container ***cret)
> > > +{
> > > +	DIR *dir;
> > > +	int nfound = 0;
> > > +	struct dirent dirent, *direntp;
> > > +
> > > +	if (!lxcpath)
> > > +		lxcpath = default_lxc_path();
> > > +
> > > +	process_lock();
> > > +	dir = opendir(lxcpath);
> > > +	process_unlock();
> > > +
> > > +	if (!dir) {
> > > +		SYSERROR("opendir on lxcpath");
> > > +		return -1;
> > > +	}
> > > +
> > > +	if (cret)
> > > +		*cret = NULL;
> > > +
> > > +	while (!readdir_r(dir, &dirent, &direntp)) {
> > > +		if (!direntp)
> > > +			break;
> > > +		if (!strcmp(direntp->d_name, "."))
> > > +			continue;
> > > +		if (!strcmp(direntp->d_name, ".."))
> > > +			continue;
> > 
> > Is it actually faster or more reliable to load the whole container
> > instead of just looking for direntp->d_name/config?
> > 
> > > +
> > > +		struct lxc_container *c = lxc_container_new(direntp->d_name, lxcpath);
> 
> Originally I had a comment there to say we might want to just
> manually check for the config file.  Originally I also had a
> char *** passed in so you could harvest only the container
> names without getting the full lxc_container structs.
> 
> What I liked about this is the ability to reuse the
> lxcapi_is_defined() function and avoid reimplementing
> the check - with potential for accidental divergence as
> the code evolves.
> 
> Should we offer the char *** option?  Or is that just
> going to complicate the lua/go/python api exports?

I don't think supporting both would be a problem for bindings, we'd
probably just bind them to two different functions (e.g.
lxc_list_defined_names).

I still think it'd be better to only lxc_container_new once we've
confirmed a config file exists, otherwise someone who has a rather messy
/var/lib/lxc or pass /usr/bin to -P (ok, that'd be stupid but still)
would cause a lot of locking calls and a ton of check for basically
nothing.

> 
> -serge

-- 
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: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131010/0352876d/attachment.pgp>


More information about the lxc-devel mailing list