Re: [libvirt] [PATCHv4 14/18] blockjob: allow mirroring under SELinux
On Fri, Apr 13, 2012 at 14:59:32 -0600, Eric Blake wrote:
On 04/13/2012 02:45 PM, Jiri Denemark wrote:
> On Mon, Apr 09, 2012 at 21:52:23 -0600, Eric Blake wrote:
>> This copies heavily from qemuDomainSnapshotCreateSingleDiskActive(),
>> in order to set the SELinux label, obtain locking manager lease, and
>> audit the fact that we hand a new file over to qemu. Alas, releasing
>> the lease and label on failure or at the end of the mirroring is a
>> trickier prospect.
>>
>> +
>> + if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
>> + goto endjob;
>> + if (virSecurityManagerSetImageLabel(driver->securityManager,
vm->def,
>> + disk) < 0) {
>> + if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
>> + VIR_WARN("Unable to release lock on %s", dest);
>> + goto endjob;
>> + }
>
> Is there a reason for calling virDomainLockDiskDetach only when SetImageLabel
> fails and not calling it if mirroring itself fails?
Simplicity of code - it's easier to code up permissions granting (and
leaking that) then it is to figure out which permissions need to be
reversed. I'll attempt to do a better job in v5, at least when
mirroring fails. But revoking permissions after 'drive-reopen' is a
bear - you can't do it immediately after the 'drive-reopen' command, but
But there's no drive-reopen involved here. We only issue drive-mirror command
that starts the process. And if it fails, i.e., we don't even start copying
data, we don't call virDomainLockDiskDetach to undo virDomainLockDiskAttach.
It's possible we don't need to call that because the file is unlinked in the
end, which is sufficient to remove the lock but I'm not sure and that's why
I'm asking :-)
Jirka