Re: [PATCHv5 13/28] locking: Add APIs to lock individual image files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/04/2014 05:29 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(-)
> 

> @@ -83,24 +82,25 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock,
>            type == VIR_STORAGE_TYPE_DIR))
>          return 0;
> 
> -    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;

Hmm. Pre-existing, and not changed here (so this patch is still good),
but we probably ought to start making the lease manager reserve backing
files as readonly, rather than limiting the reservation to just the
top-level file.  (Did I already mention this in an earlier review?)  At
any rate, I suspect that we are leaking reservations when we do
snapshots and/or commits, although I haven't played enough with the lock
manager to actually prove or disprove that.

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]