[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Andrew Price]

When using symlinks with mkfs.gfs2 it reports what the symlink points
to instead of the contents of the device, like so:

 # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
 This will destroy any data on /dev/vg/test.
 It appears to contain: symbolic link to `../dm-3'

This patch resolves symlinks before checking the device contents to make
the output more informative:

 # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
 This will destroy any data on /dev/vg/test.
 It appears to contain: GFS2 Filesystem (blocksize 4096, lockproto lock_nolock)

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index e6b00a0..f8e4741 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
 	int error;
 	int rgsize_specified = 0;
 	unsigned char uuid[16];
+	char *absname;
 
 	memset(sdp, 0, sizeof(struct gfs2_sbd));
 	sdp->bsize = -1;
@@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	absname = canonicalize_file_name(sdp->device_name);
+	if (absname == NULL) {
+		perror(_("Could not find the absolute path of the device"));
+		exit(EXIT_FAILURE);
+	}
 	if (!sdp->override) {
 		printf( _("This will destroy any data on %s.\n"), sdp->device_name);
-		check_dev_content(sdp->device_name);
+		check_dev_content(absname);
 		are_you_sure();
 	}
+	free(absname);
 
 	if (sdp->bsize == -1) {
 		if (S_ISREG(sdp->dinfo.stat.st_mode))
-- 
1.7.7.6

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Bob Peterson]

----- Original Message -----
| +	absname = canonicalize_file_name(sdp->device_name);

Hi Andy,

Thanks for the patch. I just wanted to point out that in the past we've
used realpath rather than canonicalize_file_name. For example, see this patch
we did a long time ago to gfs2_tool:

http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=e70898cfa09939a7100a057433fff3a4ad666bdd

It would be nice if our use was consistent. I'm not sure if there's an
advantage of one over the other. If canonicalize_file_name is now preferred
upstream over realpath, we should probably replace all occurrences of that.

On the other hand, if realpath is now preferred upstream, we should adjust
this patch to use it instead. AFAIK, they are the same, and I don't have a
personal preference; whatever is most favoured by the upstream community. :)

Otherwise, the patch looks good.

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Fabio M. Di Nitto]

Hi,

On 6/20/2012 6:15 PM, Bob Peterson wrote:
> ----- Original Message -----
> | +	absname = canonicalize_file_name(sdp->device_name);
> 
> Hi Andy,
> 
> Thanks for the patch. I just wanted to point out that in the past we've
> used realpath rather than canonicalize_file_name. For example, see this patch
> we did a long time ago to gfs2_tool:
> 
> http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=e70898cfa09939a7100a057433fff3a4ad666bdd
> 
> It would be nice if our use was consistent. I'm not sure if there's an
> advantage of one over the other. If canonicalize_file_name is now preferred
> upstream over realpath, we should probably replace all occurrences of that.
> 
> On the other hand, if realpath is now preferred upstream, we should adjust
> this patch to use it instead. AFAIK, they are the same, and I don't have a
> personal preference; whatever is most favoured by the upstream community. :)
> 
> Otherwise, the patch looks good.

I don?t remember what other mkfs.* tools do, but if I would prefer to
see something like:

# ./mkfs.gfs2 -p lock_nolock /dev/vg/test
WARNING: /dev/vg/test appears to be a symlink to /dev/real/device
This will destroy any data on /dev/real/device
It appears to contain: RANDOM_FS_OF_DOOM (blocksize......)

Fabio

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Andrew Price]

Hi Bob,

On 20/06/12 17:15, Bob Peterson wrote:
> ----- Original Message -----
> | +	absname = canonicalize_file_name(sdp->device_name);
>
> Hi Andy,
>
> Thanks for the patch. I just wanted to point out that in the past we've
> used realpath rather than canonicalize_file_name. For example, see this patch
> we did a long time ago to gfs2_tool:
>
> http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=e70898cfa09939a7100a057433fff3a4ad666bdd

Hmm those uses of realpath seem to have disappeared since.

> It would be nice if our use was consistent. I'm not sure if there's an
> advantage of one over the other. If canonicalize_file_name is now preferred
> upstream over realpath, we should probably replace all occurrences of that.
>
> On the other hand, if realpath is now preferred upstream, we should adjust
> this patch to use it instead. AFAIK, they are the same, and I don't have a
> personal preference; whatever is most favoured by the upstream community. :)

I couldn't find any strong arguments in preference of either function 
and we're already using _GNU_SOURCE extensions so there's no added 
portability issue.  In the current state of gfs2-utils.git we're only 
using realpath twice, in gfs2_quota, so I don't think there's a strong 
consistency argument either.  I'll push this one as-is in the morning 
unless someone can provide a convincing reason to use realpath :)

> Otherwise, the patch looks good.

Thanks for the review,

Andy

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Steven Whitehouse]

Hi,

Looks good. ACK,

Steve.

On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
> When using symlinks with mkfs.gfs2 it reports what the symlink points
> to instead of the contents of the device, like so:
> 
>  # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>  This will destroy any data on /dev/vg/test.
>  It appears to contain: symbolic link to `../dm-3'
> 
> This patch resolves symlinks before checking the device contents to make
> the output more informative:
> 
>  # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>  This will destroy any data on /dev/vg/test.
>  It appears to contain: GFS2 Filesystem (blocksize 4096, lockproto lock_nolock)
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index e6b00a0..f8e4741 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
>  	int error;
>  	int rgsize_specified = 0;
>  	unsigned char uuid[16];
> +	char *absname;
>  
>  	memset(sdp, 0, sizeof(struct gfs2_sbd));
>  	sdp->bsize = -1;
> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	absname = canonicalize_file_name(sdp->device_name);
> +	if (absname == NULL) {
> +		perror(_("Could not find the absolute path of the device"));
> +		exit(EXIT_FAILURE);
> +	}
>  	if (!sdp->override) {
>  		printf( _("This will destroy any data on %s.\n"), sdp->device_name);
> -		check_dev_content(sdp->device_name);
> +		check_dev_content(absname);
>  		are_you_sure();
>  	}
> +	free(absname);
>  
>  	if (sdp->bsize == -1) {
>  		if (S_ISREG(sdp->dinfo.stat.st_mode))

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Andrew Price]

