[lxc-devel] liblxc, lxc-browse()
Francois-Xavier Bourlet
francois-xavier.bourlet at dotcloud.com
Wed May 4 18:18:49 UTC 2011
Hi,
Ok I take note about inline patches in email.
- About your version of lxc_for_each, your version is a lots better.
My code was totally stupid and non-performant. In fact I based my code
on something else that was needed the scandir, i simply copy/pasted by
laziness. About the full path, in fact it was used only for the isdir.
Again my code was really crappy. Lets start with yours.
- About the errno = EINTR inside the for_each, the purpose was to
report easily and generically when a callback stop the loop. But
honestly, I dont care at all if that's up to the callback to set the
errno, since I dont have any use of it in my code. I thought it was
more generic and clean, beside that, again, I dont use it myself.
Let me explain what I am trying to do, so you can probably drive me to
the best solution with your deep understanding of LXC:
Since I am implementing some statistics collection for LXC
(liblxcstats, https://bitbucket.org/dotcloud/liblxcstats/src). What
informations I am retrieving is:
- # of registered containers
- # of running containers
- # of stopped containers
At first I wasn't really concerned by counting running containers that
are not registered, but hey! nothing prevent me to have more running
containers than (registered - stopped) ones in my statistics. It would
be more accurate in fact to avoid "forgetting" some running containers
because they are not registered I guess.
Maybe the best solution need to create two for_each style functions,
one for registered containers, one for running ones? I let you tell me
what's the best.
Then, for each running containers, I am accessing to the cgroup
file-system trough lxc_cgroup_get (to keep everything abstracted as
much as possible by lxc) and collect all the statistics I need. So yes
I need to browse every running containers, and I guess that even if
the container is not registered but running, I can still collect all
the statistics.
Let me know what do you think!
thanks a lots for your review
On Tue, May 3, 2011 at 3:16 PM, Daniel Lezcano <daniel.lezcano at free.fr> wrote:
> On 04/13/2011 07:49 PM, Francois-Xavier Bourlet wrote:
>>
>> Hi,
>>
>> Here's a patch with the purpose adding a way to browse containers trough
>> liblxc.
>> I added the function lxc-browse, that simply call back a function with
>> the container name as parameter.
>> It help to abstract how to browse containers without needed to know
>> the underlaying structure of LXC.
>>
>> In case this path seem too big, at least something like the second
>> path; that simply provide a way to retrieve LXCPATH; would be better
>> than nothing, trough less abstracted.
>
> Please in the future, inline the patches. One mail per patch, that will be
> easier to review.
>
> Thanks.
>
>> From 519012fa31068611096605447061bc90ffeef474 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Bourlet?= <fx at dotcloud.com>
>> Date: Wed, 13 Apr 2011 10:13:39 -0700
>> Subject: [PATCH 1/2] add lxc_browse function
>>
>> This function provide an interface to browse containers. It call a
>> callback for every containers, giving the name as an argument. This
>> function also return the number of containers browsed.
>> ---
>> src/lxc/Makefile.am | 3 +-
>> src/lxc/lxc.h | 16 +++++++
>> src/lxc/lxc_browse.c | 108
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 126 insertions(+), 1 deletions(-)
>> create mode 100644 src/lxc/lxc_browse.c
>>
>> diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
>> index df3d4dd..907b2c3 100644
>> --- a/src/lxc/Makefile.am
>> +++ b/src/lxc/Makefile.am
>> @@ -50,7 +50,8 @@ liblxc_so_SOURCES = \
>> mainloop.c mainloop.h \
>> af_unix.c af_unix.h \
>> \
>> - utmp.c utmp.h
>> + utmp.c utmp.h \
>> + lxc_browse.c
>
> Actually the lxc_* are for the cli, so I suggest the rename the file
> browse.c.
>
>>
>> AM_CFLAGS=-I$(top_srcdir)/src
>>
>> diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
>> index a091baa..b420010 100644
>> --- a/src/lxc/lxc.h
>> +++ b/src/lxc/lxc.h
>> @@ -161,6 +161,22 @@ extern int lxc_checkpoint(const char *name, int sfd,
>> int flags);
>> */
>> extern int lxc_restart(const char *, int, struct lxc_conf *, int);
>>
>> +/**
>> + * @brief Browse containers
>> + *
>> + * Browse all registered containers.
>> + *
>> + * @param [in] cb A function pointer to a callback which takes a pointer
>> to an
>> + * user context and a pointer to lxcst structure. If not null, the
>> callback
>> + * will be called with the ctx as well as the container name.
>> + * @param [in] ctx A pointer to an user defined variable which will be
>> used as
>> + * an argument to the callback. If the callback returns non zero
>> + * lxcst_browse_containers stops and return with -1 with errno set to
>> EINTR.
>> + *
>> + * @return the number of containers on success, -1 on error with errno
>> set.
>> + */
>> +int lxc_browse(int (*cb)(void *ctx, const char *name), void *ctx);
>> +
>> /*
>> * Returns the version number of the library
>> */
>> diff --git a/src/lxc/lxc_browse.c b/src/lxc/lxc_browse.c
>> new file mode 100644
>> index 0000000..194ea7a
>> --- /dev/null
>> +++ b/src/lxc/lxc_browse.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * lxc: linux Container library
>> + *
>> + * (C) Copyright IBM Corp. 2007, 2010
>> + *
>> + * Authors:
>> + * Daniel Lezcano <dlezcano at fr.ibm.com>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library 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
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>> USA
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <dirent.h>
>> +#include <string.h>
>> +#include <sys/stat.h>
>> +#include <err.h>
>> +#undef _GNU_SOURCE
>> +
>> +#include "lxc.h"
>> +#include "config.h"
>> +
>> +static int _unselect_current_and_parent(const struct dirent* d)
>> +{
>> + return (strcmp(d->d_name, ".") && strcmp(d->d_name, ".."));
>> +}
>> +
>> +static int _isdir(const char *path)
>> +{
>> + struct stat sb;
>> +
>> + if (stat(path, &sb) == -1) {
>> + warn("_isdir, can't stat path: %s", path);
>> + return 0;
>> + }
>> +
>> + return (sb.st_mode & S_IFDIR);
>> +}
>
> These two static functions are not needed, see below.
>
>> +
>> +static char *_join_path(char *dest, size_t size,
>> + const char *left, const char *right)
>> +{
>> + if (*right != '/') {
>> + if (strlen(left) + 1 + strlen(right) > size) {
>> + warn("_join_path, path overflow: %s + / + %s", left, right);
>> + return 0;
>> + }
>> + strcpy(dest, left);
>> + strcat(dest, "/");
>> + strcat(dest, right);
>> + } else {
>> + if (strlen(left) + strlen(right) > size) {
>> + warn("_join_path, path overflow: %s", right);
>> + return 0;
>> + }
>> + strcpy(dest, right);
>> + }
>> + return dest;
>> +}
>>
>> +int lxc_browse(int (*cb)(void*, const char*), void *ctx)
>> +{
>> + int i;
>> + struct dirent **c_vec;
>> + char buf[PATH_MAX];
>> + int count;
>> +
>> + count = scandir(LXCPATH, &c_vec, &_unselect_current_and_parent,
>> &versionsort);
>> + if (count == -1)
>> + return (-1);
>> +
>> + for (i = 0; i < count; ++i)
>> + {
>> + if (_isdir(
>> + _join_path(buf, sizeof(buf), LXCPATH,
>> c_vec[i]->d_name))
>> + )
>> + {
>> + if (cb(ctx, c_vec[i]->d_name)) {
>> + errno = EINTR;
>> + goto abort_by_cb;
>> + }
>> + }
>> + free(c_vec[i]);
>> + }
>> + free(c_vec);
>> + return count;
>> +
>> +abort_by_cb:
>> + for (; i < count; ++i)
>> + free(c_vec[i]);
>> + free(c_vec);
>> + return -1;
>> +}
>
>
> This function could be rewritten in a simpler way. It kills the 'isdir',
> 'joinpath' and '_unselect_current_and_parent'.
> The errno checking with EINTR should be checked within the callback, not
> outside. This is why I removed the test.
> It is preferable to let the callback to build the absolute path if it needs
> it, so only the container name is added.
>
> By the way, this function will browse only the created container not the
> running ones. Is it what you expect François-Xavier ? Or do you need also
> the running containers ? I ask you that because you can have running
> container without being present in the LXCPATH directory.
>
> int lxc_for_each(int (*callback)(const char *name, void *data))
> {
> DIR *dir;
> struct dirent dirent, *direntp;
> int ret = 0;
> int nr = 0;
>
> dir = opendir(LXCPATH);
> if (!dir)
> return -1;
>
> while (!readdir_r(dir, &dirent, &direntp)) {
>
> if (!direntp)
> break;
>
> if (direntp->d_name[0] == '.')
> continue;
>
> if (direntp->d_type != DT_DIR)
> continue;
>
> ret = callback(direntp->d_name, data);
> if (ret < 0)
> goto out;
>
> nr++;
> }
>
> ret = nr;
> out:
> closedir(dir);
>
> return ret;
> }
>
>
>> From aad1b3ccbda4c4731b2bb689f05a9607755baf11 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Bourlet?= <fx at dotcloud.com>
>> Date: Wed, 13 Apr 2011 10:13:46 -0700
>> Subject: [PATCH 2/2] add lxc_path()
>>
>> This function simply return the internal build configuration value
>> LXCPATH.
>> ---
>> src/lxc/conf.c | 5 +++++
>> src/lxc/conf.h | 7 +++++++
>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
>> index ae5b259..9a3d8d2 100644
>> --- a/src/lxc/conf.c
>> +++ b/src/lxc/conf.c
>> @@ -1748,3 +1748,8 @@ int lxc_setup(const char *name, struct lxc_conf
>> *lxc_conf)
>>
>> return 0;
>> }
>> +
>> +const char* lxc_path()
>> +{
>> + return LXCPATH;
>> +}
>> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
>> index 712cb3a..59
>
>
>
--
François-Xavier Bourlet
More information about the lxc-devel
mailing list