rbd: decouple parent info read-in from updating rbd_dev

commit c10311776f0a8ddea2276df96e255625b07045a8 upstream.

Unlike header read-in, parent info read-in is already decoupled in
get_parent_info(), but it's buried in rbd_dev_v2_parent_info() along
with the processing logic.

Separate the initial read-in and update read-in logic into
rbd_dev_setup_parent() and rbd_dev_update_parent() respectively and
have rbd_dev_v2_parent_info() just populate struct parent_image_info
(i.e. what get_parent_info() did).  Some existing QoI issues, like
flatten of a standalone clone being disregarded on refresh, remain.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b1c44c6..38c92b1 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -5595,6 +5595,14 @@ struct parent_image_info {
        u64             overlap;
 };
 
+static void rbd_parent_info_cleanup(struct parent_image_info *pii)
+{
+       kfree(pii->pool_ns);
+       kfree(pii->image_id);
+
+       memset(pii, 0, sizeof(*pii));
+}
+
 /*
  * The caller is responsible for @pii.
  */
@@ -5664,6 +5672,9 @@ static int __get_parent_info(struct rbd_device *rbd_dev,
        if (pii->has_overlap)
                ceph_decode_64_safe(&p, end, pii->overlap, e_inval);
 
+       dout("%s pool_id %llu pool_ns %s image_id %s snap_id %llu has_overlap %d overlap %llu\n",
+            __func__, pii->pool_id, pii->pool_ns, pii->image_id, pii->snap_id,
+            pii->has_overlap, pii->overlap);
        return 0;
 
 e_inval:
@@ -5702,14 +5713,17 @@ static int __get_parent_info_legacy(struct rbd_device *rbd_dev,
        pii->has_overlap = true;
        ceph_decode_64_safe(&p, end, pii->overlap, e_inval);
 
+       dout("%s pool_id %llu pool_ns %s image_id %s snap_id %llu has_overlap %d overlap %llu\n",
+            __func__, pii->pool_id, pii->pool_ns, pii->image_id, pii->snap_id,
+            pii->has_overlap, pii->overlap);
        return 0;
 
 e_inval:
        return -EINVAL;
 }
 
-static int get_parent_info(struct rbd_device *rbd_dev,
-                          struct parent_image_info *pii)
+static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev,
+                                 struct parent_image_info *pii)
 {
        struct page *req_page, *reply_page;
        void *p;
@@ -5737,7 +5751,7 @@ static int get_parent_info(struct rbd_device *rbd_dev,
        return ret;
 }
 
-static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
+static int rbd_dev_setup_parent(struct rbd_device *rbd_dev)
 {
        struct rbd_spec *parent_spec;
        struct parent_image_info pii = { 0 };
@@ -5747,37 +5761,12 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
        if (!parent_spec)
                return -ENOMEM;
 
-       ret = get_parent_info(rbd_dev, &pii);
+       ret = rbd_dev_v2_parent_info(rbd_dev, &pii);
        if (ret)
                goto out_err;
 
-       dout("%s pool_id %llu pool_ns %s image_id %s snap_id %llu has_overlap %d overlap %llu\n",
-            __func__, pii.pool_id, pii.pool_ns, pii.image_id, pii.snap_id,
-            pii.has_overlap, pii.overlap);
-
-       if (pii.pool_id == CEPH_NOPOOL || !pii.has_overlap) {
-               /*
-                * Either the parent never existed, or we have
-                * record of it but the image got flattened so it no
-                * longer has a parent.  When the parent of a
-                * layered image disappears we immediately set the
-                * overlap to 0.  The effect of this is that all new
-                * requests will be treated as if the image had no
-                * parent.
-                *
-                * If !pii.has_overlap, the parent image spec is not
-                * applicable.  It's there to avoid duplication in each
-                * snapshot record.
-                */
-               if (rbd_dev->parent_overlap) {
-                       rbd_dev->parent_overlap = 0;
-                       rbd_dev_parent_put(rbd_dev);
-                       pr_info("%s: clone image has been flattened\n",
-                               rbd_dev->disk->disk_name);
-               }
-
+       if (pii.pool_id == CEPH_NOPOOL || !pii.has_overlap)
                goto out;       /* No parent?  No problem. */
-       }
 
        /* The ceph file layout needs to fit pool id in 32 bits */
 
@@ -5789,46 +5778,34 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
        }
 
        /*
-        * The parent won't change (except when the clone is
-        * flattened, already handled that).  So we only need to
-        * record the parent spec we have not already done so.
+        * The parent won't change except when the clone is flattened,
+        * so we only need to record the parent image spec once.
         */
-       if (!rbd_dev->parent_spec) {
-               parent_spec->pool_id = pii.pool_id;
-               if (pii.pool_ns && *pii.pool_ns) {
-                       parent_spec->pool_ns = pii.pool_ns;
-                       pii.pool_ns = NULL;
-               }
-               parent_spec->image_id = pii.image_id;
-               pii.image_id = NULL;
-               parent_spec->snap_id = pii.snap_id;
-
-               rbd_dev->parent_spec = parent_spec;
-               parent_spec = NULL;     /* rbd_dev now owns this */
+       parent_spec->pool_id = pii.pool_id;
+       if (pii.pool_ns && *pii.pool_ns) {
+               parent_spec->pool_ns = pii.pool_ns;
+               pii.pool_ns = NULL;
        }
+       parent_spec->image_id = pii.image_id;
+       pii.image_id = NULL;
+       parent_spec->snap_id = pii.snap_id;
+
+       rbd_assert(!rbd_dev->parent_spec);
+       rbd_dev->parent_spec = parent_spec;
+       parent_spec = NULL;     /* rbd_dev now owns this */
 
        /*
-        * We always update the parent overlap.  If it's zero we issue
-        * a warning, as we will proceed as if there was no parent.
+        * Record the parent overlap.  If it's zero, issue a warning as
+        * we will proceed as if there is no parent.
         */
-       if (!pii.overlap) {
-               if (parent_spec) {
-                       /* refresh, careful to warn just once */
-                       if (rbd_dev->parent_overlap)
-                               rbd_warn(rbd_dev,
-                                   "clone now standalone (overlap became 0)");
-               } else {
-                       /* initial probe */
-                       rbd_warn(rbd_dev, "clone is standalone (overlap 0)");
-               }
-       }
+       if (!pii.overlap)
+               rbd_warn(rbd_dev, "clone is standalone (overlap 0)");
        rbd_dev->parent_overlap = pii.overlap;
 
 out:
        ret = 0;
 out_err:
-       kfree(pii.pool_ns);
-       kfree(pii.image_id);
+       rbd_parent_info_cleanup(&pii);
        rbd_spec_put(parent_spec);
        return ret;
 }
@@ -6978,7 +6955,7 @@ static int rbd_dev_image_probe(struct rbd_device *rbd_dev, int depth)
        }
 
        if (rbd_dev->header.features & RBD_FEATURE_LAYERING) {
-               ret = rbd_dev_v2_parent_info(rbd_dev);
+               ret = rbd_dev_setup_parent(rbd_dev);
                if (ret)
                        goto err_out_probe;
        }
