Hi,
This patch improves the 'zpool labelclear' command.
See :
https://www.illumos.org/issues/7584
for a detailed description and test cases.
Best regards,
Ganael.
You can view, comment on, or merge this pull request online at:
https://github.com/openzfs/openzfs/pull/424
-- Commit Summary
@martymac pushed 6 commits.
ccdede9 Use strtonum() instead of strtoll()
31452fc Add buflen check to nvlist_invalidate()
c876bde Move nvlist_invalidate symbol to SUNWprivate_1.1
1561da9 Simplify boolean check syntax
72b508c Avoid compilation warning related to ignored return value of memset()
Thanks for your feedback! I've just committed changes. Can you check them out ?
--
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#issuecomment-315520516
--
@martymac pushed 1 commit.
22fe1ea Use C99_ENABLE macro
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/424/files/c94b7fd8fef2a1d5ff20f540e5b0c7e338a53d3d..22fe1ea7d7b816181336970f6ba242d89bdda9ba
Hi,
Thanks for your feedback.
C99LMODE seems to be derived from C99MODE (see Makefile.master) so setting
C99MODE only should be enough ? I've modified the Makefile accordingly.
Regards,
Ganael.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly
Hi Matthew,
Thanks for your feedback.
My original idea was to extend the command while keeping its default behaviour.
The reason is simple : that command is quite new in OpenZFS world but has
already existed on FreeBSD for about 6 years now, see :
https://svnweb.freebsd.org/base?view=revision&
What I mean is that people are probably used to the fact that this command
blindly zeroes 512KiB at the beginning and the end of the disk and may (ab)use
that property in a way or another (e.g. in an install script to wipe other
kinds of metadata - shame on me, I remember having done that myself
@ahrens Hi Matthew, what do you think about my previous comments ? Can we leave
the default options as they are ?
--
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#issuecomment-32428
@martymac pushed 4 commits.
dc767f1 Check label validity by default and remove option '-c'
81bb4c6 Invalidate labels the minimal way by default
8c9eeb3 Rewrite boolean test for more readability
e0dbeda Specify -f twice to treat invalid labels as valid
--
You are receiving this because you a
@ahrens Hi Matthew,
Well, I have modified the patch to try to keep simplicity as well as
flexibility :
- As you suggest, always check label validity by default (but one can use the
-f flag twice to override that behaviour)
- Use the minimal mode by default but provide an option (-w) to wipe (ze
@ahrens Any thoughts on that new patch ?
--
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#issuecomment-326551259
--
openzfs-developer
Archives
Hi Matthew,
I've pushed fixes following your recommendations, thanks :)
Only remains warnings about abd stuff. Any hint ?
--
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#issuecomm
martymac commented on this pull request.
> @@ -47,6 +48,7 @@
#include
#include
#include
+#include
Yes, this is needed just for VDEV_LABELS. You mean moving VDEV_LABELS
definition to sys/fs/zfs.h ? I don't know what that would imply for other parts
of the code :/ Help would be welcome h
@martymac pushed 1 commit.
7485b1b Follow Matthew Ahrens' recommendations
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/424/files/e0dbedafea472876343fc49402990c0930896712..7485b1b72227ba2c14464c9a8f37b96c338388ca
@martymac pushed 1 commit.
7be4c74 Move VDEV_LABELS definition to sys/fs/zfs.h
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/424/files/7485b1b72227ba2c14464c9a8f37b96c338388ca..7be4c748e5b99478657b945bbe10dc54936
@ahrens My last commit should fix compilation warnings.
--
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#issuecomment-328064611
--
openzfs-dev
Compilation warnings have been fixed and all checks have passed.
@ahrens @rmustacc @yuripv Do you have additional comments regarding that PR ?
--
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/openzf
@ahrens Have you had time to take a deeper look at latest changes ?
--
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#issuecomment-337288816
--
@martymac pushed 1 commit.
29d743b Merge branch 'master' of https://github.com/openzfs/openzfs into
zpool-labelclear-improvements
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/424/files/7be4c748e5b99478657b945bb
Hi and happy new year :)
I've updated the patch with latest changes from master. If there are no more
comments, can it be merged upstream ?
--
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/
martymac commented on this pull request.
> @@ -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
martymac commented on this pull request.
> + if (pread64(fd, &label, sizeof (vdev_label_t),
+ label_offset(size, l)) != sizeof (vdev_label_t))
+ return (-1);
+
+ if (!force) {
+
Hi @grwilson,
As a recall, the original problem that led to the patch is described here :
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=204622
> These knobs expose the internals of the product.
Yes, but I don't think exposing the internals of the product is a problem here
because it does
Hi @grwilson,
Do you have any other concerns / comments about that patch ?
--
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#issuecomment-372652991
---
Hi @ahrens, @grwilson,
Are there news about that patch ?
Best regards,
Ganael.
--
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#issuecomment-393063362
-
25 matches
Mail list logo
