On Fri, Jul 17, 2009 at 09:04:26AM -0400, Daniel P. Berrange wrote:
> * src/qemu_driver.c: Place guest in cgroup upon startup. Remove
> cgroup upon shutdown
> ---
> src/qemu_driver.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++----
[...]
> + virReportSystemError(conn, -rc,
> + _("Unable to create cgroup for %s"), vm->def->name);
hum, there is a missing line split here, I think I saw one similar in
patch 3 but it was less obvious
> +static int qemuAddToCgroup(virDomainDefPtr def)
> +{
> + virCgroupPtr cgroup = NULL;
> + int rc;
> +
> + if (virCgroupHaveSupport() != 0)
> + return 0; /* Not supported, so claim success */
> +
> + rc = virCgroupForDomain(def, "qemu", &cgroup);
> + if (rc != 0) {
> + virReportSystemError(NULL, -rc, _("unable to find cgroup for domain %s"), def->name);
> + return -1;
> + }
> +
> + rc = virCgroupAddTask(cgroup, getpid());
> + if (rc != 0) {
> + virReportSystemError(NULL, -rc, _("unable to add domain %s task %d to cgroup"), def->name, getpid());
> + virCgroupFree(&cgroup);
> + return -1;
> + }
A couple more, and I would define itn ret = 0; and factorize the
cleanup on exit
> +
> + virCgroupFree(&cgroup);
> + return 0;
> +}
> +
> +
[...]
>
> -struct gemudHookData {
> - virConnectPtr conn;
> - virDomainObjPtr vm;
> - struct qemud_driver *driver;
> +struct qemudHookData {
> + virConnectPtr conn;
> + virDomainObjPtr vm;
> + struct qemud_driver *driver;
> };
took me a while to spot that gemu turned in qemu
Again not a cgroup expert but this looks okay to me, ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list