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

zpool add command should refuse to add non-redundant disk configurations to pools without an override option #2486

Closed
karloor opened this issue Jul 11, 2014 · 6 comments
Labels
Type: Feature Feature request or new feature
Milestone

Comments

@karloor
Copy link

karloor commented Jul 11, 2014

At the moment it's embarrassingly easy to do this:

zfs add tank sdn

when you mean to do this:

zfs add tank log sdn 

@ryao in a IRC chat suggests that adding a non-redundant vdev configuration to a pool like this should only be possible by supplying a flag to force it.

@ikiris
Copy link

ikiris commented Jul 12, 2014

I actually like this idea, compared to all the other --force requests this shouldn't be often, and it's very unlikely you really want to do it anyway, especially considering the syntax overlap of cache and log devices with add. I do think it's only really necessary as long as the lack of graceful removal of vdevs exists however, as its far to easy to irreparably poison a pool at the moment, with no clean way to resolve the issue.

@aarcane
Copy link

aarcane commented Jul 12, 2014

I'd like to add my +1 to this. I would even suggest that it simply be
implemented, then a pull request be sent upstream rather than a whole
upstream "let's pow wow on this" party that you do for other changes. It's
a good freaking idea!

Also note that it should not require force if adding a log or cache device
unless the pool version is too low to remove the cache.

@behlendorf behlendorf added this to the 0.7.0 milestone Jul 17, 2014
@behlendorf
Copy link
Contributor

Sounds like a reasonable suggestion to me. If someone wants to propose a patch for this we can get it in the queue for review.

@behlendorf behlendorf modified the milestones: 0.8.0, 0.7.0 Mar 25, 2016
@adilger
Copy link
Contributor

adilger commented Jul 19, 2017

It wouldn't be a terrible idea to structure this so zpool reports an error if adding a single-disk VDEV to a pool that does not yet have any other single-disk VDEVs, unless something like "--force-single-disk" was used (or answering "yes" at a prompt in an interactive shell). This is a problem that has hit many people (one more this week), and since there is no easy way to repair this afterward, it shouldn't be so easily done.

By only limiting the error to cases where no other single-disk VDEVs already exist in the pool, this avoids problems for cases where someone is creating a new single-disk test pool, or has previously forced a single disk into the pool and knows that is what they want.

@behlendorf
Copy link
Contributor

The original issue here was addressed in master. The fix will be in 0.7.0. When adding devices it now verifies that those devices are of a similar replication level. If they're not you must provide the - f option, for example:

# zpool create tank mirror vdb vdc
# zpool add tank vdd
invalid vdev specification
use '-f' to override the following errors:
mismatched replication level: pool uses mirror and new vdev is disk

@adilger
Copy link
Contributor

adilger commented Jul 26, 2017

Overloading "-f" to mean different things (add VDEVs of different Redundancy, add VDEVs on partitions, etc) is not really a good idea. Better to have something like "--force-nonredundant-vdev" or similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Feature request or new feature
Projects
None yet
Development

No branches or pull requests

5 participants