Merge lp:~yuningdodo/ubuntu/trusty/usb-creator/usb-creator.lp1361474+lp1300361-recreate-udisks-client into lp:ubuntu/trusty-updates/usb-creator

[Yu Ning]

[Timo Jyrinki (timo-jyrinki)]

(a backport of 0.2.62/utopic and two lines from 0.2.60/utopic)

[lp:~yuningdodo/ubuntu/trusty/usb-creator/usb-creator.lp1361474+lp1300361-recreate-udisks-client]

87. By Yu Ning on 2014-10-09

HELPER: use parted instead of udisks2 to create the new partition table
since udisks2 uses wipefs to erase the old partition table, however
partition table erasing is not supported in wipefs yet in trusty, that
would cause a timed out error.

[Yu Ning (yuningdodo)]

Updated, instead of using udisks2 to format the old partition table now we use parted directly as a workaround for (LP: #1361474)

[Sebastien Bacher (seb128)]

Thanks for the work, the first part of the second blob doesn't seem to be in trunk, is that normal? Shouldn't be applied to the vivid version first then backported? Do we have a bug about that?

[Yu Ning (yuningdodo)]

Hi Sebastien, thanks very much for the review, the two related bugs are bug #1361474 and #1300361.

Actually rev86 is the original proposed patch, however with this single patch the erasing operation will still fail due to bug #1059872 in util-linux. In such a case I ever attempted to backport a patch to util-linux [1], however I think it will be very hard for it to be accepted into util-linux, so I committed rev87 to workaround the issue without util-linux being fixed.

rev87 is useless for utopic or vivid since util-linux was already fixed in them.

Will it be better if I revert rev87 and wait for util-linux being fixed in trusty?

[1]: https://code.launchpad.net/~yuningdodo/ubuntu/trusty/util-linux/util-linux.backport-wipefs-partition-table-erasing-support/+merge/237898

[Timo Jyrinki (timo-jyrinki)]

I'm not familiar with USB Creator code as such, but #87 surely looks a simpler workaround than getting the util-linux.backport-wipefs-partition-table-erasing-support SRU:d.

Let's see if we can get Martin to comment on this SRU.

[Timo Jyrinki (timo-jyrinki)]

I've built a package from Yu's branch for testing in a PPA for Ubuntu 14.04:

sudo apt-add-repository ppa:timo-jyrinki/ppa
sudo apt update
sudo apt install usb-creator-common usb-creator-gtk

I'll test it out regarding the bugs and report back soon.

[Timo Jyrinki (timo-jyrinki)]

It works ~good to me. I was able to empty an USB both without partition table at all (/dev/zero written to the device) and with an empty partition table.

I also tested successfully the "normal" case of writing USB stick and booting from it, ie the case of partition table and one vfat partition. Seemed to work.

I did get occasionally "gi._glib.GError: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error setting partition flags on /dev/sdc1: Command-line `sfdisk --change-id "/dev/sdc" 1 0x0c' exited with non-zero exit status 1: /dev/sdc: Input/output error", but when I did I also got it with the current usb-creator version so it did not seem to be a regression. And like it says it actually sounds possibly something wrong with the SD card I'm using via USB reader.

review: Approve

[Yu Ning (yuningdodo)]

Timo, thanks very much for the approval, the issue you mentioned is never reproduced in my side, and I also think it should be a different issue.

[Brian Murray (brian-murray)]

I'll manually merge and upload this, thanks for working on it.

review: Approve

=== modified file 'bin/usb-creator-helper'
--- bin/usb-creator-helper	2014-08-19 07:46:20 +0000
+++ bin/usb-creator-helper	2014-10-09 08:46:06 +0000
@@ -46,6 +46,8 @@
     if obj.get_partition_table():
         return obj
     partition = obj.get_partition()
+    if not partition:
+        return obj
     parent = partition.get_cached_property('Table').get_string()    
     return udisks.get_object(parent)
     
@@ -235,11 +237,20 @@
             block.call_format_sync('vfat', GLib.Variant('a{sv}', {'label': GLib.Variant('s', '')}), None)
         else:
             # Create a new partition table and a FAT partition.
-            # Is this correct call to create partition table ?!
-            block.call_format_sync('dos', GLib.Variant('a{sv}', {}), None)
+            parent_block = parent_dev.get_block()
+            parent_file = parent_block.get_cached_property('Device').get_bytestring().decode('utf-8')
+            popen(['parted', '--script', '--', parent_file, 'mktable', 'msdos'])
+            # XXX: unless we re-create the udisks client it will not
+            #      return the device we created just now.
+            udisks = UDisks.Client.new_sync(None)
+            dev = udisks.get_object(device)
+            block = dev.get_block()
             size = block.get_cached_property('Size').get_uint64()
             table = dev.get_partition_table()
             partition = table.call_create_partition_sync(0, size, '0x0c', '', no_options, None) 
+            # XXX: unless we re-create the udisks client it will not
+            #      return the partition just now
+            udisks = UDisks.Client.new_sync(None)
             obj = udisks.get_object(partition)
             block = obj.get_block()
             block.call_format_sync('vfat', GLib.Variant('a{sv}', {'label': GLib.Variant('s', '')}), None)

=== modified file 'debian/changelog'
--- debian/changelog	2014-08-19 07:46:20 +0000
+++ debian/changelog	2014-10-09 08:46:06 +0000
@@ -1,3 +1,15 @@
+usb-creator (0.2.56.3) trusty-proposed; urgency=medium
+
+  * HELPER: use parent object only if there is a parent.
+  * HELPER: re-create the udisks client otherwise it will not return the newly
+    created device or partition. (LP: #1361474, #1300361)
+  * HELPER: use parted instead of udisks2 to create the new partition table
+    since udisks2 uses wipefs to erase the old partition table, however
+    partition table erasing is not supported in wipefs yet in trusty, that
+    would cause a timed out error.
+
+ -- Yu Ning <[e-mail]>  Mon, 01 Sep 2014 13:04:25 +0800
+
 usb-creator (0.2.56.2) trusty-proposed; urgency=medium
 
   * Drop invalid empty "erase" type. We don't need to erase the device (you