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

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

 



On Wed, Jun 30, 2010 at 3:21 AM, Eric Blake <eblake@xxxxxxxxxx> wrote:
> On 06/28/2010 08:49 PM, Ryota Ozaki wrote:
>>>>      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?
>
> No, I was misreading the code - I thought this was an early return
> inside the readdir loop, but re-reading it, I see it is an early exit
> because the opendir() failed.  So,

I see.

>
> ACK, and applied your patch.

Thanks!

>
>> 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...
>
> Looks clean enough to me.  Let's apply that as a separate patch; would
> you care to do the honors of writing it?

OK, I will send the patch soon.

  ozaki-r

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]