> 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 > >
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
