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