On 01/13/2016 11:51 AM, Martin Kletzander wrote: > On Wed, Jan 13, 2016 at 07:29:46AM -0500, John Ferlan wrote: >> Reposting my cgroup fixes series: >> >> http://www.redhat.com/archives/libvir-list/2016-January/msg00236.html >> >> partially because I originally forgot to CC the author (Henning Schild) >> of the original series for which these patch fix a couple of issues >> discovered during regression testing (virt-test memtune failures in >> Red Hat regression environment), but also to bring them up to date >> with the top of libvirt git. >> >> NB: I did send Henning the changes after the fact, but my resend using >> the same message-id skills so that replies are left in the onlist series >> are lacking. Henning has looked at the first patch - with a response >> here: >> >> http://www.redhat.com/archives/libvir-list/2016-January/msg00443.html >> >> Finally, I think these changes should go into 1.3.1 since that's when the >> regression was introduced. >> > > It would be nice to have them in, I really tried reviewing them, but I > can't wrap my head around last two of them. Maybe because I'm already > late for an appointment I have. > What I found happening is that by moving the virCgroupAddTask from qemuInitCgroup until later in qemuSetupCgroupForEmulator is that for "some reason" on (at least in the test environment) an older Fedora release (f20) that the /proc/$pid/cgroup file was updated "strangely". On a more recent Fedora release (f23) I didn't see the same phenomena. And by strangely - what I saw was even though the *SetupEmulator path was 'supposed to be' modifying only cpuset and cpu,cpuacct - other entries for memory, blkio, and devices were also updated - thus pointing at the wrong place (rather than the machine specific place). This caused the memtune test to fail. What was even stranger is that the code was updating the machine specific area when changing the values (e.g., internally we had the right path), but the test cannot see that so it uses the /proc/$pid/cgroup file to find the path. In any case, it's a very strange anomaly. > So unfortunately I have to leave you without the review for those two as > I really need to go, but anyone else feel free to continue. And even > re-check my reviews for 1 and 2 if you want. It would be a pity not to > fix a regression when we could. > Thanks for your time on the first two... I'll push the first one and wait on the others. > On a side note, this mail is missing "PATCH" in the subject, so people > filtering those into different folders (and not reading all of them, > i.e. not me) could easily miss it. I won't be able to get to this for > some time now, so please pursue someone just in case I won't get to that > for another day. Sorry for the inconvenience. To each their own on filters - there's a version with PATCH in the title and one that only has REPOST. The only difference is the REPOST does the CC and is 'up to date' with top of branch. The PATCH version should apply fine too. John > >> John Ferlan (4): >> cgroup: Fix possible bug as a result of code motion for vcpu cgroup >> setup >> qemu: Add check for NULL cgroup return from virCgroupNewMachine >> Revert "qemu: do not put a task into machine cgroup" >> qemu: Put the emulator cgroup pid into the right task file >> >> src/qemu/qemu_cgroup.c | 18 +++++++++++++----- >> src/qemu/qemu_process.c | 12 ++++++------ >> 2 files changed, 19 insertions(+), 11 deletions(-) >> >> -- >> 2.5.0 >> >> -- >> libvir-list mailing list >> libvir-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list