On 07/19/2013 08:54 AM, Daniel P. Berrange wrote:
>>> 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.
> +++ b/src/qemu/qemu_cgroup.c
Still incomplete - you fixed virCgroupAddTask callers, but didn't audit
for others. At least in this file, we have:
rc = virCgroupAllowDevicePath(priv->cgroup, path,
VIR_CGROUP_DEVICE_RW);
virDomainAuditCgroupPath(vm, priv->cgroup,
"allow", path, "rw", rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow access "
"for device path %s"),
path);
virDomainAuditCgroupPath() doesn't care if rc is -1 or -errno, but the
semantics of virCgroupAllowDevicePath changed, and the error message is
now wrong.
What you have looks okay to squash in, but I'm still worried that we
have more to scrub.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list