Re: [Utopia] Re: Debian/Ubuntu bug fixes for gnome-volume-manager

[Jeffrey Stedfast <fejj novell com>]

few comments below

On Tue, 2005-07-05 at 23:52 +0200, Martin Pitt wrote:
> Hi Jeffrey!
> 
> Jeffrey Stedfast [2005-07-05 11:55 -0400]:
> > On Mon, 2005-07-04 at 18:05 +0200, Martin Pitt wrote:
> > > 01_fix_nonmountable_media.patch:
> > >   - Fix a regression of 1.3.2, it does not autoplay audio CDs any
> > >     more. Do not require that a device is mountable for exercising the
> > >     removable media policy.
> > >   - This seems highly important to get upstream.
> > 
> > not quite the right fix because it breaks ipods (sorta - in that it will
> > try to mount /dev/sda1 inside gvm_media_changed which will always fail)
> > which is the reason I added the mountable check to the if-statement in
> > hal_device_added(). However my fix is obviously wrong too :)
> > 
> > I've fixed this a different way and am about to commit it
> 
> Thanks; I don't have an iPod to test it with...
> 
> > > 03_kernel_hint.patch:
> > sure, looks fine to me.
> 
> Thanks. :-)
> 
> > > 04_reconnect_on_dbus_exit.patch:
> > >   - Reconnect to dbus after restarting dbus and hal, instead of just
> > >     crashing.
> > >   - Important in package-based system when you want to continue to use
> > >     your Gnome session while doing a system upgrade.
> > 
> > concerned that this is the patch that is causing bugzilla to get flooded
> > with bug reports around dbus upgrades on ubuntu systems
> 
> Well, actually the bugs came from the time when we didn't have that
> patch yet since then g-v-m just crashed on dbus restarts. But as we
> already discussed in the other mail, if that is not a feature that
> should generally be on, and you think that making it a configure time
> option is too complicated, then I'm fine with maintaining it on my
> own.
> 
> > > 05_mount_at_start.patch:
> > >   - Do not mount devices at g-v-m start if g-v-properties configured
> > >     to not mount devices automatically.
> > 
> > would have been nicer if the coding style matched gvm and also didn't
> > use c99/c++ declare-variables-on-use tactics but the logic seems ok.
> > I'll fix it up and commit it.
> 
> Sorry, thanks.

not that big of a deal :)

> 
> > > 07_automount_enabled_hint.patch:
> > >   - Do not mount a device if storage.automount_enabled_hint == false
> > >     on the volume or drive.
> > >   - This is not really a bug fix, but rather an enhancement, see
> > >     http://mail.gnome.org/archives/utopia-list/2004-November/msg00003.html
> > >     and Gnome bug #155636.
> > 
> > +                       if (!gvm_get_automount_enabled_hint (udi))
> > +                           gvm_device_mount (udi, device, NULL);
> > +                       else
> > +                           dbg("%s has storage.automount_enabled_hint == false, not mounting\n", udi);
> >                         mounted_volumes_policy_queue = g_slist_append (mounted_volumes_policy_queue, g_strdup (udi));
> > 
> > I don't think we want to add the udi to the policy_queue if we don't
> > actually mount it.
> 
> Right, that wasn't intended. I don't think that it actually makes any
> difference, though.

yea, nothing major - it would just add a udi to the not-really-a-queue
policy_queue and it'd never get removed (not really a problem with the
average desktop but eventually it might grow large enough that it'd
consume a fair bit of memory unnecessarily).

I've actually killed off that policy_queue list in CVS anyway now, and
made it a simple hash table since really there was no point in it being
a linked list - it was only ever used as a lookup table - and a slow one
at that ;)

Jeff