On Fri, Jul 19, 2013 at 03:51:25PM +0100, Daniel P. Berrange wrote:
On Thu, Jul 18, 2013 at 09:59:47PM -0600, Eric Blake wrote:
On 07/18/2013 09:00 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Convert the remainiing methods in vircgroup.c to report errors
s/remainiing/remaining/
instead of returning errno values.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/vircgroup.c | 559 +++++++++++++++++++++++++++++---------------------- 1 file changed, 314 insertions(+), 245 deletions(-)
@@ -968,11 +964,11 @@ int virCgroupRemove(virCgroupPtr group) * @group: The cgroup to add a task to * @pid: The pid of the task to add * - * Returns: 0 on success + * Returns: 0 on success, -1 on error */ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
Ouch - at least src/qemu/qemu_cgroup.c calls this function, and still expects errno values:
rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); if (rc < 0) { virReportSystemError(-rc, _("unable to add vcpu %zu task %d to cgroup"), i, priv->vcpupids[i]); goto cleanup; }
I didn't look elsewhere; all I needed was one counterexample to state your patch is incomplete until you audit all callers impacted by semantic changes.
Fixed those.
This time with the diff attached. diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 3cec8e2..d15fa67 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -605,13 +605,8 @@ virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def) if (!(cgroup = virLXCCgroupCreate(def, true))) return NULL; - rc = virCgroupAddTask(cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to add task %d to cgroup for domain %s"), - getpid(), def->name); + if (virCgroupAddTask(cgroup, getpid()) < 0) goto cleanup; - } ret = 0; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2df80bc..c6364fc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -975,13 +975,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) goto cleanup; /* move the thread for vcpu to sub dir */ - rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); - if (rc < 0) { - virReportSystemError(-rc, - _("unable to add vcpu %zu task %d to cgroup"), - i, priv->vcpupids[i]); + if (virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]) < 0) goto cleanup; - } if (period || quota) { if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) @@ -1118,13 +1113,8 @@ qemuAddToCgroup(virDomainObjPtr vm) if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupAddTask(priv->cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("unable to add domain %s task %d to cgroup"), - vm->def->name, getpid()); + if (virCgroupAddTask(priv->cgroup, getpid()) < 0) return -1; - } return 0; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index bab7503..9d63edd 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1062,12 +1062,10 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, virErrorPtr err = virGetLastError(); /* A thread that exits between when we first read the source * tasks and now is not fatal. */ - if (err && err->code == VIR_ERR_SYSTEM_ERROR && - err->int1 == ESRCH) { + if (virLastErrorIsSystemErrno(ESRCH)) virResetLastError(); - } else { + else goto cleanup; - } } next = strchr(cur, '\n'); @@ -1711,7 +1709,7 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, } if (!S_ISBLK(sb.st_mode)) { - virReportSystemError(errno, + virReportSystemError(EINVAL, _("Path '%s' must be a block device"), path); return -1; Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|