From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Wed, 04 Jul 2012 12:02:38 +0100 Subject: [Cluster-devel] [PATCH] mkfs.gfs2: Follow symlinks before checking device contents In-Reply-To: <4FEC99CC.6010309@redhat.com> References: <1340207231-6770-1-git-send-email-anprice@redhat.com> <1340874965.2770.5.camel@menhir> <4FEC2ABB.6010804@redhat.com> <1340877709.2770.16.camel@menhir> <4FEC99CC.6010309@redhat.com> Message-ID: <4FF422CE.9060501@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 >>>>> --- >>>>> 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)) >>>> >>>> >>> >>> >> >> > >