[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