DLPX-84701 DOSE Migration: Add Object-Store vdev in running ZFS Pool (o…

…penzfs#796)

= Description

This commit allows us to add an object store bucket as a vdev in an
existing pooll and it is the first part of the DOSE Migration project.

= Note: Forcing Addition From `zpool add`

Attempting to add an object-store vdev without `-f` yields the following
error message:
```
$ sudo zpool add -o object-endpoint=etc.. testpool s3 cloudburst-data-2
invalid vdev specification
use '-f' to override the following errors:
mismatched replication level: pool uses disk and new vdev is objectstore
```

This is done on purpose for now. Adding an objects-store vdev to a pool
is an irreversible operation and should be handled with caution.

= Note: Syncing Labels & The Uberblock

When starting from a block-based pool and we add an object-store vdev
there is a point where we have an object-store vdev in our config but
that vdev is not accepting allocations and therefore we can sync the
config to it. That point is the exact TXG where the vdev is added and
we need to sync its config changes to the labels & uberblock of our
block-based vdevs.

For this reason, I adjusted all the codepaths under `vdev_config_sync()`
to be able to handle the update of the labels and uberblock of the local
devices even when there is an object-store vdev. This way, if the next
TXG fails we have the new vdev somewhere on our config. For all TXGs
from that point on, we always sync the object store's config first. This
is also the config that we always look at first when opening the pool.

With the above changes in `vdev_config_sync()` changes the behavior of
existing pure (e.g. non-hybrid) object-store pools to occasionally
update the labels of their slog devices (e.g. every time we dirty the
pool's config). This should not really have any negative effect in
existing pure object-store pools. On the contrary it should keep
their labels up to date and potentially fix any extreme corner cases
in pool import.

= Note: ZIL allocations

When the pool is in a hybrid state (e.g. backed by both an object store
and block devices) with no slog devices, we could make zil allocations
fall back to the embedded slog or normal class. I left that
functionality as future work.  This is not a prerequisite for DOSE
migration as customers are expected to add zettacache(+ slog) devices as
the first part of migration and therefore their VMs will always have at
least one slog device when the object store vdev is added.

= Note: Storage Pool Checkpoint

Pure object-based pools (e.g. not hybrid ones) do the checkpoint
rewinding process in the object agent. This is a different mechanism
from the storage pool checkpoint in block-based pools. Until we have
the need to make those two mechanism work well with each other we
avoid any migrations to the object store while a zpool checkpoint is
in effect. See `spa_ld_checkpoint_rewind()` usage in
`spa_load_impl()` for more info.

= Note: Ordering of import paths

To import a hybrid pool we need to specify two import paths: (1)
the path of the local block devices (e.g. `/dev/..etc`) and (2)
the name of the bucket in the object store. Unfortunately given
how `zpool_find_import_agent()` is implemented, importing hybrid
pools only works if we specify (2) first and then (1) but not the
opposite. Doing the opposite results in the zpool command hanging
(again this is because of the current XXX in the aforementioned
function).

= Note: Testing Lossing Power Mid-Addition of the Object Store vdev

I tested that by manually introducing panics like so:
```
diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index 5b55bb275..a82fab841 100644
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -6969,7 +6969,9 @@ spa_vdev_add(spa_t *spa, nvlist_t *nvroot)
         * if we lose power at any point in this sequence, ...
         * steps will be completed the next time we load ...
         */
+       ASSERT(B_FALSE); // <--- panic before config sync
        (void) spa_vdev_exit(spa, vd, txg, 0);
+       ASSERT(B_FALSE); // <--- panic before config sync

        mutex_enter(&spa_namespace_lock);
        spa_config_update(spa, SPA_CONFIG_UPDATE_POOL);
```

Importing the pool after the first panic we come back without
the object store vdev as expected. Importing the pool after the
second panic, we come back with the object store but don't
allocate from it nor change the spa_pool_type until we create
its metaslabs on disk.

= Next Steps

I'm planning to implement the augmented device removal logic which
evacuates data from the block devices to the object-store. Once that is
done, I'm planning to work on avoiding the insertion of all the
evacuated data in the zettacache.

Signed-off-by: Serapheim Dimitropoulos <[email protected]>

cmd/zpool/zpool_main.c

@@ -1007,8 +1007,16 @@ zpool_do_add(int argc, char **argv)
 			*propval = '\0';
 			propval++;
 
-			if ((strcmp(optarg, ZPOOL_CONFIG_ASHIFT) != 0) ||
-			    (add_prop_list(optarg, propval, &props, B_TRUE)))
+			/* Only the following properties are allowed */
+			if ((strcmp(optarg,
+			    ZPOOL_CONFIG_ASHIFT) != 0) &&
+			    (strcmp(optarg,
+			    ZPOOL_PROP_OBJ_ENDPOINT_URI) != 0) &&
+			    (strcmp(optarg,
+			    ZPOOL_PROP_OBJ_REGION_NAME) != 0))
+				usage(B_FALSE);
+
+			if (add_prop_list(optarg, propval, &props, B_TRUE))
 				usage(B_FALSE);
 			break;
 		case 'P':

contrib/pyzfs/libzfs_core/_constants.py

@@ -104,6 +104,8 @@ def enum(*sequential, **named):
         'ZFS_ERR_NOT_USER_NAMESPACE',
         'ZFS_ERR_RESUME_EXISTS',
         'ZFS_ERR_CRYPTO_NOTSUP',
+        'ZFS_ERR_OBJSTORE_EXISTS',
+        'ZFS_ERR_OBJSTORE_SINGLE_ADD',
     ],
     {}
 )

