From owner-freebsd-hackers  Thu Mar 14 10:03:38 1996
Return-Path: owner-hackers
Received: (from root@localhost)
          by freefall.freebsd.org (8.7.3/8.7.3) id KAA22487
          for hackers-outgoing; Thu, 14 Mar 1996 10:03:38 -0800 (PST)
Received: from phaeton.artisoft.com (phaeton.Artisoft.COM [198.17.250.211])
          by freefall.freebsd.org (8.7.3/8.7.3) with SMTP id KAA22481
          for <freebsd-hackers@FreeBSD.ORG>; Thu, 14 Mar 1996 10:03:36 -0800 (PST)
Received: (from terry@localhost) by phaeton.artisoft.com (8.6.11/8.6.9) id KAA11338; Thu, 14 Mar 1996 10:59:15 -0700
From: Terry Lambert <terry@lambert.org>
Message-Id: <199603141759.KAA11338@phaeton.artisoft.com>
Subject: Re: PANIC ( remove a file on the read-only FS)
To: m_tanaka@pa.yokogawa.co.jp (Mihoko Tanaka)
Date: Thu, 14 Mar 1996 10:59:15 -0700 (MST)
Cc: freebsd-hackers@FreeBSD.ORG
In-Reply-To: <9603141233.AA20013@cabbage.pa.yokogawa.co.jp> from "Mihoko Tanaka" at Mar 14, 96 09:33:15 pm
X-Mailer: ELM [version 2.4 PL24]
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-hackers@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

> The function 'VOP_LOOKUP()' calls xxx_lookup() (xxx : file system name).
> When I remove a file which is on a read-only file system, 
> VOP_LOOKUP() returns a error(EROFS) and ndp->ni_vp isn't NULL
> because it exist.
> 
> So if I compile my kernel with 'DIAGNOSTIC', panic occurs .
> If without 'DIAGNOSTIC', the system conflicts somewhere and gets into panic.
> 
> 
> I made some patch for this trouble.
> Is it correct?  Does anyone have another good idea?
> -----------------(cut here)------------------------------------------
> --- vfs_lookup.c	Wed Oct 25 18:17:15 1995
> +++ vfs_lookup.c.new	Thu Mar 14 20:23:05 1996
> @@ -399,6 +399,9 @@
>  	error = VOP_LOOKUP(dp, &ndp->ni_vp, cnp);
>  	if (error) {
>  #ifdef DIAGNOSTIC
> +		if (error == EROFS)
> +			goto bad;
> +
>  		if (ndp->ni_vp != NULL)
>  			panic("leaf should be empty");
>  #endif
> @@ -422,7 +425,7 @@
>  		 * If creating and at end of pathname, then can consider
>  		 * allowing file to be created.
>  		 */
> -		if (rdonly) {
> +		if (rdonly || (dp->v_mount->mnt_flag & MNT_RDONLY)) {
>  			error = EROFS;
>  			goto bad;
>  		}
> -----------------(cut here)------------------------------------------

The error check should be *outside* the #ifdef DIAGNOSTIC, IMO.  8-).

Other than that, this seems to be the correct response to the move
of the read-only down into the FS proper.

The read-only error code return move was the right thing to do because
of overlay FS's, like translucent, union (not the union causing the
problem in this case, which is the option, but the real FS), etc..  The
EROFS needs to be returned by the underlying FS of an FS pair only if
it is applicable; otherwise a union mount on a CDROM rendered the
writable non-CDROM FS involved non-writeable (or cause the panic you
saw).


					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.