Created attachment 106488 [details]
k850i-mtp-mode-listing.txt

Yeah, this would be nice to add. There's a few complications here

 - We want a separate GVolume for each storage head
   - means the volume monitor bits need to know about this
   - means right now linking the volume monitor with libgphoto2
   - this can't work; the vm bits are in-process and only a single
     process can access the device
   - so having the volume monitor out-of-process is a 
     necessity for this to work. That's bug 520132

 - So with two GVolume objects; one for each head
   - we would have two GMount objects when both heads are mounted
   - however the code for these needs to run in the same process
    - Since they need to share access to the device
    - I think that is possible today with gvfs otherwise we need to fix it

Btw, the only thing that breaks with this is x-content/* detection of DCIM/ and other x-content/* types.

Bastien, is it possible for you to hack the gvfsbackendgphoto2.c file to print out information about all the heads cf.

http://www.gphoto.org/doc/api/struct__CameraStorageInformation.html

and paste it here? Should be easy to add to do_query_fs_info(). Thanks!

Created attachment 106497 [details] [review]
gvfs-gphoto-multi-storage-debug.patch

And replace the [0] with [i] obviously, stoopid cut'n'paste:

** Message: storage 0
** Message: basedir: /store_00010001
** Message: label: '' description: 'Phone Memory'
** Message: type: 3 fstype: 2 accesstype: 0

** Message: storage 1
** Message: basedir: /store_00020001
** Message: label: '' description: 'Memory Stick (TM)'
** Message: type: 4 fstype: 2 accesstype: 0

Let's move the problem of moving the hal monitor out of process aside for this bug. It's necessary for when we want to merge the code, but not for testing.

I've fudged the hal monitor into creating 2 volumes (I'm not sure that's what we want, we'd probably rather have 2 mounts, for the same volume, but gvfs/gio don't allow that).

But how do I make sure that:
1) the mounts have the right roots
2) both mounts are handled by the same backend

The gphoto2 volume monitor is now a separate remote volume monitor in trunk - so it's now easy to make it use gphoto2 to probe for multiple storage heads.

2008-07-21  David Zeuthen  <davidz@redhat.com>

	* configure.ac:
	* monitor/Makefile.am:
	* monitor/hal/*:
	* monitor/gphoto2/*:
	Separate the hal and gphoto2 volume monitors - this is in part
	needed to solve bug #520123.

Note that the problem remaining is being able to handle 2 mounts from a single backend. At the same time, we don't want _all_ the gphoto mounts to be handled by the same backend (as the HTTP daemon does right now).

(In reply to comment #9)
> Note that the problem remaining is being able to handle 2 mounts from a single
> backend. 

Right.

> At the same time, we don't want _all_ the gphoto mounts to be handled
> by the same backend (as the HTTP daemon does right now).

I'm actually not sure this would be a huge problem:

 - you rarely have more than one PTP/MTP device attached
 - gphoto2 fails anyway
   - http://sourceforge.net/tracker/?func=detail&atid=108874&aid=1904987&group_id=8874
 - performance may suffer due to how the libgphoto2 API works
   - but you rarely need to do IO on multiple devices at the same time
   - some day we might switch to another PTP/MTP library with a proper async API

so I think, based on this, we just make the gphoto2 mount daemon handle all mounts. Thoughts?

(In reply to comment #10)
<snip>
> so I think, based on this, we just make the gphoto2 mount daemon handle all
> mounts. Thoughts?

Already working towards that now, if only as a stop-gap. If it ends up being good enough, then we'll see :)

Created attachment 119943 [details] [review]
First pass

This is a first pass at multi storage head support.

A couple of questions:
- Not really sure I'm using the libgphoto2 API properly using the same context and camera. I wonder what libgphoto2's locking is like, if any, otherwise we'll need to lock every call to the device
- The head detection part of the gphoto2 volume monitor needs proper implementing
- Is that piece of code supposed to avoid mounting music players for Rhythmbox/Banshee's sake?:
   if (!hal_device_has_capability (device, "camera") ||
       (hal_device_has_capability (device, "portable_audio_player") &&
       hal_device_get_property_bool (device, "camera.libgphoto2.support")))

I forgot to mention:
- free/used space query only ever queries the first storage head (easy to fix, got a patch here)
- fuse name, and UI names need fixing to use the storage description

Hold on; I have an (almost working) patch-set that allows multiple GVolume objects to share the same GMount. It turns out we need that anyway for a better "Connect to server" feature (two "servers" (e.g. "My Movies", "My Home Directory on quad.local") pointing to the underlying GMount just different directories).

Anyway, with that feature I envision the gphoto2 volume monitor can simply create two GVolume objects that point to a single mount.

Will follow up with a link to the patches shortly.

Here's the patches along with a description of the changes

 http://mail.gnome.org/archives/gvfs-list/2008-October/msg00002.html

Created attachment 120007 [details] [review]
And we'd use a patch like that to detect the storage heads

Needs a bit of cleaning up.

Created attachment 123746 [details] [review]
gvfs-gphoto-multihead-1.patch

First patch ready for mainline.

Yay, am happy to see that ignore_prefix code go; it really added complexity. We only call get_stores_for_camera() for new devices right? If so, the patch looks good.

Btw, it's also worth checking that there's no race condition in Nautilus if you use a PTP device without any stores (e.g. when the GDaemonMount and GProxyMount both have a DCIM folder in their mount root); e.g. that you won't get two "hai! you haz new photos" dialogs...

(In reply to comment #18)
> Yay, am happy to see that ignore_prefix code go; it really added complexity. We
> only call get_stores_for_camera() for new devices right? If so, the patch looks
> good.

Yep.

> Btw, it's also worth checking that there's no race condition in Nautilus if you
> use a PTP device without any stores (e.g. when the GDaemonMount and GProxyMount
> both have a DCIM folder in their mount root); e.g. that you won't get two "hai!
> you haz new photos" dialogs...

It's also worth noting that all the devices actually have stores, they just weren't visible underneath the hacks in the gphoto2 backend.

I haven't looked much at the gphoto code, so I'm not an ideal reviewer. However, this seems like a nice cleanup and should work. So, please commit. If there are any issues we can fix them when they show up.

Hopefully, nothing's broken :)

2008-12-09  Bastien Nocera  <hadess@hadess.net>

	* monitor/gphoto2/Makefile.am:
	* monitor/gphoto2/ggphoto2volumemonitor.c (get_stores_for_camera),
	(update_cameras): For each camera discovered, create shadow mounts
	for each one of the top-level storage heads, makes each of the
	device's stores appear separately

	* daemon/gvfsbackendgphoto2.c (monitors_emit_internal),
	(release_device), (split_filename), (file_get_info), (do_mount),
	(do_open_for_read_real), (do_query_info), (try_query_info),
	(do_enumerate), (try_enumerate), (do_make_directory),
	(do_set_display_name), (do_delete), (do_create_internal),
	(do_replace), (do_append_to), (do_move), (do_create_dir_monitor),
	(do_create_file_monitor): Remove all the hacks to handle
	a single storage head differently from multiple storage heads,
	this makes photo directories detection and the likes work
	for multiple storage-heads

	(Closes: #520123)