xfs
[Top] [All Lists]

Re: [PATCH 4/10] sort out opening and closing of the block devices

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 4/10] sort out opening and closing of the block devices
From: David Chinner <dgc@xxxxxxx>
Date: Mon, 12 May 2008 12:04:21 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20080501220105.GD2315@lst.de>
References: <20080501220105.GD2315@lst.de>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
On Fri, May 02, 2008 at 12:01:05AM +0200, Christoph Hellwig wrote:
> +STATIC int
> +xfs_open_devices(
> +     struct xfs_mount        *mp,
> +     struct xfs_mount_args   *args)
> +{
> +     struct block_device     *ddev = mp->m_super->s_bdev;
> +     struct block_device     *logdev = NULL, *rtdev = NULL;
> +     int                     error;
> +
> +     /*
> +      * Open real time and log devices - order is important.
> +      */
> +     if (args->logname[0]) {
> +             error = xfs_blkdev_get(mp, args->logname, &logdev);
> +             if (error)
> +                     goto out;
> +     }
> +
> +     if (args->rtname[0]) {
> +             error = xfs_blkdev_get(mp, args->rtname, &rtdev);
> +             if (error)
> +                     goto out_close_logdev;
> +
> +             if (rtdev == ddev || rtdev == logdev) {
> +                     cmn_err(CE_WARN,
> +     "XFS: Cannot mount filesystem with identical rtdev and ddev/logdev.");

error needs to be set to something here.

> +                     goto out_close_rtdev;
> +             }
> +     }
> +
> +     /*
> +      * Setup xfs_mount buffer target pointers
> +      */
> +     error = ENOMEM;
> +     mp->m_ddev_targp = xfs_alloc_buftarg(ddev, 0);
> +     if (!mp->m_ddev_targp)
> +             goto out_close_rtdev;
> +
> +     if (rtdev) {
> +             mp->m_rtdev_targp = xfs_alloc_buftarg(rtdev, 1);
> +             if (!mp->m_rtdev_targp)
> +                     goto out_free_ddev_targ;
> +     }
> +
> +     if (logdev && logdev != ddev) {
> +             mp->m_logdev_targp = xfs_alloc_buftarg(logdev, 1);
> +             if (!mp->m_logdev_targp)
> +                     goto out_free_rtdev_targ;
> +     } else {
> +             mp->m_logdev_targp = mp->m_ddev_targp;
> +     }
> +
> +     return 0;
> +
> + out_free_rtdev_targ:
> +     if (mp->m_rtdev_targp)
> +             xfs_free_buftarg(mp->m_rtdev_targp);
> + out_free_ddev_targ:
> +     xfs_free_buftarg(mp->m_ddev_targp);
> + out_close_rtdev:
> +     if (mp->m_rtdev_targp)
> +             xfs_blkdev_put(mp->m_rtdev_targp->bt_bdev);

And that looks broken - we either haven't allocated m_rtdev_targp
yet or we've freed it above.

incremental patch to fix is fine...

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group


<Prev in Thread] Current Thread [Next in Thread>