On Tue, Apr 05, 2016 at 06:11:40PM +0200, Andrea Bolognani wrote: > On Tue, 2016-04-05 at 17:32 +0200, Ján Tomko wrote: > > On Tue, Apr 05, 2016 at 03:08:41PM +0200, Andrea Bolognani wrote: > > > The existing code was built with the assumption that no cgroup > > > name could appear as part of another cgroup name. > > > > This is not true - the code checked if there is a comma or a null > > character after the cgroup name. > > But not whether there was a comma *before* the cgroup name. So > an ipotetic cgroup called 'fakecpu' would match when looking > for the 'cpu' cgroup. > > More importantly, only the first match is taken into account: > on my system, the 'cpu' and 'cpuacct' are mounted to the same > location and the corresponding mtab entry looks like > > cgroup /sys/fs/cgroup/cpu,cpuacct \ > cgroup rw,nosuid,nodev,noexec,relatime,cpu,cpuacct 0 0 > > but it's been reported that, on other systems, the 'cpuacct' > mount flag is listed *before* the 'cpu' mount flag. > > When that's the case, strstr() will match the first occurrence > of 'cpu', which happens to be that at the beginning of 'cpuacct', > and the subsequent check for that match be followed by a comma > will fail. > > To sum up: the commit message is not good enough and should be > improved, but the change itself (or a variation thereof) is > still needed to make the cgroup mount point detection robust. > Right. ACK to the patch as-is. Jan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list