Hello Buteo team!

I'm happy to report that Paul's gadgetfs-based transport for the Media
Transfer Protocol (MTP) passes initial tests and that we obtained
permission to publish the code. We'll do so shortly and hope to get this
included in buteo-mtp.

For those who are not aware of it, at the moment MTP does not work in
MeeGo because it relies on a Nokia kernel driver which Linux upstream
didn't want to see included in the Linux kernel. The new transport
instead relies on the gadgetfs API, which is part of upstream Linux.
There may be performance drawbacks (no tests done) when using gadgetfs,
but even slow performance would be better than no MTP at all ;-}

But there are still some significant gaps, so let me ask some questions
so that we can resolve those.

Permissions
===========

Currently the code runs as part of mtp_test when started as root. That's
necessary because /dev/gadget must be read/writable. But eventually the
code needs to run as plugin as normal user inside msycnd. Initially the
process accesses /dev/gadget/<driver> to configure USB, then some new
entries (like /dev/gadget/ep-a) which are created by gadgetfs based on
that configuration. We could do a "chmod" as root to grant the msyncd
process read/write permission for /dev/gadget/<driver>, but that doesn't
help with the additional files, which are exclusively for root. If
anyone knows how to solve this, ideally without patching the kernel,
please speak up...

Even if there is a solution, which permissions would be suitable? We
don't want to grant all user-space processes the right to modify the
device's USB config. Here's one possibility:
      * add a group "sync"
      * make msyncd setgid "sync"
      * grant the "sync" group read/write access

The drawback is that everything running inside msyncd, including
third-party plugins, will have the possibility to reconfigure USB. I
don't see how this can be avoided, except by separating different sync
sessions into different processes. Splitting out the setup phase into a
separate setuid or setgid binary does not work, because the MTP
transport in msyncd needs full read/write access to everything,
including the main /dev/gadget/<driver> file descriptor.

IMHO granting privileges as part of the MeeGo Security Framework has
similar issues and perhaps changing msyncd such that it runs each sync
inside a separate process would be worthwhile in the long run (also
better for stability), but this is clearly out of scope right now.

Starting the MTP server plugin
==============================

mtp.xml specifies:
<profile name="mtp" type="server" >
    <key name="usb_transport" value="true"/>
</profile>

Under which circumstances will that server be activated right now in
MeeGo's msyncd? If I read the buteo-syncfw code correctly, it would be
activated if USB gets connected, which in turn is detected if usbmoded
support is enabled - which as far as I know, is not the case in MeeGo
because usbmoded is not in MeeGo.

Would it be acceptable to create a pseudo Sync::CONNECTIVITY_ALWAYS
transport? A server plugin specifying that in its XML config then would
be activated as soon as msyncd starts and remain active as long as
msyncd runs, and mtp.xml could be changed to use that.

Paths, configuration
====================

Why is /usr/share/mtp/deviceinfo.xml copied
to /home/user/.mtpdeviceinfo.xml? The code says that it must be
writable, but it did not become clear to me why the file must be
writable:

    // Kludge : till we know how and where to securely install a file
    // that can be modifed by an apllication.
    QFile fileDst(m_devinceInfoXmlPath);
    QFile fileSrc("/usr/share/mtp/deviceinfo.xml");
    if( "/home/user/.mtpdeviceinfo.xml" == m_devinceInfoXmlPath && 
!fileDst.exists() )
    {
        fileSrc.copy(m_devinceInfoXmlPath);
    }

Without protecting this .mtpdeviceinfo.xml against writes from other
apps, any code running on the device can change the USB config. Can we
remove this kludge from MeeGo until this is settled, in favor of
reading /usr/share/mtp/deviceinfo.xml directly? Or at least hide it
behind an ifdef, so that it is not enabled in MeeGo?

/usr/share/mtp/deviceinfo.xml contains information like hardware vendor
and model. We need a way how this can be modified without recompiling
buteo-mtp. Perhaps the file could be installed in and read
from /etc/mtp, where device images or scripts can modify it?

The path under which files are made accessible via MTP is currently
hard-coded to /home/user/MyDocs. "PlayLists" as sub-directory of that is
created by the MTP code.

