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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list