include/libzfs.h

@@ -153,6 +153,8 @@ typedef enum zfs_error {
 	EZFS_NOT_USER_NAMESPACE,	/* a file is not a user namespace */
 	EZFS_CKSUM,		/* insufficient replicas */
 	EZFS_RESUME_EXISTS,	/* Resume on existing dataset without force */
+	EZFS_OBJSTORE_EXISTS, /* pool is already backed by an object store */
+	EZFS_OBJSTORE_ADD_ALONE, /* can't add extra vdevs with objstore */
 	EZFS_UNKNOWN
 } zfs_error_t;
 

include/sys/fs/zfs.h

@@ -725,6 +725,17 @@ typedef struct zpool_load_policy {
 #define	ZPOOL_CONFIG_VDEV_STATS		"vdev_stats"	/* not stored on disk */
 #define	ZPOOL_CONFIG_INDIRECT_SIZE	"indirect_size"	/* not stored on disk */
 
+/*
+ * These are user-visible property names that are specific to the object
+ * store. We declare those here because they happen to be temporarily
+ * added to the config in-memory, but they're never actually saved on
+ * disk with the config.
+ */
+#define	ZPOOL_PROP_OBJ_PROTOCOL_NAME	"object-protocol"
+#define	ZPOOL_PROP_OBJ_ENDPOINT_URI	"object-endpoint"
+#define	ZPOOL_PROP_OBJ_REGION_NAME	"object-region"
+#define	ZPOOL_PROP_OBJ_CRED_PROFILE_REF	"object-credentials-profile"
+
 /* container nvlist of extended stats */
 #define	ZPOOL_CONFIG_VDEV_STATS_EX	"vdev_stats_ex"
 
@@ -1571,6 +1582,8 @@ typedef enum {
 	ZFS_ERR_NOT_USER_NAMESPACE,
 	ZFS_ERR_RESUME_EXISTS,
 	ZFS_ERR_CRYPTO_NOTSUP,
+	ZFS_ERR_OBJSTORE_EXISTS,
+	ZFS_ERR_OBJSTORE_ADD_ALONE,
 } zfs_errno_t;
 
 /*

include/sys/spa_impl.h

@@ -469,7 +469,8 @@ extern int param_set_deadman_failmode_common(const char *val);
 extern void spa_set_deadman_synctime(hrtime_t ns);
 extern void spa_set_deadman_ziotime(hrtime_t ns);
 extern const char *spa_history_zone(void);
-extern void spa_set_pool_type(spa_t *);
+extern void spa_set_pool_type_from_vdev_tree(spa_t *);
+extern void spa_set_pool_type_from_vdev_metadata(spa_t *);
 
 #ifdef	__cplusplus
 }

include/sys/vdev.h

@@ -69,6 +69,7 @@ extern int vdev_validate_aux(vdev_t *vd);
 extern zio_t *vdev_probe(vdev_t *vd, zio_t *pio);
 extern boolean_t vdev_is_concrete(vdev_t *vd);
 extern boolean_t vdev_is_object_based(vdev_t *vd);
+extern boolean_t vdev_is_object_store(vdev_t *vd);
 extern boolean_t vdev_is_bootable(vdev_t *vd);
 extern vdev_t *vdev_lookup_top(spa_t *spa, uint64_t vdev);
 extern vdev_t *vdev_lookup_by_guid(vdev_t *vd, uint64_t guid);

lib/libzfs/libzfs_util.c

@@ -307,6 +307,12 @@ libzfs_error_description(libzfs_handle_t *hdl)
 	case EZFS_RESUME_EXISTS:
 		return (dgettext(TEXT_DOMAIN, "Resuming recv on existing "
 		    "dataset without force"));
+	case EZFS_OBJSTORE_EXISTS:
+		return (dgettext(TEXT_DOMAIN, "pool already has an "
+		    "object store vdev"));
+	case EZFS_OBJSTORE_ADD_ALONE:
+		return (dgettext(TEXT_DOMAIN, "one vdev entry allowed "
+		    "when adding an object store vdev"));
 	case EZFS_UNKNOWN:
 		return (dgettext(TEXT_DOMAIN, "unknown error"));
 	default:
@@ -753,6 +759,12 @@ zpool_standard_error_fmt(libzfs_handle_t *hdl, int error, const char *fmt, ...)
 	case ZFS_ERR_IOC_ARG_BADTYPE:
 		zfs_verror(hdl, EZFS_IOC_NOTSUPPORTED, fmt, ap);
 		break;
+	case ZFS_ERR_OBJSTORE_EXISTS:
+		zfs_verror(hdl, EZFS_OBJSTORE_EXISTS, fmt, ap);
+		break;
+	case ZFS_ERR_OBJSTORE_ADD_ALONE:
+		zfs_verror(hdl, EZFS_OBJSTORE_ADD_ALONE, fmt, ap);
+		break;
 	default:
 		zfs_error_aux(hdl, "%s", strerror(error));
 		zfs_verror(hdl, EZFS_UNKNOWN, fmt, ap);

lib/libzutil/zutil_import.c

@@ -1761,12 +1761,11 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg)
 			return (NULL);
 		}
 
-		nvlist_t *tree;
-		uint_t c, children;
-		nvlist_t **child;
-
-		tree = fnvlist_lookup_nvlist(src, ZPOOL_CONFIG_VDEV_TREE);
+		nvlist_t *tree =
+		    fnvlist_lookup_nvlist(src, ZPOOL_CONFIG_VDEV_TREE);
 
+		uint_t children;
+		nvlist_t **child;
 		if (nvlist_lookup_nvlist_array(tree, ZPOOL_CONFIG_CHILDREN,
 		    &child, &children) != 0) {
 			fprintf(stderr, gettext("cannot import '%s': invalid "
@@ -1775,7 +1774,7 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg)
 		}
 
 		boolean_t object_store_pool_found = B_FALSE;
-		for (c = 0; c < children; c++) {
+		for (uint_t c = 0; c < children; c++) {
 			const char *type;
 			if (nvlist_lookup_string(child[c], ZPOOL_CONFIG_TYPE,
 			    &type) != 0) {
@@ -1848,11 +1847,14 @@ zpool_find_import_agent(libpc_handle_t *hdl, importargs_t *iarg,
 	if (bucket == NULL && iarg->path != NULL) {
 		bucket = iarg->path[0];
 	}
-	nvlist_lookup_string(iarg->props, "object-protocol", &protocol);
-	nvlist_lookup_string(iarg->props, "object-endpoint", &endpoint);
-	nvlist_lookup_string(iarg->props, "object-region", &region);
-	nvlist_lookup_string(iarg->props, "object-credentials-profile",
-	    &profile);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_PROTOCOL_NAME, &protocol);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_ENDPOINT_URI, &endpoint);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_REGION_NAME, &region);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_CRED_PROFILE_REF, &profile);
 
 	nvlist_t *msg = fnvlist_alloc();
 	fnvlist_add_string(msg, AGENT_REQUEST_TYPE, AGENT_TYPE_GET_POOLS);
@@ -1911,14 +1913,17 @@ zpool_find_import_agent(libpc_handle_t *hdl, importargs_t *iarg,
 		 * them to the config here.
 		 */
 		if (region != NULL)
-			fnvlist_add_string(tree, "object-region", region);
+			fnvlist_add_string(tree,
+			    ZPOOL_PROP_OBJ_REGION_NAME, region);
 		if (endpoint != NULL)
-			fnvlist_add_string(tree, "object-endpoint", endpoint);
+			fnvlist_add_string(tree,
+			    ZPOOL_PROP_OBJ_ENDPOINT_URI, endpoint);
 		if (protocol != NULL)
-			fnvlist_add_string(tree, "object-protocol", protocol);
+			fnvlist_add_string(tree,
+			    ZPOOL_PROP_OBJ_PROTOCOL_NAME, protocol);
 		if (profile != NULL)
-			fnvlist_add_string(tree, "object-credentials-profile",
-			    profile);
+			fnvlist_add_string(tree,
+			    ZPOOL_PROP_OBJ_CRED_PROFILE_REF, profile);
 
 		slice->rn_vdev_guid = guid;
 		slice->rn_lock = lock;
@@ -2161,11 +2166,14 @@ zoa_resume_destroy(void *hdl, importargs_t *iarg)
 	if (bucket == NULL) {
 		return (-1);
 	}
-	nvlist_lookup_string(iarg->props, "object-protocol", &protocol);
-	nvlist_lookup_string(iarg->props, "object-endpoint", &endpoint);
-	nvlist_lookup_string(iarg->props, "object-region", &region);
-	nvlist_lookup_string(iarg->props, "object-credentials-profile",
-	    &profile);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_PROTOCOL_NAME, &protocol);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_ENDPOINT_URI, &endpoint);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_REGION_NAME, &region);
+	nvlist_lookup_string(iarg->props,
+	    ZPOOL_PROP_OBJ_CRED_PROFILE_REF, &profile);
 
 	// Resume destroy
 	nvlist_t *msg = fnvlist_alloc();

