Commit 686238b7 authored by Ilya Dryomov's avatar Ilya Dryomov
Browse files

rbd: remove snapshot existence validation code 



RBD_DEV_FLAG_EXISTS check in rbd_queue_workfn() is racy and leads to
inconsistent behaviour.  If the object (or its snapshot) isn't there,
the OSD returns ENOENT.  A read submitted before the snapshot removal
notification is processed would be zero-filled and ended with status
OK, while future reads would be failed with IOERR.  It also doesn't
handle a case when an image that is mapped read-only is removed.

On top of this, because watch is no longer established for read-only
mappings, we no longer get notifications, so rbd_exists_validate() is
effectively dead code.  While failing requests rather than returning
zeros is a good thing, RBD_DEV_FLAG_EXISTS is not it.

Signed-off-by: default avatarIlya Dryomov <idryomov@gmail.com>
Reviewed-by: default avatarJason Dillaman <dillaman@redhat.com>
Reviewed-by: default avatarDongsheng Yang <dongsheng.yang@easystack.cn>
parent b9ef2b88

drivers/block/rbd.c

+3 −39
Original line number Original line Diff line number Diff line
@@ -462,7 +462,7 @@ struct rbd_device { 
 *   by rbd_dev->lock

 *   by rbd_dev->lock

 */

 */

enum rbd_dev_flags {

enum rbd_dev_flags {

	RBD_DEV_FLAG_EXISTS,	/* mapped snapshot has not been deleted */

	RBD_DEV_FLAG_EXISTS,	/* rbd_dev_device_setup() ran */

	RBD_DEV_FLAG_REMOVING,	/* this mapping is being removed */

	RBD_DEV_FLAG_REMOVING,	/* this mapping is being removed */

	RBD_DEV_FLAG_READONLY,  /* -o ro or snapshot */

	RBD_DEV_FLAG_READONLY,  /* -o ro or snapshot */

};

};
@@ -4865,19 +4865,6 @@ static void rbd_queue_workfn(struct work_struct *work) 
		rbd_assert(!rbd_is_snap(rbd_dev));

		rbd_assert(!rbd_is_snap(rbd_dev));

	}

	}





	/*

	 * Quit early if the mapped snapshot no longer exists.  It's

	 * still possible the snapshot will have disappeared by the

	 * time our request arrives at the osd, but there's no sense in

	 * sending it if we already know.

	 */

	if (!test_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags)) {

		dout("request for non-existent snapshot");

		rbd_assert(rbd_is_snap(rbd_dev));

		result = -ENXIO;

		goto err_rq;

	}



	if (offset && length > U64_MAX - offset + 1) {

	if (offset && length > U64_MAX - offset + 1) {

		rbd_warn(rbd_dev, "bad request range (%llu~%llu)", offset,

		rbd_warn(rbd_dev, "bad request range (%llu~%llu)", offset,

			 length);

			 length);
@@ -5057,25 +5044,6 @@ out: 
	return ret;

	return ret;

}

}





/*

 * Clear the rbd device's EXISTS flag if the snapshot it's mapped to

 * has disappeared from the (just updated) snapshot context.

 */

static void rbd_exists_validate(struct rbd_device *rbd_dev)

{

	u64 snap_id;



	if (!test_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags))

		return;



	snap_id = rbd_dev->spec->snap_id;

	if (snap_id == CEPH_NOSNAP)

		return;



	if (rbd_dev_snap_index(rbd_dev, snap_id) == BAD_SNAP_INDEX)

		clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);

}



static void rbd_dev_update_size(struct rbd_device *rbd_dev)

static void rbd_dev_update_size(struct rbd_device *rbd_dev)

{

{

	sector_t size;

	sector_t size;
@@ -5116,12 +5084,8 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev) 
			goto out;

			goto out;

	}

	}





	if (!rbd_is_snap(rbd_dev)) {

	rbd_assert(!rbd_is_snap(rbd_dev));

	rbd_dev->mapping.size = rbd_dev->header.image_size;

	rbd_dev->mapping.size = rbd_dev->header.image_size;

	} else {

		/* validate mapped snapshot's EXISTS flag */

		rbd_exists_validate(rbd_dev);

	}





out:

out:

	up_write(&rbd_dev->header_rwsem);

	up_write(&rbd_dev->header_rwsem);

CRA Git | Maintained and supported by SUSTech CRA and CCSE