On Jun 10, 2010, at 10:06 AM, Benny Halevy wrote:
On Jun. 10, 2010, 16:35 +0300, "J. Bruce Fields"
<bfields@xxxxxxxxxxxx> wrote:
On Thu, Jun 10, 2010 at 09:33:37AM -0400, bfields wrote:
The patch "pnfsd: fix test in nfsd4_find_pnfs_dlm_device" was
obviously
correct; before that patch, nfsd4_find_pnfs_dlm_device() returned
the
first device which did *not* match, instead of the first device
which
did.
Other callers previous worked because they passed int the incorrect
name, and because we never had more than one element in the list, so
returning the first non-match would return the right thing.
But then I don't know what point the "/dev/" is serving here. May as
well just change the interface so that what's passed in is just the
name
("sdc2" or whatever)--no use pretending this is some kind of path
name
if it's not really.
However, I don't know whether passing in the disk name is really a
sensible way to refer to a filesystem in a user<->kernel api.
up to you... I think the whole scheme is currently GFS2 specific
No, it's DLM specific, with GFS2 being the only current user.
and it assumes the device exists symmetrically on /dev/... on all
DSs.
No. This identifies the MDS exported DLM pNFS file system, and has
nothing to do with the local DS file system.
The /dev prefix seems to be completely redundant.
It makes more sense to get rid of it in the user->kernel
API as passed down to nfsd4_set_pnfs_dlm_device() rather than
adding it here.
Sure.
Benny
--b.
But now we need to fix those callers.
From: Eric Anderle <eanderle@xxxxxxxxx>
---
fs/nfsd/nfs4pnfsdlm.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index 84caa6e..5a66a71 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -268,10 +268,13 @@ static int nfsd4_pnfs_dlm_getdevinfo(struct
super_block *sb,
/*
* If the DS list has not been established, return -EINVAL
*/
- dlm_pdev = nfsd4_find_pnfs_dlm_device(sb->s_bdev->bd_disk-
>disk_name);
+ /* Long enough to hold "/dev/" + disk name */
+ char full_disk_name[6 + strlen(sb->s_bdev->bd_disk->disk_name)];
+ sprintf(full_disk_name, "/dev/%s", sb->s_bdev->bd_disk-
>disk_name);
+ dlm_pdev = nfsd4_find_pnfs_dlm_device(full_disk_name);
if (!dlm_pdev) {
dprintk("%s: DEBUG: disk %s Not Found\n", __func__,
- sb->s_bdev->bd_disk->disk_name);
+ full_disk_name);
return err;
}
@@ -364,7 +367,10 @@ static int dlm_ino_hash(struct inode *ino)
/* If can't find the inode block device in the pnfs_dlm_deivce list
* then don't hand out a layout
*/
- de = nfsd4_find_pnfs_dlm_device(ino->i_sb->s_bdev->bd_disk-
>disk_name);
+ /* Long enough to hold "/dev/" + disk name */
+ char full_disk_name[6 + strlen(ino->i_sb->s_bdev->bd_disk-
>disk_name)];
+ sprintf(full_disk_name, "/dev/%s", ino->i_sb->s_bdev->bd_disk-
>disk_name);
+ de = nfsd4_find_pnfs_dlm_device(full_disk_name);
if (!de)
return -1;
hash_mask = de->num_ds - 1;
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs"
in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
