usb-creator

Merge lp://staging/~yuningdodo/usb-creator/usb-creator.lp1413494-auto-mount-after-format into lp://staging/usb-creator

  1. usb-creator.lp1413494-auto-mount-after-format
  2. Merge into trunk
Proposed by Yu Ning
Status: Rejected
Rejected by: Mathieu Trudel-Lapierre
Proposed branch: lp://staging/~yuningdodo/usb-creator/usb-creator.lp1413494-auto-mount-after-format
Merge into: lp://staging/usb-creator
Diff against target: 34 lines (+9/-1)
1 file modified
bin/usb-creator-helper (+9/-1)
To merge this branch: bzr merge lp://staging/~yuningdodo/usb-creator/usb-creator.lp1413494-auto-mount-after-format
Reviewer Review Type Date Requested Status
Yu Ning (community) Disapprove
Mathieu Trudel-Lapierre Needs Fixing
Brian Murray Pending
Review via email: mp+250256@code.staging.launchpad.net

Description of the change

I checked the source code and found usb-creator can only calculate the free space for a mounted partition, however the newly formated partition isn't mounted.

So to fix the issue we could mount it automatically after the formating. In the attached patch this job is done in the dbus method Format() in usb-creator-helper. In my own test it works fine.

To post a comment you must log in.
Revision history for this message
Yu Ning (yuningdodo) wrote :

One problem for this patch is it will mount the new partition as root, so the normal user can not unmount it.

I have proposed another patch to mount the new partition as the normal user:
https://code.launchpad.net/~yuningdodo/usb-creator/usb-creator.lp1413494v2-update-free-space-after-format/+merge/250258

Anyway, I think below change is still needed, although it's a different issue:

8 - unmount_all(udisks, dev)
9 + unmount_all(udisks, parent_dev)

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

In this case, could you please either update this request appropriately to only keep that change, or provide it in a new merge proposal (along with a bug link if possible?)

Which bug would the umount_all change fix?

review: Needs Fixing
Revision history for this message
Yu Ning (yuningdodo) wrote :

Mathieu, thanks for the review. I checked the code again, looks like I had a misunderstanding about the umount_all() function, we shouldn't change the argument from dev to parent_dev. Please help to reject this merge proposal.

review: Disapprove

Unmerged revisions

463. By Yu Ning

* auto mount the new partition to calc free space correctly. (LP: #1413494)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: