Re: [libvirt] [PATCH v3 6/7] util: vircgroupv2: don't error out if enabling controller fails

On Wed, Jun 26, 2019 at 09:21:59AM +0200, Peter Krempa wrote: > On Tue, Jun 25, 2019 at 13:16:25 +0200, Pavel Hrdina wrote: > > Currently CPU controller cannot be enabled if there is any real-time > > task running and is assigned to non-root cgroup which is the case on > > several distributions with graphical environment. > > > > Instead of erroring out treat it as the controller is not available. > > > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > > --- > > src/util/vircgroupv2.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c > > index 133a8e0e66..348c12d5c6 100644 > > --- a/src/util/vircgroupv2.c > > +++ b/src/util/vircgroupv2.c > > @@ -433,6 +433,8 @@ virCgroupV2MakeGroup(virCgroupPtr parent ATTRIBUTE_UNUSED, > > } else { > > size_t i; > > for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { > > + int rc; > > + > > if (!virCgroupV2HasController(parent, i)) > > continue; > > > > @@ -440,8 +442,17 @@ virCgroupV2MakeGroup(virCgroupPtr parent ATTRIBUTE_UNUSED, > > if (i == VIR_CGROUP_CONTROLLER_CPUACCT) > > continue; > > > > - if (virCgroupV2EnableController(parent, i) < 0) > > + rc = virCgroupV2EnableController(parent, i); > > + if (rc < 0) { > > + if (rc == -2) { > > + virResetLastError(); > > Instead of doing this you should not report the error in the first > place. Given that the refactor in the previous commit adds the error > report in the called function it should be trivial to do so. > > Without that the logs would be spammed by an error which does not help > the users much. Works for me, this was based on our in person discussion but I guess there was some misunderstanding. Do you need v4?