[Cluster-devel] [PATCH][GFS2] Given device ID rather than s_id in "id" sysfs file

[Cluster-devel] [PATCH][GFS2] Given device ID rather than s_id in "id" sysfs file

[Bob Peterson]

Hi,

This patch changes the /sys/fs/gfs2/<s_id>/id file to give the device
id "major:minor" rather than the s_id.  That enables gfs2_tool to
match devices properly (by id, not name) when locating the tuning files.

Regards,

Bob Peterson
--
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
--
 fs/gfs2/sys.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 06e0b77..10807b7 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -32,7 +32,8 @@ spinlock_t gfs2_sys_margs_lock;
 
 static ssize_t id_show(struct gfs2_sbd *sdp, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%s\n", sdp->sd_vfs->s_id);
+	return snprintf(buf, PAGE_SIZE, "%u:%u\n",
+			MAJOR(sdp->sd_vfs->s_dev), MINOR(sdp->sd_vfs->s_dev));
 }
 
 static ssize_t fsname_show(struct gfs2_sbd *sdp, char *buf)

[Cluster-devel] Re: [PATCH][GFS2] Given device ID rather than s_id in "id" sysfs file

[Steven Whitehouse]

Hi,

Now in the -nmw git tree. Thanks,

Steve

On Fri, 2007-11-02 at 09:37 -0500, Bob Peterson wrote:
> Hi,
> 
> This patch changes the /sys/fs/gfs2/<s_id>/id file to give the device
> id "major:minor" rather than the s_id.  That enables gfs2_tool to
> match devices properly (by id, not name) when locating the tuning files.
> 
> Regards,
> 
> Bob Peterson
> --
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> --
>  fs/gfs2/sys.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index 06e0b77..10807b7 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -32,7 +32,8 @@ spinlock_t gfs2_sys_margs_lock;
>  
>  static ssize_t id_show(struct gfs2_sbd *sdp, char *buf)
>  {
> -	return snprintf(buf, PAGE_SIZE, "%s\n", sdp->sd_vfs->s_id);
> +	return snprintf(buf, PAGE_SIZE, "%u:%u\n",
> +			MAJOR(sdp->sd_vfs->s_dev), MINOR(sdp->sd_vfs->s_dev));
>  }
>  
>  static ssize_t fsname_show(struct gfs2_sbd *sdp, char *buf)
> 
> 

[Cluster-devel] [PATCH][GFS2] Given device ID rather than s_id in "id" sysfs file

[David Teigland]

On Fri, Nov 02, 2007 at 09:37:15AM -0500, Bob Peterson wrote:
> Hi,
> 
> This patch changes the /sys/fs/gfs2/<s_id>/id file to give the device
> id "major:minor" rather than the s_id.  That enables gfs2_tool to
> match devices properly (by id, not name) when locating the tuning files.

We have to be extremely cautious when changing the kernel abi like this;
have you verified that it doesn't break any existing programs?

> 
> Regards,
> 
> Bob Peterson
> --
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> --
>  fs/gfs2/sys.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index 06e0b77..10807b7 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -32,7 +32,8 @@ spinlock_t gfs2_sys_margs_lock;
>  
>  static ssize_t id_show(struct gfs2_sbd *sdp, char *buf)
>  {
> -	return snprintf(buf, PAGE_SIZE, "%s\n", sdp->sd_vfs->s_id);
> +	return snprintf(buf, PAGE_SIZE, "%u:%u\n",
> +			MAJOR(sdp->sd_vfs->s_dev), MINOR(sdp->sd_vfs->s_dev));
>  }
>  
>  static ssize_t fsname_show(struct gfs2_sbd *sdp, char *buf)
> 

[Cluster-devel] [PATCH][GFS2] Given device ID rather than s_id in "id" sysfs file

[Bob Peterson]

On Fri, 2007-11-02 at 09:55 -0500, David Teigland wrote:
> We have to be extremely cautious when changing the kernel abi like this;
> have you verified that it doesn't break any existing programs?

Dave has a good point here.  I've verified that no other gfs2 util
uses the "id" except for gfs2_tool.  However, this kernel change
makes previous userland versions of gfs2_tool stop working in some
cases, for example, gfs2_tool gettune and settune.
Customers would need to upgrade their gfs2_tool to use the new
gfs2 kernel module.

For that reason, perhaps we should revert this patch and instead
export the device id to a separate /sys/fs file called
/sys/fs/gfs2/<s_id>/device_id or some such.  That would ensure
backward compatibility with older userland tools.

BTW, this also led me to discover that the gfs2 quota tool
uses /sys/fs/gfs2/<lock table>/xxx which, in the case of stand-alone
file systems, would be a NULL string.  The quota tool should
be changed to use the same interface as gfs2_tool to determine
the proper path in sysfs.  Perhaps I'll open a bugzilla record on it.

Regards,

Bob Peterson
Red Hat Cluster Suite