Review Request 112040: Do not create duplicate UMS collections for encrypted volumes

[Matěj Laitl]

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


Looks good, altough I'd suggest a tweak, see below.


src/core-impl/collections/umscollection/UmsCollection.cpp
<http://git.reviewboard.kde.org/r/112040/#comment27782>

    This makes sense, altough looking at other UsageType enum values we want really the FileSystem one and no other. So please convert the logic from "not encypted" to "only FileSystem". Code-wise, it is best to create a switch()/case with all the variants and no default: label to get notified on new enum values by the compiler in future.


- Matěj Laitl


On Aug. 12, 2013, 8:44 p.m., Frank Meerkoetter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112040/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2013, 8:44 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> When I plug an encrypted USB-stick amarok is creating two UMS collections for it. Both have the same mount point.
> 
> $ solid-hardware list details
> [...]
> udi = '/org/freedesktop/UDisks/devices/dm_2d5'
>   parent = '/org/freedesktop/UDisks/devices/sdd1'  (string)
>   vendor = ''  (string)
>   product = ''  (string)
>   description = '29.9 GiB Removable Media'  (string)
>   Block.major = 252  (0xfc)  (int)
>   Block.minor = 5  (0x5)  (int)
>   Block.device = '/dev/dm-5'  (string)
>   StorageAccess.accessible = true  (bool)
>   StorageAccess.filePath = '/media/62a745fa-6350-4ee5-ba37-0462dfa3530f'  (string)
>   StorageAccess.ignored = false  (bool)
>   StorageVolume.ignored = false  (bool)
>   StorageVolume.usage = 'FileSystem'  (0x2)  (enum)
>   StorageVolume.fsType = 'ext4'  (string)
>   StorageVolume.label = ''  (string)
>   StorageVolume.uuid = '62a745fa-6350-4ee5-ba37-0462dfa3530f'  (string)
>   StorageVolume.size = 32125222912  (0x77ad00000)  (qulonglong)
> [...]
> udi = '/org/freedesktop/UDisks/devices/sdd1'
>   parent = '/org/freedesktop/UDisks/devices/sdd'  (string)
>   vendor = 'JetFlash'  (string)
>   product = 'Transcend 32GB'  (string)
>   description = '29.9 GiB Encrypted Container'  (string)
>   Block.major = 8  (0x8)  (int)
>   Block.minor = 49  (0x31)  (int)
>   Block.device = '/dev/sdd1'  (string)
>   StorageAccess.accessible = true  (bool)
>   StorageAccess.filePath = '/media/62a745fa-6350-4ee5-ba37-0462dfa3530f'  (string)
>   StorageAccess.ignored = false  (bool)
>   StorageVolume.ignored = false  (bool)
>   StorageVolume.usage = 'Encrypted'  (0x5)  (enum)
>   StorageVolume.fsType = 'crypto_LUKS'  (string)
>   StorageVolume.label = ''  (string)
>   StorageVolume.uuid = '1a38165b-2eee-41d0-acd1-6d34032f47fd'  (string)
>   StorageVolume.size = 32127320064  (0x77af00000)  (qulonglong)
> 
> This patch is filtering out the storage volume where the usage field is set to "Encrypted" (as opposed to "Filesystem").
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/umscollection/UmsCollection.cpp 028966e 
> 
> Diff: http://git.reviewboard.kde.org/r/112040/diff/
> 
> 
> Testing
> -------
> 
> I have tested plugin an USB-stick containing an dmcrypt/luks encrypted ext4fs. I also tested with an USB-stick that was not encrypted.
> 
> 
> Thanks,
> 
> Frank Meerkoetter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130812/37b9473a/attachment-0001.html>