DAOS-18368 rebuild: fix use before check of ec_agg_boundary

1. fix a bug of using ec_agg_boundary before checking its valid
2. add some more logs for rebuild fetch getting zero iod_size,
   to provide some hints for layout information.

 Signed-off-by: Xuezhao Liu <[email protected]>

Author: liuxuezhao

src/object/obj_internal.h

@@ -1181,6 +1181,8 @@ iov_alloc_for_csum_info(d_iov_t *iov, struct dcs_csum_info *csum_info);
 /* obj_layout.c */
 int
 obj_pl_grp_idx(uint32_t layout_gl_ver, uint64_t hash, uint32_t grp_nr);
+void
+obj_dump_grp_layout(daos_handle_t oh, uint32_t shard);
 
 int
 obj_pl_place(struct pl_map *map, uint16_t layout_ver, struct daos_obj_md *md,

src/object/obj_layout.c

@@ -1,5 +1,6 @@
 /*
  *  (C) Copyright 2016-2023 Intel Corporation.
+ *  (C) Copyright 2025 Hewlett Packard Enterprise Development LP
  *
  * SPDX-License-Identifier: BSD-2-Clause-Patent
  */
@@ -95,3 +96,36 @@ obj_layout_diff(struct pl_map *map, daos_unit_oid_t oid, uint32_t new_ver, uint3
 
 	return rc;
 }
+
+void
+obj_dump_grp_layout(daos_handle_t oh, uint32_t shard)
+{
+	struct dc_object    *obj;
+	struct dc_obj_shard *obj_shard;
+	uint32_t             grp_idx, i, nr;
+
+	obj = obj_hdl2ptr(oh);
+	if (obj == NULL) {
+		D_INFO("invalid oh");
+		return;
+	}
+	if (shard >= obj->cob_shards_nr) {
+		D_ERROR("bad shard %d, cob_shards_nr %d", shard, obj->cob_shards_nr);
+		goto out;
+	}
+
+	grp_idx = shard / obj->cob_grp_size;
+	D_INFO(DF_OID " shard %d, grp_idx %d, grp_size %d", DP_OID(obj->cob_md.omd_id), shard,
+	       grp_idx, obj->cob_grp_size);
+	for (i = grp_idx * obj->cob_grp_size, nr = 0; nr < obj->cob_grp_size; i++, nr++) {
+		obj_shard = &obj->cob_shards->do_shards[i];
+		D_INFO("shard %d/%d/%d, tgt_id %d, rank %d, tgt_idx %d, "
+		       "rebuilding %d, reintegrating %d, fseq %d",
+		       i, obj_shard->do_shard_idx, obj_shard->do_shard, obj_shard->do_target_id,
+		       obj_shard->do_target_rank, obj_shard->do_target_idx,
+		       obj_shard->do_rebuilding, obj_shard->do_reintegrating, obj_shard->do_fseq);
+	}
+
+out:
+	obj_decref(obj);
+}

src/object/srv_obj.c

@@ -701,6 +701,20 @@ obj_set_reply_sizes(crt_rpc_t *rpc, daos_iod_t *iods, int iod_nr, uint8_t *skips
 		sizes[i] = iods[idx].iod_size;
 		D_DEBUG(DB_IO, DF_UOID" %d:"DF_U64"\n", DP_UOID(orw->orw_oid),
 			i, iods[idx].iod_size);
+		if ((orw->orw_flags & ORF_FOR_MIGRATION) && sizes[i] == 0) {
+			D_INFO(DF_CONT " obj " DF_UOID "rebuild fetch zero iod_size, i:%d/idx:%d, "
+				       "iod_nr %d, orw_epoch " DF_X64 ", orw_epoch_first " DF_X64
+				       " may cause DER_DATA_LOSS",
+			       DP_CONT(orw->orw_pool_uuid, orw->orw_co_uuid), DP_UOID(orw->orw_oid),
+			       i, idx, iods[idx].iod_nr, orw->orw_epoch, orw->orw_epoch_first);
+			if (iods[idx].iod_type == DAOS_IOD_ARRAY) {
+				int j;
+
+				for (j = 0; j < min(8, iods[idx].iod_nr); j++)
+					D_INFO("recx[%d] - " DF_RECX, j,
+					       DP_RECX(iods[idx].iod_recxs[j]));
+			}
+		}
 		idx++;
 	}
 
@@ -1368,7 +1382,7 @@ struct ec_agg_boundary_arg {
 };
 
 static int
-obj_fetch_ec_agg_boundary(void *data)
+obj_fetch_ec_agg_boundary_ult(void *data)
 {
 	struct ec_agg_boundary_arg *arg = data;
 	int                         rc;
@@ -1381,6 +1395,33 @@ obj_fetch_ec_agg_boundary(void *data)
 	return rc;
 }
 
+static int
+obj_fetch_ec_agg_boundary(struct obj_io_context *ioc, daos_unit_oid_t *uoid)
+{
+	struct ec_agg_boundary_arg arg;
+	int                        rc;
+
+	arg.eab_pool = ioc->ioc_coc->sc_pool->spc_pool;
+	uuid_copy(arg.eab_co_uuid, ioc->ioc_coc->sc_uuid);
+	rc = dss_ult_execute(obj_fetch_ec_agg_boundary_ult, &arg, NULL, NULL, DSS_XS_SYS, 0, 0);
+	if (rc) {
+		DL_ERROR(rc, DF_CONT ", " DF_UOID " fetch ec_agg_boundary failed.",
+			 DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid),
+			 DP_UOID(*uoid));
+		return rc;
+	}
+	if (ioc->ioc_coc->sc_ec_agg_eph_valid == 0) {
+		rc = -DER_FETCH_AGAIN;
+		DL_INFO(rc, DF_CONT ", " DF_UOID " zero ec_agg_boundary.",
+			DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid), DP_UOID(*uoid));
+		return rc;
+	}
+	D_DEBUG(DB_IO, DF_CONT ", " DF_UOID " fetched ec_agg_eph_boundary " DF_X64 "\n",
+		DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid), DP_UOID(*uoid),
+		ioc->ioc_coc->sc_ec_agg_eph_boundary);
+	return 0;
+}
+
 static int
 obj_local_rw_internal(crt_rpc_t *rpc, struct obj_io_context *ioc, daos_iod_t *iods,
 		      struct dcs_iod_csums *iod_csums, uint64_t *offs, uint8_t *skips,
@@ -1503,29 +1544,14 @@ obj_local_rw_internal(crt_rpc_t *rpc, struct obj_io_context *ioc, daos_iod_t *io
 		}
 		if ((ec_deg_fetch || (ec_recov && get_parity_list)) &&
 		    ioc->ioc_coc->sc_ec_agg_eph_valid == 0) {
-			struct ec_agg_boundary_arg arg;
-
-			arg.eab_pool = ioc->ioc_coc->sc_pool->spc_pool;
-			uuid_copy(arg.eab_co_uuid, ioc->ioc_coc->sc_uuid);
-			rc = dss_ult_execute(obj_fetch_ec_agg_boundary, &arg, NULL, NULL,
-					     DSS_XS_SYS, 0, 0);
+			rc = obj_fetch_ec_agg_boundary(ioc, &orw->orw_oid);
 			if (rc) {
 				DL_ERROR(rc, DF_CONT ", " DF_UOID " fetch ec_agg_boundary failed.",
 					 DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid),
 					 DP_UOID(orw->orw_oid));
 				goto out;
 			}
-			if (ioc->ioc_coc->sc_ec_agg_eph_valid == 0) {
-				rc = -DER_FETCH_AGAIN;
-				DL_INFO(rc, DF_CONT ", " DF_UOID " zero ec_agg_boundary.",
-					DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid),
-					DP_UOID(orw->orw_oid));
-				goto out;
-			}
-			D_DEBUG(DB_IO,
-				DF_CONT ", " DF_UOID " fetched ec_agg_eph_boundary " DF_X64 "\n",
-				DP_CONT(ioc->ioc_coc->sc_pool_uuid, ioc->ioc_coc->sc_uuid),
-				DP_UOID(orw->orw_oid), ioc->ioc_coc->sc_ec_agg_eph_boundary);
+			D_ASSERT(ioc->ioc_coc->sc_ec_agg_eph_valid);
 		}
 		if (get_parity_list) {
 			D_ASSERT(!ec_deg_fetch);
@@ -3030,6 +3056,20 @@ ds_obj_rw_handler(crt_rpc_t *rpc)
 		if (orw->orw_flags & ORF_FETCH_EPOCH_EC_AGG_BOUNDARY) {
 			uint64_t rebuild_epoch;
 
+			if (ioc.ioc_coc->sc_ec_agg_eph_valid == 0) {
+				rc = obj_fetch_ec_agg_boundary(&ioc, &orw->orw_oid);
+				if (rc) {
+					DL_ERROR(rc,
+						 DF_CONT ", " DF_UOID " fetch ec_agg_boundary "
+							 "failed.",
+						 DP_CONT(ioc.ioc_coc->sc_pool_uuid,
+							 ioc.ioc_coc->sc_uuid),
+						 DP_UOID(orw->orw_oid));
+					goto out;
+				}
+				D_ASSERT(ioc.ioc_coc->sc_ec_agg_eph_valid);
+			}
+
 			D_ASSERTF(orw->orw_epoch <= orw->orw_epoch_first,
 				  "bad orw_epoch " DF_X64 ", orw_epoch_first " DF_X64 "\n",
 				  orw->orw_epoch, orw->orw_epoch_first);

src/object/srv_obj_migrate.c

@@ -1156,6 +1156,28 @@ migrate_fetch_update_parity(struct migrate_one *mrone, daos_handle_t oh,
 	return rc;
 }
 
+static void
+mrone_dump_info(struct migrate_one *mrone, daos_handle_t oh, daos_iod_t *iod)
+{
+	int i;
+
+	if (daos_is_dkey_uint64(mrone->mo_oid.id_pub) && mrone->mo_dkey.iov_len == 8)
+		D_INFO(DF_RB ": " DF_UOID " int dkey " DF_U64 ", akey " DF_KEY ", iod_type %d, "
+			     " iod_nr %d, iod_size " DF_U64,
+		       DP_RB_MPT(mrone->mo_tls), DP_UOID(mrone->mo_oid),
+		       *(uint64_t *)mrone->mo_dkey.iov_buf, DP_KEY(&iod->iod_name), iod->iod_type,
+		       iod->iod_nr, iod->iod_size);
+	else
+		D_INFO(DF_RB ": " DF_UOID " dkey " DF_KEY ", akey " DF_KEY ", iod_type %d, "
+			     " iod_nr %d, iod_size " DF_U64,
+		       DP_RB_MPT(mrone->mo_tls), DP_UOID(mrone->mo_oid), DP_KEY(&mrone->mo_dkey),
+		       DP_KEY(&iod->iod_name), iod->iod_type, iod->iod_nr, iod->iod_size);
+	if (iod->iod_type == DAOS_IOD_ARRAY)
+		for (i = 0; i < min(8, iod->iod_nr); i++)
+			D_INFO("recxs[%d] - " DF_RECX, i, DP_RECX(iod->iod_recxs[i]));
+	obj_dump_grp_layout(oh, mrone->mo_oid.id_shard);
+}
+
 static int
 migrate_fetch_update_single(struct migrate_one *mrone, daos_handle_t oh,
 			    struct ds_cont_child *ds_cont)
@@ -1224,6 +1246,8 @@ migrate_fetch_update_single(struct migrate_one *mrone, daos_handle_t oh,
 		daos_iod_t	*iod = &mrone->mo_iods[i];
 
 		if (mrone->mo_iods[i].iod_size == 0) {
+			static __thread int log_nr;
+
 			/* zero size iod will cause assertion failure
 			 * in VOS, so let's check here.
 			 * So the object is being destroyed between
@@ -1236,11 +1260,16 @@ migrate_fetch_update_single(struct migrate_one *mrone, daos_handle_t oh,
 			 */
 			rc = -DER_DATA_LOSS;
 			D_DEBUG(DB_REBUILD,
-				DF_RB ": " DF_UOID " %p dkey " DF_KEY " " DF_KEY
-				      " nr %d/%d eph " DF_U64 " " DF_RC "\n",
-				DP_RB_MRO(mrone), DP_UOID(mrone->mo_oid), mrone,
-				DP_KEY(&mrone->mo_dkey), DP_KEY(&mrone->mo_iods[i].iod_name),
-				mrone->mo_iod_num, i, mrone->mo_epoch, DP_RC(rc));
+				DF_RB ": cont " DF_UUID " obj " DF_UOID " dkey " DF_KEY " " DF_KEY
+				      " nr %d/%d eph " DF_X64 " " DF_RC "\n",
+				DP_RB_MRO(mrone), DP_UUID(mrone->mo_cont_uuid),
+				DP_UOID(mrone->mo_oid), DP_KEY(&mrone->mo_dkey),
+				DP_KEY(&mrone->mo_iods[i].iod_name), mrone->mo_iod_num, i,
+				mrone->mo_epoch, DP_RC(rc));
+			if (log_nr <= 128) {
+				mrone_dump_info(mrone, oh, &mrone->mo_iods[i]);
+				log_nr++;
+			}
 			D_GOTO(out, rc);
 		}
 
@@ -1407,6 +1436,8 @@ __migrate_fetch_update_bulk(struct migrate_one *mrone, daos_handle_t oh,
 
 	for (i = 0; rc == 0 && i < iod_num; i++) {
 		if (iods[i].iod_size == 0) {
+			static __thread int log_nr;
+
 			/* zero size iod will cause assertion failure
 			 * in VOS, so let's check here.
 			 * So the object is being destroyed between
@@ -1418,11 +1449,16 @@ __migrate_fetch_update_bulk(struct migrate_one *mrone, daos_handle_t oh,
 			 * the rebuild and retry.
 			 */
 			rc = -DER_DATA_LOSS;
-			D_INFO(DF_RB ": " DF_UOID " %p dkey " DF_KEY " " DF_KEY
-				     " nr %d/%d eph " DF_U64 " " DF_RC "\n",
-			       DP_RB_MRO(mrone), DP_UOID(mrone->mo_oid), mrone,
-			       DP_KEY(&mrone->mo_dkey), DP_KEY(&iods[i].iod_name), iod_num, i,
-			       mrone->mo_epoch, DP_RC(rc));
+			DL_INFO(rc,
+				DF_RB ": cont " DF_UUID " obj " DF_UOID " dkey " DF_KEY " " DF_KEY
+				      " nr %d/%d mo_epoch " DF_X64 " fetch_eph " DF_X64,
+				DP_RB_MRO(mrone), DP_UUID(mrone->mo_cont_uuid),
+				DP_UOID(mrone->mo_oid), DP_KEY(&mrone->mo_dkey),
+				DP_KEY(&iods[i].iod_name), iod_num, i, mrone->mo_epoch, fetch_eph);
+			if (log_nr <= 128) {
+				mrone_dump_info(mrone, oh, &mrone->mo_iods[i]);
+				log_nr++;
+			}
 			D_GOTO(end, rc);
 		}
 	}