Bug 22707

Right, makes sense. I'll cook a patch for this.

I tested this with my /dev/sda3 which is an unused ext3 partition:

ENV{ID_FS_TYPE}=="ext3", ENV{ID_FS_LABEL}=="test", ENV{DKD_PRESENTATION_HIDE}="1"

This works for udev (I get exactly one match):

$ udevadm info --export-db|grep PRES
E: DKD_PRESENTATION_HIDE=1

and I confirm that the current gdu exports this property correctly and gvfs honors it ("test" does not appear in computer place and places menu any more).

Created attachment 27563 [details] [review]
patch

Here's the git format-patch. Can't commit this yet (requested in bug 22578).

Hi, thanks for working on this.

> +# Apple Bootstrap partitions
> +ENV{ID_FS_TYPE}=="hfs", ENV{ID_FS_LABEL}=="bootstrap", ENV{DKD_PRESENTATION_HIDE}="1"

It's probably better keying off partition type, here's the relevant udev data from my Powerbook 12" G4

 E: DKD_PARTITION=1
 E: DKD_PARTITION_SCHEME=apm
 E: DKD_PARTITION_NUMBER=2
 E: DKD_PARTITION_TYPE=Apple_Bootstrap
 E: DKD_PARTITION_SIZE=1048576
 E: DKD_PARTITION_LABEL=untitled
 E: DKD_PARTITION_FLAGS=allocated allow_read allow_write

So I think we can just key off SCHEME=="apm" and TYPE=="Apple_Bootstrap".

> +
> +# EFI firmware partitions
> +ENV{ID_FS_TYPE}=="vfat", ENV{ID_FS_LABEL}=="EFI", > ENV{DKD_PRESENTATION_HIDE}="1"

Would also be better to key off partition type here, see below

> +# recovery partitions

This looks fine. Except that the line is really long; does udev support breaking string literals into multiple lines the same way you can do in C? If not, it's not a big deal...

Also For MBR, we should set PRESENTATION_IGNORE to 1 if

 - (DKD_PARTITION_TYPE=0x00 or
    DKD_PARTITION_TYPE=0x11 or
    DKD_PARTITION_TYPE=0x14 or
    DKD_PARTITION_TYPE=0x16 or
    DKD_PARTITION_TYPE=0x17 or
    DKD_PARTITION_TYPE=0x1b or
    DKD_PARTITION_TYPE=0x1c or
    DKD_PARTITION_TYPE=0x1e or
    DKD_PARTITION_TYPE=0x27 or
    DKD_PARTITION_TYPE=0x3d or
    DKD_PARTITION_TYPE=0x84 or
    DKD_PARTITION_TYPE=0x8d or
    DKD_PARTITION_TYPE=0x90 or
    DKD_PARTITION_TYPE=0x91 or
    DKD_PARTITION_TYPE=0x92 or
    DKD_PARTITION_TYPE=0x93 or
    DKD_PARTITION_TYPE=0x97 or
    DKD_PARTITION_TYPE=0x98 or
    DKD_PARTITION_TYPE=0x9a or
    DKD_PARTITION_TYPE=0x9b or
    DKD_PARTITION_TYPE=0xbb or
    DKD_PARTITION_TYPE=0xc2 or
    DKD_PARTITION_TYPE=0xc3 or
    DKD_PARTITION_TYPE=0xdd or
    DKD_PARTITION_TYPE=0xfe)
   and
    DKD_PARTITION_SCHEME=mbr

cf. http://www.win.tue.nl/~aeb/partitions/partition_types-1.html

and for GPT

 - (DKD_PARTITION_TYPE=C12A7328-F81F-11D2-BA4B-00A0C93EC93B or
    DKD_PARTITION_TYPE=21686148-6449-6E6F-744E-656564454649)
   and
    DKD_PARTITION_SCHEME=gpt

   cf. http://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs

I think the former match should take care of EFI partitions.

Would also be good to reference those websites in the rules file. Probably also easier to use udev's | or operator e.g. TYPE=0x00|0x11|0x14|...

You should be able to change the partition type with Palimpsest to check that things are hidden/unhidden on the fly...

> Except that the line is really long; does udev support breaking string literals > into multiple lines the same way you can do in C

Unfortunately it doesn't know about the

   "string"
   "continuation"

schema; you can do

ENV{ID_FS_TYPE}=="ntfs|vfat|ext3", ENV{ID_FS_LABEL}=="RECOVERY|PQSERVICE|HP_RECOVERY|test|\
Recovery Partition|DellUtility|DellRestore|IBM_SERVICE|SERVICEV001|SERVICEV002", \
  ENV{DKD_PRESENTATION_HIDE}="1"

but IMHO that looks worse.

(In reply to comment #5)
> Unfortunately it doesn't know about the
> 
>    "string"
>    "continuation"
> 
> schema; you can do
> 
> ENV{ID_FS_TYPE}=="ntfs|vfat|ext3",
> ENV{ID_FS_LABEL}=="RECOVERY|PQSERVICE|HP_RECOVERY|test|\
> Recovery
> Partition|DellUtility|DellRestore|IBM_SERVICE|SERVICEV001|SERVICEV002", \
>   ENV{DKD_PRESENTATION_HIDE}="1"
> 
> but IMHO that looks worse.
> 

Agreed, let's just go for a single line then.

Sorry, bugzilla broke lines unfortunately. So, you can break a line (including string literals) with \, but nothing else (see udev/udev-rules.c, parse_file()).

Thanks for the bootstrap udev/dk values and the links, I'll update the patch accordingly.

Taking notes...

 - EFI mbr partition is 0xef, not 0xfe
 - we can drop PQSERVICE label check, it's mbr type 0x27

Created attachment 27569 [details] [review]
now with checking schema/type

I asked the original reporter in https://launchpad.net/bugs/394088 to supply an udev db dump, just to make double-sure that his EFI partition indeed matches these properties.

I tested these rules on my system by changing the partition type in palimpsest, and confirm that GNOME hides/shows partitions on the fly according to their type.

Hah, nice. The original reporter sent his udev db dump (http://launchpadlibrarian.net/28891060/udev-db.txt) which perfectly matches your URLs and the patch:

E: DEVNAME=/dev/sda1
E: ID_FS_LABEL=EFI
E: ID_FS_LABEL_ENC=EFI
E: DKD_PARTITION_SCHEME=gpt
E: DKD_PARTITION_NUMBER=1
E: DKD_PARTITION_TYPE=C12A7328-F81F-11D2-BA4B-00A0C93EC93B
E: DKD_PARTITION_LABEL=EFI System Partition

Pushed to master, thanks!