9:54 a.m.
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(a)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(a)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 :|