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 673246 - Attempt Data Rescue - Failed to open read-only view
Attempt Data Rescue - Failed to open read-only view
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
0.12.0
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2012-03-31 17:12 UTC by Curtis Gedak
Modified: 2012-04-09 20:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GParted attempt data rescue - failed to open read only view (82.34 KB, image/png)
2012-03-31 17:12 UTC, Curtis Gedak
  Details
Dialog_Rescue_Data::on_view_clicked(): String::ucompose() no longer used due to a bug with some locales (1.03 KB, patch)
2012-04-03 18:18 UTC, Joan Lledó
none Details | Review

Description Curtis Gedak 2012-03-31 17:12:45 UTC
Created attachment 211035 [details]
GParted attempt data rescue - failed to open read only view

The problem occurs when using the menu option "Device --> Attempt Data Rescue".  More specifically when a file system is found and you click on the "View" button, GParted always seems to fail to create the Read-Only view.

I have performed some preliminary debugging and discovered the following problem with the mount command:

# mount -o ro,loop,offset=1,048,576,sizelimit=24,117,248 /dev/sde /tmp/gparted-roview-JvVarj
mount: you must specify the filesystem type
#

In the above case I was trying to rescue an NTFS file system.

A similar error occurs when I try to rescue an ext2 file system.

The testing was performed on kubuntu 11.04 with Linux kernel 2.6.38.x and the mount command from util-linux-ng 2.17.2.  The same problem occurs using recent GParted Live releases such as 0.12.0-5.

My suspicion is that something might have changed in the way the mount command works, and now it seems to require the file system type for commands such as listed above.

Fixing the problem might involve adding a "-t FS-TYPE" option to the command line.
Comment 1 Joan Lledó 2012-04-01 10:02:46 UTC
Hi Curtis, I couldn't reproduce the error on a Debian squeeze whit the same version of util-linux. I've been taking a look at the code and I have a question:

- In Win_GParted::thread_mount_partition() you're using this for mount a partition:

*succes = ! Utils::execute_command( "mount -v " + selected_partition .get_path() + " \"" + mountpoint + "\"",  dummy, *error );

This code doesn't fail also? This piece of code is not providing the -t option to the mount command, like the Dialog_Rescue_Data::on_view_clicked() function where the error os ocurring.
Comment 2 Curtis Gedak 2012-04-01 18:12:49 UTC
(In reply to comment #1)
> - In Win_GParted::thread_mount_partition() you're using this for mount a
> partition:
> 
> *succes = ! Utils::execute_command( "mount -v " + selected_partition
> .get_path() + " \"" + mountpoint + "\"",  dummy, *error );
> 
> This code doesn't fail also? This piece of code is not providing the -t option
> to the mount command, like the Dialog_Rescue_Data::on_view_clicked() function
> where the error os ocurring.

This code works properly, but is sort of a special case.  The only way that GParted knows how to mount a partition is if there is an entry for it in /etc/fstab.  Hence the partitions with mount actions will all have entries in /etc/fstab.

Regarding the problem I have done some more testing using both Debian (I think Squeeze) with Linux kernel 2.6.32-5-686 in a Virtual Machine, and also with my Kubuntu 11.04 installation.

Both of these work correctly with GParted versions 0.8.0 up to and including 0.9.1.  Starting with version 0.10.0 is where the problem appears to have crept in.

One of the relevant changes made between version 0.9.1 and 0.10.0 that might impact "attempt data rescue" is the update of String::ucompose().


Update String::ucompose library to version 1.0.5
http://git.gnome.org/browse/gparted/commit/?id=a739afc9a1d1d94a8c95446db762db93cff15572


My default language is set to en_CA.  Coincidentally, the mount command in Dialog_Rescue_Data::on_view_clicked() uses String::ucompose().  So I tried "export LANG=C", tested again, and surprise it worked!

If you set "export LANG=en_CA", do gparted versions >= 0.10.0 fail for you?

If so, then perhaps we should not build command lines using the String::ucompose() method?
Comment 3 Joan Lledó 2012-04-02 21:49:35 UTC
Hi Curtis, indeed, using LANG=en_CA fails on my computer. I test also with LANG=en_US and the error persists, but if I change to ca_ES or es_ES the problem goes away.

Following your link, it seems that the only differences between the versions 1.0.4 and 1.0.5 of the compose library are two little changes, and only one related with the locales. This one:

#if __GNUC__ >= 3
- //see #157871
- //os.imbue(std::locale("")); // use the user's locale for the stream
+ os.imbue(std::locale("")); // use the user's locale for the stream
#endif

I try to re-comment this line and all works correctly again...

Using the debugger I catch the bug, look at this two  mount commands:

1) mount -o ro,loop,offset=42991616,sizelimit=61865984 /dev/sdf /tmp/gparted-roview-l2nr0k

2) mount -o ro,loop,offset=42,991,616,sizelimit=61,865,984 /dev/sdf /tmp/gparted-roview-lEBF3i

The first one, generated by gparted using ca_ES works ok. The second one, using en_US fails with the "specify the filesystem type" message.

The problem is the ucompose function formats the numbers passed as parameters by adding the coma as thousands separator. So the problem is not in gparted, but we have 3 solutions:

1) Go back to the 1.0.4 version of the compose library
2) Comment the line like I did above
3) Don't use ucompose for this purpose

What do you think?
Comment 4 Curtis Gedak 2012-04-02 22:37:22 UTC
Thank you Joan for testing and confirming the problem.

My preference would be option 3 because in text string situations it would be useful to have the correct format for each language.  Hence I would like to keep the new String::ucompose mthod.

For command lines we do not wish anything to change the values.

I think there might be some other commands that use String::ucompose so these should be looked at some time in the future too.

Would you like to change the command line to not use ucompose?
Comment 5 Joan Lledó 2012-04-03 09:46:33 UTC
I will use std::stringstream for this purpose. Do you have any other option?
Comment 6 Curtis Gedak 2012-04-03 15:17:54 UTC
There is a Utils::num_to_str() method to convert from numbers to string.
Coincidentally it uses the same std::stringstream idea as you had.  :-)

You can see it in use on the btrfs::resize() and ext2::resize() methods.
Comment 7 Joan Lledó 2012-04-03 18:18:19 UTC
Created attachment 211240 [details] [review]
Dialog_Rescue_Data::on_view_clicked(): String::ucompose() no longer used due to a bug with some locales

Patch: Dialog_Rescue_Data::on_view_clicked(): String::ucompose() no longer used due to a bug with some locales
Comment 8 Curtis Gedak 2012-04-03 18:45:41 UTC
Thank you Joan for this patch to address the problem.  I hope you don't mind but I took the liberty placing a new title on the git commit so that it was less than 80 characters.

Your patch has been committed to the git repository for inclusion in the next release of GParted.

The relevant git commit can be viewed at the following link:

Fix attempt data rescue fail to open read-only view (#673246)
http://git.gnome.org/browse/gparted/commit/?id=e494eca1f7798d98cc4e83fb2875f1c409d0ac98
Comment 9 Joan Lledó 2012-04-03 20:22:13 UTC
Sure, no problem.
Comment 10 Curtis Gedak 2012-04-09 20:15:12 UTC
The enhancements for this bug report have been included in GParted 0.12.1 released on April 9, 2012.