Bastian Blank wrote:
> On Tue, Oct 13, 2009 at 05:45:27PM +0200, Michael Biebl wrote:
>> What we can not remove currently, is the call to devkit-disks-dm-export,
>> as it makes additional information available like (DKM_)DM_TARGET_TYPES,
>> which is used inside the dk-disks code.
> 
> Please explain which highlevel information you need.
> 
> DM_TARGET_TYPES is only used to find the string "crypt" in it and I
> already explained several times that DM_UUID==CRYPT-* is a quite better
> indicator, completely functional in the near future with cryptsetup 1.1.

Finally some useful information, thanks.

Attached is a patch which makes use of DM_UUID making it possible to completely
remove devkit-disks-dm-export, and it seems to work nicely.

Is the DM_UUID=CRYPT-* naming scheme documented somewhere? I'd like to send this
patch upstream and would need to know if it is a Debian specific feature.
If not, since what version of cryptsetup/dmsetup is this naming interface 
stable?


Thanks,
Michael

P.S: I have to retract my statement wrt to mdadm. I double checked the Debian
mdadm udev rules files, and it uses the upstream udev rules file that calls
"mdadm --detail", whereas the dk-disks udev rule calls "mdadm --examine". The
former seems to operate on the actual md device only, the latter on the
underlying physical device.
So I don't think it's safe to remove the mdadm section from
95-devkit-disks.rules just yet without further investigation.
I CCed the mdadm maintainers for their input. Maybe we could enable that "mdadm
--examine" in mdadm itself?

Anyways, this should probably be handled in a separate bug report as this one is
about dm devices. I just mentioned it, as you brought up the mdadm issue.

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
From 9aca432753e8e4fbaacf09b47bdf374765397232 Mon Sep 17 00:00:00 2001
From: Michael Biebl <bi...@debian.org>
Date: Thu, 15 Oct 2009 14:26:17 +0200
Subject: [PATCH 1/2] Stop probing device-mapper devices

Use DM_NAME and DM_UUID which are already setup by the dmsetup udev
rules.
See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=545032
---
 data/95-devkit-disks.rules |   21 ---------------------
 src/devkit-disks-device.c  |   17 +++++++++--------
 2 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/data/95-devkit-disks.rules b/data/95-devkit-disks.rules
index fbb85a0..f8fad85 100644
--- a/data/95-devkit-disks.rules
+++ b/data/95-devkit-disks.rules
@@ -29,27 +29,6 @@ LABEL="probe_parttable_end"
 
 ##############################################################################################################
 
-# pick up device-mapper data; this REALLY should be done by rules installed
-# by the device-mapper package
-#
-KERNEL!="dm-*", GOTO="device_mapper_end"
-ACTION!="add|change", GOTO="device_mapper_end"
-
-IMPORT{program}="devkit-disks-dm-export %M %m"
-ENV{DKD_DM_NAME}!="?*", GOTO="device_mapper_end"
-
-ENV{DKD_DM_STATE}=="SUSPENDED", GOTO="device_mapper_end"
-ENV{DKD_DM_TARGET_TYPES}=="|*error*", GOTO="device_mapper_end"
-
-# avoid probing if it has already been done earlier
-#
-ENV{ID_FS_USAGE}!="", GOTO="device_mapper_end"
-IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
-
-LABEL="device_mapper_end"
-
-##############################################################################################################
-
 # pick up data from MD components; this REALLY should be done by rules installed
 # by mdadm or the kernel package
 #
diff --git a/src/devkit-disks-device.c b/src/devkit-disks-device.c
index 5af03ce..58de2fd 100644
--- a/src/devkit-disks-device.c
+++ b/src/devkit-disks-device.c
@@ -2200,21 +2200,22 @@ static gboolean
 update_info_luks_cleartext (DevkitDisksDevice *device)
 {
         uid_t unlocked_by_uid;
-        const gchar *dkd_dm_name;
-        const gchar *dkd_dm_target_types;
+        const gchar *dm_name;
+        const gchar *dm_uuid;
         gboolean ret;
 
         ret = FALSE;
 
-        dkd_dm_name = g_udev_device_get_property (device->priv->d, "DKD_DM_NAME");
-        dkd_dm_target_types = g_udev_device_get_property (device->priv->d, "DKD_DM_TARGET_TYPES");
-        if (dkd_dm_name != NULL && g_strcmp0 (dkd_dm_target_types, "crypt") == 0 &&
+        dm_name = g_udev_device_get_property (device->priv->d, "DM_NAME");
+        dm_uuid = g_udev_device_get_property (device->priv->d, "DM_UUID");
+        if (dm_name != NULL && g_str_has_prefix (dm_uuid, "CRYPT-") &&
             device->priv->slaves_objpath->len == 1) {
 
                 /* TODO: might be racing with setting is_drive earlier */
                 devkit_disks_device_set_device_is_drive (device, FALSE);
 
-                if (g_str_has_prefix (dkd_dm_name, "temporary-cryptsetup-")) {
+                /* Debian already hides temporary cryptsetup devices, so this check could probably be removed? */
+                if (g_str_has_prefix (dm_name, "temporary-cryptsetup-")) {
                         /* ignore temporary devices created by /sbin/cryptsetup */
                         goto out;
                 }
@@ -2223,12 +2224,12 @@ update_info_luks_cleartext (DevkitDisksDevice *device)
 
                 devkit_disks_device_set_luks_cleartext_slave (device, ((gchar **) device->priv->slaves_objpath->pdata)[0]);
 
-                if (luks_get_uid_from_dm_name (dkd_dm_name, &unlocked_by_uid)) {
+                if (luks_get_uid_from_dm_name (dm_name, &unlocked_by_uid)) {
                         devkit_disks_device_set_luks_cleartext_unlocked_by_uid (device, unlocked_by_uid);
                 }
 
                 /* TODO: export this at some point */
-                devkit_disks_device_set_dm_name (device, dkd_dm_name);
+                devkit_disks_device_set_dm_name (device, dm_name);
         } else {
                 devkit_disks_device_set_device_is_luks_cleartext (device, FALSE);
                 devkit_disks_device_set_luks_cleartext_slave (device, NULL);
-- 
1.6.5

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to