Re: Re: General question

"Also sprach Stephan von Krawczynski:"
> > > > 
> > > > > :~ # lsmod
> > > > > Module                  Size  Used by    Not tainted
> > > > > fr1                    18356   1 
> > > > > bitmap                  6416   0  [fr1]
> > > > > md                     49664   2  [fr1]
> > > > > enbd                   71788   8 
> > 
> > > It wouldn't open fr1 module if it were the wrong version of md, not?
> > 
> > It would load and run fine either way, as far as I know. The change to
> > md is to allow the hotrepair ioctl/technique plus a notification
> > mechanism (again ioctl) for device error.
> 
> I checked the sources and the modules. It _is_ the correct module. For testing
> I even compiled in some debug messages. It is really the correct, patched md
> module.
> And the fr1 module does correctly sync by bitmap, btw, as my logs show.

OK - the patched md.c code should have code like this in it:

+static void
+notify_device (mddev_t * mddev, dev_t dev)
+{
+#ifndef BLKMDNTFY
+#define BLKMDNTFY _IOW(0x12,133,int)
+#endif
+        struct block_device *bdev;
+
+        bdev = bdget (dev);
+        if (!bdev)
+                return;
+        printk (KERN_INFO "%s: notifying dev %x it is now in array\n",
+                mdname(mddev), dev);
+        ioctl_by_bdev (bdev, BLKMDNTFY, MKDEV (MD_MAJOR, mddev->__minor));
+#ifndef BLKMDRGTR
+#define BLKMDRGTR _IOW(0x12,135,unsigned long)
+#endif
+        ioctl_by_bdev (bdev, BLKMDRGTR, (unsigned long)md_hot_cmd_disk);
+       bdput(bdev);
+}

Whenever a device is added to an array, that device is sent the new
BLKMDNTFY and then BLKMDRGTR ioctls.


@@ -1018,6 +1165,9 @@
        list_add(&rdev->same_set, &mddev->disks);
        rdev->mddev = mddev;
        printk(KERN_INFO "md: bind<%s>\n", bdevname(rdev->bdev,b));
+#ifdef MD_BITMAP_SUPPORT
+        notify_device(mddev, rdev->bdev->bd_inode->i_rdev);
+#endif /* MD_BITMAP_SUPPORT */
        return 0;
 }
 
@@ -1028,6 +1178,9 @@


The device may understand the ioctls or not.  If the device understands
BLKMDRGTR (and enbd does), then it takes the argument of that ioctl as a
callback function to use whenever the device changes its state (in the
case of enbd, whenever the network goes down or up).

This callback function pushes the device in or out of the raid array.
You can see its functionality defined in md.c:

+/*
+ * This is registered to other devices as a callback
+ */
+static int
+md_hot_cmd_disk(dev_t dev, int cmd) {
+
+        static int hot_add_disk(mddev_t * mddev, dev_t dev);
+        static int set_disk_faulty(mddev_t *mddev, dev_t dev);
+
+        mdk_rdev_t *rdev;
+        mddev_t *mddev;
+        int res;
+
+        rdev = find_rdev_all(dev);
+        if (!rdev)
+                return -EINVAL;
+        mddev = rdev->mddev;
+        if (!mddev)
+                return -EINVAL;
+
+        switch(cmd) {
+            case HOT_ADD_DISK:
+                res = hot_add_disk(mddev, dev);
+                return res;
+            case SET_DISK_FAULTY:
+               res = set_disk_faulty(mddev, dev);
+                return res;
+        }
+        return -EINVAL;
+}

Now, when enbd is in show_errs mode, then it will sense a network
outage and use this callback function to notify the overlying array of
the state of the device, and the array will repond by pulling it in or
out of the array.

As for enbd going into or out of show_errs mode, you can see that
notify_device() of md.c first sends the BLKMDNTFY ioctl to a component
device of the raid before BLKMDRGTR. Enbd in particular understands
this ioctl and sets the RAID_SHOW_ERRS flag in response:

#ifndef BLKMDNTFY
#define BLKMDNTFY _IOW(0x12,133,int)
#endif
              case BLKMDNTFY:
                ENBD_INFO ("received BLKMDNTFY, am now in raid %x\n",
                        (unsigned) arg);
                err = enbd_md.inc(&enbd_md);
                if (err < 0)
                        return err;
                // PTB count the individual partition and whole disk
                // inclusions
                if (slot)
                        slot->md_count++;
                atomic_inc(&lo->md_count);
                if (!atomic_test_and_set_mask (&lo->flags, ENBD_RAID_SHOW_ERRS)
) {
                        ENBD_INFO ("set show_errs on nd%s\n", lo->devnam);
                }
                return 0;

You should SEE these messages in the logs.

One thing I do not like here though is that I don't see the full SHOW_ERRS
flag being forced as a result. It may not be necessary. I see that the
enbd_set_remote_invalid() function does some forcing of the show_error flag
on its own. But it may be worth trying the following experiment in
enbd_base.c ... 

+                       if (!atomic_test_and_set_mask (&lo->flags, ENBD_SHOW_ERRS)) {
+                               atomic_set_mask (ENBD_SET_SHOW_ERRS, &lo->flags);
+                       }
                        ENBD_INFO ("set show_errs on nd%s\n", lo->devnam);


and a bit lower down:

+                       if (atomic_test_and_clear_mask(&lo->flags, ENBD_SET_SHOW_ERRS)) {
+                                atomic_clear_mask (ENBD_SHOW_ERRS, &lo->flags);
+                       }
                        ENBD_INFO ("cleared show_errs on nd%s\n", lo->devnam);

And the info calls migt want to go inside the new if blocks too.

>  
> > But it's still not right. It should happen autmatically. I suspect the
> > md module is the old one, not the new one.
> 
> Can you please point to the source where it does this? Maybe the fr1 patch I
> used does not? 

See above.

Peter