FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 09-27-2011, 04:37 PM
Bob Peterson
 
Default mkfs: Remove unneeded open/close fd test from are_you_sure()

----- Original Message -----
| are_you_sure() function should not make an open/close
| test on the underlying device. The pourpose of this
| function is just to ask a yes/no question to the user.
| If we need a function to do this test, we should create
| a new function with its specific pourpose.
| This patch also fix a call to are_you_sure() function and
| move the are_you_sure() function to the begining of the
| file, allowing it to be used in another part of the code
|
| Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

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.

Regards,

Bob Peterson
Red Hat File Systems
 
Old 09-27-2011, 05:37 PM
Carlos Maiolino
 
Default mkfs: Remove unneeded open/close fd test from are_you_sure()

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
"),
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
"),
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
 
Old 09-27-2011, 06:01 PM
Bob Peterson
 
Default mkfs: Remove unneeded open/close fd test from are_you_sure()

----- Original Message -----
| Hi Bob,
|
| 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):

Hi Carlos,

Thanks for clearing that up. I should have looked myself; sorry.
Yes, the patches look good to push as far as I'm concerned.

Regards,

Bob Peterson
Red Hat File System
 

Thread Tools




All times are GMT. The time now is 10:09 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org