After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784533 - Add support for UDF file system
Add support for UDF file system
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal enhancement
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2017-07-05 00:17 UTC by Pali
Modified: 2017-08-07 17:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] Add support for UDF file system (12.70 KB, text/plain)
2017-07-05 00:17 UTC, Pali
  Details
[PATCH v2 1/2] Add support for UDF file system (#784533) (12.34 KB, patch)
2017-07-07 17:08 UTC, Pali
none Details | Review
[PATCH v2 2/2] Do not show error message about missing software package when there is no such software (2.79 KB, patch)
2017-07-07 17:09 UTC, Pali
none Details | Review
Add support for UDF file system (v3) (14.26 KB, patch)
2017-07-09 11:55 UTC, Mike Fleetwood
none Details | Review
[PATCH] Add support for long UDF labels and add check for old mkudffs version (5.90 KB, patch)
2017-07-23 12:31 UTC, Pali
none Details | Review
Add support for long UDF labels etc. (v2) (12.56 KB, patch)
2017-07-29 08:43 UTC, Mike Fleetwood
none Details | Review

Description Pali 2017-07-05 00:17:23 UTC
Created attachment 354904 [details]
[PATCH] Add support for UDF file system

This patch adds support for detecting UDF file system and allow formatting
hard disk partition to UDF file system of revision 2.01. Formatting optical
disks or any other media types is not supported yet. Changing label or
UUID after formatting is not supported too as there is no tool for it yet.
Comment 1 Pali 2017-07-05 00:42:08 UTC
There is one big limitation in GParted: file system needs to be created on partitioned disk (e.g. MBR, GPT). But UDF is supposed to be formatted on the whole disk device without any partition scheme/header.

Apple Mac OS X systems enforce this rule and reject to UDF file system unless it is formatted on the whole unpartitioned hard disk. To have things complicated, Microsoft Windows systems do not detect any file system on *non*-removable devices unless it MBR or GPT partitioned. Removable devices could be without partition scheme. Linux do not have any limitation.

To achieve maximal compatibility between operating systems, removable devices should be formatted to UDF without MBR/GPT partitioning. For non-removable devices can be misused fact that UDF file system has reserved first 32kB for boot area and fact that MBR partition can starts at sector 0. This allows to store MBR table with one partition which covers whole disk device, starting at sector 0 as MBR is smaller then 32kB (boot area, which is not used by UDF).

mkudffs 1.4 would have format option --bootarea=mbr to generate such MBR table.

For better OS compatibility, GParted should allow to format whole disk to allowed file systems without partitioning it and possible warn people if they try to format partition to UDF, because it would not be detected by Apple systems.
Comment 2 Mike Fleetwood 2017-07-05 14:19:05 UTC
Hi Pali,

Thank you for going to the effort of producing a patch to support UDF.


Firstly please note that GParted since 0.22.0 does supports file systems
on unpartitioned drives.
https://git.gnome.org/browse/gparted/tree/NEWS?h=GPARTED_0_22_0

The GParted Manual explains how to do this:
http://gparted.org/display-doc.php?name=help-manual#gparted-create-partition-table
"Note
 To use a disk without a partition table, choose loop to create a
 virtual partition that spans the disk.  Then format to the desired file
 system.


I'll have a look at your patch.

Thanks,
Mike
Comment 3 Pali 2017-07-05 15:18:40 UTC
I'm using version from git. I tried that "loop" partition table, but after I formatted disk as UDF, then gparted detected it again as unformatted and have not seen UDF file system on it... Even it is there.

But first step is support for UDF with MBR and GPT which is enough for Linux & Windows compatibility.
Comment 4 Mike Fleetwood 2017-07-07 09:14:32 UTC
Question:
From my testing it appears that mkudffs automatically detects the block
size of the device and uses it by default.  Even with old udftools
1.0.0b3 from 2002.  Is it necessary to specify the block size?


That is a good patch for a first go.  Just a few items to address:

1) Put GParted_Core::set_used_sectors() change into a separate commit
   with appropriate commit message explaining why.  It is not directly
   related to adding udf support.  Use this wording for the user
   message:
      _("Reading the usage of this file system is only supported while
      mounted.")

2) Change MIN/MAX_UDF_BLOCKS* #defines to C++ constants of type
   Byte_Value.

3) Add translation entry into po/POTFILES.in
      src/udf.cc

4) In the partition too large/small user messages change "maximal" and
   "minimal" to "maximum" and "minimum".


There are a few minor issues I'll just change when committing so you
don't need to bother with:
* Add bug references to commit message as per examples in git
* Darken the colour for UDF to #0E5B0E to better distinguish from EXFAT


