rbd: remove snapshot existence validation code
authorIlya Dryomov <idryomov@gmail.com>
Mon, 18 Nov 2019 11:51:02 +0000 (12:51 +0100)
committerIlya Dryomov <idryomov@gmail.com>
Mon, 25 Nov 2019 10:44:03 +0000 (11:44 +0100)
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: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
drivers/block/rbd.c

index 2a22bc2..ee2b15e 100644 (file)
@@ -462,7 +462,7 @@ struct rbd_device {
  *   by rbd_dev->lock
  */
 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_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));
        }
 
-       /*
-        * 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) {
                rbd_warn(rbd_dev, "bad request range (%llu~%llu)", offset,
                         length);
@@ -5057,25 +5044,6 @@ out:
        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)
 {
        sector_t size;
@@ -5116,12 +5084,8 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev)
                        goto out;
        }
 
-       if (!rbd_is_snap(rbd_dev)) {
-               rbd_dev->mapping.size = rbd_dev->header.image_size;
-       } else {
-               /* validate mapped snapshot's EXISTS flag */
-               rbd_exists_validate(rbd_dev);
-       }
+       rbd_assert(!rbd_is_snap(rbd_dev));
+       rbd_dev->mapping.size = rbd_dev->header.image_size;
 
 out:
        up_write(&rbd_dev->header_rwsem);