Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improve 'zpool labelclear' command #8548

Closed

Conversation

martymac
Copy link

Hello,

That patch follows : #8500 to further improve the labelclear command.

It is an updated version of my my previous patch for OpenZFS :

openzfs/openzfs#424

Description

Basically, the patch adds the following :

  • ability to clear a specific label (options -b, -e and -i)
  • label invalidation using a single-byte modification (use -w to zeroe the label as before)
  • option -ff to force invalidation even for invalid labels

How Has This Been Tested?

The patch has been tested on Debian 9 with a file-backed zpool.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

That patch follows:
  openzfs#8500
and is a port to ZoL of:
  openzfs/openzfs#424

It adds the following:
  - ability to clear a specific label (options -b, -e and -i)
  - label invalidation using a single-byte modification (wipe with option -w)
  - option -ff to force invalidation even for invalid labels

OpenZFS-issue: https://www.illumos.org/issues/7584
@behlendorf behlendorf added the Status: Design Review Needed Architecture or design is under discussion label Mar 29, 2019
@ahrens
Copy link
Member

ahrens commented Jun 4, 2021

I haven't often had need for zpool labelclear, but when I have, it's worked just fine. Personally, I don't see the use case for these expanded options, but maybe I just haven't run into enough weird situations :-). Can you elaborate on when this flexibility is needed? In the absence of additional input, I would be inclined to close this PR.

@martymac
Copy link
Author

martymac commented Jun 6, 2021

Hello Matthew,

Thanks for the update.

The original idea was :

  • to avoid breaking an existing underlying filesystem by destroying the pool with minimal writes (in case of a mistake)
  • to add flexibility by allowing manipulating specific labels only
    but there have been nearly no reaction to this patch so I presume I am the only one feeling this could be useful :p I have no special case in mind but just wanted to prevent our customers from destroying data by mistake.

If you think this is not worth updating that patch, feel free to close that PR.

Best regards,

Ganael.

@tonyhutter
Copy link
Contributor

@martymac I have a PR (#12474) that adds a zpool labelclear -f --unsafe option to allow clearing the label on active pools. I'm not clear if that's the same as your -ff option. Just wanted to give you a heads-up.

@martymac
Copy link
Author

Hello @tonyhutter,

Thanks !

Our patches do not act on the same things : your patch focuses on allowing clearing valid labels from active pools while mine allows clearing invalid labels (see: 7c62008#diff-8f1a174ac28c3b0269336b69d4cbd5d057fb6e65b62321f54c4dce5c8ab2d14bR1168 and 7c62008#diff-4d1ee8409af09d86d05eb396741ef7c3a1ba2cad77a58777164c4866084fb140R177). They can be seen as complementary if we really want to avoid using dd to clear labels.

Cheers,

Ganael.

@ahrens
Copy link
Member

ahrens commented Nov 9, 2021

I think that we would need to find a specific use case for this, otherwise we'll close this PR.

@martymac
Copy link
Author

Hello Matthew,

As there has been no interest shown in the features brought by the patch, I presume we can close that PR.

Best regards,

Ganael.

@ahrens ahrens closed this Nov 10, 2021
@ahrens
Copy link
Member

ahrens commented Nov 10, 2021

Thanks Ganael, if you have other ideas for how to improve OpenZFS please let us know by opening a PR or discussing them at our monthly meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Design Review Needed Architecture or design is under discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants