Regression: vfat disks mounted with incorrect shortname setting

Bug #428174 reported by Malcolm Scott
52
This bug affects 9 people
Affects Status Importance Assigned to Milestone
obsolete
Fix Released
Medium
devicekit-disks (Ubuntu)
Medium
Martin Pitt
Karmic
Medium
Martin Pitt

Bug Description

Binary package hint: devicekit-disks

Since the move to DeviceKit for karmic, my removable VFAT-formatted disks are being mounted with shortnames=lower. This breaks e.g. rsync: if a file called "FOO" is created, it will actually appear named "foo". Past bugs -- e.g. bug 15525 and bug 76200, amongst others -- concluded that shortnames=winnt (or perhaps shortnames=mixed) was the correct setting, and this was indeed the case in jaunty.

shortnames=lower appears to be set in devicekit-disks's src/devkit-disks-device.c.

Revision history for this message
Malcolm Scott (malcscott) wrote :

Apologies, the setting is "shortname" not "shortnames".

summary: - Regression: vfat disks mounted with incorrect shortnames setting
+ Regression: vfat disks mounted with incorrect shortname setting
Revision history for this message
Fraser Murray (fraserm) wrote :

I am also getting this.

Possibly related, setting the GConf key /system/storage/default_options/vfat/mount_options/ to shortname=mixed or shortname=winnt does not make any difference

Revision history for this message
Niels Kristian Bech Jensen (nkbjensen) wrote :

I can confirm this problem on karmic. Alle upper case file names shows up as lower case (i.e. "UPPER" becomes "upper") while mixed case file names stays unchanged.

Changed in devicekit-disks (Ubuntu):
status: New → Confirmed
Fraser Murray (fraserm)
Changed in devicekit-disks (Ubuntu):
assignee: nobody → fraser (fraser-m-murray)
status: Confirmed → In Progress
Revision history for this message
Fraser Murray (fraserm) wrote :

Assuming you know how to actually compile devicekit-disks properly, which I don't, obviously, I think this should work.

Revision history for this message
Fraser Murray (fraserm) wrote :

This does actually work.

Fraser Murray (fraserm)
Changed in devicekit-disks (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
In , Martin Pitt (pitti) wrote :

From Ubuntu bug report:

  "Since the move to DeviceKit for karmic, my removable VFAT-formatted disks are being mounted with shortnames=lower. This breaks e.g. rsync: if a file called "FOO" is created, it will actually appear named "foo". Past bugs -- e.g. https://launchpad.net/bugs/15525 and https://launchpad.net/bugs/76200, amongst others -- concluded that shortnames=winnt (or perhaps shortnames=mixed) was the correct setting, and this was indeed the case in earlier releases.

  shortnames=lower appears to be set in devicekit-disks's src/devkit-disks-device.c."

The proposed patch is pretty straightforward, just changing "shortname=lower" to "shortname=winnt" in src/devkit-disks-device.c. However, we should have a consensus about this and fix it upstream, or not at all.

Was this deliberate or just an oversight? Thanks!

Revision history for this message
In , Martin Pitt (pitti) wrote :

It's not actually a regression from an upstream perspective, though. In Ubuntu we had a gnome-mount patch to change to shortname=mixed by default. However, I'd really like to avoid piling up patches again. There must be a generally applicable default for this, after all.

Revision history for this message
Martin Pitt (pitti) wrote :

Can you please open a bug at https://bugs.freedesktop.org/enter_bug.cgi?product=DeviceKit-disks and discuss it there with David? I wouldn't like to go down the bad path of carring a mess of Ubuntu specific patches again wit DeviceKit-disks. Thanks!

Revision history for this message
Martin Pitt (pitti) wrote :

Nevermind, I filed it. Please feel free to subscribe there and join the discussion.

Revision history for this message
Martin Pitt (pitti) wrote :

I unsubscribed sponsors, I'll upload this once we dicsussed it upstream. Thanks!

Changed in devicekit-disks (Ubuntu):
status: Fix Committed → Triaged
Changed in devicekit:
status: Unknown → Confirmed
Revision history for this message
In , Zeuthen (zeuthen) wrote :

Hmm, it's shortname=lower because that's what the (upstream) kernel defaults to, see Documentation/filesystems/vfat.txt. If we change it, then e.g. various (obscure?) rsync usage patterns are fixed - but what do we break?

Revision history for this message
In , Martin Pitt (pitti) wrote :

It's not that obscure, though. With "lower", you don't preserve case:

$ rm *; touch foo; ls
foo
$ rm *; touch FOO; ls
foo

but with "winnt" you do:

$ rm *; touch foo; ls
foo
$ rm *; touch FOO; ls
FOO

If you want to open a particular file, both variants are clever enough to ignore case:

$ rm *; echo xx > FOO; cat foo
xx
$ rm *; echo xx > foo; cat FOO
xx

So we don't break file access, and behaviour for lower-case and MixedCase is identical, it just fixes ls/readdir() for ALLUPPERCASE file names.

Revision history for this message
In , Martin Pitt (pitti) wrote :

So obviously this change would break software which relies on an all-uppercase file name being actually saved as all-lowercase name.

The other issue is that if you create a file in all uppercase under Windows XP, it gets displayed in all-lowercase in Linux with "shortname=lower", whic is unexpected as well.

Revision history for this message
In , Martin Pitt (pitti) wrote :

I mailed the kernel vfat maintainer for input.

Revision history for this message
In , Martin Pitt (pitti) wrote :

Answer from the kernel vfat maintainer:

> Timely question :) Primary reason is historically compatible. However,
> at 2.6.32, I've decided to change default from "lower" to "mixed".
>
> 2.6.32 will use shortname=mixed as default.

