Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Josef Bacik <jbacik@fusionio.com>]

On Tue, Aug 13, 2013 at 03:14:08PM -0400, Josef Bacik wrote:
> On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > ---
> >  mkfs.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/mkfs.c b/mkfs.c
> > index 60f906c..66f558a 100644
> > --- a/mkfs.c
> > +++ b/mkfs.c
> > @@ -1570,6 +1570,8 @@ int main(int ac, char **av)
> >  		 * occur by the following processing.
> >  		 * (btrfs_register_one_device() fails if O_EXCL is on)
> >  		 */
> > +		if (fd > 0)
> > +			close(fd);
> >  		fd = open(file, O_RDWR);
> >  		if (fd < 0) {
> >  			fprintf(stderr, "unable to open %s: %s\n", file,
> > @@ -1581,7 +1583,6 @@ int main(int ac, char **av)
> >  		if (ret) {
> >  			fprintf(stderr, "skipping duplicate device %s in FS\n",
> >  				file);
> > -			close(fd);
> >  			continue;
> >  		}
> >  		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,
> 
> This breaks mkfs with multiple disks.  Please for the love of all that is holy
> just do a xfstests run with your patches to make sure they don't break anything
> so when I go to try to test something completely different I don't have to waste
> time bisecting down to figure out wtf you broke today?  David can you kick this
> one out of integration for the time being please?  Thanks,
> 

In fact this isn't right at all, we pass this fd into the device, so we aren't
"overwriting" it, we're assigning it to the device and moving on to the next
thing, and then the close_ctree() stuff closes all the devices as normal.  So
just no, this isn't right at all and can be evicted permanently.  Thanks,

Josef