[patch] Add advisory locks to HAL devices

[David Zeuthen]

On Fri, 2004-09-10 at 13:43 -0400, Joe Shaw wrote:
> Hi,
> 

Hey, 

Sorry for the lag,

> I took a stab at implementing the advisory locks, as mentioned in the
> TODO and referenced in this thread:
> 
> 	http://freedesktop.org/pipermail/hal/2004-July/000579.html
> 
> This patch adds the infrastructure for the locks, but they're not yet
> used anywhere (like not polling a CD recorder while it's burning, for
> instance).  It adds Lock and Unlock methods to libhal, which trigger
> certain properties being set in hald.  hald indicates that it's locked
> with the "info.locked" property and sets "info.locked.dbus_service" to
> indicate who took the lock and "info.locked.reason" to a user-visible
> string as to why it is locked.  hald tracks the lifecycle of the service
> in "info.locked.dbus_service" and releases the lock if it is deleted
> before it voluntarily unlocks the device.
> 
> Check it out, let me know what you think.
> 

Very cool, I just played around with it and it looks good. The only real
comment I have is that the Lock() method shouldn't return anything and
instead just throw an exception if the device is already locked.

Right now it returns either TRUE or [FALSE, reason] which is a bit
difficult to catch when using language bindings. With the change that it
throws the exception DeviceAlreadyLocked I was able to write this tiny
python program:

        #!/usr/bin/python
        
        # locking.py <numsecs>
        
        import dbus
        import sys
        import time
        import os
        
        dev_udi = "/org/freedesktop/Hal/devices/computer"
        duration = int(sys.argv[1])
        pid = os.getpid()
        reason = "locking.py pid %d"%pid
        
        bus = dbus.Bus(dbus.Bus.TYPE_SYSTEM)
        hal_service = bus.get_service("org.freedesktop.Hal")
        
        print "I am %s with pid %d"%(bus.get_connection().get_base_service(), pid)
        print
        
        dev = hal_service.get_object (dev_udi, "org.freedesktop.Hal.Device")
        
        print "Attempting to lock %s for %d secs"%(dev_udi, duration)
        print "with reason '%s'"%reason
        print
        
        try:
            dev.Lock(reason)
        except Exception, e:
            print "Locking failed"
            reason =   dev.GetProperty("info.locked.reason")
            lock_svc = dev.GetProperty("info.locked.dbus_service")
            print "Reason:                   '%s'"%reason
            print "Locked by D-BUS basesvc:  '%s'"%lock_svc
            sys.exit(1)
        
        print "Lock acquired; sleeping %d seconds"%duration    
        time.sleep(duration)
        print "Releasing lock"
        dev.Unlock()
        print "Lock released"
        
A few small comments

> +/** Raise the org.freedesktop.Hal.NotLocked error
> + *

This should probably be org.freedesktop.Hal.DeviceNotLocked.

> +/** Grab an advisory lock on a device.
> + *
> + *  <pre>
> + *  bool Device.Lock()
> + *
> + *    raises org.freedesktop.Hal.NoSuchDevice,
> + *  </pre>

This should probably be 'void Device.Lock(string reason)' and should
also throw org.freedesktop.Hal.DeviceAlreadyLocked. 

The libhal parts looks good although the libhal.h patch is missing - I'm
assuming it's only the prototypes for the two new functions. Also the
spec should probably be updated to show the D-BUS API additions.

Much thanks,
David
_______________________________________________
hal mailing list
hal at freedesktop.org
http://freedesktop.org/mailman/listinfo/hal