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

[Andrew Price <anprice@redhat.com>]

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))
>>>
>>>
>>
>>
>
>