Opened 8 years ago

Last modified 8 years ago

#2630 assigned enhancement

Not enough feedback for unmount failure

Reported by: Zack Owned by: rwh
Priority: high Milestone: Opportunity
Component: journal-activity Version:
Keywords: Cc: Zack, Eben, tomeu, kimquirk
Blocked By: Blocking:
Deployments affected: Action Needed:
Verified: no


There should probably be some feedback for an unmount failure other than "after the UI stopped responding for several seconds, the disk icon didn't go away".

Attachments (4)

badge-busy.svg (1.1 KB) - added by Eben 8 years ago.
badge-warning.svg (758 bytes) - added by Eben 8 years ago.
journal_async_unmount.diff (11.4 KB) - added by rwh 8 years ago.
ds_unmount_async.diff (1.1 KB) - added by rwh 8 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 8 years ago by jg

  • Cc Eben added
  • Milestone changed from Untriaged to Trial-3

comment:2 Changed 8 years ago by Eben

Lets do two things:

  1. Render the device icon in white outline stroke, no fill
  2. Apply the warning badge to the icon (it's a triangle with a '!' inside...)

I think this visual feedback should be enough; something more obtrusive would be annoying. I'm not sure the actual effect premature unmount has. At one point, someone was mentioning caching the files so that it could resume automatically when it was mounted again. If that's the case, this device should remain in the tray; otherwise, we should remove it after some reasonable timeout. In the former case, it seems we need the option to force it to go away (in rollover), since the device could be lost, stolen, broke, or otherwise unavailable.

comment:3 Changed 8 years ago by Zack

This ticket was for feedback when the user tries to unmount a disk via the "Unmount" menu item, and the unmount fails due to the device being busy. The feedback you're describing sounds good for the case of a user prematurely and physically yanking the disk from the system without successfully unmounting, but not quite right for this case since the disk is still physically there, it's just still not safe to remove yet.

This is what I've poorly described in the initial ticket:
1   User plugs a removable disk in
(stuff happens)
2   User wants to unplug disk
3   User selects 'umount' from the disk's menu in the Journal
4   Journal tries to unmount the disk but fails, because it has open files

What I'm proposing is something like:
5   Journal sticks a Warning badge on the Disk's icon

What you're proposing isn't the same thing, but could fit like:
6A  User yanks disk while it's still mounted, losing data
7A  Disk's icon goes "white stroke/no fill" with a Warning badge
8A  User reinserts disk
9A* Disk is rebuild to correct data loss/corruption

Although #9A isn't possible yet.

And then, I'd like to propose what happens when the user doesn't yank the disk:
6B  When the disk is able to be unmounted, Journal unmounts it automatically
7B  User removes disk and loses no data

Man, is WikiFormatting a pain sometimes.

comment:4 Changed 8 years ago by Eben

You're right, I misunderstood the use case. It sounds like using the same warning badge, as you mention, will work quite well. As such, this all fits under one ticket, too.

I like the similarity, because in both cases you'll get a warning badge, and the state of the icon itself (colored, or stroked) will alert the user to the actual presence or absence of the device.

comment:5 Changed 8 years ago by tomeu

Eben, in which cases could fail the unmounting? The DS could refuse to unmount the device if a write is going on. Is this your idea?

comment:6 Changed 8 years ago by Eben

Alright, reconsidering the use case here, I think there's one more obvious thing we should do first. If the device is busy, we shouldn't even provide an unmount option within the secondary palette (or, at least, we should disable it...I'm not sure which is better). We should do this when either a) a write is going on or b) files on the disk are open. I don't think the automatic unmounting is a good idea; it would be better to have them wait until it's no longer busy to actually unmount, so they get direct feedback when it's safe to remove the device.

It seems that it would be best to put a badge on the device to indicate that it's in use. I'll have to think about what that should look like.

comment:7 Changed 8 years ago by Zack

I agree that it would be good to have some indication that a device is busy beforehand. I can't attest to how feasible that is, but I'm not the best person to do that anyway.

I'm not as big a fan of completely disallowing unmounting while the device is busy, because if the only "business" is a background task that isn't really critical (like indexing, which can just resume next time the disk is inserted), it would be really great to be able to say "unmount this, please" and have that task stop itself so I can unplug the disk and do whatever I'm in so much of a hurry to do.

Now, sometimes a disk is *really* busy, and you can't do anything but wait. But we've already handled that case, in terms of design anyway.

comment:8 Changed 8 years ago by Eben

  • Owner changed from tomeu to Eben

I'm taking this one on so I remember to create the badge icons.

Changed 8 years ago by Eben

Changed 8 years ago by Eben

comment:9 Changed 8 years ago by Eben

  • Owner changed from Eben to tomeu

comment:10 Changed 8 years ago by tomeu

  • Owner changed from tomeu to marco

Marco is working on adding badge support to radio buttons.

comment:11 Changed 8 years ago by marco

  • Owner changed from marco to tomeu

Added badge support to Icon. I don't think we need to add it to the buttons. We can do button.set_image(Icon())

comment:12 Changed 8 years ago by tomeu

Don't know of any way the system can notify the journal that the device is busy. Perhaps the best we can do is notify when an unmount is not immediate (some activity is going on) and use a badge for this.

comment:13 Changed 8 years ago by kimquirk

  • Milestone changed from Trial-3 to First Deployment, V1.0

comment:14 Changed 8 years ago by marco

  • Milestone changed from First Deployment, V1.0 to Untriaged
  • Type changed from defect to enhancement

comment:15 Changed 8 years ago by jg

  • Cc tomeu added
  • Milestone changed from Untriaged to V1.1

tomeu: dbus?

comment:16 Changed 8 years ago by tomeu

  • Cc kimquirk added
  • Milestone changed from V1.1 to Untriaged

Who could tell us through dbus that the device is being used? AFAIK, hal doesn't give us that service. Am I wrong?

I would like Kim or somebody close to testing to say if the usability implications of this are worth FRS or not. I'm not sure, but perhaps this is a big problem for users.

I'm putting this untriaged again because we really need to decide this now. Having to add this feature after code freeze would be very unfortunate.

comment:17 Changed 8 years ago by tomeu

My suggestion for FRS in case it has enough relevance:

  1. When calling DataStore.Unmount async, put a busy badge on the volume icon.
  2. If the call returns successfully, call HAL.Unmount async. If returns an error, set the warning badge.
  3. If the second call returns successfully, remove the icon. If returns an error, set the warning badge.

After any error, the user can try unmounting again.

comment:18 Changed 8 years ago by Eben

My fear here is that the warning badge would remain on the icon, seemingly indicated that something is continuously wrong, which is not the case. Maybe we could try something different altogether:

  1. When the user tries to unmount, convert to a white outline, no fill (like the XOs about to leave the mesh will be)
  2. If the unmount completes successfully, remove the icon completely.
  3. If the unmount fails for any reason, fill it back in to make it clear that it's still mounted on the system.

comment:19 Changed 8 years ago by marco

Yeah, Eben approach sounds reasonable to me.

comment:20 Changed 8 years ago by walter

  • Milestone changed from Untriaged to Opportunity (please help!)

comment:21 Changed 8 years ago by rwh

  • Owner changed from tomeu to rwh
  • Status changed from new to assigned

Changed 8 years ago by rwh

comment:22 Changed 8 years ago by rwh

I attached a patch to implement async unmounting and Volume state handling in the UI. See also my personal git tree.

The Volume class has a state added, and emits a signal when this is changed. The VolumesManager forwards this signal, and the VolumesToolbar connect()s to that to modify the icon accordingly. Visual feedback is currently by badges, but could be the suggestion of Even as well with changing colors. Btw, Eben: the warning badge is not in the current artwork; that might be useful for other purposes as well.

I distinguish several different states:
*STATE_REMOVE: Not plugged in
*STATE_INSERTED: Plugged in, not mounted in system or DS
*STATE_PRESENT: mounted in system
*STATE_MOUNTED: mounted in system and DS
*STATE_BUSY: changing state (from STATE_MOUNTED to STATE_REMOVED for example)
*STATE_WARNING, STATE_ERROR: something went wrong. Lasts 10secs [now, I suggest 20s or so] and gets replaced by a more appropriate final state.

I think this gives us the flexibility to give the user all the feedback he needs. Note that Tomeu suggested that in general we should never be in the state where the system has the device mounted and the DS is unmounted; this should only happen if the user accesses the device outside of the DS.

We do not need this many visual icon states, but can map several states to the same icon + badge.

Changed 8 years ago by rwh

Note: See TracTickets for help on using tickets.