On Thu, Oct 10, 2013 at 08:33:28AM -0500, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgra...@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.hal...@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

Attachment: signature.asc
Description: Digital signature

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to