[RFC] Add persistent storage capability to HAL

David Zeuthen david at fubar.dk
Mon Aug 21 08:57:06 PDT 2006


On Thu, 2006-08-17 at 22:52 +0100, Matthew Garrett wrote:
> The attached patch adds the ability to save devices (optionally 
> recursively) and then reload them into HAL at a later stage. This is 
> useful for Bluetooth, where it's impractical to rediscover devices when 
> the device is no longer discoverable - instead, the device state should 
> be stored and recreated when the HCI is connected. Somewhat draft code, 
> but comments welcome.

Sounds conceptually good to me, some comments

 - preferably don't mix var decl with assignments

 - use /* comment style */

 - shouldn't hardcode "/var/lib/hal/persistent", should use
   $localstatedir

 - would prefer not to have recursive; too easy of a trap to fall into,
   caller should do this manually if it's really needed (plus you have
   little or no guarantee that there is a variable "LibHalContext *ctx"

 - so.. should pass LibHalContext *ctx to hal_save_device() and
   hal_load_device()

 - also, hal_load_device() should error out if the device is not there
   instead of creating it

 - hal_load_device() should use LibHalChangeSet to reduce number of
   round trips.

 - should be able to do both hal_load_device() and hal_save_device()
   with only a single roundtrip (e.g libhal_device_get_all_properties()
   and libhal_device_commit_changeset())

 - hal_save_device() should take a parameter, "prefix" such that only
   properties starting with value of "prefix" are saved

 - should be part of libhal (some day we might move libhal functions
   that require uid 0 to a new libhal-private that addons etc. use)

 - need to return some bool whether it succeeded or not. Suggest to
   fill in DBusError for process local errors too, e.g. can not
   write to file etc.

 - should use new more descriptive names
   - gboolean libhal_device_save_properties (LibHalContext *ctx, 
                                             const char *udi,
                                             const char *prefix,
                                             DBusError *error)
   - gboolean libhal_device_load_properties (LibHalContext *ctx,
                                             const char *udi,
                                             DBusError *error)

 - should add functioning for deleting persistent properties on disk
   - gboolean libhal_device_delete_properties (LibHalContext *ctx,
                                               const char *udi)

    David




More information about the hal mailing list