Re: [libvirt] [PATCH 1/2] qemu: add vfio devices to cgroup ACL when appropriate

>> +int qemuSetupHostdevCGroup(virDomainObjPtr vm, >> + virDomainHostdevDefPtr dev) ATTRIBUTE_RETURN_CHECK; >> +int qemuTeardownHostdevCgroup(virDomainObjPtr vm, >> + virDomainHostdevDefPtr dev); > A bit odd that setup is ATTRIBUTE_RETURN_CHECK but teardown is not. I'd > rather see both require a use... I didn't include it for teardown because I wasn't checking the return (see below), and didn't want the extra line length caused by ignore_value(). (also, I notice that none of the other functions in qemu_cgroup.h have any sort of ATTRIBUTE_* usage at all).

> >> @@ -1257,6 +1260,9 @@ error: >> vm->def, hostdev, NULL) < 0) >> VIR_WARN("Unable to restore host device labelling on hotplug fail"); >> >> +teardown_cgroup: >> + qemuTeardownHostdevCgroup(vm, hostdev); > ...and here, on cleanup paths after an earlier error, stick a VIR_WARN() > that logs any failure trying to clean up (as we already did on the line > before). But the teardown function itself is already logging an error message (as is the security manager function preceding it), so I don't really see the point of the additional VIR_WARN (I had seen the VIR_WARN on failure of the security manager, and concluded that it was redundant, so didn't replicate it). At any rate, I want to get this in before RC2, so I'll add a VIR_WARN and you can convince me (or I can convince you) later.

I'm not certain I agree, but what you're requesting won't hurt anything, so in the interest of time I'll modify it that way and push.