adilger@clusterfs.com
2006-Dec-21 17:27 UTC
[Lustre-devel] [Bug 11135] NFS exports problem with symbolic link
Please don''t reply to lustre-devel. Instead, comment in Bugzilla by using the following link: https://bugzilla.lustre.org/show_bug.cgi?id=11135 What |Removed |Added ---------------------------------------------------------------------------- Attachment #9118|review?(adilger@clusterfs.co|review+ Flag|m) | (From update of attachment 9118)>+static int ll_new_node(struct inode *dir, struct qstr *name, >+ struct qstr *tgt, int mode, >+ int rdev, struct dentry *dchild)In the end I''m not sure if passing a qstr is an improvement or not. It does make the API marginally better, at the expense of extra stack space. New way is (qstr = pointer + len + hash) on the stack and qstr pointer as parameter vs. (pointer + int) as parameter, so new way is 8 or 12 bytes more stack usage. We don''t use the qstr directly anywhere internal to ll_new_inode() so there isn''t a win in that regard. Lustre is very stack constrained, and it is usually best to avoid stack usage where possible, so I would just pass the tgt_name and tgt_len directly as we did before.> { > struct ptlrpc_request *request = NULL; > struct inode *inode = NULL; > struct ll_sb_info *sbi = ll_i2sbi(dir); > struct mdc_op_data op_data; > int err; >+ >+ ENTRY; >+ >+ ll_prepare_mdc_op_data(&op_data, dir, NULL, name->name, >+ name->len, 0); >+ err = mdc_create(sbi->ll_mdc_exp, &op_data, tgt->name, tgt->len, >+ mode, current->fsuid, current->fsgid, >+ current->cap_effective, rdev, &request); >+ if (err) >+ goto err_exit;In Lustre code it is preferred to use GOTO(err_exit, err) so that the debug logs record which exit path was taken.>+ ll_update_times(request, 0, dir); >+ >+ if (dchild) { >+ err = ll_prep_inode(sbi->ll_osc_exp, &inode, request, 0, >+ dchild->d_sb); >+ if (err) >+ goto err_exit; >+ >+ d_instantiate(dchild, inode); >+ }Here you would put an EXIT, we know that err = 0 here.>+err_exit: >+ ptlrpc_req_finished(request); >+ >+ RETURN(err);And here you would just have "return err", since we know from the GOTO() macro what the error is and it is considered an "exit". Otherwise it looks good and nice clean code. Is there a test that we can use for this in sanity.sh (e.g. checking on client if there is an NFS mount, creating a symlink to that, doing ls).