[libvirt] [PATCH] qemu: don't label anything before locking the domain

[Daniel P. Berrange]

On Thu, Jun 26, 2014 at 12:42:52PM +0100, Daniel P. Berrange wrote:
> On Thu, Jun 26, 2014 at 01:20:02PM +0200, Martin Kletzander wrote:
> > If locking the domain failed, files were already labelled and thus we
> > restored the previous label on them.  Having disks on NFS means the
> > domain having the lock already gets permission denial.
> > 
> > This code moves the labelling part into the command hook since it's
> > still privileged, and also moves the clearing of
> > VIR_QEMU_PROCESS_STOP_NO_RELABEL from stop_flags right after the
> > handshare after hook.
> 
> This problem description / fix doesn't make much sense to me.
> 
> IIUC the control flow is
> 
>   - Parent runs fork()
>   - Parent waits for handshake notify
>   - Child runs hook
>       - Hook *only* registers with lock daemon
>   - Child sends handshake notify to parent
>   - Child waits for handshake response
>   - Parent received handshake notify
>   - Parent does labelling
>   - Parent sends handshake response
>   - Child execs QEMU
>   - QEMU launches but CPUs are paused
>   - Parent acquires disk locks
>   - Parent tells QEMU to start CPUs
> 
> Note that the hook does not acquire any locks - it merely connects
> to the lock daemon. Locks are not acquired until the CPUs are ready
> to be started. So I don't see how moving labelling into the hook
> solves anything.
> 
> Note that the goal of the locking code as it is today, was only to
> prevent the content of the disk image being corrupted by 2 QEMUs
> running concurrently. The design as it is succeeds in this. Stopping
> changes to the labelling was not attempted. Yes, this will result
> in a running QEMU loosing access to a disk if another QEMU attempts
> to start and use those disks, but the content is protected in this
> way.
> 
> It isn't actually possible to protect against concurrent changes
> to both the content and the labelling with a single lock because
> there are differing lock ordering & protection rules requires for
> these.
> 
> To do that, we actually need to incorporate use of the lock manager
> into the security drivers using a separate lock space and use locking
> rules that apply explicitly to the needs of the labelling.

Specifically what the security drivers would have todo is

  - Acquire exclusive lock on the image
  - If not already labelled
       - Label image
    Else
       - See if current labelling is readonly or shared
         and this matches desired labelling
  - Release the exclusive lock on the image

So see that the lock only has to be held for the short time
that the labelling is being changed. This is very different
from the existing content lock which must be held for the
entire time the guest is running.

This all really ties back into the previous problem we've tried to
solve of tracking the original image label so we can correctly
restore upon guest shutdown. Both the locking and that tracking
have to be solved at the same time - two facets of the same problem.

Regards,
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 :|