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

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,

ACK, and applied your patch.

> 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?

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