Revision history for this message
In , Martin Pitt (pitti) wrote :
Revision history for this message
In , Martin Pitt (pitti) wrote :

Created an attachment (id=30176)
git formatted patch

Trivial patch, but for the sake of fast "git am" love and having a meaningful changelog (and also to make my local "git rebase" work :) ), I'll attach it here.

Revision history for this message
Martin Pitt (pitti) wrote :

I asked the kernel vfat maintainer, and he said that 2.6.32 will use "mixed" as default, so I think we should do that for Karmic as well.

Changed in devicekit-disks (Ubuntu Karmic):
assignee: Fraser Murray (fraserm) → Martin Pitt (pitti)
importance: Undecided → Medium
tags: added: regression-potential
Changed in devicekit-disks (Ubuntu Karmic):
status: Triaged → In Progress
Revision history for this message
Martin Pitt (pitti) wrote :

Patch sent to upstream for review.

Changed in devicekit-disks (Ubuntu Karmic):
status: In Progress → Fix Committed
Revision history for this message
In , Zeuthen (zeuthen) wrote :

Committed. Thanks for doing this.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package devicekit-disks - 007-1ubuntu1

---------------
devicekit-disks (007-1ubuntu1) karmic; urgency=low

  * Upload current Debian git head to pick up recent bug fixes.
  * debian/rules: Enable quilt patch system. Add quilt build dependency.
  * Add 01-mkfs-tempdir.patch: Daemon does not create /var/run/DeviceKit-disks/,
    so mkfs jobs fail. Just create the directory in /tmp, this is what /tmp is
    for, after all. (See https://bugs.freedesktop.org/show_bug.cgi?id=24265)
  * Add 00git-fix-inhibit.patch: Actually make Inhibit() work again. Taken
    from upstream git head. (LP: #428133)
  * Add 02-unlock-CD-trays-after-mounting.patch: Unlike in the hal world, we
    do not have a daemon polling CD drives for eject button presses. In order
    to make hardware tray eject buttons work, unlock the tray after
    mounting a CD. This is pretty much equivalent to yanking out USB sticks,
    which we already handle reasonably (detecting disappeared device,
    force-unmounting). (https://bugs.freedesktop.org/show_bug.cgi?id=24052,
    LP: #397734)
  * Add 03-fix-subsystem-check-for-firewire.patch: Firewire subsystem is
    called "ieee1394" in current Linux. Now check for both "ieee1394" and
    "firewire". This fixes firewire drives to not be considered system
    internal any more. (https://bugs.freedesktop.org/show_bug.cgi?id=24351,
    LP: #442604)
  * Add 04-mount-vfat-with-shortname-mixed-by-default.patch: The previous
    default, shortname=lower, breaks all-uppercase file names ("touch
    FOO" creates "foo"), thus breaks rsync, and Windows compatibility. The
    default was changed in the Linux kernel for 2.6.32 as well.
    (https://bugs.freedesktop.org/show_bug.cgi?id=24129, LP: #428174)
  * Add 05-dont-remove-NULL-values-from-hash-tables.patch: In device_remove(),
    device_file, object_path, or dev are sometimes NULL. Do not attempt to
    remove NULL from the hash tables, since this crashes. This is mainly a
    robustification patch, it does not really fix the underlying
    state bookkeeping. (http://bugs.freedesktop.org/show_bug.cgi?id=24264,
    LP: #414407)

 -- Martin Pitt <email address hidden> Fri, 09 Oct 2009 09:55:41 +0200

Changed in devicekit-disks (Ubuntu Karmic):
status: Fix Committed → Fix Released
Changed in devicekit:
status: Confirmed → Fix Released
Changed in devicekit:
importance: Unknown → Medium
Changed in devicekit:
importance: Medium → Unknown
Changed in devicekit:
importance: Unknown → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.