[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] cgroup: Add missing errno == ENOENT check in virCgroupRemoveRecursively



On Tue, Jun 29, 2010 at 1:41 AM, Eric Blake <eblake redhat com> wrote:
> On 06/26/2010 11:21 AM, Ryota Ozaki wrote:
>> ENOENT happens normally when a subsystem is enabled with any other
>> subsystems and the directory of the target group has already removed
>> in a prior loop. In that case, the function should just return without
>> leaving an error message.
>>
>> NB this is the same behavior as before introducing virCgroupRemoveRecursively.
>> ---
>>  src/util/cgroup.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/util/cgroup.c b/src/util/cgroup.c
>> index 62b1446..9fa64dc 100644
>> --- a/src/util/cgroup.c
>> +++ b/src/util/cgroup.c
>> @@ -616,6 +616,8 @@ static int virCgroupRemoveRecursively(char *grppath)
>>
>>      grpdir = opendir(grppath);
>>      if (grpdir == NULL) {
>> +        if (errno == ENOENT)
>> +            return 0;
>
> Shouldn't this be continue instead of return 0, so as to go on to the
> next readdir() in case there is anything else in the directory?

The next readdir() and mkdir() following to it are for the directory (e.g.,
/a/b/c) and its inclusions (e.g., /a/b/c/d). We cannot go on if the directory
(a/b/c) does not present. Other sibling directories of the directory (e.g,
/a/b/d) will be handled in the caller function.

Well, am I missing your question?

>
> Also, right now, your cgroup recursive delete is breaking compilation on
> mingw, which lacks d_type.  Obviously, since mingw also lacks cgroup
> altogether, we can make things conditional, but I haven't had time to
> look into the correct fix for that yet.

If we follow the same workaround as doing for virCgroupForDriver,
it'll be like this:

#if defined _DIRENT_HAVE_D_TYPE
static int virCgroupRemoveRecursively(char *grppath)
{
    ...
}
#else
static int virCgroupRemoveRecursively(char *grppath ATTRIBUTE_UNUSED)
{
    /* Claim no support */
    return -ENXIO;
}
#endif

I'm not sure it's sane though, it'll work...
  ozaki-r

>
> --
> Eric Blake   eblake redhat com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]