Re: [libvirt] [PATCH 11/23] Introduce an internal API for handling file based lockspaces
On Thu, Aug 16, 2012 at 04:31:28PM -0600, Eric Blake wrote:
On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
> eg to lock /var/lib/libvirt/images/foo.img the application
> might do
>
> virLockSpacePtr lockspace =
virLockSpaceNew("/var/lib/libvirt/imagelocks");
> lockname = md5sum("/var/lib/libvirt/images/foo.img")
> virLockSpaceAcquireLock(lockspace, lockname)
s/)$/),/g
Don't you want to ensure that the canonical name is used (that is,
symlinks can allow two different names to resolve to the same file, but
you want to hash the name without symlinks).
Yes, I added a note that the filename should be canonicalized
in this example
> +static void virLockSpaceResourceFree(virLockSpaceResourcePtr
res)
> +{
> + if (!res)
> + return;
> +
> + if (res->lockHeld &&
> + (res->flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE)) {
> + if (res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) {
> + /* We must upgrade to an exclusive lock to ensure
> + * no one else still has it before trying to delete */
> + if (virFileLock(res->fd, false, 0, 1) < 0) {
> + VIR_DEBUG("Could not upgrade shared lease to exclusive, not
deleting");
> + } else {
> + unlink(res->path);
Should we log failures to unlink()?
Yeah, worth while, so I changed it to
if (unlink(res->path) < 0 &&
errno != ENOENT) {
char ebuf[1024];
VIR_WARN("Failed to unlink resource %s: %s",
res->path, virStrerror(ioError, ebuf, sizeof(ebuf)));
}
> +
> +void virLockSpaceFree(virLockSpacePtr lockspace)
> +{
> + if (!lockspace)
> + return;
> +
> + virHashFree(lockspace->resources);
> + VIR_FREE(lockspace->dir);
Should we first try to rmdir() the directory, and silently ignore errors
related to the directory still being full?
I think it is best to just leave it alone. In most production
setups I expect this directory would in fact be as NFS mount.
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 :|