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

[Serge Hallyn]

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?

-serge