lvm/dm - clean up debug, fix open bug
authorAlex Hornung <ahornung@gmail.com>
Fri, 9 Jul 2010 08:14:31 +0000 (09:14 +0100)
committerAlex Hornung <ahornung@gmail.com>
Sun, 11 Jul 2010 21:05:54 +0000 (22:05 +0100)
* Clean up debug code and change it to log_verbose, mostly. Also remove
  some of the older, now irrelevant, comments.

* For dm, add a sysctl and tunable to enable debugging (debug.dm_debug)

* in the dragonfly filter, open the device early enough, and definitely
  before closing it ;)

contrib/lvm2/dist/lib/device/dev-cache.c
contrib/lvm2/dist/lib/filters/filter_dragonfly.c
contrib/lvm2/dist/libdm/ioctl/libdm-dragonfly-iface.c
sys/dev/disk/dm/device-mapper.c
sys/dev/disk/dm/dm.h
sys/dev/disk/dm/dm_ioctl.c
sys/dev/disk/dm/dm_target_crypt.c

index fc73c13..6bedb29 100644 (file)
@@ -431,7 +431,7 @@ static int _insert(const char *path, int rec)
 
        } else {
                /* add a device */
-#ifdef __NetBSD__              
+#ifdef __NetBSD__
                /*
                 * In NetBSD we have two different types of devices
                 * raw and block. I can insert only  existing
@@ -465,13 +465,29 @@ static int _insert(const char *path, int rec)
                        return_0;
                }
 
+               if (!strncmp(path, "/dev/tun", strlen("/dev/tun"))) {
+                       log_debug("%s: Not adding tun devices");
+                       return_0;
+               }
+
+               if (!strncmp(path, "/dev/cd", strlen("/dev/cd")) ||
+                   !strncmp(path, "/dev/acd", strlen("/dev/acd"))) {
+                       log_debug("%s: Not adding CD drives");
+                       return_0;
+               }
+
+               if (!strcmp(path, "/dev/vn")) {
+                       log_debug("%s: Not adding vn autoclone dev");
+                       return_0;
+               }
+
                if (!strncmp(path, "/dev/md", strlen("/dev/md"))) {
                        log_debug("%s: Not adding malloc disks");
                        return_0;
                }
 
-               if (!strncmp(path, "/dev/fd0", strlen("/dev/fd0"))) {
-                       log_debug("%s: Not adding floppy disks");
+               if (!strncmp(path, "/dev/fd", strlen("/dev/fd"))) {
+                       log_debug("%s: Not adding floppy disks or fds");
                        return_0;
                }
 #else
index 305da51..a58ba1a 100644 (file)
@@ -76,7 +76,7 @@ static int _passes_lvm_type_device_filter(struct dev_filter *f __attribute((unus
 {
        const char *name = dev_name(dev);
        int fd, type;
-       int ret = 0;
+       int ret = LVM_FAILURE;
        uint64_t size;
 
        log_debug("Checking: %s", name);
@@ -106,12 +106,18 @@ static int _passes_lvm_type_device_filter(struct dev_filter *f __attribute((unus
                return LVM_FAILURE;
        }
 
+       /* Check it's accessible */
+       if (!dev_open_flags(dev, O_RDONLY, 0, 1)) {
+               log_debug("%s: Skipping: open failed", name);
+               return LVM_FAILURE;
+       }
+
        /* Check it's not too small */
        if (!dev_get_size(dev, &size)) {
-               log_debug("%s: Skipping: dev_get_size failed", name);
+               log_debug("%s: Skipping: dev_get_size failed", name);
                goto out;
        }
-       
+
        if (size < PV_MIN_SIZE) {
                log_debug("%s: Skipping: Too small to hold a PV", name);
                goto out;
@@ -123,12 +129,6 @@ static int _passes_lvm_type_device_filter(struct dev_filter *f __attribute((unus
                goto out;
        }
 
-       /* Check it's accessible */
-       if (!dev_open_flags(dev, O_RDONLY, 0, 1)) {
-               log_debug("%s: Skipping: open failed", name);
-               return LVM_FAILURE;
-       }
-
        ret = LVM_SUCCESS;
 
       out:
index 92d5116..4bdda49 100644 (file)
@@ -423,9 +423,6 @@ int dm_format_dev(char *buf, int bufsize, uint32_t dev_major,
        size_t val_len,i;
 
        dev = MKDEV(dev_major,dev_minor);
-
-       log_error("dangerous zone...\n");
-
        mode |= S_IFCHR;
 
        name = devname(dev,mode);
@@ -595,7 +592,7 @@ struct target *create_target(uint64_t start, uint64_t len, const char *type,
                             const char *params)
 {
        struct target *t = dm_malloc(sizeof(*t));
-       log_error("params is at create_target: %s", params);
+       log_verbose("params is at create_target: %s", params);
        if (!t) {
                log_error("create_target: malloc(%" PRIsize_t ") failed",
                          sizeof(*t));
@@ -655,12 +652,10 @@ static int _flatten(struct dm_task *dmt, prop_dictionary_t dm_dict)
                strlcpy(type,t->type,DM_MAX_TYPE_NAME);
 
                prop_dictionary_set_cstring(target_spec,DM_TABLE_TYPE,type);
-               log_error("t->params: %s\n", t->params);
                prop_dictionary_set_cstring(target_spec,DM_TABLE_PARAMS,t->params);
 
                prop_dictionary_get_cstring(target_spec,
                    DM_TABLE_PARAMS, (char **) &str);
-               log_error("t->params from dict: %s\n", str);
 
                prop_array_set(cmd_array,count,target_spec);
 
@@ -848,7 +843,7 @@ static int _create_and_load_v4(struct dm_task *dmt)
        struct dm_task *task;
        int r;
 
-       printf("create and load called \n");
+       log_verbose("create and load called");
        
        /* Use new task struct to create the device */
        if (!(task = dm_task_create(DM_DEVICE_CREATE))) {
@@ -1159,7 +1154,7 @@ int dm_task_run(struct dm_task *dmt)
 
        switch (dmt->type) {
        case DM_DEVICE_CREATE:
-               printf("create dmt->dev_name = %s\n", dmt->dev_name);
+               log_verbose("create dmt->dev_name = %s", dmt->dev_name);
                /* XXX: ideally kernel takes care of this */
 #if 0
                add_dev_node(dmt->dev_name, MAJOR(dmi->dev), MINOR(dmi->dev),
@@ -1168,7 +1163,7 @@ int dm_task_run(struct dm_task *dmt)
                break;
 
        case DM_DEVICE_REMOVE:
-               printf("remove dmt->dev_name = %s\n", dmt->dev_name);
+               log_verbose("remove dmt->dev_name = %s", dmt->dev_name);
                /* XXX: ideally kernel takes care of this */
 #if 0
                /* FIXME Kernel needs to fill in dmi->name */
@@ -1179,7 +1174,7 @@ int dm_task_run(struct dm_task *dmt)
 
        case DM_DEVICE_RENAME:
                /* FIXME Kernel needs to fill in dmi->name */
-               printf("rename dmt->dev_name = %s\n", dmt->dev_name);
+               log_verbose("rename dmt->dev_name = %s", dmt->dev_name);
                 _dm_rename_kern(dmt);
 #if 0
                if (dmt->dev_name)
@@ -1194,7 +1189,7 @@ int dm_task_run(struct dm_task *dmt)
                break;
        
        case DM_DEVICE_MKNODES:
-               printf("mknodes dmt->dev_name = %s\n", dmt->dev_name);
+               log_verbose("mknodes dmt->dev_name = %s", dmt->dev_name);
 #if 0
                if (dmi->flags & DM_EXISTS_FLAG)
                        add_dev_node(dmi->name, MAJOR(dmi->dev),
index 01dd8be..2b24fb6 100644 (file)
@@ -45,6 +45,7 @@
 #include <sys/ioccom.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
+#include <sys/sysctl.h>
 
 #include "netbsd-dm.h"
 #include "dm.h"
@@ -85,6 +86,8 @@ struct dev_ops dm_ops = {
 
 MALLOC_DEFINE(M_DM, "dm", "Device Mapper allocations");
 
+int dm_debug_level = 0;
+
 extern uint64_t dm_dev_counter;
 
 static cdev_t dmcdev;
@@ -351,7 +354,6 @@ disk_ioctl_switch(cdev_t dev, u_long cmd, void *data)
                        return ENOTSUP;
                } else {
                        size = dm_table_size(&dmv->table_head);
-                       kprintf("fooi, size = %"PRIu64" ...\n", size);
                        dpart->media_offset  = 0;
                        dpart->media_size    = size * DEV_BSIZE;
                        dpart->media_blocks  = size;
@@ -575,4 +577,9 @@ dmgetproperties(struct disk *disk, dm_table_head_t *head)
        if (odisk_info != NULL)
                prop_object_release(odisk_info);
 #endif
-}      
+}
+
+TUNABLE_INT("debug.dm_debug", &dm_debug_level);
+SYSCTL_INT(_debug, OID_AUTO, dm_debug, CTLFLAG_RW, &dm_debug_level,
+              0, "Eanble device mapper debugging");
+
index 2dfbf14..eed29fe 100644 (file)
@@ -391,8 +391,10 @@ int dm_pdev_destroy(void);
 int dm_pdev_init(void);
 dm_pdev_t* dm_pdev_insert(const char *);
 
-/* XXX: temporary hacks */
-#define aprint_debug   while(0) kprintf
+extern int dm_debug_level;
+
+#define aprint_debug(format, ...)      \
+    do { if (dm_debug_level) kprintf(format, ## __VA_ARGS__); } while(0)
 #define aprint_normal  kprintf
 
 #endif /*_KERNEL*/
index 3e407c5..fd6ef2d 100644 (file)
@@ -792,7 +792,7 @@ dm_table_load_ioctl(prop_dictionary_t dm_dict)
                 * therfore I have to pass it to target init
                 * routine and parse parameters there.
                 */
-               kprintf("DM: str passed in is: %s", str);
+               aprint_debug("DM: str passed in is: %s", str);
                if ((ret = target->init(dmv, &table_en->target_config,
                            str)) != 0) {
 
index a7534d1..fc2aa4d 100644 (file)
@@ -57,7 +57,7 @@ typedef void ivgen_t(struct target_crypt_config *, u_int8_t *, size_t, off_t);
 typedef struct target_crypt_config {
        size_t  params_len;
        dm_pdev_t *pdev;
-       char    *status_str;
+       char    *status_str;
        int     crypto_alg;
        int     crypto_klen;
        u_int8_t        crypto_key[512>>3];
@@ -68,7 +68,6 @@ typedef struct target_crypt_config {
        SHA512_CTX      essivsha512_ctx;
        struct cryptoini        crypto_session;
        ivgen_t *crypto_ivgen;
-       /* XXX: uuid */
 } dm_target_crypt_config_t;
 
 struct dmtc_helper {
@@ -355,15 +354,14 @@ dm_target_crypt_crypto_done_read(struct cryptop *crp)
 
        bio = (struct bio *)crp->crp_opaque;
        KKASSERT(bio != NULL);
-       
+
        n = atomic_fetchadd_int(&bio->bio_caller_info3.value, -1);
 #if 0
        kprintf("dm_target_crypt_crypto_done_read %p, n = %d\n", bio, n);
 #endif
        if (crp->crp_etype != 0) {
                kprintf("dm_target_crypt: dm_target_crypt_crypto_done_read crp_etype = %d\n", crp->crp_etype);
-               /* XXX: Print something out */
-               bio->bio_buf->b_error = crp->crp_etype; 
+               bio->bio_buf->b_error = crp->crp_etype;
        }
        if (n == 1) {
                kfree(bio->bio_caller_info2.ptr, M_DMCRYPT);
@@ -388,15 +386,14 @@ dm_target_crypt_crypto_done_write(struct cryptop *crp)
 
        bio = (struct bio *)crp->crp_opaque;
        KKASSERT(bio != NULL);
-       
+
        n = atomic_fetchadd_int(&bio->bio_caller_info3.value, -1);
 #if 0
        kprintf("dm_target_crypt_crypto_done_write %p, n = %d\n", bio, n);
 #endif
        if (crp->crp_etype != 0) {
                kprintf("dm_target_crypt: dm_target_crypt_crypto_done_write crp_etype = %d\n", crp->crp_etype);
-               /* XXX: Print something out */
-               bio->bio_buf->b_error = crp->crp_etype; 
+               bio->bio_buf->b_error = crp->crp_etype;
        }
        if (n == 1) {
                /* This is the last chunk of the write */
@@ -568,7 +565,7 @@ dm_target_crypt_init(dm_dev_t * dmv, void **target_config, char *params)
        }
 
        if (strcmp(crypto_mode, "cbc") != 0) {
-               kprintf("dm_target_crypt: only support 'cbc' chaining mode, invalid mode '%s'\n", crypto_mode);         
+               kprintf("dm_target_crypt: only support 'cbc' chaining mode, invalid mode '%s'\n", crypto_mode);
                goto notsup;
        }
 
@@ -643,7 +640,7 @@ dm_target_crypt_init(dm_dev_t * dmv, void **target_config, char *params)
                error = essiv_hash_mkey(priv, iv_opt);
                if (error) {
                        kprintf("dm_target_crypt: essiv_hash_mkey failed\n");
-                       goto notsup;            
+                       goto notsup;
                }
                priv->crypto_ivgen = essiv_ivgen;
        } else if (!strcmp(iv_mode, "plain")) {
@@ -731,7 +728,7 @@ dm_target_crypt_strategy(dm_table_entry_t * table_en, struct buf * bp)
                break;
 
        default:
-               vn_strategy(priv->pdev->pdev_vnode, &bp->b_bio1);               
+               vn_strategy(priv->pdev->pdev_vnode, &bp->b_bio1);
        }
 
        return 0;