[Kde-hardware-devel] Re: Review Request: Sort out ejecting of devices

[Jacopo De Simoi]

> On May 30, 2011, 3:18 p.m., Lukáš Tinkl wrote:
> > Sry, but the patch doesn't make sense to me; I don't like the timeout approach and secondly, the check for optical drive is there on purpose, they are handled separately elsewhere. This special case is because of bug https://bugs.kde.org/show_bug.cgi?id=267398

I messed up with post-review; the timeout thing belongs to this other review request, namely 
https://git.reviewboard.kde.org/r/101428/
please move your comments to that review so that we can start a discussion there.

More on topic, I don't really understand what the code does; maybe you could help me:
As far as I can understand the following should happen:

We are unmounting a device; if the device is an optical media, we should call the appropriate program on each os,
i.e. cdio / cdcontrol / eject
It seems  to me that the codepath executing this is called whenever the device is _not_ an optical disc, 
where it should be callew whenever the device _is_ an optical disk. Am I missing something?

If the device is not an optical media, then we should check if it is ejectable and, in case,
eject it using the udisks API. What the code seems to do now is try to eject it anyways, which leads to 
weird issues such as bug 270490. Again, am I missing something?

The patch attempts to fix both issues; imho if a device is not marked as ejectable, it should not be ejected.
If there exist devices that do require to be ejected but do not mark themselves as ejectable, this issue should be probably 
fixed somewhere else via udev rules or whatever else. 


- Jacopo De


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101453/#review3596
-----------------------------------------------------------


On May 30, 2011, 6:39 p.m., Jacopo De Simoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101453/
> -----------------------------------------------------------
> 
> (Updated May 30, 2011, 6:39 p.m.)
> 
> 
> Review request for Solid, Lukáš Tinkl and Ozan Çağlayan.
> 
> 
> Summary
> -------
> 
> Call DeviceEject only if the drive actually requires to be ejected, 
> moreover call the special eject routine if the drive IS an optical Disc, not if it is NOT an optical disc…
> 
> 
> This addresses bug 270490.
>     http://bugs.kde.org/show_bug.cgi?id=270490
> 
> 
> Diffs
> -----
> 
>   solid/solid/backends/udisks/udisksstorageaccess.cpp 4cb0f7c 
> 
> Diff: http://git.reviewboard.kde.org/r/101453/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacopo De
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20110530/10cc45d5/attachment.htm