Hi Steve,

On 06/28/2012 10:16 AM, Steven Whitehouse wrote:
> Hi,
>
> Looks good. ACK,

This one was obsoleted by the v2 patch I posted on the 21st. If you 
prefer this one though I can push it instead.

Andy

>
> Steve.
>
> On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
>> When using symlinks with mkfs.gfs2 it reports what the symlink points
>> to instead of the contents of the device, like so:
>>
>>   # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>   This will destroy any data on /dev/vg/test.
>>   It appears to contain: symbolic link to `../dm-3'
>>
>> This patch resolves symlinks before checking the device contents to make
>> the output more informative:
>>
>>   # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>   This will destroy any data on /dev/vg/test.
>>   It appears to contain: GFS2 Filesystem (blocksize 4096, lockproto lock_nolock)
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>>   gfs2/mkfs/main_mkfs.c |    9 ++++++++-
>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
>> index e6b00a0..f8e4741 100644
>> --- a/gfs2/mkfs/main_mkfs.c
>> +++ b/gfs2/mkfs/main_mkfs.c
>> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
>>   	int error;
>>   	int rgsize_specified = 0;
>>   	unsigned char uuid[16];
>> +	char *absname;
>>
>>   	memset(sdp, 0, sizeof(struct gfs2_sbd));
>>   	sdp->bsize = -1;
>> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
>>   		exit(EXIT_FAILURE);
>>   	}
>>
>> +	absname = canonicalize_file_name(sdp->device_name);
>> +	if (absname == NULL) {
>> +		perror(_("Could not find the absolute path of the device"));
>> +		exit(EXIT_FAILURE);
>> +	}
>>   	if (!sdp->override) {
>>   		printf( _("This will destroy any data on %s.\n"), sdp->device_name);
>> -		check_dev_content(sdp->device_name);
>> +		check_dev_content(absname);
>>   		are_you_sure();
>>   	}
>> +	free(absname);
>>
>>   	if (sdp->bsize == -1) {
>>   		if (S_ISREG(sdp->dinfo.stat.st_mode))
>
>

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Steven Whitehouse]

Hi,

Yes, v2 is ok too. I think I missed that in my first pass over my email
today. To do this properly, we should be opening the path that we are
passed, and then doing the remaining operations with just the fd. We
could do that with "file" if we used the -f - option and passed the fd
as standard input to the process.

That way we also remove any possible race conditions too, but this is
good enough for now,

Steve.


On Thu, 2012-06-28 at 10:58 +0100, Andrew Price wrote:
> Hi Steve,
> 
> On 06/28/2012 10:16 AM, Steven Whitehouse wrote:
> > Hi,
> >
> > Looks good. ACK,
> 
> This one was obsoleted by the v2 patch I posted on the 21st. If you 
> prefer this one though I can push it instead.
> 
> Andy
> 
> >
> > Steve.
> >
> > On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
> >> When using symlinks with mkfs.gfs2 it reports what the symlink points
> >> to instead of the contents of the device, like so:
> >>
> >>   # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
> >>   This will destroy any data on /dev/vg/test.
> >>   It appears to contain: symbolic link to `../dm-3'
> >>
> >> This patch resolves symlinks before checking the device contents to make
> >> the output more informative:
> >>
> >>   # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
> >>   This will destroy any data on /dev/vg/test.
> >>   It appears to contain: GFS2 Filesystem (blocksize 4096, lockproto lock_nolock)
> >>
> >> Signed-off-by: Andrew Price <anprice@redhat.com>
> >> ---
> >>   gfs2/mkfs/main_mkfs.c |    9 ++++++++-
> >>   1 files changed, 8 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> >> index e6b00a0..f8e4741 100644
> >> --- a/gfs2/mkfs/main_mkfs.c
> >> +++ b/gfs2/mkfs/main_mkfs.c
> >> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
> >>   	int error;
> >>   	int rgsize_specified = 0;
> >>   	unsigned char uuid[16];
> >> +	char *absname;
> >>
> >>   	memset(sdp, 0, sizeof(struct gfs2_sbd));
> >>   	sdp->bsize = -1;
> >> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
> >>   		exit(EXIT_FAILURE);
> >>   	}
> >>
> >> +	absname = canonicalize_file_name(sdp->device_name);
> >> +	if (absname == NULL) {
> >> +		perror(_("Could not find the absolute path of the device"));
> >> +		exit(EXIT_FAILURE);
> >> +	}
> >>   	if (!sdp->override) {
> >>   		printf( _("This will destroy any data on %s.\n"), sdp->device_name);
> >> -		check_dev_content(sdp->device_name);
> >> +		check_dev_content(absname);
> >>   		are_you_sure();
> >>   	}
> >> +	free(absname);
> >>
> >>   	if (sdp->bsize == -1) {
> >>   		if (S_ISREG(sdp->dinfo.stat.st_mode))
> >
> >
> 
> 

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Andrew Price]

