[Cluster-devel] [PATCH 2/3] mkfs: Remove unneeded open/close fd test from are_you_sure()

Carlos Maiolino cmaiolino at redhat.com
Tue Sep 27 17:37:07 UTC 2011


Hi Bob,

> Hi,
> 
> This all looks good and makes sense.  I am concerned
> that removing the open/close might change the behavior
> when errors occur: For example, if you try to mkfs.gfs2
> for a device you don't have permission to.  Or for a
> device that doesn't exist.  Please test those two cases
> to make sure nothing weird happens.
> 
Maybe my mistake to not have written it before, but, I've checked the 
are_you_sure() calls into the code, and before any call to are_you_sure()
we have a open/close test, so, we had the are_you_sure() doing a second
test.

Portions of code in the main_mkfs() function (almost at the begining of the function):

553         sdp->device_fd = open(sdp->device_name, O_RDWR | O_CLOEXEC);
554         if (sdp->device_fd < 0)
555                 die( _("can't open device %s: %s\n"),
556                     sdp->device_name, strerror(errno));
557
558         if (fstat(sdp->device_fd, &st_buf) < 0) {
559                 fprintf(stderr, _("could not fstat fd %d: %s\n"),
560                         sdp->device_fd, strerror(errno));
561                 exit(-1);
562         }
563 
564         if (!sdp->override)
565                 are_you_sure();
.
.
591         verify_bsize(sdp);


After that, we have a call to are_you_sure() inside the verify_bsize()
function, but, the verify_bsize() is called once, in the main_mkfs() too,
after the check specified on the code snippet above. 
What do you think? Are these 3 patches ok to push?

-- 
--Carlos




More information about the Cluster-devel mailing list