module/os/freebsd/zfs/spa_os.c

@@ -246,7 +246,7 @@ spa_import_rootpool(const char *name, bool checkpointrewind)
 	spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER);
 	error = spa_config_parse(spa, &rvd, nvtop, NULL, 0,
 	    VDEV_ALLOC_ROOTPOOL);
-	spa_set_pool_type(spa);
+	spa_set_pool_type_from_vdev_metadata(spa);
 	spa_config_exit(spa, SCL_ALL, FTAG);
 	if (error) {
 		mutex_exit(&spa_namespace_lock);

module/os/linux/zfs/vdev_object_store.c

@@ -2403,7 +2403,18 @@ vdev_object_store_open(vdev_t *vd, uint64_t *psize, uint64_t *max_psize,
 	vos->vos_send_thread  = thread_create(NULL, 0,
 	    vdev_object_store_send_thread, vd, 0, &p0, TS_RUN, defclsyspri);
 
-	if (vd->vdev_spa->spa_load_state == SPA_LOAD_CREATE) {
+	/*
+	 * There are two cases where we want to create an "object-store pool":
+	 * [1] We are creating a brand new ZFS pool on top of an object store
+	 *     (e.g. SPA_LOAD_CREATE).
+	 * [2] We are about to add an object-based vdev as part of migrating a
+	 *     block-based ZFS pool to an object-based one (e.g. this is the
+	 *     spa_pending_vdev and the spa pool type is not object based yet).
+	 */
+	spa_t *spa = vd->vdev_spa;
+	if ((spa->spa_load_state == SPA_LOAD_CREATE) ||
+	    (vdev_is_object_based(spa->spa_pending_vdev) &&
+	    !spa_is_object_based(spa))) {
 		vos->vos_create_completed = B_FALSE;
 		error = object_store_create_pool(vd);
 		if (error != 0) {

module/zcommon/zpool_prop.c

@@ -82,16 +82,20 @@ zpool_prop_init(void)
 	zprop_register_string(ZPOOL_PROP_COMPATIBILITY, "compatibility",
 	    "off", PROP_DEFAULT, ZFS_TYPE_POOL,
 	    "<file[,file...]> | off | legacy", "COMPATIBILITY", sfeatures);
-	zprop_register_string(ZPOOL_PROP_OBJ_PROTOCOL, "object-protocol", NULL,
-	    PROP_DEFAULT, ZFS_TYPE_POOL, "<protocol>", "OBJ_PROTOCOL",
-	    sfeatures);
-	zprop_register_string(ZPOOL_PROP_OBJ_ENDPOINT, "object-endpoint", NULL,
-	    PROP_DEFAULT, ZFS_TYPE_POOL, "<uri>", "OBJ_ENDPOINT", sfeatures);
-	zprop_register_string(ZPOOL_PROP_OBJ_REGION, "object-region", NULL,
-	    PROP_DEFAULT, ZFS_TYPE_POOL, "<region>", "OBJ_REGION", sfeatures);
+	zprop_register_string(ZPOOL_PROP_OBJ_PROTOCOL,
+	    ZPOOL_PROP_OBJ_PROTOCOL_NAME, NULL,
+	    PROP_DEFAULT, ZFS_TYPE_POOL, "<protocol>",
+	    "OBJ_PROTOCOL", sfeatures);
+	zprop_register_string(ZPOOL_PROP_OBJ_ENDPOINT,
+	    ZPOOL_PROP_OBJ_ENDPOINT_URI, NULL,
+	    PROP_DEFAULT, ZFS_TYPE_POOL, "<uri>",
+	    "OBJ_ENDPOINT", sfeatures);
+	zprop_register_string(ZPOOL_PROP_OBJ_REGION,
+	    ZPOOL_PROP_OBJ_REGION_NAME, NULL, PROP_DEFAULT, ZFS_TYPE_POOL,
+	    "<region>", "OBJ_REGION", sfeatures);
 	zprop_register_string(ZPOOL_PROP_OBJ_CRED_PROFILE,
-	    "object-credentials-profile", NULL, PROP_DEFAULT, ZFS_TYPE_POOL,
-	    "<profile>", "OBJ_CRED_PROFILE", sfeatures);
+	    ZPOOL_PROP_OBJ_CRED_PROFILE_REF, NULL, PROP_DEFAULT,
+	    ZFS_TYPE_POOL, "<profile>", "OBJ_CRED_PROFILE", sfeatures);
 
 	/* readonly number properties */
 	zprop_register_number(ZPOOL_PROP_SIZE, "size", 0, PROP_READONLY,

module/zfs/spa.c

@@ -3592,7 +3592,7 @@ spa_ld_parse_config(spa_t *spa, spa_import_type_t type)
 	parse = (type == SPA_IMPORT_EXISTING ?
 	    VDEV_ALLOC_LOAD : VDEV_ALLOC_SPLIT);
 	error = spa_config_parse(spa, &rvd, nvtree, NULL, 0, parse);
-	spa_set_pool_type(spa);
+	spa_set_pool_type_from_vdev_metadata(spa);
 	spa_config_exit(spa, SCL_ALL, FTAG);
 
 	if (error != 0) {
@@ -3964,7 +3964,7 @@ spa_ld_trusted_config(spa_t *spa, spa_import_type_t type,
 		    error);
 		return (spa_vdev_err(rvd, VDEV_AUX_CORRUPT_DATA, error));
 	}
-	spa_set_pool_type(spa);
+	spa_set_pool_type_from_vdev_metadata(spa);
 
 	/*
 	 * Vdev paths in the MOS may be obsolete. If the untrusted config was
@@ -4859,6 +4859,10 @@ spa_ld_checkpoint_rewind(spa_t *spa)
 			if (svdcount == SPA_SYNC_MIN_VDEVS)
 				break;
 		}
+
+		ASSERT(!spa_is_object_based(spa));
+		ASSERT(!vdev_is_object_based(spa->spa_root_vdev));
+
 		error = vdev_config_sync(svd, svdcount, spa->spa_first_txg);
 		if (error == 0)
 			spa->spa_last_synced_guid = rvd->vdev_guid;
@@ -6068,7 +6072,7 @@ spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
 
 	error = spa_config_parse(spa, &rvd, nvroot, NULL, 0, VDEV_ALLOC_ADD);
 	if (error == 0)
-		spa_set_pool_type(spa);
+		spa_set_pool_type_from_vdev_tree(spa);
 
 	ASSERT(error != 0 || rvd != NULL);
 	ASSERT(error != 0 || spa->spa_root_vdev == rvd);
@@ -6414,6 +6418,11 @@ spa_import(char *pool, nvlist_t *config, nvlist_t *props, uint64_t flags)
 	if (spa_writeable(spa)) {
 		/*
 		 * Update the config cache to include the newly-imported pool.
+		 *
+		 * This may also create metaslabs for newly added vdevs (see
+		 * `spa_vdev_add()`. This normally happens through the async
+		 * thread request of `spa_ld_check_for_config_update()` calling
+		 * `spa_config_update()` but this code path may get there first.
 		 */
 		spa_config_update(spa, SPA_CONFIG_UPDATE_POOL);
 	}
@@ -6801,6 +6810,55 @@ spa_vdev_add(spa_t *spa, nvlist_t *nvroot)
 
 	spa->spa_pending_vdev = vd;	/* spa_vdev_exit() will clear this */
 
+	if (vdev_is_object_based(vd)) {
+		/*
+		 * Can't add a second object-store vdev to object-based
+		 * pool.
+		 *
+		 * Note: we check if there is an object-store vdev under the
+		 * root_vdev instead of using the `spa_is_object_based()`
+		 * helper because it is possible that we just added an object
+		 * store vdev in this pool but we haven't yet converted the
+		 * pool's type.
+		 */
+		if (vdev_is_object_based(spa->spa_root_vdev)) {
+			return (spa_vdev_exit(spa, vd, txg,
+			    SET_ERROR(ZFS_ERR_OBJSTORE_EXISTS)));
+		}
+
+		/*
+		 * If we are adding an object-store vdev to migrate a
+		 * block-based pool to the object store, ensure that
+		 * zpool-add just passed that vdev and nothing else.
+		 * This is a lazy way of making sure that we don't add
+		 * two object store vdevs at once. Note, it also
+		 * prevents us from adding any other type of vdev within
+		 * the same zpool-add invocation.
+		 */
+		if (vd->vdev_children > 1) {
+			return (spa_vdev_exit(spa, vd, txg,
+			    SET_ERROR(ZFS_ERR_OBJSTORE_ADD_ALONE)));
+		}
+
+		/*
+		 * Pure object-based pools (e.g. not hybrid ones) do the
+		 * checkpoint rewinding process in the object agent. This
+		 * is a different mechanism from the storage pool checkpoint
+		 * in block-based pools. Until we have the need to make those
+		 * two mechanism work well with each other we avoid any
+		 * migrations to the object store while a zpool checkpoint
+		 * is in effect. See `spa_ld_checkpoint_rewind()` usage in
+		 * `spa_load_impl()` for more info.
+		 */
+		if (spa_feature_is_active(spa, SPA_FEATURE_POOL_CHECKPOINT)) {
+			error = (spa_has_checkpoint(spa)) ?
+			    ZFS_ERR_CHECKPOINT_EXISTS :
+			    ZFS_ERR_DISCARDING_CHECKPOINT;
+			return (spa_vdev_exit(spa, NULL, txg,
+			    SET_ERROR(error)));
+		}
+	}
+
 	if (nvlist_lookup_nvlist_array(nvroot, ZPOOL_CONFIG_SPARES, &spares,
 	    &nspares) != 0)
 		nspares = 0;
@@ -8381,9 +8439,12 @@ spa_async_thread(void *arg)
 		uint64_t old_space, new_space;
 
 		mutex_enter(&spa_namespace_lock);
+		/*
+		 * There is no point in introducing the object store class
+		 * here as it would ruin any reasonable calculation for the
+		 * history log message below and won't be helpful in any way.
+		 */
 		old_space = metaslab_class_get_space(spa_normal_class(spa));
-		old_space +=
-		    metaslab_class_get_space(spa_object_store_class(spa));
 		old_space += metaslab_class_get_space(spa_special_class(spa));
 		old_space += metaslab_class_get_space(spa_dedup_class(spa));
 		old_space += metaslab_class_get_space(
@@ -9566,7 +9627,8 @@ spa_sync(spa_t *spa, uint64_t txg)
 	VERIFY(spa_writeable(spa));
 
 	if (spa_is_object_based(spa)) {
-		object_store_begin_txg(spa->spa_root_vdev->vdev_child[0], txg);
+		object_store_begin_txg(vdev_find_leaf(spa->spa_root_vdev,
+		    &vdev_object_store_ops), txg);
 	}
 
 	/*