Re: Re: General question

"Also sprach Stephan von Krawczynski:"
> On Mon, 6 Jun 2005 17:36:43 +0200 (MET DST) "Peter T. Breuer" <[email protected]> wrote:
> > 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.
> 
> Busted. Here is _my_ enbd:

Oh - it appears you are running a 2.4 kernel. Sorry - I was looking at
the 2.6 code.


> #ifndef BLKMDNTFY
> #define BLKMDNTFY _IOW(0x12,133,int)
> #endif
>       case BLKMDNTFY:
>         ENBD_INFO ("received BLKMDNTFY, am now in raid %x\n", (unsigned) arg);
>         spin_lock (&md_access_lock);
>         if (md_doing_notify && md_notify_pid != current->pid) {
>                 // PTB don't add if we didn't remove
>                 spin_unlock (&md_access_lock);
>                 return -EBUSY;
>         }
>         md_count++;
>         // PTB count the individual partition and whole disk inclusions
>         if (slot) {
>             atomic_inc(&slot->md_count);
>         }
>         atomic_inc(&lo->md_count);
>         spin_unlock (&md_access_lock);
>         return 0;

Yeah, it looks as though the 2.4 code doesn't set SHOW_ERRS there.
Well, the 2.6 code apparently didn't do it either (as things were) but
it didn't matter because the set_remote_invalid() code that gets run
when things go bad across the net sets the flag. And I see the same
mechanism in the 2.4 code, so maybe nthing is wrong.  Nevertheless, I
don't think it would do any harm to set SHOW_ERRS here in the 2.4 code
(and in the 2.6 code).  One would need the same mechanism as I outlined
in the 2.6 case ...  namely


          atomic_inc(&lo->md_count);
+         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);
+         }
          spin_unlock (&md_access_lock);
          return 0;


and then in the BLKMDUNTFY case:

          atomic_dec(&lo->md_count);
+         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);
+         }
          spin_unlock (&md_access_lock);
          return 0;

I'm not altogether sure if SHOW_ERRS is a perfect thing for a pure
raid1 array, but with FR1 (which will retry read errors on its own
without faulting the device) it will be just peachy.


> This is from nbd-2.4.32pre.tgz I downloaded around 6th April 2005.

Looks fine - seems you just have the 2.4 code, and anyway it seems
maybe I left it up to the admin to set show_errs.  But I think it is
probably OK to set it on going into a raid array when its a FR1 array.
And since only FR1 sends the two ioctls above, it should be fine to add
the appropriate code (sketched above) there.

I've done it now in my copy.

> Is this a newer feature?

Peter