On 06/25/2014 10:54 AM, Peter Krempa wrote: > Add helper APIs to manage individual image files rather than disks. To > simplify the addition some parts of the code were refactored in this > patch. > --- > src/libvirt_private.syms | 2 ++ > src/locking/domain_lock.c | 65 ++++++++++++++++++++++++++++++----------------- > src/locking/domain_lock.h | 8 ++++++ > 3 files changed, 52 insertions(+), 23 deletions(-) > > +static int virDomainLockManagerAddImage(virLockManagerPtr lock, > + virStorageSourcePtr src) > > - if (disk->src->readonly) > + if (src->readonly) > diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY; > - if (disk->src->shared) > + if (src->shared) > diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; Don't you also need to ensure that backing files in the chain are marked readonly regardless of the setting on the active layer? Oh, I see - on the lock manager, we are currently only ever locking the active image. Hmm, I wonder if this is actually correct in the presence of snapshots - are we leaving a file locked when it becomes the backing element of a new file due to an external snapshot? We may have pre-existing bugs (while I know I got SELinux and cgroup issues sorted out in my testing, I haven't personally played much with the lease manager at the same time as playing with snapshots). But as far as I can tell, this patch makes no semantic difference, so it is no worse than what we already had. Therefore, ACK -- 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