c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2026-05-22 18:40:43 +03:00
Code Issues Packages Projects Releases Wiki Activity

Disable 'zfs remap' command

Browse Source 
The implementation of 'zfs remap' has proven to be problematic since
it modifies the objset (but not its logical contents) by dirtying
metadata without owning it.  The consequence of which is that
dmu_objset_remap_indirects() is vulnerable to certain races.

For example, if we are in the middle of receiving into the filesystem
while it is being remapped.  Then it is possible we could evict the
objset when the receive completes (see dsl_dataset_clone_swap_sync_impl,
or dmu_recv_end_sync), but dmu_objset_remap_indirects() may be still
using the objset.  The result of which would be a panic.

Extended runs of ztest(8) have exposed other possible races which
can occur when using 'zfs remap'.  Several of these have been fixed
but there may be others which have not yet been encountered and
diagnosed.

Furthermore, the ability to manually remap a filesystem is no longer
particularly useful now that the removal code can map large chunks.
Coupled with the fact that explaining what this command does and why
it may be useful requires a detailed understanding of the internals
of device removal.  These are details users should not be bothered
with.

Therefore, the 'zfs remap' command is being disabled but not entirely
removed.  It may be removed in the future or potentially reworked
to address the issues described above.  Since 'zfs remap' has never
been part of a tagged release its removal is expected to have
minimal impact.

The ZTS tests have been updated to continue to exercise the command
to prevent atrophy, but it has been removed entirely from ztest(8).

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Tom Caputi <tcaputi@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #8238
This commit is contained in:
Brian Behlendorf
2019-01-15 15:46:58 -08:00
committed by GitHub
parent 5e7f3ace58
commit 6e91a72fe3
10 changed files with 57 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/zfs/zfs_main.c
+38 -1
View File
@@ -7083,6 +7083,21 @@ zfs_do_unshare(int argc, char **argv)
return (unshare_unmount(OP_SHARE, argc, argv));
}
static int
disable_command_idx(char *command)
{
for (int i = 0; i < NCOMMAND; i++) {
if (command_table[i].name == NULL)
continue;
if (strcmp(command, command_table[i].name) == 0) {
command_table[i].name = NULL;
return (0);
}
}
return (1);
}
static int
find_command_idx(char *command, int *idx)
{
@@ -7180,7 +7195,22 @@ zfs_do_diff(int argc, char **argv)
/*
* zfs remap <filesystem | volume>
*
* Remap the indirect blocks in the given fileystem or volume.
* N.B. The remap command has been disabled and may be removed in the future.
*
* Remap the indirect blocks in the given filesystem or volume so that they no
* longer reference blocks on previously removed vdevs and we can eventually
* shrink the size of the indirect mapping objects for the previously removed
* vdevs. Note that remapping all blocks might not be possible and that
* references from snapshots will still exist and cannot be remapped.
*
* This functionality is no longer particularly useful now that the removal
* code can map large chunks. Furthermore, explaining what this command
* does and why it may be useful requires a detailed understanding of the
* internals of device removal. These are details users should not be
* bothered with. If required, the remap command can be re-enabled by
* setting the ZFS_REMAP_ENABLED environment variable.
*
* > ZFS_REMAP_ENABLED=yes zfs remap <filesystem | volume>
*/
static int
zfs_do_remap(int argc, char **argv)
@@ -8006,6 +8036,13 @@ main(int argc, char **argv)
if (strcmp(cmdname, "snap") == 0)
cmdname = "snapshot";
/*
* The 'remap' command has been disabled and may be removed in the
* future. See the comment above zfs_do_remap() for details.
*/
if (!libzfs_envvar_is_set("ZFS_REMAP_ENABLED"))
disable_command_idx("remap");
/*
* Special case '-?'
*/
cmd/ztest/ztest.c
-23
View File
@@ -216,7 +216,6 @@ extern int dmu_object_alloc_chunk_shift;
extern boolean_t zfs_force_some_double_word_sm_entries;
extern unsigned long zio_decompress_fail_fraction;
extern unsigned long zfs_reconstruct_indirect_damage_fraction;
extern int zfs_object_remap_one_indirect_delay_ms;
static ztest_shared_opts_t *ztest_shared_opts;
@@ -373,7 +372,6 @@ ztest_func_t ztest_split_pool;
ztest_func_t ztest_reguid;
ztest_func_t ztest_spa_upgrade;
ztest_func_t ztest_device_removal;
ztest_func_t ztest_remap_blocks;
ztest_func_t ztest_spa_checkpoint_create_discard;
ztest_func_t ztest_initialize;
ztest_func_t ztest_fletcher;
@@ -427,7 +425,6 @@ ztest_info_t ztest_info[] = {
ZTI_INIT(ztest_vdev_class_add, 1, &ztest_opts.zo_vdevtime),
ZTI_INIT(ztest_vdev_aux_add_remove, 1, &ztest_opts.zo_vdevtime),
ZTI_INIT(ztest_device_removal, 1, &zopt_sometimes),
ZTI_INIT(ztest_remap_blocks, 1, &zopt_sometimes),
ZTI_INIT(ztest_spa_checkpoint_create_discard, 1, &zopt_rarely),
ZTI_INIT(ztest_initialize, 1, &zopt_sometimes),
ZTI_INIT(ztest_fletcher, 1, &zopt_rarely),
@@ -5550,20 +5547,6 @@ ztest_dsl_prop_get_set(ztest_ds_t *zd, uint64_t id)
(void) pthread_rwlock_unlock(&ztest_name_lock);
}
/* ARGSUSED */
void
ztest_remap_blocks(ztest_ds_t *zd, uint64_t id)
{
(void) pthread_rwlock_rdlock(&ztest_name_lock);
int error = dmu_objset_remap_indirects(zd->zd_name);
if (error == ENOSPC)
error = 0;
ASSERT0(error);
(void) pthread_rwlock_unlock(&ztest_name_lock);
}
/* ARGSUSED */
void
ztest_spa_prop_get_set(ztest_ds_t *zd, uint64_t id)
@@ -6629,12 +6612,6 @@ ztest_resume_thread(void *arg)
*/
if (ztest_random(10) == 0)
zfs_abd_scatter_enabled = ztest_random(2);
/*
* Periodically inject remapping delays (10% of the time).
*/
zfs_object_remap_one_indirect_delay_ms =
ztest_random(10) == 0 ? ztest_random(1000) + 1 : 0;
}
thread_exit();