Messages in this thread Patch in this message | | | Date | Fri, 09 Jan 1998 14:28:30 -0500 | From | Bill Hawes <> | Subject | patch for 2.1.78 knfsd |
| |
The attached patch will hopefully fix some of the problems reported with incorrect file lengths for certain clients. If not, it will at least help gather more debugging information.
The changes are as follows: (1) The nfsd_proc_create routine seems to be ignoring the size flag in the iattr when the file already exists. The comments state clearly what is desired, but the code appears to do the opposite. I've changed it so that nfsd_setattr is called only if the ATTR_SIZE flag is set. (Though probably it should always truncate to 0 length?)
(2) In nfsd_setattr, after changing the file size the ia_valid flags were being cleared, instead of just clearing the size bit. I think this was causing the modes to be incorrect for HPUX clients.
(3) I've made a lookup of "xyzzx" toggle just the nfsd debugging flag, so that it can be turned on and off selectively.
(4) The debugging output for some of the procedures now has more information.
Please test against the previously reported problem cases and let me know if this helps at all.
Regards, Bill--- linux-2.1.78/include/linux/nfsd/nfsfh.h.old Tue Jan 6 12:15:39 1998 +++ linux-2.1.78/include/linux/nfsd/nfsfh.h Fri Jan 9 12:40:10 1998 @@ -70,6 +70,8 @@ * Shorthand for dprintk()'s */ #define SVCFH_DENTRY(f) ((f)->fh_dentry) +#define SVCFH_INO(f) ((f)->fh_handle.fh_ino) +#define SVCFH_DEV(f) ((f)->fh_handle.fh_dev) /* * Function prototypes --- linux-2.1.78/fs/nfsd/vfs.c.old Tue Jan 6 11:39:08 1998 +++ linux-2.1.78/fs/nfsd/vfs.c Fri Jan 9 13:43:04 1998 @@ -190,20 +190,24 @@ inode = dentry->d_inode; /* The size case is special... */ - if ((iap->ia_valid & ATTR_SIZE) && S_ISREG(inode->i_mode)) { + if (iap->ia_valid & ATTR_SIZE) { +if (!S_ISREG(inode->i_mode)) +printk("nfsd_setattr: size change??\n"); if (iap->ia_size < inode->i_size) { err = nfsd_permission(fhp->fh_export, dentry, MAY_TRUNC); if (err != 0) goto out; } - if ((err = get_write_access(inode)) != 0) - return nfserrno(-err); + err = get_write_access(inode); + if (err) + goto out_nfserr; + /* N.B. Should we update the inode cache here? */ inode->i_size = iap->ia_size; if (inode->i_op && inode->i_op->truncate) inode->i_op->truncate(inode); mark_inode_dirty(inode); put_write_access(inode); - iap->ia_valid &= ATTR_SIZE; + iap->ia_valid &= ~ATTR_SIZE; iap->ia_valid |= ATTR_MTIME; iap->ia_mtime = CURRENT_TIME; } @@ -232,13 +236,17 @@ iap->ia_ctime = CURRENT_TIME; err = notify_change(dentry, iap); if (err) - return nfserrno(-err); + goto out_nfserr; if (EX_ISSYNC(fhp->fh_export)) write_inode_now(inode); } err = 0; out: return err; + +out_nfserr: + err = nfserrno(-err); + goto out; } /* --- linux-2.1.78/fs/nfsd/nfsfh.c.old Sun Nov 30 11:33:43 1997 +++ linux-2.1.78/fs/nfsd/nfsfh.c Fri Jan 9 15:08:05 1998 @@ -1089,8 +1089,10 @@ { struct inode * inode = dentry->d_inode; - dprintk("nfsd: fh_compose(exp %x/%ld dentry %p)\n", - exp->ex_dev, exp->ex_ino, dentry); + dprintk("nfsd: fh_compose(exp %x/%ld %s/%s, ino=%ld)\n", + exp->ex_dev, exp->ex_ino, + dentry->d_parent->d_name.name, dentry->d_name.name, + (inode ? inode->i_ino : 0)); /* * N.B. We shouldn't need to init the fh -- the call to fh_compose --- linux-2.1.78/fs/nfsd/nfsproc.c.old Sun Nov 30 11:26:24 1997 +++ linux-2.1.78/fs/nfsd/nfsproc.c Fri Jan 9 14:57:46 1998 @@ -56,7 +56,8 @@ nfsd_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp, struct nfsd_attrstat *resp) { - dprintk("nfsd: GETATTR %p\n", SVCFH_DENTRY(&argp->fh)); + dprintk("nfsd: GETATTR %d/%ld\n", + SVCFH_DEV(&argp->fh), SVCFH_INO(&argp->fh)); fh_copy(&resp->fh, &argp->fh); RETURN(fh_verify(rqstp, &resp->fh, 0, MAY_NOP)); @@ -70,7 +71,9 @@ nfsd_proc_setattr(struct svc_rqst *rqstp, struct nfsd_sattrargs *argp, struct nfsd_attrstat *resp) { - dprintk("nfsd: SETATTR %p\n", SVCFH_DENTRY(&argp->fh)); + dprintk("nfsd: SETATTR %d/%ld, valid=%x, size=%ld\n", + SVCFH_DEV(&argp->fh), SVCFH_INO(&argp->fh), + argp->attrs.ia_valid, (long) argp->attrs.ia_size); fh_copy(&resp->fh, &argp->fh); RETURN(nfsd_setattr(rqstp, &resp->fh, &argp->attrs)); @@ -88,7 +91,12 @@ { int nfserr; - dprintk("nfsd: LOOKUP %p %s\n", SVCFH_DENTRY(&argp->fh), argp->name); + dprintk("nfsd: LOOKUP %d/%ld %s\n", + SVCFH_DEV(&argp->fh), SVCFH_INO(&argp->fh), argp->name); +#ifdef RPC_DEBUG + if (!strcmp(argp->name, "xyzzx")) + nfsd_debug = ~nfsd_debug; +#endif nfserr = nfsd_lookup(rqstp, &argp->fh, argp->name, argp->len, &resp->fh); @@ -131,8 +139,8 @@ u32 * buffer; int nfserr, avail; - dprintk("nfsd: READ %p %d bytes at %d\n", - SVCFH_DENTRY(&argp->fh), + dprintk("nfsd: READ %d/%ld %d bytes at %d\n", + SVCFH_DEV(&argp->fh), SVCFH_INO(&argp->fh), argp->count, argp->offset); /* Obtain buffer pointer for payload. 19 is 1 word for @@ -168,8 +176,8 @@ { int nfserr; - dprintk("nfsd: WRITE %p %d bytes at %d\n", - SVCFH_DENTRY(&argp->fh), + dprintk("nfsd: WRITE %d/%ld %d bytes at %d\n", + SVCFH_DEV(&argp->fh), SVCFH_INO(&argp->fh), argp->len, argp->offset); nfserr = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh), @@ -192,24 +200,21 @@ nfsd_proc_create(struct svc_rqst *rqstp, struct nfsd_createargs *argp, struct nfsd_diropres *resp) { - struct inode *dirp, *inode = NULL; - struct iattr *attr; - svc_fh *dirfhp, *newfhp; + svc_fh *dirfhp = &argp->fh; + svc_fh *newfhp = &resp->fh; + struct iattr *attr = &argp->attrs; + struct inode *inode = NULL; int nfserr, type, mode; int rdonly = 0, exists; dev_t rdev = NODEV; - dprintk("nfsd: CREATE %p %s\n", SVCFH_DENTRY(&argp->fh), argp->name); - - dirfhp = &argp->fh; - newfhp = &resp->fh; - attr = &argp->attrs; + dprintk("nfsd: CREATE %d/%ld %s\n", + SVCFH_DEV(dirfhp), SVCFH_INO(dirfhp), argp->name); /* Get the directory inode */ nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, MAY_EXEC); if (nfserr) goto done; /* must fh_put dirfhp even on error */ - dirp = dirfhp->fh_dentry->d_inode; /* Check for MAY_WRITE separately. */ nfserr = nfsd_permission(dirfhp->fh_export, dirfhp->fh_dentry, @@ -247,10 +252,9 @@ } /* This is for "echo > /dev/null" a la SunOS. Argh. */ - if (rdonly && (!exists || type == S_IFREG)) { - nfserr = nfserr_rofs; + nfserr = nfserr_rofs; + if (rdonly && (!exists || type == S_IFREG)) goto done; - } attr->ia_valid |= ATTR_MODE; attr->ia_mode = type | mode; @@ -292,11 +296,19 @@ nfserr = nfsd_create(rqstp, dirfhp, argp->name, argp->len, attr, type, rdev, newfhp); } else if (type == S_IFREG) { + dprintk("nfsd: existing %s, valid=%x, size=%ld\n", + argp->name, attr->ia_valid, (long) attr->ia_size); /* File already exists. We ignore all attributes except * size, so that creat() behaves exactly like * open(..., O_CREAT|O_TRUNC|O_WRONLY). */ +#if 0 + /* N.B. What is this doing? ignores size?? */ if ((attr->ia_valid &= ~(ATTR_SIZE)) != 0) + nfserr = nfsd_setattr(rqstp, newfhp, attr); +#endif + attr->ia_valid &= ATTR_SIZE; + if (attr->ia_valid) nfserr = nfsd_setattr(rqstp, newfhp, attr); } | |