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