What is the purpose of "PlayLists", or in other words, how is it
special? How can the /home/user/MyDocs path be made configurable? Would
deviceinfo.xml be a good place?

"media" and "Phone Memory" are also hard-coded as description of that
directory. Are these presented to the MTP host?

What about providing access to more than one directory?

Are symbolic links followed? In other words, if I set up
  ~/MyDocs/
      Videos -> ~/Videos
      Pictures -> ~/Pictures
      Music -> ~/Music
      ...
would that provide access also to the content of ~/Videos and
~/Pictures?

The code currently tries to load the libfsstorage.so module
from /usr/share/mtp, but it is installed in /usr/lib/mtp, which is
indeed where it should be. Okay to change the code?

MTP transport API
=================

This is from an email that Paul sent privately earlier, still needs an
answer as far as I know.

---------------------> snip <--------------------------

I [Paul] ran into a design issue around Zero-Length-Packets while doing
this.  The USB spec (paraphrased) says that transfers exceeding 1 packet
size get transmitted and ended as follows:

() All packets up to the last shall be the maximum packet size.  If your
higher-level protocol lets you know the size of the transfer ahead of
time (e.g., fixed-size, info from a prior transaction, etc.) then you
are done when all the data is transferred.  No additional logic or
packets necessary.

() However, if you don't know the length ahead of time (e.g., the length
is variable, or the sender decides to truncate early) then the
transmission ends when a shorter-than-maximum packet is
transmitted/received.  This works great unless the whole transmission is
an exact multiple of the packet size in which case:

() A Zero-Length-Packet is transmitted/received to mark the end.  This
is really just a special case of the "short packet" rule preceeding.

A USB 2.0 clarification document points out that this same batch of
rules apply for bulk, interrupt, and control endpoint.  The issue
doesn't really arise for isochronous endpoints.  In the case of MTP
protocol, I think only the bulk endpoint(s) can actually encounter this
issue, since all the control and interrupt messages "fit" into a packet.

The MTPTransporter super-class defines a defaultable boolean in its
"sendData()" (bulk data) method to call for a a Zero-Length Packet
(comments are a bit ambiguous).  In the MTPTransporter subclasses, this
parameter has been turned into a "isLastPacket" boolean (undocumented).
Apparently the design intentions were as follows:

(1) Make the MTPResponder layer (above MTPTransporter) be independent of
what sort of transport is underneath.  For example, if it is Bluetooth
which supposedly doesn't have this Zero-Length-Packet nonsense, then
MTPResponder shouldn't have to know the difference or do anything
different.

(2) Make MTPTransporter know strictly about the transport, with no
knowledge of the higher-level protocol that it is transporting.

The trouble with this is that MTPTransporter (subclasses) then cannot
know when they should send a Zero-Length-Packet.  When sendData() gets
called with "isLastPacket" set to true, the implementation does not know
whether this is (1) the end of a known-length transmission so no ZLP is
required, or (2) the end of a unknown-length transmission so a ZLP is
required if we're right at a packet-size boundary.

I'm starting to come around to the belief that the MTPTransporter (sub)
class can't figure this out on its own.  I think the higher-level
MTPResponder really needs to know about the Zero-Length-Packet
requirement and tell MTPTransporter about it when needed.

I'm flexible how this could happen.  It could be (1) MTPResponder calls
sendData() with a pre-formatted Zero-Length-Packet when needed, (2)
sendData() adds a new "please add a ZLP" parameter and the Transporter
creates and sends one, (3) a whole new "sendZeroLengthPacket()" function
gets added to the MTPTransporter superclass, called by MTPResponder when
needed, and subclasses either send such packets or do a no-op depending
on actual underlying medium, (4) or some other cool solution.  Opinions?

---------------------> snip <--------------------------

-- 
Best Regards

Patrick Ohly
Senior Software Engineer

Intel GmbH
Open Source Technology Center               
Pützstr. 5                              Phone: +49-228-2493652
53129 Bonn
Germany


_______________________________________________
MeeGo-dev mailing list
MeeGo-dev@meego.com
http://lists.meego.com/listinfo/meego-dev

Reply via email to