Skip to content

Commit

Permalink
rbd: decouple parent info read-in from updating rbd_dev
Browse files Browse the repository at this point in the history 
commit c103117 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>
  • Loading branch information
@idryomov @gregkh
idryomov authored and gregkh committed Oct 10, 2023
1 parent 377d261 commit 698039a
Showing 1 changed file with 80 additions and 62 deletions.
142 changes: 80 additions & 62 deletions drivers/block/rbd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
Expand All @@ -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 */

Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -7060,6 +7077,7 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev)
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;
}
Expand Down

0 comments on commit 698039a

Please sign in to comment.