From 721b31ca6200bd8891737abffd1b03eb58385743 Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Sun, 5 Aug 2012 02:49:34 +0200 Subject: [PATCH 01/14] First try to implement ashift property. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit tries to implement the ashift pool property as used in ZFSonLinux and the ZEVO beta releases. It is based on the ZFSonLinux implementation, and follows this comment: https://github.com/zfsonlinux/zfs/pull/195#issuecomment-1029278 and the resulting final patch for ZFSonLinux found here: https://github.com/zfsonlinux/zfs/commit/df30f56639f96175ba71d83b4456ccf410c46542 The original inventors are - Darik Horn - Richard Laager - Christian Kohlschütter Due to the large distance between macZFS and ZFSonLinux (pool version 8 vs. 28) it was necessary to redo and adapt their approach instead of just copying the changes. Note that this solution is technical not fully correct: (a) it introduces a pool property that should not exist in pool version 8 (b) it introduces a new property type, PROP_ONTIME that should not exist in pool version 8 (c) the ashift value is really a property of individual top-level vdevs, and not a pool-wide value. A pool can (and should under certain conditions) have top-level vdevs of different block size. However, using a pool-wide property is what other implementations do, so we do the same for the sake of compatibility. --- usr/src/cmd/zpool/zpool_main.c | 67 ++++++++++++++++++++-- usr/src/cmd/zpool/zpool_util.h | 2 +- usr/src/cmd/zpool/zpool_vdev.c | 26 +++++++-- usr/src/common/zfs/zfs_prop.c | 31 ++++++++-- usr/src/lib/libzfs/common/libzfs_dataset.c | 2 +- usr/src/uts/common/fs/zfs/dsl_prop.c | 7 ++- usr/src/uts/common/sys/fs/zfs.h | 1 + 7 files changed, 119 insertions(+), 17 deletions(-) diff --git a/usr/src/cmd/zpool/zpool_main.c b/usr/src/cmd/zpool/zpool_main.c index 247865ec..19a55c25 100644 --- a/usr/src/cmd/zpool/zpool_main.c +++ b/usr/src/cmd/zpool/zpool_main.c @@ -445,6 +445,52 @@ print_vdev_tree(zpool_handle_t *zhp, const char *name, nvlist_t *nv, int indent, } } +/* + * Add a property pair (name, string-value) into a property nvlist. + */ +static int +add_prop_list(const char *propname, char *propval, nvlist_t **props, + boolean_t poolprop) +{ + zpool_prop_t prop = ZPOOL_PROP_INVAL; + zfs_prop_t fprop; + nvlist_t *proplist; + const char *normnm; + char *strval; + + if (*props == NULL && + nvlist_alloc(props, NV_UNIQUE_NAME, 0) != 0) { + (void) fprintf(stderr, + gettext("internal error: out of memory\n")); + return (1); + } + + proplist = *props; + + if (poolprop) { + if ((prop = zpool_name_to_prop(propname)) == ZPOOL_PROP_INVAL) { + (void) fprintf(stderr, gettext("property '%s' is " + "not a valid pool property\n"), propname); + return (2); + } + normnm = zpool_prop_to_name(prop); + } else { + if ((fprop = zfs_name_to_prop(propname)) != ZFS_PROP_INVAL) { + normnm = zfs_prop_to_name(fprop); + } else { + normnm = propname; + } + } + + if (nvlist_add_string(proplist, normnm, propval) != 0) { + (void) fprintf(stderr, gettext("internal " + "error: out of memory\n")); + return (1); + } + + return (0); +} + /* * zpool add [-fn] ... * @@ -513,7 +559,7 @@ zpool_do_add(int argc, char **argv) } /* pass off to get_vdev_spec for processing */ - nvroot = make_root_vdev(zhp, force, !force, B_FALSE, argc, argv); + nvroot = make_root_vdev(zhp, NULL, force, !force, B_FALSE, argc, argv); if (nvroot == NULL) { zpool_close(zhp); return (1); @@ -616,9 +662,11 @@ zpool_do_create(int argc, char **argv) char *mountpoint = NULL; nvlist_t **child; uint_t children; + nvlist_t *props = NULL; + char *propval = NULL; /* check options */ - while ((c = getopt(argc, argv, ":fnR:m:")) != -1) { + while ((c = getopt(argc, argv, ":fnR:m:o:")) != -1) { switch (c) { case 'f': force = B_TRUE; @@ -632,6 +680,17 @@ zpool_do_create(int argc, char **argv) case 'm': mountpoint = optarg; break; + case 'o': + if ((propval = strchr(optarg, '=')) == NULL) { + (void) fprintf(stderr, gettext("missing " + "'=' for -o option\n")); + usage(B_FALSE); + } + *propval = '\0'; + propval++; + if (add_prop_list(optarg, propval, &props, B_TRUE)) + usage(B_FALSE); + break; case ':': (void) fprintf(stderr, gettext("missing argument for " "'%c' option\n"), optopt); @@ -672,7 +731,7 @@ zpool_do_create(int argc, char **argv) } /* pass off to get_vdev_spec for bulk processing */ - nvroot = make_root_vdev(NULL, force, !force, B_FALSE, argc - 1, + nvroot = make_root_vdev(NULL, props, force, !force, B_FALSE, argc - 1, argv + 1); if (nvroot == NULL) return (1); @@ -2322,7 +2381,7 @@ zpool_do_attach_or_replace(int argc, char **argv, int replacing) return (1); } - nvroot = make_root_vdev(zhp, force, B_FALSE, replacing, argc, argv); + nvroot = make_root_vdev(zhp, NULL, force, B_FALSE, replacing, argc, argv); if (nvroot == NULL) { zpool_close(zhp); return (1); diff --git a/usr/src/cmd/zpool/zpool_util.h b/usr/src/cmd/zpool/zpool_util.h index 2a1aca03..f4e2c154 100644 --- a/usr/src/cmd/zpool/zpool_util.h +++ b/usr/src/cmd/zpool/zpool_util.h @@ -47,7 +47,7 @@ uint_t num_logs(nvlist_t *nv); * Virtual device functions */ -nvlist_t *make_root_vdev(zpool_handle_t *zhp, int force, int check_rep, +nvlist_t *make_root_vdev(zpool_handle_t *zhp, nvlist_t *props, int force, int check_rep, boolean_t isreplace, int argc, char **argv); /* diff --git a/usr/src/cmd/zpool/zpool_vdev.c b/usr/src/cmd/zpool/zpool_vdev.c index 63b93a19..d8a8c5e7 100644 --- a/usr/src/cmd/zpool/zpool_vdev.c +++ b/usr/src/cmd/zpool/zpool_vdev.c @@ -413,7 +413,7 @@ is_whole_disk(const char *arg) * xxx Shorthand for /dev/dsk/xxx */ static nvlist_t * -make_leaf_vdev(const char *arg, uint64_t is_log) +make_leaf_vdev(nvlist_t *props, const char *arg, uint64_t is_log) { char path[MAXPATHLEN]; struct stat statbuf; @@ -538,6 +538,20 @@ make_leaf_vdev(const char *arg, uint64_t is_log) (void) close(fd); } + if (props != NULL) { + + uint64_t ashift = 0; + char *value = NULL; + + if (nvlist_lookup_string(props, + zpool_prop_to_name(ZPOOL_PROP_ASHIFT), &value) == 0) + zfs_nicestrtonum(NULL, value, &ashift); + + if (ashift > 0) + verify(nvlist_add_uint64(vdev, ZPOOL_CONFIG_ASHIFT, + ashift) == 0); + } + return (vdev); } @@ -1158,7 +1172,7 @@ is_grouping(const char *type, int *mindev) * because the program is just going to exit anyway. */ nvlist_t * -construct_spec(int argc, char **argv) +construct_spec(nvlist_t *props, int argc, char **argv) { nvlist_t *nvroot, *nv, **top, **spares; int t, toplevels, mindev, nspares, nlogs; @@ -1234,7 +1248,7 @@ construct_spec(int argc, char **argv) children * sizeof (nvlist_t *)); if (child == NULL) zpool_no_memory(); - if ((nv = make_leaf_vdev(argv[c], B_FALSE)) + if ((nv = make_leaf_vdev(props, argv[c], B_FALSE)) == NULL) return (NULL); child[children - 1] = nv; @@ -1279,7 +1293,7 @@ construct_spec(int argc, char **argv) * We have a device. Pass off to make_leaf_vdev() to * construct the appropriate nvlist describing the vdev. */ - if ((nv = make_leaf_vdev(argv[0], is_log)) == NULL) + if ((nv = make_leaf_vdev(props, argv[0], is_log)) == NULL) return (NULL); if (is_log) nlogs++; @@ -1342,7 +1356,7 @@ construct_spec(int argc, char **argv) * added, even if they appear in use. */ nvlist_t * -make_root_vdev(zpool_handle_t *zhp, int force, int check_rep, +make_root_vdev(zpool_handle_t *zhp, nvlist_t *props, int force, int check_rep, boolean_t isreplacing, int argc, char **argv) { nvlist_t *newroot; @@ -1354,7 +1368,7 @@ make_root_vdev(zpool_handle_t *zhp, int force, int check_rep, * that we have a valid specification, and that all devices can be * opened. */ - if ((newroot = construct_spec(argc, argv)) == NULL) + if ((newroot = construct_spec(props, argc, argv)) == NULL) return (NULL); if (zhp && ((poolconfig = zpool_get_config(zhp, NULL)) == NULL)) diff --git a/usr/src/common/zfs/zfs_prop.c b/usr/src/common/zfs/zfs_prop.c index 4bb2d3d7..3d30d64b 100644 --- a/usr/src/common/zfs/zfs_prop.c +++ b/usr/src/common/zfs/zfs_prop.c @@ -70,7 +70,14 @@ typedef enum { PROP_DEFAULT, PROP_READONLY, - PROP_INHERIT + PROP_INHERIT, + /* + * ONETIME properties are a sort of conglomeration of READONLY + * and INHERIT. They can be set only during object creation, + * after that they are READONLY. If not explicitly set during + * creation, they can be inherited. + */ + PROP_ONETIME } prop_attr_t; typedef struct zfs_index { @@ -326,10 +333,15 @@ zfs_prop_init(void) register_number(ZFS_PROP_COMPRESSRATIO, "compressratio", 0, PROP_READONLY, ZFS_TYPE_ANY, "<1.00x or higher if compressed>", "RATIO"); + + /* readonly onetime number properties */ + register_number(ZPOOL_PROP_ASHIFT, "ashift", 0, PROP_ONETIME, + ZFS_TYPE_POOL, "", "ASHIFT"); register_number(ZFS_PROP_VOLBLOCKSIZE, "volblocksize", 8192, - PROP_READONLY, + PROP_ONETIME, ZFS_TYPE_VOLUME, "512 to 128k, power of 2", "VOLBLOCK"); + /* default number properties */ register_number(ZFS_PROP_QUOTA, "quota", 0, PROP_DEFAULT, ZFS_TYPE_FILESYSTEM, " | none", "QUOTA"); @@ -616,7 +628,17 @@ zpool_prop_default_numeric(zpool_prop_t prop) int zfs_prop_readonly(zfs_prop_t prop) { - return (zfs_prop_table[prop].pd_attr == PROP_READONLY); + return (zfs_prop_table[prop].pd_attr == PROP_READONLY || + zfs_prop_table[prop].pd_attr == PROP_ONETIME); +} + +/* + * Returns TRUE if the property is only allowed to be set once. + */ +boolean_t +zfs_prop_setonce(zfs_prop_t prop) +{ + return (zfs_prop_table[prop].pd_attr == PROP_ONETIME); } /* @@ -645,7 +667,8 @@ zpool_prop_to_name(zpool_prop_t prop) int zfs_prop_inheritable(zfs_prop_t prop) { - return (zfs_prop_table[prop].pd_attr == PROP_INHERIT); + return (zfs_prop_table[prop].pd_attr == PROP_INHERIT || + zfs_prop_table[prop].pd_attr == PROP_ONETIME); } /* diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c index 58abeb60..6e7b6ddf 100644 --- a/usr/src/lib/libzfs/common/libzfs_dataset.c +++ b/usr/src/lib/libzfs/common/libzfs_dataset.c @@ -780,7 +780,7 @@ zfs_validate_properties(libzfs_handle_t *hdl, zfs_type_t type, char *pool_name, } if (zfs_prop_readonly(prop) && - (prop != ZFS_PROP_VOLBLOCKSIZE || zhp != NULL)) { + (!zfs_prop_setonce(prop) || zhp != NULL)) { zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "'%s' is readonly"), propname); diff --git a/usr/src/uts/common/fs/zfs/dsl_prop.c b/usr/src/uts/common/fs/zfs/dsl_prop.c index e814fe27..3fd2fae9 100644 --- a/usr/src/uts/common/fs/zfs/dsl_prop.c +++ b/usr/src/uts/common/fs/zfs/dsl_prop.c @@ -44,8 +44,13 @@ dodefault(const char *propname, int intsz, int numint, void *buf) { zfs_prop_t prop; + /* + * The setonce properties are read-only, BUT they still + * have a default value that can be used as the initial + * value. + */ if ((prop = zfs_name_to_prop(propname)) == ZFS_PROP_INVAL || - zfs_prop_readonly(prop)) + (zfs_prop_readonly(prop) && !zfs_prop_setonce(prop))) return (ENOENT); if (zfs_prop_get_type(prop) == PROP_TYPE_STRING) { diff --git a/usr/src/uts/common/sys/fs/zfs.h b/usr/src/uts/common/sys/fs/zfs.h index 2dbb168f..5ae19988 100644 --- a/usr/src/uts/common/sys/fs/zfs.h +++ b/usr/src/uts/common/sys/fs/zfs.h @@ -106,6 +106,7 @@ typedef enum { ZPOOL_PROP_DELEGATION, ZFS_PROP_VERSION, ZPOOL_PROP_NAME, + ZPOOL_PROP_ASHIFT, ZFS_NUM_PROPS } zfs_prop_t; From fd8a53f592bec2983733a99201031e6b20b4fa8d Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Wed, 8 Aug 2012 23:48:30 +0200 Subject: [PATCH 02/14] added missing input sanitization for ashift property --- usr/src/lib/libzfs/common/libzfs_pool.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/usr/src/lib/libzfs/common/libzfs_pool.c b/usr/src/lib/libzfs/common/libzfs_pool.c index c14f202a..c15f90c5 100644 --- a/usr/src/lib/libzfs/common/libzfs_pool.c +++ b/usr/src/lib/libzfs/common/libzfs_pool.c @@ -233,6 +233,7 @@ zpool_get_prop(zpool_handle_t *zhp, zpool_prop_t prop, char *buf, size_t len, case ZPOOL_PROP_SIZE: case ZPOOL_PROP_USED: case ZPOOL_PROP_AVAILABLE: + case ZPOOL_PROP_ASHIFT: (void) zfs_nicenum(intval, buf, len); break; @@ -353,6 +354,23 @@ zpool_validate_properties(libzfs_handle_t *hdl, const char *poolname, } break; + case ZPOOL_PROP_ASHIFT: + if (!flags.create) { + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "property '%s' can only be set at " + "creation time"), propname); + (void) zfs_error(hdl, EZFS_BADPROP, errbuf); + goto error; + } + if (intval != 0 && (intval < 9 || intval > 17)) { + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "property '%s' number %d is invalid."), + propname, intval); + (void) zfs_error(hdl, EZFS_BADPROP, errbuf); + goto error; + } + break; + case ZPOOL_PROP_BOOTFS: if (create_or_import) { zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, From d3df1442ba8d69f8219604ed89f95ac1c9195bc1 Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Sun, 2 Sep 2012 00:12:08 +0200 Subject: [PATCH 03/14] Fixed missing cv_init() / cv_destroy() calls in space_map_load(), txg_init() and txg_fini(). --- usr/src/uts/common/fs/zfs/space_map.c | 4 ++++ usr/src/uts/common/fs/zfs/txg.c | 28 +++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/usr/src/uts/common/fs/zfs/space_map.c b/usr/src/uts/common/fs/zfs/space_map.c index a15e5ff8..754535de 100644 --- a/usr/src/uts/common/fs/zfs/space_map.c +++ b/usr/src/uts/common/fs/zfs/space_map.c @@ -67,6 +67,8 @@ space_map_create(space_map_t *sm, uint64_t start, uint64_t size, uint8_t shift, sm->sm_size = size; sm->sm_shift = shift; sm->sm_lock = lp; + + cv_init(&sm->sm_load_cv, NULL, CV_DEFAULT, NULL); } void @@ -75,6 +77,8 @@ space_map_destroy(space_map_t *sm) ASSERT(!sm->sm_loaded && !sm->sm_loading); VERIFY3U(sm->sm_space, ==, 0); avl_destroy(&sm->sm_root); + + cv_destroy(&sm->sm_load_cv); } void diff --git a/usr/src/uts/common/fs/zfs/txg.c b/usr/src/uts/common/fs/zfs/txg.c index 70f24c90..f6c5e192 100644 --- a/usr/src/uts/common/fs/zfs/txg.c +++ b/usr/src/uts/common/fs/zfs/txg.c @@ -53,12 +53,24 @@ txg_init(dsl_pool_t *dp, uint64_t txg) tx->tx_cpu = kmem_zalloc(max_ncpus * sizeof (tx_cpu_t), KM_SLEEP); - for (c = 0; c < max_ncpus; c++) + for (c = 0; c < max_ncpus; c++) { mutex_init(&tx->tx_cpu[c].tc_lock, NULL, MUTEX_DEFAULT, NULL); + int cv; + for (cv=0; cv < TXG_SIZE; cv++) { + cv_init(&tx->tx_cpu[c].tc_cv[cv], NULL, CV_DEFAULT, NULL); + } + } rw_init(&tx->tx_suspend, NULL, RW_DEFAULT, NULL); mutex_init(&tx->tx_sync_lock, NULL, MUTEX_DEFAULT, NULL); + cv_init(&tx->tx_sync_more_cv, NULL, CV_DEFAULT, NULL); + cv_init(&tx->tx_sync_done_cv, NULL, CV_DEFAULT, NULL); + cv_init(&tx->tx_quiesce_more_cv, NULL, CV_DEFAULT, NULL); + cv_init(&tx->tx_quiesce_done_cv, NULL, CV_DEFAULT, NULL); + cv_init(&tx->tx_timeout_exit_cv, NULL, CV_DEFAULT, NULL); + cv_init(&tx->tx_exit_cv, NULL, CV_DEFAULT, NULL); + tx->tx_open_txg = txg; } @@ -76,8 +88,20 @@ txg_fini(dsl_pool_t *dp) rw_destroy(&tx->tx_suspend); mutex_destroy(&tx->tx_sync_lock); - for (c = 0; c < max_ncpus; c++) + cv_destroy(&tx->tx_sync_more_cv); + cv_destroy(&tx->tx_sync_done_cv); + cv_destroy(&tx->tx_quiesce_more_cv); + cv_destroy(&tx->tx_quiesce_done_cv); + cv_destroy(&tx->tx_timeout_exit_cv); + cv_destroy(&tx->tx_exit_cv); + + for (c = 0; c < max_ncpus; c++) { mutex_destroy(&tx->tx_cpu[c].tc_lock); + int cv; + for (cv=0; cv < TXG_SIZE; cv++) { + cv_destroy(&tx->tx_cpu[c].tc_cv[cv]); + } + } kmem_free(tx->tx_cpu, max_ncpus * sizeof (tx_cpu_t)); From e85d2cca67fd0fb8e160f2c534653f60ecc4ecb9 Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Sun, 2 Sep 2012 02:36:52 +0200 Subject: [PATCH 04/14] Fixed type mismatch between lib/libzpool/common/kernel.c and zfs_context.h --- usr/src/lib/libzpool/common/kernel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usr/src/lib/libzpool/common/kernel.c b/usr/src/lib/libzpool/common/kernel.c index e8ce11b2..bec46aca 100644 --- a/usr/src/lib/libzpool/common/kernel.c +++ b/usr/src/lib/libzpool/common/kernel.c @@ -826,13 +826,13 @@ size_t u8_textprep_str(char *i, size_t *il, char *o, size_t *ol, int nf, } uid_t -crgetuid(cred_t *cr) +crgetuid(const cred_t *cr) { return (0); } gid_t -crgetgid(cred_t *cr) +crgetgid(const cred_t *cr) { return (0); } From 339b734191a26b3ebd794cdb753ae78ab6b4daab Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Mon, 3 Sep 2012 22:16:49 +0200 Subject: [PATCH 05/14] Fixed unneeded memcpy on it self in zio_ready(zio_t *zio). In zio_ready(), under some condition a blockpointer is copied into itself, i.e. bptr_t bp1, *bp2; bp2 = &bp; bp1 = *bp2; This produces a memcpy overlap error in valgrind. The real question of course is, if trying to copy the blockpointer into itself is a legitimate operation here. Not sure. Call tree for the offending copy: ==00:00:00:02.705 27867== Source and destination overlap in memcpy(0x1004b94f8, 0x1004b94f8, 128) ==00:00:00:02.705 27867== at 0x10016AD73: memcpy (mc_replace_strmem.c:523) ==00:00:00:02.705 27867== by 0x1000B954C: zio_ready (zio.c:919) ==00:00:00:02.705 27867== by 0x1000BE103: zio_next_stage (zio.c:2024) ==00:00:00:02.705 27867== by 0x1000B9192: zio_wait_for_children (zio.c:846) ==00:00:00:02.705 27867== by 0x1000B93BA: zio_wait_children_ready (zio.c:874) ==00:00:00:02.705 27867== by 0x1000BE38A: zio_next_stage_async (zio.c:2073) ==00:00:00:02.705 27867== by 0x1000B9111: zio_nowait (zio.c:833) ==00:00:00:02.705 27867== by 0x10009B7D8: vdev_label_read (vdev_label.c:168) ==00:00:00:02.705 27867== by 0x10009C69D: vdev_label_read_config (vdev_label.c:336) ==00:00:00:02.705 27867== by 0x100096FD3: vdev_validate (vdev.c:1009) ==00:00:00:02.705 27867== by 0x100096F80: vdev_validate (vdev.c:999) ==00:00:00:02.705 27867== by 0x100096F80: vdev_validate (vdev.c:999) ==00:00:00:02.705 27867== --- usr/src/uts/common/fs/zfs/zio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/usr/src/uts/common/fs/zfs/zio.c b/usr/src/uts/common/fs/zfs/zio.c index e4f45c49..7cce153b 100644 --- a/usr/src/uts/common/fs/zfs/zio.c +++ b/usr/src/uts/common/fs/zfs/zio.c @@ -916,7 +916,8 @@ zio_ready(zio_t *zio) &pio->io_children_notready); if (zio->io_bp) - zio->io_bp_copy = *zio->io_bp; + if (&zio->io_bp_copy != zio->io_bp) /* do not copy block over itself. */ + zio->io_bp_copy = *zio->io_bp; zio_next_stage(zio); } From 9b24f0fe87f4de390455f3dc237f20607a508082 Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Mon, 3 Sep 2012 23:02:52 +0200 Subject: [PATCH 06/14] Fixed potential NULL pointer dereference in zap_lookup_norm() In zap_lookup_norm(objset_t *os, uint64_t zapobj, const char *name, uint64_t integer_size, uint64_t num_integers, void *buf, matchtype_t mt, char *realname, int rn_len, boolean_t *ncp) strlcpy() is called with a size of 0, which results in undefined behavior. Calltree (example): ==00:00:00:07.711 27867== Invalid write of size 1 ==00:00:00:07.711 27867== at 0x10016AFFA: strlcpy (mc_replace_strmem.c:373) ==00:00:00:07.711 27867== by 0x1000AF568: zap_lookup_norm (zap_micro.c:683) ==00:00:00:07.711 27867== by 0x1000AF428: zap_lookup (zap_micro.c:645) ==00:00:00:07.711 27867== by 0x10007A78B: dsl_pool_open (dsl_pool.c:92) ==00:00:00:07.711 27867== by 0x100083AB0: spa_load (spa.c:1124) ==00:00:00:07.711 27867== by 0x100084538: spa_open_common (spa.c:1456) ==00:00:00:07.711 27867== by 0x100084783: spa_open (spa.c:1528) ==00:00:00:07.711 27867== by 0x100008404: main (zdb.c:2399) ==00:00:00:07.711 27867== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==00:00:00:07.711 27867== ==00:00:00:07.711 27867== ==00:00:00:07.711 27867== Process terminating with default action of signal 11 (SIGSEGV) ==00:00:00:07.711 27867== Access not within mapped region at address 0x0 Note that zap_lookup() *always* calls zap_lookup_norm() with a zero size argument, which is correct. So zap_lookup_norm() must not depend on implementation-defined behavior regarding strlcpy with zero size. --- usr/src/uts/common/fs/zfs/zap_micro.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/usr/src/uts/common/fs/zfs/zap_micro.c b/usr/src/uts/common/fs/zfs/zap_micro.c index 421e0473..487a78ec 100644 --- a/usr/src/uts/common/fs/zfs/zap_micro.c +++ b/usr/src/uts/common/fs/zfs/zap_micro.c @@ -680,8 +680,9 @@ zap_lookup_norm(objset_t *os, uint64_t zapobj, const char *name, err = EINVAL; } else { *(uint64_t *)buf = mze->mze_phys.mze_value; - (void) strlcpy(realname, - mze->mze_phys.mze_name, rn_len); + if (rn_len > 0) /* strlcpy may not be called with size == 0 */ + (void) strlcpy(realname, + mze->mze_phys.mze_name, rn_len); if (ncp) { *ncp = mzap_normalization_conflict(zap, zn, mze); From 34cbc198c3016b2868290bbd5df670e866439367 Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Wed, 5 Sep 2012 00:33:50 +0200 Subject: [PATCH 07/14] Fix a missing cv_init() cv_destroy() pair in spa_add() / spa_remove(). spa_add() does not fully initialize the spa structure, resulting in undefined behavior due to uninitialized memory later on. Calltree at time of crash ==00:00:02:46.240 31470== Process terminating with default action of signal 4 (SIGILL) ==00:00:02:46.240 31470== Illegal opcode at address 0x10023291D ==00:00:02:46.240 31470== at 0x10023291D: __abort (in /usr/lib/libSystem.B.dylib) ==00:00:02:46.240 31470== by 0x100232980: abort (in /usr/lib/libSystem.B.dylib) ==00:00:02:46.240 31470== by 0x1000A21E8: cv_wait (kernel.c:348) ==00:00:02:46.240 31470== by 0x10000D65B: ztest_suspend_monitor (ztest.c:3174) ==00:00:02:46.240 31470== by 0x10017D455: _pthread_start (in /usr/lib/libSystem.B.dylib) ==00:00:02:46.240 31470== by 0x10017D308: thread_start (in /usr/lib/libSystem.B.dylib) here the conditional variable spa->spa_zio_cv is used uninitialized, causing cv_wait to fail and the program to raising SIGABORT. --- usr/src/uts/common/fs/zfs/spa_misc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/usr/src/uts/common/fs/zfs/spa_misc.c b/usr/src/uts/common/fs/zfs/spa_misc.c index 09553550..d352652a 100644 --- a/usr/src/uts/common/fs/zfs/spa_misc.c +++ b/usr/src/uts/common/fs/zfs/spa_misc.c @@ -268,6 +268,7 @@ spa_add(const char *name, const char *altroot) cv_init(&spa->spa_async_cv, NULL, CV_DEFAULT, NULL); cv_init(&spa->spa_scrub_cv, NULL, CV_DEFAULT, NULL); cv_init(&spa->spa_scrub_io_cv, NULL, CV_DEFAULT, NULL); + cv_init(&spa->spa_zio_cv, NULL, CV_DEFAULT, NULL); spa->spa_name = spa_strdup(name); spa->spa_state = POOL_STATE_UNINITIALIZED; @@ -331,6 +332,7 @@ spa_remove(spa_t *spa) cv_destroy(&spa->spa_async_cv); cv_destroy(&spa->spa_scrub_cv); cv_destroy(&spa->spa_scrub_io_cv); + cv_destroy(&spa->spa_zio_cv); mutex_destroy(&spa->spa_uberblock_lock); mutex_destroy(&spa->spa_async_lock); From e35f948f848ad30e23d60caa599fc1673f27539c Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Wed, 5 Sep 2012 13:28:51 +0200 Subject: [PATCH 08/14] Fixed missing #ifdef __APPLE__ around change name type in call to zfs_dirent_lock(). The call to zfs_dirent_lock() in zfs_create() was missing an replacing the "char *name" parameter with Apples"componentname_t *cnp". This triggered a kerel panic on file create. panic trace: 0x21b455 : mov 0x8011d0,%eax 0x2a8ab2 : jmp 0x2a8ace 0x29e9a8 : mov %edi,%esp 0x4d614177 : movzbl (%eax),%eax 0x4d6240ca : mov %eax,-0x28(%ebp) 0x4d624820 : leave 0x4d614177 is in zfs_dirent_lock (/Users/bj/new-mac-zfs-bjoka/usr/src/uts/common/fs/zfs/zfs_dir.c:197). 192 ASSERT(name[cnp->cn_namelen] == '\0'); 193 #endif 194 /* 195 * Verify that we are not trying to lock '.', '..', or '.zfs' 196 */ 197 if (name[0] == '.' && 198 (name[1] == '\0' || (name[1] == '.' && name[2] == '\0')) || 199 zfs_has_ctldir(dzp) && strcmp(name, ZFS_CTLDIR_NAME) == 0) 200 return (EEXIST); 201 0x4d6240ca is in zfs_create (/Users/bj/new-mac-zfs-bjoka/usr/src/uts/common/fs/zfs/zfs_vnops.c:1674). 1669 #ifndef __APPLE__ 1670 if (flag & FIGNORECASE) 1671 zflg |= ZCILOOK; 1672 #endif 1673 1674 error = zfs_dirent_lock(&dl, dzp, name, &zp, zflg, 1675 NULL, NULL); 1676 if (error) { 1677 if (strcmp(name, "..") == 0) 1678 error = EISDIR; --- usr/src/uts/common/fs/zfs/zfs_vnops.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/usr/src/uts/common/fs/zfs/zfs_vnops.c b/usr/src/uts/common/fs/zfs/zfs_vnops.c index 5fd2f920..f9ab4720 100644 --- a/usr/src/uts/common/fs/zfs/zfs_vnops.c +++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c @@ -1671,8 +1671,13 @@ zfs_create(vnode_t *dvp, char *name, vattr_t *vap, vcexcl_t excl, zflg |= ZCILOOK; #endif +#ifdef __APPLE__ + error = zfs_dirent_lock(&dl, dzp, ct->componentname, &zp, zflg, + NULL, NULL); +#else error = zfs_dirent_lock(&dl, dzp, name, &zp, zflg, NULL, NULL); +#endif if (error) { if (strcmp(name, "..") == 0) error = EISDIR; From cb6a432fb7b930d8f0b9b1d2ecb9f1f82ffd5e1f Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Sat, 8 Sep 2012 23:23:49 +0200 Subject: [PATCH 09/14] Fixed missing clean of ZFS106. Due to a typo, build/ZFS105 was cleaned twice and build/ZFS106 no at all. --- support/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/release.sh b/support/release.sh index 2e418437..ecf1f513 100755 --- a/support/release.sh +++ b/support/release.sh @@ -36,7 +36,7 @@ USER=`whoami` ( mkdir -p ${DIR}/../${BUILD} rm -rf ${DIR}/../${BUILD}/ZFS105 -rm -rf ${DIR}/../${BUILD}/ZFS105 +rm -rf ${DIR}/../${BUILD}/ZFS106 cp -R ${DIR}/MacZFS.pmdoc ${DIR}/../${BUILD} sed -i~ -e "s/0\\.0\\.0/${VERSION}/g" ${DIR}/../${BUILD}/MacZFS.pmdoc/* From 448671a15ba177ff4093e2643c1a5fe55175304e Mon Sep 17 00:00:00 2001 From: BjoKaSH Date: Sat, 8 Sep 2012 23:25:49 +0200 Subject: [PATCH 10/14] Updated installer texts based on Daniel Bethe's suggestions, except credits. Reworded most texts in the installer based on Daniel Bethe's suggestions. File CREDITS not added, need further work. --- support/MacZFS.pmdoc/index.xml | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/support/MacZFS.pmdoc/index.xml b/support/MacZFS.pmdoc/index.xml index 1e184cec..f818424e 100644 --- a/support/MacZFS.pmdoc/index.xml +++ b/support/MacZFS.pmdoc/index.xml @@ -11,6 +11,16 @@ + Welcome to MacZFS, a free implementation of the ZFS file system for Mac OSX Leopard and newer. + +MacZFS offers nearly unlimited storage capacity, only limited by the amount and size of hard disks connected to an computer. It offers data replication in mirror or raidz configuration, dynamic stripping and build-in storage pool based volume management. + +MacZFS has end-to-end compression and strong data checksumming, including self-healing of bit-rot or any other read problem in redundant pools and offers unlimited, nearly-instantaneous snapshot support for backup and ecovery. Snapshots can be mount for read-only inspection, efficiently transfered between storage pools for off-site backups or promoted to full read-write filesystems. + +MacZFS is based on the original Sun implementation of the ZFS file system and Apple's initial port to Leopard whch was discontinued before public release. + +Today, MacZFS is developed by a community effort on www.maczfs.org and released under the CDDL and APSL liecenses. + - @@ -40,7 +50,7 @@ selected="no" enabled="no" hidden="yes" startSelected="unchanged" startEnabled="unchanged" startHidden="unchanged" /> -