Date: Fri, 7 Jun 2002 02:00:06 -0700 (PDT) From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/38963: Unable to newfs vinum volumes Message-ID: <200206070900.g57906f48108@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/38963; it has been noted by GNATS.
From: Bruce Evans <bde@zeta.org.au>
To: KT Sin <ktsin@acm.org>
Cc: freebsd-gnats-submit@FreeBSD.ORG
Subject: Re: bin/38963: Unable to newfs vinum volumes
Date: Fri, 7 Jun 2002 18:53:13 +1000 (EST)
On Thu, 6 Jun 2002, KT Sin wrote:
> The attempt to remove -v flag in revision 1.60 of newfs.c broke newfs when newfsing vinum volumes.
>
> A snippet of the diff to rev 1.59:
>
> @@ -279,11 +274,11 @@ main(int argc, char *argv[])
> if (lp != NULL) {
> cp = strchr(special, '\0');
> cp--;
> - if (!vflag && (*cp < 'a' || *cp > 'h') && !isdigit(*cp))
> + if ((*cp < 'a' || *cp > 'h') && !isdigit(*cp))
> errx(1, "%s: can't figure out file system partition",
> special);
> - if (vflag || isdigit(*cp))
> - pp = &lp->d_partitions[0];
> + if (isdigit(*cp))
> + pp = &lp->d_partitions[RAW_PART];
> else
> pp = &lp->d_partitions[*cp - 'a'];
> oldpartition = *pp;
>
> We should not have removed vflags from all conditional statements blindly. A better approach is to implicitly set vflag to 1 since we want to make -v the default behaviour.
I think it is not so simple. -v can't be the default behaviour, because it
is inconsistent with the use of a partition other than RAW_PART. This
wasn't a problem when there was a -v flag. You could just not use -v
when newfs'ing a partition.
> >Fix:
> --- sbin/newfs/newfs.c.orig Thu May 16 15:36:55 2002
> +++ sbin/newfs/newfs.c Fri Jun 7 09:52:02 2002
> @@ -274,13 +274,7 @@
> if (lp != NULL) {
> cp = strchr(special, '\0');
> cp--;
> - if ((*cp < 'a' || *cp > 'h') && !isdigit(*cp))
> - errx(1, "%s: can't figure out filesystem partition",
> - special);
> - if (isdigit(*cp))
> - pp = &lp->d_partitions[RAW_PART];
> - else
> - pp = &lp->d_partitions[*cp - 'a'];
> + pp = &lp->d_partitions[RAW_PART];
> oldpartition = *pp;
> if (pp->p_size == 0)
> errx(1, "%s: `%c' partition is unavailable",
This gives the RAW partition in all cases, so at least the check of the
partition size is wrong if the device is a partition but not the RAW one
(but at least this wrongness is unimportant in most cases, since we
have checked the correct size early after getting it with DIOCMEDIASIZE).
Bringing back the -v flag seems to be the best way to fix this, short of
dropping support for partitions.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200206070900.g57906f48108>