* [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. @ 2010-06-10 13:33 J. Bruce Fields 2010-06-10 13:35 ` J. Bruce Fields 0 siblings, 1 reply; 7+ messages in thread From: J. Bruce Fields @ 2010-06-10 13:33 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs 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 now we need to fix those callers. From: Eric Anderle <eanderle@umich.edu> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 13:33 [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name J. Bruce Fields @ 2010-06-10 13:35 ` J. Bruce Fields 2010-06-10 14:06 ` Benny Halevy 0 siblings, 1 reply; 7+ messages in thread From: J. Bruce Fields @ 2010-06-10 13:35 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs, eanderle 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. --b. > > But now we need to fix those callers. > > From: Eric Anderle <eanderle@umich.edu> > > --- > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 13:35 ` J. Bruce Fields @ 2010-06-10 14:06 ` Benny Halevy 2010-06-10 14:18 ` Benny Halevy 2010-06-10 14:36 ` Andy Adamson 0 siblings, 2 replies; 7+ messages in thread From: Benny Halevy @ 2010-06-10 14:06 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, eanderle On Jun. 10, 2010, 16:35 +0300, "J. Bruce Fields" <bfields@fieldses.org> 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 and it assumes the device exists symmetrically on /dev/... on all DSs. 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. Benny > --b. > >> >> But now we need to fix those callers. >> >> From: Eric Anderle <eanderle@umich.edu> >> >> --- >> 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 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 14:06 ` Benny Halevy @ 2010-06-10 14:18 ` Benny Halevy 2010-06-10 14:36 ` Andy Adamson 1 sibling, 0 replies; 7+ messages in thread From: Benny Halevy @ 2010-06-10 14:18 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, eanderle On Jun. 10, 2010, 17:06 +0300, Benny Halevy <bhalevy@panasas.com> wrote: > On Jun. 10, 2010, 16:35 +0300, "J. Bruce Fields" <bfields@fieldses.org> 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 > and it assumes the device exists symmetrically on /dev/... on all > DSs. 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. A more through design could use the fs uuid rather than the device name which is kinda volatile... Benny > > Benny > >> --b. >> >>> >>> But now we need to fix those callers. >>> >>> From: Eric Anderle <eanderle@umich.edu> >>> >>> --- >>> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 14:06 ` Benny Halevy 2010-06-10 14:18 ` Benny Halevy @ 2010-06-10 14:36 ` Andy Adamson 2010-06-10 14:52 ` Eric Anderle 1 sibling, 1 reply; 7+ messages in thread From: Andy Adamson @ 2010-06-10 14:36 UTC (permalink / raw) To: Benny Halevy; +Cc: J. Bruce Fields, linux-nfs, eanderle On Jun 10, 2010, at 10:06 AM, Benny Halevy wrote: > On Jun. 10, 2010, 16:35 +0300, "J. Bruce Fields" > <bfields@fieldses.org> 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@umich.edu> >>> >>> --- >>> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 14:36 ` Andy Adamson @ 2010-06-10 14:52 ` Eric Anderle 2010-06-10 17:31 ` J. Bruce Fields 0 siblings, 1 reply; 7+ messages in thread From: Eric Anderle @ 2010-06-10 14:52 UTC (permalink / raw) To: Andy Adamson; +Cc: Benny Halevy, J. Bruce Fields, linux-nfs What if we added a struct block_device member to dlm_device_list? That way we could look it up by major:minor numbers. On Thu, 2010-06-10 at 10:36 -0400, Andy Adamson wrote: > On Jun 10, 2010, at 10:06 AM, Benny Halevy wrote: > > > On Jun. 10, 2010, 16:35 +0300, "J. Bruce Fields" > > <bfields@fieldses.org> 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@umich.edu> > >>> > >>> --- > >>> 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- Eric Anderle <eanderle@umich.edu> U-M Class of 2013 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name. 2010-06-10 14:52 ` Eric Anderle @ 2010-06-10 17:31 ` J. Bruce Fields 0 siblings, 0 replies; 7+ messages in thread From: J. Bruce Fields @ 2010-06-10 17:31 UTC (permalink / raw) To: Eric Anderle; +Cc: Andy Adamson, Benny Halevy, linux-nfs On Thu, Jun 10, 2010 at 10:52:52AM -0400, Eric Anderle wrote: > What if we added a struct block_device member to dlm_device_list? That > way we could look it up by major:minor numbers. I don't know. By the way: > > >>> 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; There's some duplicated code there; it should probably go into a common function called from both places. --b. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-06-10 17:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-10 13:33 [PATCH] Fixed getdevinfo() and dlm_ino_hash() to search for the correct disk name J. Bruce Fields 2010-06-10 13:35 ` J. Bruce Fields 2010-06-10 14:06 ` Benny Halevy 2010-06-10 14:18 ` Benny Halevy 2010-06-10 14:36 ` Andy Adamson 2010-06-10 14:52 ` Eric Anderle 2010-06-10 17:31 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox