grwilson commented on this pull request. I have concerns about the ultimate goal for this command. It's unclear if we really want to "wipe" or just "forget" about this device.
> @@ -709,8 +751,12 @@ zpool_do_labelclear(int argc, char **argv) } if (zpool_read_label(fd, &config) != 0) { - (void) fprintf(stderr, - gettext("failed to read label from %s\n"), vdev); + if (force) Shouldn't this check force_invalid since it's possible for zpool_read_label to return -1 for reasons other than the inability to read the label. If just force is used then we could wipe out the label of a legit pool. > + if (pread64(fd, &label, sizeof (vdev_label_t), + label_offset(size, l)) != sizeof (vdev_label_t)) + return (-1); + + if (!force) { + nvlist_t *config = NULL; + if (nvlist_unpack(buf, buflen, &config, 0) != 0) + return (-1); + nvlist_free(config); + } + } + + if (wipe) { + (void) memset(&label, 0, sizeof (vdev_label_t)); + } else { + if (nvlist_invalidate(buf, buflen) != 0) If the goal is to prevent zpool import from showing the pool, then we should set the TXG value to 0 which is how ZFS clears a label while still leaving some information around for debugging. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/424#pullrequestreview-94746416 ------------------------------------------ openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T59e810f5492df392-Mb71b9d84a4c53f6d4b400daf Powered by Topicbox: https://topicbox.com