Subject: Re: My changes to subr_disc_mbr (Was Re: default generated disklabels get overwritten?)
To: None <tech-kern@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 12/15/2005 00:24:03
On Wed, Dec 14, 2005 at 11:41:31PM +0100, Reinoud Zandijk wrote:
> I traced the problem back to the readdisklabel() in kern/subr_disk_mbr.c 
> where the function returns NULL regardless if there is a disklabel on disc 
> or not. It only returns an error string if there was an error reading 
> sectors or when the read in disklabel was corrupt.
> 
> This makes it impossible for its callers (disc device drivers) to determine 
> if there was indeed a disklabel on disc or not or that it needs to create a 
> default label for the disc. The basic change i made was:
> 
> -----------------
> -__KERNEL_RCSID(0, "$NetBSD: subr_disk_mbr.c,v 1.11 2005/12/14 21:40:32 dsl Exp $");
> +__KERNEL_RCSID(0, "$NetBSD: subr_disk_mbr.c,v 1.7 2005/12/11 12:24:30 christos Exp $");
>  
>  #include <sys/param.h>
>  #include <sys/systm.h>
> @@ -297,6 +297,8 @@
>         }
>  
>         brelse(a.bp);
> +       if (rval == SCAN_CONTINUE)
> +               return "No disclabel found";
>         if (rval == SCAN_ERROR)
>                 return a.msg;
>         return NULL;
> -------------------
> 
> It was reversed later the same day without discussion. I propably should 
> have mailed first to this list but IMHO its broken in the origional form.

Yes, you should have started a discussion first. Your change will cause 
problems, see dev/isa/fd.c for example. It's probably not the only driver
using readdisklabel this way.

> The result is that a disk driver like sys/dev/scsipi/cd.c can not create 
> its own default label while still supporting `normal' disklabels to be on 
> the disc. It can't determine if there is a label on the disk or not.

The real problem is that readdisklabel() can't return detailled informations
on what happended. Sounds like you want to change its interface, with
the error string returned in a char* passed as parameter and the return
value being an error code with multiple values: got a label from disk,
no label on disk (provided a fake label), error reading label from disk
(provided a fake label), maybe more.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--