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

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.