On Thu, Nov 19, 2009 at 09:45:48PM +0900, Ryota Ozaki wrote: > Even if qemudStartVMDaemon suceeds, an error is logged such as > 'qemuRemoveCgroup:1778 : internal error Unable to find cgroup for'. > This is because qemudStartVMDaemon calls qemuRemoveCgroup to > ensure that old cgroup does not remain. This workaround makes > sense but leaving an error message may confuse users. > > This patch simply adds an option to suppress the error > log if not needed. > --- >  src/qemu/qemu_driver.c |   16 +++++++++------- >  1 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index a4a87ac..84ee942 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -1763,7 +1763,8 @@ cleanup: > >  static int qemuRemoveCgroup(virConnectPtr conn, >                              struct qemud_driver *driver, > -                            virDomainObjPtr vm) > +                            virDomainObjPtr vm, > +                            int quiet) >  { >      virCgroupPtr cgroup; >      int rc; > @@ -1773,9 +1774,10 @@ static int qemuRemoveCgroup(virConnectPtr conn, > >      rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); >      if (rc != 0) { > -        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, > -                         _("Unable to find cgroup for %s\n"), > -                         vm->def->name); > +        if (!quiet) > +            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, > +                             _("Unable to find cgroup for %s\n"), > +                             vm->def->name); >          return rc; >      } > > @@ -2142,7 +2144,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, >          return -1; > >      /* Ensure no historical cgroup for this VM is lieing around bogus settings */ > -    qemuRemoveCgroup(conn, driver, vm); > +    qemuRemoveCgroup(conn, driver, vm, 1); > >      if ((vm->def->ngraphics == 1) && >          vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && > @@ -2327,7 +2329,7 @@ cleanup: >          VIR_FREE(vm->def->seclabel.label); >          VIR_FREE(vm->def->seclabel.imagelabel); >      } > -    qemuRemoveCgroup(conn, driver, vm); > +    qemuRemoveCgroup(conn, driver, vm, 0); >      if ((vm->def->ngraphics == 1) && >          vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && >          vm->def->graphics[0]->data.vnc.autoport) > @@ -2416,7 +2418,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, >      qemuDomainReAttachHostDevices(conn, driver, vm->def); > >  retry: > -    if ((ret = qemuRemoveCgroup(conn, driver, vm)) < 0) { > +    if ((ret = qemuRemoveCgroup(conn, driver, vm, 0)) < 0) { >          if (ret == -EBUSY && (retries++ < 5)) { >              usleep(200*1000); >              goto retry;  ACK, looks fine to me, but I would rather apply this after the 0.7.3 release as this is just a superfluous error message  Please remind me to push this next week if I forget :-)

  thanks ! Daniel -- Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/ daniel(a)veillard.com  | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library  http://libvirt.org/

On Thu, Nov 19, 2009 at 11:27 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: > On Thu, Nov 19, 2009 at 09:45:48PM +0900, Ryota Ozaki wrote: >> Even if qemudStartVMDaemon suceeds, an error is logged such as >> 'qemuRemoveCgroup:1778 : internal error Unable to find cgroup for'. >> This is because qemudStartVMDaemon calls qemuRemoveCgroup to >> ensure that old cgroup does not remain. This workaround makes >> sense but leaving an error message may confuse users. >> >> This patch simply adds an option to suppress the error >> log if not needed. >> --- >>  src/qemu/qemu_driver.c |   16 +++++++++------- >>  1 files changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index a4a87ac..84ee942 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -1763,7 +1763,8 @@ cleanup: >> >>  static int qemuRemoveCgroup(virConnectPtr conn, >>                              struct qemud_driver *driver, >> -                            virDomainObjPtr vm) >> +                            virDomainObjPtr vm, >> +                            int quiet) >>  { >>      virCgroupPtr cgroup; >>      int rc; >> @@ -1773,9 +1774,10 @@ static int qemuRemoveCgroup(virConnectPtr conn, >> >>      rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); >>      if (rc != 0) { >> -        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, >> -                         _("Unable to find cgroup for %s\n"), >> -                         vm->def->name); >> +        if (!quiet) >> +            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, >> +                             _("Unable to find cgroup for %s\n"), >> +                             vm->def->name); >>          return rc; >>      } >> >> @@ -2142,7 +2144,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, >>          return -1; >> >>      /* Ensure no historical cgroup for this VM is lieing around bogus settings */ >> -    qemuRemoveCgroup(conn, driver, vm); >> +    qemuRemoveCgroup(conn, driver, vm, 1); >> >>      if ((vm->def->ngraphics == 1) && >>          vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && >> @@ -2327,7 +2329,7 @@ cleanup: >>          VIR_FREE(vm->def->seclabel.label); >>          VIR_FREE(vm->def->seclabel.imagelabel); >>      } >> -    qemuRemoveCgroup(conn, driver, vm); >> +    qemuRemoveCgroup(conn, driver, vm, 0); >>      if ((vm->def->ngraphics == 1) && >>          vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && >>          vm->def->graphics[0]->data.vnc.autoport) >> @@ -2416,7 +2418,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, >>      qemuDomainReAttachHostDevices(conn, driver, vm->def); >> >>  retry: >> -    if ((ret = qemuRemoveCgroup(conn, driver, vm)) < 0) { >> +    if ((ret = qemuRemoveCgroup(conn, driver, vm, 0)) < 0) { >>          if (ret == -EBUSY && (retries++ < 5)) { >>              usleep(200*1000); >>              goto retry; > >  ACK, looks fine to me, but I would rather apply this after the 0.7.3 > release as this is just a superfluous error message >  Please remind me to push this next week if I forget :-) I remember this patch ;-) Could you apply it?