Add option to zpool status to print guids

[ryao]

The use of vdev GUIDs are a necessary workaround in edge cases where the
names provided by zpool status are not accepted by the zpool
detach/offline/remove/replace commands. The current method of obtaining
them uses zdb, but this does not work in all cases (see
#1530).

This provides a method of obtaining vdev GUIDs that is more reliable and
straightforward than zdb. It would be better to fix all edge cases that
require the use of GUIDs as a workaround, but Linux's /dev design makes
it difficult to anticipate such edge cases, which makes this option
necessary.

Note that this adds a new boolean parameter to zpool_vdev_name, which
changes the libzfs interface.

Closes #2011

Signed-off-by: Richard Yao ryao@gentoo.org

[ryao]

I emailed the Open ZFS list about this last night and the first (and so far only) reply is extremely positive:

http://lists.open-zfs.org:8080/pipermail/developer/2013-December/000439.html

There appears to be no need to worry about objections from other Open ZFS implementations. Their multipath implementations also benefit from this.

[ryao]

wca in #open-zfs on freenode asked that I change zpool_vdev_name() to be less fragile. In specific, he would like me to modify the interface to accept an nvlist of parameters so that additional ones can be added without modification to the libzfs interface. I plan to do this in a few days. I will keep the pull request open pending that update.

[thisisnotmyrealname]

Got this compiled and worked perfectly.

sheppard@ubuntu:~/zfs/cmd/zpool$ sudo ./zpool status -g
  pool: big
 state: ONLINE
status: One or more devices could not be used because the label is missing or
        invalid.  Sufficient replicas exist for the pool to continue
        functioning in a degraded state.
action: Replace the device using 'zpool replace'.
   see: http://zfsonlinux.org/msg/ZFS-8000-4J
config:

        NAME                      STATE     READ WRITE CKSUM
        big                       ONLINE       0     0     0
...
        cache
          6180157807563045896     UNAVAIL      0     0     0
          13167722250894959249    UNAVAIL      0     0     0
          16837174335567101628    ONLINE       0     0     0
          10715406852195698685    ONLINE       0     0     0
          9524106851775155867     ONLINE       0     0     0
          11284432760070873792    ONLINE       0     0     0

errors: No known data errors
sheppard@ubuntu:~/zfs/cmd/zpool$ sudo zpool remove big 6180157807563045896
sheppard@ubuntu:~/zfs/cmd/zpool$ sudo zpool remove big 13167722250894959249
sheppard@ubuntu:~/zfs/cmd/zpool$ sudo zpool status
  pool: big
 state: ONLINE
config:
...
       cache
          16837174335567101628    ONLINE       0     0     0
          10715406852195698685    ONLINE       0     0     0
          9524106851775155867     ONLINE       0     0     0
          11284432760070873792    ONLINE       0     0     0

Thank you very much Mr Yao.

[behlendorf]

@ryao I'm all for this as well, once you've made the requested changes I'll give it a look!

[behlendorf]

@ryao What's the status of this?

[ryao]

@behlendorf I wrote it to help a user and it has been on the back burner since. It is on my list of things to revise as soon as I push out the taskq refactoring. The taskq refactoring was delayed as other things came up, but I have the prototype running on m live system with only a few regressions that left to address before pushing it. In specific, I need to fix the zvol dispatch to avoid taking a mutex in an interrupt context (which is okay on Illumos/Solaris), implement a subset of the cyclic interface in the SPL (partially done) and rework the deadman code to use it.

[ryao]

@behlendorf I have pushed an updated commit that addresses @wca's comment.

[wca]

Excellent! Thanks!

Really minor nit: zpool_vdev_name()'s 'args' arg is probably more of an 'opts'; it only contains options.

[ryao]

@wca I have updated the commit to use opts. Thanks for suggesting it.

By the way, should I append your Reviewed-by: to the commit message?

[wca]

I have not reviewed the entire patch, I was merely offering an improvement in the name of API stability. For the file scope functions it doesn't matter, but exported APIs deserve a bit more care. In any case, the spirit of this change is fine by me.

[ryao]

@behlendorf I have done a refresh to fix cstyle warnings. This version should be merge-able.

[ryao]

The sole failure is openzfs/spl#371, which is unrelated to this patch. It should be safe to merge this.

[behlendorf]

I'd like to get this in to 0.6.4. If we could get some additional testing and reviews it would help speed up getting this merged.

[ilovezfs]

Thanks to @rottegift's championing of this pull request, it is now cherry picked in OpenZFS on OS X master:
openzfsonosx/zfs@1390502

[rottegift]

Works great. It'd be nice if zpool status -g also gives the pool guid, though.

[behlendorf]

It'd be nice if zpool status -g also gives the pool guid, though.

I like that idea. Tweak the patch to print the pool guid instead of it's name when -g is used.

[DeHackEd]

A recent commit, e02b533, broke this patch. It adds references to zpool_vdev_name which need the additional argument added.

[behlendorf]

I had to chance to look these patches over and while the definitely work and are safe to use, I think we need to explore a cleaner way to add this support. The bulk of the changes in the patch are caused by trying to pass the -g option all the way down to zpool_vdev_name() as a function argument. What if instead we either:

• Passed this flag in the zpool_handle_t structure which is already passed to max_width(), print_status_config(), print_logs(), print_l2cache(), and print_spares(). Or,
• Added an optional ZPOOL_CONFIG_VDEV_FORMAT entry to the nvroot configuration nvlist.

Either option would also allow us to extend the -g flag beyond zpool status. It would be nice if this functionality was also available to things like zpool add -n.

[behlendorf]

@ryao @DeHackEd since this is a very valuable feature I'd love to see it in the next tag. Any change either of you can find the time to exploring reworking this in to a form which can be merged. I'm fairly sure this code can be significantly simplified.

[behlendorf]

Closing is favor or #4343.