@@ -7027,9 +7004,47 @@ static void rbd_dev_update_header(struct rbd_device *rbd_dev,
        }
 }
 
+static void rbd_dev_update_parent(struct rbd_device *rbd_dev,
+                                 struct parent_image_info *pii)
+{
+       if (pii->pool_id == CEPH_NOPOOL || !pii->has_overlap) {
+               /*
+                * Either the parent never existed, or we have
+                * record of it but the image got flattened so it no
+                * longer has a parent.  When the parent of a
+                * layered image disappears we immediately set the
+                * overlap to 0.  The effect of this is that all new
+                * requests will be treated as if the image had no
+                * parent.
+                *
+                * If !pii.has_overlap, the parent image spec is not
+                * applicable.  It's there to avoid duplication in each
+                * snapshot record.
+                */
+               if (rbd_dev->parent_overlap) {
+                       rbd_dev->parent_overlap = 0;
+                       rbd_dev_parent_put(rbd_dev);
+                       pr_info("%s: clone has been flattened\n",
+                               rbd_dev->disk->disk_name);
+               }
+       } else {
+               rbd_assert(rbd_dev->parent_spec);
+
+               /*
+                * Update the parent overlap.  If it became zero, issue
+                * a warning as we will proceed as if there is no parent.
+                */
+               if (!pii->overlap && rbd_dev->parent_overlap)
+                       rbd_warn(rbd_dev,
+                                "clone has become standalone (overlap 0)");
+               rbd_dev->parent_overlap = pii->overlap;
+       }
+}
+
 static int rbd_dev_refresh(struct rbd_device *rbd_dev)
 {
        struct rbd_image_header header = { 0 };
+       struct parent_image_info pii = { 0 };
        u64 mapping_size;
        int ret;
 
@@ -7045,12 +7060,14 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev)
         * mapped image getting flattened.
         */
        if (rbd_dev->parent) {
-               ret = rbd_dev_v2_parent_info(rbd_dev);
+               ret = rbd_dev_v2_parent_info(rbd_dev, &pii);
                if (ret)
                        goto out;
        }
 
        rbd_dev_update_header(rbd_dev, &header);
+       if (rbd_dev->parent)
+               rbd_dev_update_parent(rbd_dev, &pii);
 
        rbd_assert(!rbd_is_snap(rbd_dev));
        rbd_dev->mapping.size = rbd_dev->header.image_size;
@@ -7060,6 +7077,7 @@ out:
        if (!ret && mapping_size != rbd_dev->mapping.size)
                rbd_dev_update_size(rbd_dev);
 
+       rbd_parent_info_cleanup(&pii);
        rbd_image_header_cleanup(&header);
        return ret;
 }