Commit b82d167b authored by Alex Elder's avatar Alex Elder Committed by Sage Weil

rbd: prevent open for image being removed

An open request for a mapped rbd image can arrive while removal of
that mapping is underway.  We need to prevent such an open request
from succeeding.  (It appears that Maciej Galkiewicz ran into this
problem.)

Define and use a "removing" flag to indicate a mapping is getting
removed.  Set it in the remove path after verifying nothing holds
the device open.  And check it in the open path before allowing the
open to proceed.  Acquire the rbd device's lock around each of these
spots to avoid any races accessing the flags and open_count fields.

This addresses:
    http://tracker.newdream.net/issues/3427Reported-by: default avatarMaciej Galkiewicz <maciejgalkiewicz@ragnarson.com>
Signed-off-by: default avatarAlex Elder <elder@inktank.com>
Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
parent 6d292906
...@@ -261,10 +261,10 @@ struct rbd_device { ...@@ -261,10 +261,10 @@ struct rbd_device {
char name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */ char name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */
spinlock_t lock; /* queue lock */ spinlock_t lock; /* queue, flags, open_count */
struct rbd_image_header header; struct rbd_image_header header;
unsigned long flags; unsigned long flags; /* possibly lock protected */
struct rbd_spec *spec; struct rbd_spec *spec;
char *header_name; char *header_name;
...@@ -289,13 +289,19 @@ struct rbd_device { ...@@ -289,13 +289,19 @@ struct rbd_device {
/* sysfs related */ /* sysfs related */
struct device dev; struct device dev;
unsigned long open_count; unsigned long open_count; /* protected by lock */
}; };
/* Flag bits for rbd_dev->flags */ /*
* Flag bits for rbd_dev->flags. If atomicity is required,
* rbd_dev->lock is used to protect access.
*
* Currently, only the "removing" flag (which is coupled with the
* "open_count" field) requires atomic access.
*/
enum rbd_dev_flags { enum rbd_dev_flags {
RBD_DEV_FLAG_EXISTS, /* mapped snapshot has not been deleted */ RBD_DEV_FLAG_EXISTS, /* mapped snapshot has not been deleted */
RBD_DEV_FLAG_REMOVING, /* this mapping is being removed */
}; };
static DEFINE_MUTEX(ctl_mutex); /* Serialize open/close/setup/teardown */ static DEFINE_MUTEX(ctl_mutex); /* Serialize open/close/setup/teardown */
...@@ -383,14 +389,23 @@ static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver); ...@@ -383,14 +389,23 @@ static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver);
static int rbd_open(struct block_device *bdev, fmode_t mode) static int rbd_open(struct block_device *bdev, fmode_t mode)
{ {
struct rbd_device *rbd_dev = bdev->bd_disk->private_data; struct rbd_device *rbd_dev = bdev->bd_disk->private_data;
bool removing = false;
if ((mode & FMODE_WRITE) && rbd_dev->mapping.read_only) if ((mode & FMODE_WRITE) && rbd_dev->mapping.read_only)
return -EROFS; return -EROFS;
spin_lock(&rbd_dev->lock);
if (test_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags))
removing = true;
else
rbd_dev->open_count++;
spin_unlock(&rbd_dev->lock);
if (removing)
return -ENOENT;
mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
(void) get_device(&rbd_dev->dev); (void) get_device(&rbd_dev->dev);
set_device_ro(bdev, rbd_dev->mapping.read_only); set_device_ro(bdev, rbd_dev->mapping.read_only);
rbd_dev->open_count++;
mutex_unlock(&ctl_mutex); mutex_unlock(&ctl_mutex);
return 0; return 0;
...@@ -399,10 +414,14 @@ static int rbd_open(struct block_device *bdev, fmode_t mode) ...@@ -399,10 +414,14 @@ static int rbd_open(struct block_device *bdev, fmode_t mode)
static int rbd_release(struct gendisk *disk, fmode_t mode) static int rbd_release(struct gendisk *disk, fmode_t mode)
{ {
struct rbd_device *rbd_dev = disk->private_data; struct rbd_device *rbd_dev = disk->private_data;
unsigned long open_count_before;
spin_lock(&rbd_dev->lock);
open_count_before = rbd_dev->open_count--;
spin_unlock(&rbd_dev->lock);
rbd_assert(open_count_before > 0);
mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
rbd_assert(rbd_dev->open_count > 0);
rbd_dev->open_count--;
put_device(&rbd_dev->dev); put_device(&rbd_dev->dev);
mutex_unlock(&ctl_mutex); mutex_unlock(&ctl_mutex);
...@@ -4083,10 +4102,14 @@ static ssize_t rbd_remove(struct bus_type *bus, ...@@ -4083,10 +4102,14 @@ static ssize_t rbd_remove(struct bus_type *bus,
goto done; goto done;
} }
if (rbd_dev->open_count) { spin_lock(&rbd_dev->lock);
if (rbd_dev->open_count)
ret = -EBUSY; ret = -EBUSY;
else
set_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags);
spin_unlock(&rbd_dev->lock);
if (ret < 0)
goto done; goto done;
}
rbd_remove_all_snaps(rbd_dev); rbd_remove_all_snaps(rbd_dev);
rbd_bus_del_dev(rbd_dev); rbd_bus_del_dev(rbd_dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment