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 status" doesn't give reliable device names #4341

Closed
chadmiller opened this issue Feb 16, 2016 · 8 comments
Closed

"zpool status" doesn't give reliable device names #4341

chadmiller opened this issue Feb 16, 2016 · 8 comments
Milestone

Comments

@chadmiller
Copy link

In truncating everything before the last '/' in vdev names, we lose information that users and utilities would like to be able to use to work on the device outside of zpool.

I'm thinking of "grub" now, in that it wants a list of devices that make up a pool. "zpool status" gives some name that could be found in /dev/ or in /dev/disk/by-id/ or /dev/mapper/ ....

By discarding the path before printing, we make utilities and users guess, and that's a bad thing.

@chadmiller
Copy link
Author

A new parameter will work, but enforces a dependency on a particular version. This is what I hope to use in Ubuntu.

--- zfs-linux-0.6.5.4.orig/lib/libzfs/libzfs_pool.c
+++ zfs-linux-0.6.5.4/lib/libzfs/libzfs_pool.c
@@ -3484,8 +3484,11 @@ zpool_vdev_name(libzfs_handle_t *hdl, zp
         */
        verify(nvlist_lookup_string(nv, ZPOOL_CONFIG_TYPE, &type) == 0);
        if (strcmp(type, VDEV_TYPE_DISK) == 0) {
+         char *full_paths = getenv("WITH_FULL_PATHS");
+         if ((full_paths == NULL) || (strcmp(full_paths, "") == 0) || (strcmp(full_paths, "0") == 0)) {
            path = strrchr(path, '/');
            path++;
+         }
        }

        /*

@behlendorf
Copy link
Contributor

At least in spirit this is related to #2012 which adds an option to print the guid instead of the short name. It would be nice to refactor that previous work so it could be used to optionally print either the guid, short name, or full name of a vdev.

@ilovezfs
Copy link
Contributor

@behlendorf We've had this on OS X for a while:
openzfsonosx/zfs@87943e6
openzfsonosx/zfs@1390502

@behlendorf
Copy link
Contributor

@ilovezfs nice! I wasn't aware you'd built on Richard's work.

behlendorf pushed a commit to behlendorf/zfs that referenced this issue Feb 18, 2016
The following options have been added to the zpool add, iostat,
list, status, and split subcommands.  The default behavior was
not modified, from zfs(8).

  -g    Display vdev GUIDs  instead  of  the  normal  short
        device  names.  These GUIDs can be used in-place of
        device   names   for    the    zpool    detach/off‐
        line/remove/replace commands.

  -L    Display real paths for vdevs resolving all symbolic
        links. This can be used to lookup the current block
        device  name regardless of the /dev/disk/ path used
        to open it.

  -p    Display  full  paths  for vdevs instead of only the
        last component of the path.  This can  be  used  in
        conjunction with the -L flag.

This change is based on worked originally started by Richard Yao
to add a -g option.  Then extended by @ilovezfs to add a -L option
for openzfsonosx.  Those changes have been merged, re-factored,
a -p option added and extended to all relevant zpool subcommands.

Original-patch-by: Richard Yao <[email protected]>
Extended-by: ilovezfs <[email protected]>
Extended-by: Brian Behlendorf <[email protected]>
Issue openzfs#2011
Issue openzfs#4341
@arvidjaar
Copy link

Would you still consider out-of-band way to enable full paths, like environment variable, suggested originally? Using options is compatibility problem - how can we reliably detect whether zpool supports new options or not before calling it? Environment variable allows us to ask "please give us full names if possible" and will be ignored if zpool version in use does not support it.

behlendorf pushed a commit to behlendorf/zfs that referenced this issue Feb 18, 2016
The following options have been added to the zpool add, iostat,
list, status, and split subcommands.  The default behavior was
not modified, from zfs(8).

  -g    Display vdev GUIDs  instead  of  the  normal  short
        device  names.  These GUIDs can be used in-place of
        device   names   for    the    zpool    detach/off‐
        line/remove/replace commands.

  -L    Display real paths for vdevs resolving all symbolic
        links. This can be used to lookup the current block
        device  name regardless of the /dev/disk/ path used
        to open it.

  -p    Display  full  paths  for vdevs instead of only the
        last component of the path.  This can  be  used  in
        conjunction with the -L flag.

This behavior may also be enabled using the following environment
variables.

  ZPOOL_VDEV_NAME_GUID
  ZPOOL_VDEV_NAME_FOLLOW_LINKS
  ZPOOL_VDEV_NAME_PATH

This change is based on worked originally started by Richard Yao
to add a -g option.  Then extended by @ilovezfs to add a -L option
for openzfsonosx.  Those changes have been merged, re-factored,
a -p option added and extended to all relevant zpool subcommands.

Original-patch-by: Richard Yao <[email protected]>
Extended-by: ilovezfs <[email protected]>
Extended-by: Brian Behlendorf <[email protected]>
Issue openzfs#2011
Issue openzfs#4341
@behlendorf
Copy link
Contributor

@arvidjaar I'd think you'd want to test the command (or check the version) to determine if the option was supported. However, I do think adding the environment variables would be a nice way to customize the default behavior. I've added the following environment variables to control this.

ZPOOL_VDEV_NAME_GUID
ZPOOL_VDEV_NAME_FOLLOW_LINKS
ZPOOL_VDEV_NAME_PATH

For those who need this feature it would be helpful if you could test the patch in #4343 and verify it meets your needs.

behlendorf pushed a commit to behlendorf/zfs that referenced this issue Feb 18, 2016
The following options have been added to the zpool add, iostat,
list, status, and split subcommands.  The default behavior was
not modified, from zfs(8).

  -g    Display vdev GUIDs  instead  of  the  normal  short
        device  names.  These GUIDs can be used in-place of
        device   names   for    the    zpool    detach/off‐
        line/remove/replace commands.

  -L    Display real paths for vdevs resolving all symbolic
        links. This can be used to lookup the current block
        device  name regardless of the /dev/disk/ path used
        to open it.

  -p    Display  full  paths  for vdevs instead of only the
        last component of the path.  This can  be  used  in
        conjunction with the -L flag.

This behavior may also be enabled using the following environment
variables.

  ZPOOL_VDEV_NAME_GUID
  ZPOOL_VDEV_NAME_FOLLOW_LINKS
  ZPOOL_VDEV_NAME_PATH

This change is based on worked originally started by Richard Yao
to add a -g option.  Then extended by @ilovezfs to add a -L option
for openzfsonosx.  Those changes have been merged, re-factored,
a -p option added and extended to all relevant zpool subcommands.

Original-patch-by: Richard Yao <[email protected]>
Extended-by: ilovezfs <[email protected]>
Extended-by: Brian Behlendorf <[email protected]>
Issue openzfs#2011
Issue openzfs#4341
@rlaager
Copy link
Member

rlaager commented Feb 24, 2016

@arvidjaar, @chadmiller Is it particularly problematic to just call zpool status -p and if the exit status is non-zero, try again with zpool status?

@arvidjaar
Copy link

@rlaager We run it on multiple platforms (Linux, *BSD, OS X, may be others I have not even heard of). There is no guarantee that each platform gives the same meaning to zpool status -p as each develops zfs independently.

Anyway, variables are already there, it is more work to remove the code now :)

nedbass pushed a commit that referenced this issue Mar 23, 2016
The following options have been added to the zpool add, iostat,
list, status, and split subcommands.  The default behavior was
not modified, from zfs(8).

  -g    Display vdev GUIDs  instead  of  the  normal  short
        device  names.  These GUIDs can be used in-place of
        device   names   for    the    zpool    detach/off‐
        line/remove/replace commands.

  -L    Display real paths for vdevs resolving all symbolic
        links. This can be used to lookup the current block
        device  name regardless of the /dev/disk/ path used
        to open it.

  -p    Display  full  paths  for vdevs instead of only the
        last component of the path.  This can  be  used  in
        conjunction with the -L flag.

This behavior may also be enabled using the following environment
variables.

  ZPOOL_VDEV_NAME_GUID
  ZPOOL_VDEV_NAME_FOLLOW_LINKS
  ZPOOL_VDEV_NAME_PATH

This change is based on worked originally started by Richard Yao
to add a -g option.  Then extended by @ilovezfs to add a -L option
for openzfsonosx.  Those changes have been merged, re-factored,
a -p option added and extended to all relevant zpool subcommands.

Original-patch-by: Richard Yao <[email protected]>
Extended-by: ilovezfs <[email protected]>
Extended-by: Brian Behlendorf <[email protected]>
Signed-off-by: ilovezfs <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #2011
Closes #4341
@behlendorf behlendorf added this to the 0.6.5.6 milestone Mar 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants