Bug 52332

Summary: Switch away from mtools (to dosfstools) for changing/creating VFAT labels
Product: udisks Reporter: Michael Biebl <mbiebl>
Component: generalAssignee: David Zeuthen (not reading bugmail) <zeuthen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: zeuthen
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: [PATCH] Use dosfstools instead of mtools
[PATCH] Use dosfstools instead of mtools

Description Michael Biebl 2012-07-21 05:48:08 UTC
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=680683

We already require dosfstools for mkfs.vfat. There were bug fixes in dosfslabel so it now seems to work at least as well as mlabel (from mtools) so we should consider dropping this additinal dependency and switching back to dosfslabel.

For a full discussion, see the above Debian bug report. The Debian package was switched to use dosfslabel.

There were some encoding issues with both mlabel and dosfslabel when I created labels with German umlauts under Linux and using them under Windows.
This probably needs further investigation/fixes in dosfstools. I would prefer that approach over implementing that functinality in udisks directly.
Comment 1 David Zeuthen (not reading bugmail) 2012-09-28 16:54:55 UTC
Sounds good, I'd love to get the VFAT label encoding problems fixed once and for all. How do we proceed?
Comment 2 David Zeuthen (not reading bugmail) 2012-09-28 17:41:56 UTC
*** Bug 20983 has been marked as a duplicate of this bug. ***
Comment 3 Michael Biebl 2013-01-31 01:14:38 UTC
(In reply to comment #1)
> Sounds good, I'd love to get the VFAT label encoding problems fixed once and
> for all. How do we proceed?

Attached is the patch we currently use in Debian.
Comment 4 Michael Biebl 2013-01-31 01:15:21 UTC
Created attachment 73968 [details] [review]
[PATCH] Use dosfstools instead of mtools
Comment 5 Martin Pitt 2013-05-24 07:27:42 UTC
This now fails with current dosfstools 3.0.16:

ERROR: test_vfat (__main__.FS)
fs: FAT
----------------------------------------------------------------------
Traceback (most recent call last):
  File "src/tests/integration-test", line 655, in test_vfat
    self._do_fs_check('vfat')
  File "src/tests/integration-test", line 834, in _do_fs_check
    self._do_udisks_check(type)
  File "src/tests/integration-test", line 978, in _do_udisks_check
    fs.call_set_label_sync(l, no_options, None)
  File "/usr/lib/python3/dist-packages/gi/types.py", line 113, in function
    return info.invoke(*args, **kwargs)
gi._glib.GError: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error setting label: Command-line `dosfslabel "/dev/sdb" "n@a$me"' exited with non-zero exit status 1: dosfslabel: labels cannot contain lower case characters

VFAT has always worked with uppercase characters only, but current dosfstools now enforces that. So we should do that conversion in udisks, or return an error in that case.
Comment 6 Martin Pitt 2013-05-24 07:29:14 UTC
Created attachment 79740 [details] [review]
[PATCH] Use dosfstools instead of mtools

Updated patch. Note that this still fails further on when trying to clear the label:

  File "src/tests/integration-test", line 1001, in _do_udisks_check
    fs.call_set_label_sync('', no_options, None)
  File "/usr/lib/python3/dist-packages/gi/types.py", line 113, in function
    return info.invoke(*args, **kwargs)
gi._glib.GError: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error setting label: Command-line `dosfslabel "/dev/sdb" ""' exited with non-zero exit status 1: dosfslabel: labels cannot contain lower case characters

But this is a bug in dosfstools itself:

# dosfslabel /dev/sdb ''
dosfslabel: labels cannot contain lower case characters

I'll report/fix that now.
Comment 7 Martin Pitt 2013-05-24 07:47:47 UTC
(In reply to comment #6)
> But this is a bug in dosfstools itself:
> 
> # dosfslabel /dev/sdb ''
> dosfslabel: labels cannot contain lower case characters
> 
> I'll report/fix that now.

Done, bug/patch at http://bugs.debian.org/709587 (the Debian maintainer is the upstream).

So wrt. udisks2, with this fix (or dosfstools <= 3.0.14) the tests are all happy again.

David, do you have any objections to switching to dosfstools for VFAT labels? It would be nice to drop the mtools dependency.
Comment 8 Martin Pitt 2013-11-25 07:14:24 UTC
This was fixed in dosfstools 3.0.18 back in June, I think it's now time to push this.

http://cgit.freedesktop.org/udisks/commit/?id=306b56b7

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.