[libvirt] [PATCH 11/23] Introduce an internal API for handling file based lockspaces
Daniel P. Berrange
berrange at redhat.com
Tue Aug 21 14:51:24 UTC 2012
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 :|
More information about the libvir-list
mailing list