Thanks,
Mike
Comment 5 Pali 2017-07-07 09:39:27 UTC
> From my testing it appears that mkudffs automatically detects the block
size of the device and uses it by default.  Even with old udftools
1.0.0b3 from 2002.

Nope it does not. Support for blocksize detection was added in commit https://github.com/pali/udftools/commit/cfbdf06250ebe728195f4309357e532d4ce84666 and it was after udftools was moved from sourceforge. It was introduced in udftools 1.1.

> Is it necessary to specify the block size?

As more linux distributions still uses udftools from sourceforge, yes it is needed to format disk to UDF correctly.

Another option would be to add check that mkudffs is at least in version 1.1 to prevent generating incorrect file system. But specifying blocksize manually in gparted is easier as writie runtime check for mkudffs 1.1 in gparted.

1) OK

2) Is Byte_Value storage large enough to store 2^32-1?

3) OK

4) OK

About color, choose which you want, I do not mind.
Comment 6 Mike Fleetwood 2017-07-07 09:58:43 UTC
OK.  Keep the block size setting on the mkudffs command line.

Yes, Byte_Value is large enough to store 2^32-1.

$ fgrep Byte_Value include/Utils.h
typedef long long Byte_Value;
const Byte_Value KIBIBYTE=1024;
const Byte_Value MEBIBYTE=(KIBIBYTE * KIBIBYTE);
const Byte_Value GIBIBYTE=(MEBIBYTE * KIBIBYTE);
const Byte_Value TEBIBYTE=(GIBIBYTE * KIBIBYTE);
const Byte_Value PEBIBYTE=(TEBIBYTE * KIBIBYTE);
const Byte_Value EXBIBYTE=(PEBIBYTE * KIBIBYTE);
Comment 7 Pali 2017-07-07 17:08:38 UTC
Created attachment 355103 [details] [review]
[PATCH v2 1/2] Add support for UDF file system (#784533)
Comment 8 Pali 2017-07-07 17:09:52 UTC
Created attachment 355104 [details] [review]
[PATCH v2 2/2] Do not show error message about missing software package when there is no such software
Comment 9 Pali 2017-07-07 17:10:32 UTC
Fixed everything, new patches are attached.
Comment 10 Mike Fleetwood 2017-07-09 11:55:49 UTC
Created attachment 355206 [details] [review]
Add support for UDF file system (v3)

For future reference you can create a single file containing multiple
git commits like this, so you only need to upload a single file to
bugzilla:
    git format-patch --stdout FROMREV[..TOREV] > mychanges

For patch v2 2/2 from comment 8:
I only intended you to replace the user message text, not the associated
logic.  Anyway, the logic doesn't work from either patch v1 or v2.  If
the software package is not installed for a supported file system then
it is now saying:
  Reading the usage of this file system is only supported while mounted.
and not:
  The cause might be a missing software package.
  The following list of software packages is required for ${FS} file
  system support: ${PACKAGES}.
I don't see a simple way GParted can differentiate between software to
read FS block usage is not found so not installed and there isn't any
software (or GParted doesn't yet support it) to read the FS block usage.

Attached is patchset v3.  In the absence of being able to resolve this I
intend to only commit the first patch.
Comment 11 Pali 2017-07-09 12:32:57 UTC
Ok, patch v3 looks good.
Comment 12 Mike Fleetwood 2017-07-10 18:26:05 UTC
Thank you Pali,  The following relevant commits has been pushed to the
GParted GIT repo to add UDF support and recognise your contribution.

Add support for UDF file system (#784533)
https://git.gnome.org/browse/gparted/commit/?id=5f327feb25bcb8b55ffa2eab1bb86d5f75d75fba

Update AUTHORS file and Help > About > Credits
https://git.gnome.org/browse/gparted/commit/?id=62a7465fd24bb3c6333cf7c223003c6fd37c1da3
Comment 13 Pali 2017-07-23 12:31:18 UTC
Created attachment 356229 [details] [review]
[PATCH] Add support for long UDF labels and add check for old mkudffs version

Here is patch which address FIXME that older mkudffs versions damage label and also it enables support for long 126-characters labels.
Comment 14 Mike Fleetwood 2017-07-24 19:25:12 UTC
That label length code is complicated and the rules being implemented
hard to understand.

So just checking that the UDF label can either be encoded as Latin1 or
UCS-2 and have these maximum sizes?

Encoding | Logical Volume ID (--lvid) | Volume ID (--vid)
---------|----------------------------|------------------
 Latin1  |                        126 |                30
 UCS-2   |                         63 |                15

Also what are the maximum label sizes with mkudffs pre v1.1 with ASCII
only characters?
Comment 15 Pali 2017-07-24 19:42:32 UTC
(In reply to Mike Fleetwood from comment #14)
> That label length code is complicated and the rules being implemented
> hard to understand.
> 
> So just checking that the UDF label can either be encoded as Latin1 or
> UCS-2 and have these maximum sizes?
> 
> Encoding | Logical Volume ID (--lvid) | Volume ID (--vid)
> ---------|----------------------------|------------------
>  Latin1  |                        126 |                30
>  UCS-2   |                         63 |                15

Right!

> Also what are the maximum label sizes with mkudffs pre v1.1 with ASCII
> only characters?

Same as for Latin1, 126 and 30.

mkudffs takes UTF-8 strings in argv[] (when --utf8 is specified in argv[1]) and re-decodes them either to Latin1 (8bit OSTA Compressed Unicode) or UCS-2BE (16bit OSTA Compresses Unicode). Latin1 is preferred if there are only U+01 ... U+FF code points.

mkudffs pre v1.1 has bug in UTF-8 decoder which cause that only UTF-8 invariants (ASCII chars U+01 ... U+7F) are read correctly.
Comment 16 Mike Fleetwood 2017-07-24 20:10:59 UTC
Is all that complicated label code just doing this?

If old mkudffs and non-ASCII chars
    error
vid = label.substr(0,30)
if vid contains any chars outside Latin1
    vid = label.substr(0,15)
Comment 17 Pali 2017-07-24 20:43:01 UTC
(In reply to Mike Fleetwood from comment #16)
> Is all that complicated label code just doing this?
> 
> If old mkudffs and non-ASCII chars
>     error

Yes

> vid = label.substr(0,30)
> if vid contains any chars outside Latin1
>     vid = label.substr(0,15)

Not exaclty, it is something like:

vid = label.substr(0, 30)
if vid contains any char outside Latin1:
    if that char outside Latin1 is at position 0..14:
        vid = vid.substr(0, 15)
    else
        vid = prefix of vid with only Latin1 chars (prefix length: 15..29)

What I want to achieve is to put as larger as possible prefix of label into vid.

And I'm not sure if Glib::ustring substr function work also with if parameter range is out of string (e.g. if substr(0,N) works also for strings which length is << N). Glib documentation does not say it and after quick look into source code it looks like that N >= length must pass...

Anyway, if you have better idea how to rewrite it, I'm fine with ut.
Comment 18 Mike Fleetwood 2017-07-29 08:43:08 UTC
Created attachment 356548 [details] [review]
Add support for long UDF labels etc. (v2)

Here is UDF long label support patchset v2.  It makes the following
changes to Pali's patch from comment 13:

1) Initialise old_mkudffs in udf constructor rather than leaving it
   undefined.

2) Calls "mkudffs --help" with use_C_locale=true in
   udf::get_filesystem_support().

3) Removes unnecessary "\n" between output+error when searching output
   from "mkudffs --help".

4) Wordsmiths the comments a bit.

Also adds a patch by me to simplify the logic manipulating the label
using two helper functions: contains_only_ascii() and
find_first_non_latin1() which stops my head hurting understanding the
code to truncate the label for the VID.


Tested successfully on Fedora 26 and CentOS.  Errors on non-ascii label
with old mkudffs and truncates the label to fit in the 30 byte VID as
intended.


Will be committing these patches in a couple of days unless I here
otherwise.
Comment 19 Curtis Gedak 2017-07-29 15:36:56 UTC
Hi Mike,

My testing of patch set v2 from comment #18 has gone well on kubuntu 16.04.

I found only one issue with a comment in the first patch.  The last paragraph of the comment doesn't read quite right.

----- Begin comment -----

Because versions of mkudffs prior to 1.1 damage the label if it contains
non-ASCII characters, make sure GParted does not such versions of
                                               ^^^ 
mkudffs with a non-ASCII character label.

----- End comment -----

Other than that, all looks good to me.

Curtis
Comment 20 Pali 2017-07-30 08:27:42 UTC
Looks good, thank you!
Comment 21 Mike Fleetwood 2017-07-30 08:52:39 UTC
Thank you both for checking the code.  I've fixed that commit message
with:
    ... make sure GParted does not call such versions of ...
                                   ++++

The following commits have been pushed to the public GIT repo ready for
inclusion in the next release of GParted.

Add support for long UDF labels and check for old versions of mkudffs (#784533)
https://git.gnome.org/browse/gparted/commit/?id=861bc8df5d65c1df500161fc9904b0285773357c

Refactor UDF label manipulation code (#784533)
https://git.gnome.org/browse/gparted/commit/?id=a0c44d766d44afbde6b0c729125223c06b944278
Comment 22 Curtis Gedak 2017-08-07 17:22:36 UTC
This enhancement was included in the GParted 0.29.0 release on August 7, 2017.