On 06/28/2012 11:01 AM, Steven Whitehouse wrote:
> Hi,
>
> Yes, v2 is ok too. I think I missed that in my first pass over my email
> today. To do this properly, we should be opening the path that we are
> passed, and then doing the remaining operations with just the fd. We
> could do that with "file" if we used the -f - option and passed the fd
> as standard input to the process.

The only way I can think of passing the fd to file -f - would be the C 
equivalent of:

   printf "/proc/$pid/fd/$fd" | file -f -

Which would mean adding another pipe() and fork(). So we might as well 
just shorten it to:

   file "/proc/$pid/fd/$fd"

Would this solve the race condition problem? Or am I misunderstanding 
how you intended it to be done?

Andy


> That way we also remove any possible race conditions too, but this is
> good enough for now,
>
> Steve.
>
>
> On Thu, 2012-06-28 at 10:58 +0100, Andrew Price wrote:
>> Hi Steve,
>>
>> On 06/28/2012 10:16 AM, Steven Whitehouse wrote:
>>> Hi,
>>>
>>> Looks good. ACK,
>>
>> This one was obsoleted by the v2 patch I posted on the 21st. If you
>> prefer this one though I can push it instead.
>>
>> Andy
>>
>>>
>>> Steve.
>>>
>>> On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
>>>> When using symlinks with mkfs.gfs2 it reports what the symlink points
>>>> to instead of the contents of the device, like so:
>>>>
>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>>>    This will destroy any data on /dev/vg/test.
>>>>    It appears to contain: symbolic link to `../dm-3'
>>>>
>>>> This patch resolves symlinks before checking the device contents to make
>>>> the output more informative:
>>>>
>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>>>    This will destroy any data on /dev/vg/test.
>>>>    It appears to contain: GFS2 Filesystem (blocksize 4096, lockproto lock_nolock)
>>>>
>>>> Signed-off-by: Andrew Price <anprice@redhat.com>
>>>> ---
>>>>    gfs2/mkfs/main_mkfs.c |    9 ++++++++-
>>>>    1 files changed, 8 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
>>>> index e6b00a0..f8e4741 100644
>>>> --- a/gfs2/mkfs/main_mkfs.c
>>>> +++ b/gfs2/mkfs/main_mkfs.c
>>>> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
>>>>    	int error;
>>>>    	int rgsize_specified = 0;
>>>>    	unsigned char uuid[16];
>>>> +	char *absname;
>>>>
>>>>    	memset(sdp, 0, sizeof(struct gfs2_sbd));
>>>>    	sdp->bsize = -1;
>>>> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
>>>>    		exit(EXIT_FAILURE);
>>>>    	}
>>>>
>>>> +	absname = canonicalize_file_name(sdp->device_name);
>>>> +	if (absname == NULL) {
>>>> +		perror(_("Could not find the absolute path of the device"));
>>>> +		exit(EXIT_FAILURE);
>>>> +	}
>>>>    	if (!sdp->override) {
>>>>    		printf( _("This will destroy any data on %s.\n"), sdp->device_name);
>>>> -		check_dev_content(sdp->device_name);
>>>> +		check_dev_content(absname);
>>>>    		are_you_sure();
>>>>    	}
>>>> +	free(absname);
>>>>
>>>>    	if (sdp->bsize == -1) {
>>>>    		if (S_ISREG(sdp->dinfo.stat.st_mode))
>>>
>>>
>>
>>
>
>

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Andrew Price]

On 06/28/2012 06:52 PM, Andrew Price wrote:
> On 06/28/2012 11:01 AM, Steven Whitehouse wrote:
>> Hi,
>>
>> Yes, v2 is ok too. I think I missed that in my first pass over my email
>> today. To do this properly, we should be opening the path that we are
>> passed, and then doing the remaining operations with just the fd. We
>> could do that with "file" if we used the -f - option and passed the fd
>> as standard input to the process.
>
> The only way I can think of passing the fd to file -f - would be the C
> equivalent of:
>
>    printf "/proc/$pid/fd/$fd" | file -f -
>
> Which would mean adding another pipe() and fork(). So we might as well
> just shorten it to:
>
>    file "/proc/$pid/fd/$fd"
>
> Would this solve the race condition problem? Or am I misunderstanding
> how you intended it to be done?

I didn't get a reply to this. Does the above method seem sensible or 
would we need to read the start of the file/device ourselves and pipe it 
into 'file -' to avoid opening the file twice?

Andy

>> That way we also remove any possible race conditions too, but this is
>> good enough for now,
>>
>> Steve.
>>
>>
>> On Thu, 2012-06-28 at 10:58 +0100, Andrew Price wrote:
>>> Hi Steve,
>>>
>>> On 06/28/2012 10:16 AM, Steven Whitehouse wrote:
>>>> Hi,
>>>>
>>>> Looks good. ACK,
>>>
>>> This one was obsoleted by the v2 patch I posted on the 21st. If you
>>> prefer this one though I can push it instead.
>>>
>>> Andy
>>>
>>>>
>>>> Steve.
>>>>
>>>> On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
>>>>> When using symlinks with mkfs.gfs2 it reports what the symlink points
>>>>> to instead of the contents of the device, like so:
>>>>>
>>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>>>>    This will destroy any data on /dev/vg/test.
>>>>>    It appears to contain: symbolic link to `../dm-3'
>>>>>
>>>>> This patch resolves symlinks before checking the device contents to
>>>>> make
>>>>> the output more informative:
>>>>>
>>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
>>>>>    This will destroy any data on /dev/vg/test.
>>>>>    It appears to contain: GFS2 Filesystem (blocksize 4096,
>>>>> lockproto lock_nolock)
>>>>>
>>>>> Signed-off-by: Andrew Price <anprice@redhat.com>
>>>>> ---
>>>>>    gfs2/mkfs/main_mkfs.c |    9 ++++++++-
>>>>>    1 files changed, 8 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
>>>>> index e6b00a0..f8e4741 100644
>>>>> --- a/gfs2/mkfs/main_mkfs.c
>>>>> +++ b/gfs2/mkfs/main_mkfs.c
>>>>> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
>>>>>        int error;
>>>>>        int rgsize_specified = 0;
>>>>>        unsigned char uuid[16];
>>>>> +    char *absname;
>>>>>
>>>>>        memset(sdp, 0, sizeof(struct gfs2_sbd));
>>>>>        sdp->bsize = -1;
>>>>> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
>>>>>            exit(EXIT_FAILURE);
>>>>>        }
>>>>>
>>>>> +    absname = canonicalize_file_name(sdp->device_name);
>>>>> +    if (absname == NULL) {
>>>>> +        perror(_("Could not find the absolute path of the device"));
>>>>> +        exit(EXIT_FAILURE);
>>>>> +    }
>>>>>        if (!sdp->override) {
>>>>>            printf( _("This will destroy any data on %s.\n"),
>>>>> sdp->device_name);
>>>>> -        check_dev_content(sdp->device_name);
>>>>> +        check_dev_content(absname);
>>>>>            are_you_sure();
>>>>>        }
>>>>> +    free(absname);
>>>>>
>>>>>        if (sdp->bsize == -1) {
>>>>>            if (S_ISREG(sdp->dinfo.stat.st_mode))
>>>>
>>>>
>>>
>>>
>>
>>
>
>

[Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents

[Steven Whitehouse]

Hi,

On Wed, 2012-07-04 at 12:02 +0100, Andrew Price wrote:
> On 06/28/2012 06:52 PM, Andrew Price wrote:
> > On 06/28/2012 11:01 AM, Steven Whitehouse wrote:
> >> Hi,
> >>
> >> Yes, v2 is ok too. I think I missed that in my first pass over my email
> >> today. To do this properly, we should be opening the path that we are
> >> passed, and then doing the remaining operations with just the fd. We
> >> could do that with "file" if we used the -f - option and passed the fd
> >> as standard input to the process.
> >
> > The only way I can think of passing the fd to file -f - would be the C
> > equivalent of:
> >
> >    printf "/proc/$pid/fd/$fd" | file -f -
> >
> > Which would mean adding another pipe() and fork(). So we might as well
> > just shorten it to:
> >
> >    file "/proc/$pid/fd/$fd"
> >
> > Would this solve the race condition problem? Or am I misunderstanding
> > how you intended it to be done?
> 
> I didn't get a reply to this. Does the above method seem sensible or 
> would we need to read the start of the file/device ourselves and pipe it 
> into 'file -' to avoid opening the file twice?
> 
> Andy
> 
Yes, that should work too. The intent is to avoid any issues with the
name changing from when mkfs opens the device to when the device is
checked, so any method based on using the open fd, rather than parsing
the path a second time is ok,

Steve.

> >> That way we also remove any possible race conditions too, but this is
> >> good enough for now,
> >>
> >> Steve.
> >>
> >>
> >> On Thu, 2012-06-28 at 10:58 +0100, Andrew Price wrote:
> >>> Hi Steve,
> >>>
> >>> On 06/28/2012 10:16 AM, Steven Whitehouse wrote:
> >>>> Hi,
> >>>>
> >>>> Looks good. ACK,
> >>>
> >>> This one was obsoleted by the v2 patch I posted on the 21st. If you
> >>> prefer this one though I can push it instead.
> >>>
> >>> Andy
> >>>
> >>>>
> >>>> Steve.
> >>>>
> >>>> On Wed, 2012-06-20 at 16:47 +0100, Andrew Price wrote:
> >>>>> When using symlinks with mkfs.gfs2 it reports what the symlink points
> >>>>> to instead of the contents of the device, like so:
> >>>>>
> >>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
> >>>>>    This will destroy any data on /dev/vg/test.
> >>>>>    It appears to contain: symbolic link to `../dm-3'
> >>>>>
> >>>>> This patch resolves symlinks before checking the device contents to
> >>>>> make
> >>>>> the output more informative:
> >>>>>
> >>>>>    # ./mkfs.gfs2 -p lock_nolock /dev/vg/test
> >>>>>    This will destroy any data on /dev/vg/test.
> >>>>>    It appears to contain: GFS2 Filesystem (blocksize 4096,
> >>>>> lockproto lock_nolock)
> >>>>>
> >>>>> Signed-off-by: Andrew Price <anprice@redhat.com>
> >>>>> ---
> >>>>>    gfs2/mkfs/main_mkfs.c |    9 ++++++++-
> >>>>>    1 files changed, 8 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> >>>>> index e6b00a0..f8e4741 100644
> >>>>> --- a/gfs2/mkfs/main_mkfs.c
> >>>>> +++ b/gfs2/mkfs/main_mkfs.c
> >>>>> @@ -531,6 +531,7 @@ void main_mkfs(int argc, char *argv[])
> >>>>>        int error;
> >>>>>        int rgsize_specified = 0;
> >>>>>        unsigned char uuid[16];
> >>>>> +    char *absname;
> >>>>>
> >>>>>        memset(sdp, 0, sizeof(struct gfs2_sbd));
> >>>>>        sdp->bsize = -1;
> >>>>> @@ -560,11 +561,17 @@ void main_mkfs(int argc, char *argv[])
> >>>>>            exit(EXIT_FAILURE);
> >>>>>        }
> >>>>>
> >>>>> +    absname = canonicalize_file_name(sdp->device_name);
> >>>>> +    if (absname == NULL) {
> >>>>> +        perror(_("Could not find the absolute path of the device"));
> >>>>> +        exit(EXIT_FAILURE);
> >>>>> +    }
> >>>>>        if (!sdp->override) {
> >>>>>            printf( _("This will destroy any data on %s.\n"),
> >>>>> sdp->device_name);
> >>>>> -        check_dev_content(sdp->device_name);
> >>>>> +        check_dev_content(absname);
> >>>>>            are_you_sure();
> >>>>>        }
> >>>>> +    free(absname);
> >>>>>
> >>>>>        if (sdp->bsize == -1) {
> >>>>>            if (S_ISREG(sdp->dinfo.stat.st_mode))
> >>>>
> >>>>
> >>>
> >>>
> >>
> >>
> >
> >
> 
>