Author: jpaetzel
Date: Fri Apr 21 19:53:52 2017
New Revision: 317267
URL: https://svnweb.freebsd.org/changeset/base/317267

Log:
  MFV 316891
  
  7386 zfs get does not work properly with bookmarks
  
  illumos/illumos-gate@edb901aab9c738b5eb15aa55933e82b0f2f9d9a2
  
https://github.com/illumos/illumos-gate/commit/edb901aab9c738b5eb15aa55933e82b0f2f9d9a2
  
  https://www.illumos.org/issues/7386
    The zfs get command does not work with the bookmark parameter while it works
    properly with both filesystem and snapshot:
    # zfs get -t all -r creation rpool/test
    NAME               PROPERTY  VALUE                  SOURCE
    rpool/test         creation  Fri Sep 16 15:00 2016  -
    rpool/test@snap    creation  Fri Sep 16 15:00 2016  -
    rpool/test#bkmark  creation  Fri Sep 16 15:00 2016  -
    # zfs get -t all -r creation rpool/test@snap
    NAME             PROPERTY  VALUE                  SOURCE
    rpool/test@snap  creation  Fri Sep 16 15:00 2016  -
    # zfs get -t all -r creation rpool/test#bkmark
    cannot open 'rpool/test#bkmark': invalid dataset name
    #
    The zfs get command should be modified to work properly with bookmarks too.
  
  Reviewed by: Simon Klinkert <simon.klink...@gmail.com>
  Reviewed by: Paul Dagnelie <p...@delphix.com>
  Approved by: Matthew Ahrens <mahr...@delphix.com>
  Author: Marcel Telka <mar...@telka.sk>

Modified:
  head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
  head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c
  head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c
  head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h
Directory Properties:
  head/cddl/contrib/opensolaris/   (props changed)
  head/cddl/contrib/opensolaris/cmd/zfs/   (props changed)
  head/cddl/contrib/opensolaris/lib/libzfs/   (props changed)
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Fri Apr 21 19:41:33 2017        
(r317266)
+++ head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Fri Apr 21 19:53:52 2017        
(r317267)
@@ -25,13 +25,13 @@
 .\" Copyright (c) 2013 by Saso Kiselkov. All rights reserved.
 .\" Copyright (c) 2014, Joyent, Inc. All rights reserved.
 .\" Copyright (c) 2013, Steven Hartland <s...@freebsd.org>
-.\" Copyright (c) 2014 Nexenta Systems, Inc. All Rights Reserved.
+.\" Copyright (c) 2016 Nexenta Systems, Inc. All Rights Reserved.
 .\" Copyright (c) 2014, Xin LI <delp...@freebsd.org>
 .\" Copyright (c) 2014-2015, The FreeBSD Foundation, All Rights Reserved.
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 31, 2016
+.Dd September 16, 2016
 .Dt ZFS 8
 .Os
 .Sh NAME
@@ -114,7 +114,7 @@
 .Op Fl t Ar type Ns Oo , Ns type Ns Oc Ns ...
 .Oo Fl s Ar property Oc Ns ...
 .Oo Fl S Ar property Oc Ns ...
-.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot
+.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot | Ns Ar bookmark Ns ...
 .Nm
 .Cm set
 .Ar property Ns = Ns Ar value Oo Ar property Ns = Ns Ar value Oc Ns ...
@@ -2156,7 +2156,7 @@ section.
 .Op Fl t Ar type Ns Oo , Ns Ar type Oc Ns ...
 .Op Fl s Ar source Ns Oo , Ns Ar source Oc Ns ...
 .Ar all | property Ns Oo , Ns Ar property Oc Ns ...
-.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns ...
+.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns | Ns Ar bookmark Ns ...
 .Xc
 .Pp
 Displays properties for the given datasets. If no datasets are specified, then

Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c    Fri Apr 21 19:41:33 
2017        (r317266)
+++ head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c    Fri Apr 21 19:53:52 
2017        (r317267)
@@ -243,7 +243,7 @@ get_usage(zfs_help_t idx)
                    "[-o \"all\" | field[,...]]\n"
                    "\t    [-t type[,...]] [-s source[,...]]\n"
                    "\t    <\"all\" | property[,...]> "
-                   "[filesystem|volume|snapshot] ...\n"));
+                   "[filesystem|volume|snapshot|bookmark] ...\n"));
        case HELP_INHERIT:
                return (gettext("\tinherit [-rS] <property> "
                    "<filesystem|volume|snapshot> ...\n"));
@@ -1622,7 +1622,7 @@ zfs_do_get(int argc, char **argv)
 {
        zprop_get_cbdata_t cb = { 0 };
        int i, c, flags = ZFS_ITER_ARGS_CAN_BE_PATHS;
-       int types = ZFS_TYPE_DATASET;
+       int types = ZFS_TYPE_DATASET | ZFS_TYPE_BOOKMARK;
        char *value, *fields;
        int ret = 0;
        int limit = 0;

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c    Fri Apr 
21 19:41:33 2017        (r317266)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c    Fri Apr 
21 19:53:52 2017        (r317267)
@@ -103,7 +103,7 @@ zfs_validate_name(libzfs_handle_t *hdl, 
        char what;
 
        (void) zfs_prop_get_table();
-       if (dataset_namecheck(path, &why, &what) != 0) {
+       if (entity_namecheck(path, &why, &what) != 0) {
                if (hdl != NULL) {
                        switch (why) {
                        case NAME_ERR_TOOLONG:
@@ -132,9 +132,10 @@ zfs_validate_name(libzfs_handle_t *hdl, 
                                    "'%c' in name"), what);
                                break;
 
-                       case NAME_ERR_MULTIPLE_AT:
+                       case NAME_ERR_MULTIPLE_DELIMITERS:
                                zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-                                   "multiple '@' delimiters in name"));
+                                   "multiple '@' and/or '#' delimiters in "
+                                   "name"));
                                break;
 
                        case NAME_ERR_NOLETTER:
@@ -165,7 +166,7 @@ zfs_validate_name(libzfs_handle_t *hdl, 
        if (!(type & ZFS_TYPE_SNAPSHOT) && strchr(path, '@') != NULL) {
                if (hdl != NULL)
                        zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-                           "snapshot delimiter '@' in filesystem name"));
+                           "snapshot delimiter '@' is not expected here"));
                return (0);
        }
 
@@ -176,6 +177,20 @@ zfs_validate_name(libzfs_handle_t *hdl, 
                return (0);
        }
 
+       if (!(type & ZFS_TYPE_BOOKMARK) && strchr(path, '#') != NULL) {
+               if (hdl != NULL)
+                       zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+                           "bookmark delimiter '#' is not expected here"));
+               return (0);
+       }
+
+       if (type == ZFS_TYPE_BOOKMARK && strchr(path, '#') == NULL) {
+               if (hdl != NULL)
+                       zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+                           "missing '#' delimiter in bookmark name"));
+               return (0);
+       }
+
        if (modifying && strchr(path, '%') != NULL) {
                if (hdl != NULL)
                        zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
@@ -613,8 +628,36 @@ make_bookmark_handle(zfs_handle_t *paren
        return (zhp);
 }
 
+struct zfs_open_bookmarks_cb_data {
+       const char *path;
+       zfs_handle_t *zhp;
+};
+
+static int
+zfs_open_bookmarks_cb(zfs_handle_t *zhp, void *data)
+{
+       struct zfs_open_bookmarks_cb_data *dp = data;
+
+       /*
+        * Is it the one we are looking for?
+        */
+       if (strcmp(dp->path, zfs_get_name(zhp)) == 0) {
+               /*
+                * We found it.  Save it and let the caller know we are done.
+                */
+               dp->zhp = zhp;
+               return (EEXIST);
+       }
+
+       /*
+        * Not found.  Close the handle and ask for another one.
+        */
+       zfs_close(zhp);
+       return (0);
+}
+
 /*
- * Opens the given snapshot, filesystem, or volume.   The 'types'
+ * Opens the given snapshot, bookmark, filesystem, or volume.   The 'types'
  * argument is a mask of acceptable types.  The function will print an
  * appropriate error message and return NULL if it can't be opened.
  */
@@ -623,6 +666,7 @@ zfs_open(libzfs_handle_t *hdl, const cha
 {
        zfs_handle_t *zhp;
        char errbuf[1024];
+       char *bookp;
 
        (void) snprintf(errbuf, sizeof (errbuf),
            dgettext(TEXT_DOMAIN, "cannot open '%s'"), path);
@@ -630,20 +674,68 @@ zfs_open(libzfs_handle_t *hdl, const cha
        /*
         * Validate the name before we even try to open it.
         */
-       if (!zfs_validate_name(hdl, path, ZFS_TYPE_DATASET, B_FALSE)) {
-               zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-                   "invalid dataset name"));
+       if (!zfs_validate_name(hdl, path, types, B_FALSE)) {
                (void) zfs_error(hdl, EZFS_INVALIDNAME, errbuf);
                return (NULL);
        }
 
        /*
-        * Try to get stats for the dataset, which will tell us if it exists.
+        * Bookmarks needs to be handled separately.
         */
-       errno = 0;
-       if ((zhp = make_dataset_handle(hdl, path)) == NULL) {
-               (void) zfs_standard_error(hdl, errno, errbuf);
-               return (NULL);
+       bookp = strchr(path, '#');
+       if (bookp == NULL) {
+               /*
+                * Try to get stats for the dataset, which will tell us if it
+                * exists.
+                */
+               errno = 0;
+               if ((zhp = make_dataset_handle(hdl, path)) == NULL) {
+                       (void) zfs_standard_error(hdl, errno, errbuf);
+                       return (NULL);
+               }
+       } else {
+               char dsname[ZFS_MAX_DATASET_NAME_LEN];
+               zfs_handle_t *pzhp;
+               struct zfs_open_bookmarks_cb_data cb_data = {path, NULL};
+
+               /*
+                * We need to cut out '#' and everything after '#'
+                * to get the parent dataset name only.
+                */
+               assert(bookp - path < sizeof (dsname));
+               (void) strncpy(dsname, path, bookp - path);
+               dsname[bookp - path] = '\0';
+
+               /*
+                * Create handle for the parent dataset.
+                */
+               errno = 0;
+               if ((pzhp = make_dataset_handle(hdl, dsname)) == NULL) {
+                       (void) zfs_standard_error(hdl, errno, errbuf);
+                       return (NULL);
+               }
+
+               /*
+                * Iterate bookmarks to find the right one.
+                */
+               errno = 0;
+               if ((zfs_iter_bookmarks(pzhp, zfs_open_bookmarks_cb,
+                   &cb_data) == 0) && (cb_data.zhp == NULL)) {
+                       (void) zfs_error(hdl, EZFS_NOENT, errbuf);
+                       zfs_close(pzhp);
+                       return (NULL);
+               }
+               if (cb_data.zhp == NULL) {
+                       (void) zfs_standard_error(hdl, errno, errbuf);
+                       zfs_close(pzhp);
+                       return (NULL);
+               }
+               zhp = cb_data.zhp;
+
+               /*
+                * Cleanup.
+                */
+               zfs_close(pzhp);
        }
 
        if (zhp == NULL) {

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c       Fri Apr 
21 19:41:33 2017        (r317266)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c       Fri Apr 
21 19:53:52 2017        (r317267)
@@ -947,9 +947,10 @@ zpool_name_valid(libzfs_handle_t *hdl, b
                                    "trailing slash in name"));
                                break;
 
-                       case NAME_ERR_MULTIPLE_AT:
+                       case NAME_ERR_MULTIPLE_DELIMITERS:
                                zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-                                   "multiple '@' delimiters in name"));
+                                   "multiple '@' and/or '#' delimiters in "
+                                   "name"));
                                break;
 
                        default:

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c       Fri Apr 
21 19:41:33 2017        (r317266)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c       Fri Apr 
21 19:53:52 2017        (r317267)
@@ -718,7 +718,7 @@ zfs_get_pool_handle(const zfs_handle_t *
  * Given a name, determine whether or not it's a valid path
  * (starts with '/' or "./").  If so, walk the mnttab trying
  * to match the device number.  If not, treat the path as an
- * fs/vol/snap name.
+ * fs/vol/snap/bkmark name.
  */
 zfs_handle_t *
 zfs_path_to_zhandle(libzfs_handle_t *hdl, char *path, zfs_type_t argtype)

Modified: head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c        Fri Apr 
21 19:41:33 2017        (r317266)
+++ head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c        Fri Apr 
21 19:53:52 2017        (r317267)
@@ -120,9 +120,9 @@ permset_namecheck(const char *path, name
 }
 
 /*
- * Dataset names must be of the following form:
+ * Entity names must be of the following form:
  *
- *     [component][/]*[component][@component]
+ *     [component/]*[component][(@|#)component]?
  *
  * Where each component is made up of alphanumeric characters plus the 
following
  * characters:
@@ -133,10 +133,10 @@ permset_namecheck(const char *path, name
  * names for temporary clones (for online recv).
  */
 int
-dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
+entity_namecheck(const char *path, namecheck_err_t *why, char *what)
 {
-       const char *loc, *end;
-       int found_snapshot;
+       const char *start, *end;
+       int found_delim;
 
        /*
         * Make sure the name is not too long.
@@ -161,12 +161,13 @@ dataset_namecheck(const char *path, name
                return (-1);
        }
 
-       loc = path;
-       found_snapshot = 0;
+       start = path;
+       found_delim = 0;
        for (;;) {
                /* Find the end of this component */
-               end = loc;
-               while (*end != '/' && *end != '@' && *end != '\0')
+               end = start;
+               while (*end != '/' && *end != '@' && *end != '#' &&
+                   *end != '\0')
                        end++;
 
                if (*end == '\0' && end[-1] == '/') {
@@ -176,25 +177,8 @@ dataset_namecheck(const char *path, name
                        return (-1);
                }
 
-               /* Zero-length components are not allowed */
-               if (loc == end) {
-                       if (why) {
-                               /*
-                                * Make sure this is really a zero-length
-                                * component and not a '@@'.
-                                */
-                               if (*end == '@' && found_snapshot) {
-                                       *why = NAME_ERR_MULTIPLE_AT;
-                               } else {
-                                       *why = NAME_ERR_EMPTY_COMPONENT;
-                               }
-                       }
-
-                       return (-1);
-               }
-
                /* Validate the contents of this component */
-               while (loc != end) {
+               for (const char *loc = start; loc != end; loc++) {
                        if (!valid_char(*loc) && *loc != '%') {
                                if (why) {
                                        *why = NAME_ERR_INVALCHAR;
@@ -202,43 +186,64 @@ dataset_namecheck(const char *path, name
                                }
                                return (-1);
                        }
-                       loc++;
                }
 
-               /* If we've reached the end of the string, we're OK */
-               if (*end == '\0')
-                       return (0);
-
-               if (*end == '@') {
-                       /*
-                        * If we've found an @ symbol, indicate that we're in
-                        * the snapshot component, and report a second '@'
-                        * character as an error.
-                        */
-                       if (found_snapshot) {
+               /* Snapshot or bookmark delimiter found */
+               if (*end == '@' || *end == '#') {
+                       /* Multiple delimiters are not allowed */
+                       if (found_delim != 0) {
                                if (why)
-                                       *why = NAME_ERR_MULTIPLE_AT;
+                                       *why = NAME_ERR_MULTIPLE_DELIMITERS;
                                return (-1);
                        }
 
-                       found_snapshot = 1;
+                       found_delim = 1;
+               }
+
+               /* Zero-length components are not allowed */
+               if (start == end) {
+                       if (why)
+                               *why = NAME_ERR_EMPTY_COMPONENT;
+                       return (-1);
                }
 
+               /* If we've reached the end of the string, we're OK */
+               if (*end == '\0')
+                       return (0);
+
                /*
-                * If there is a '/' in a snapshot name
+                * If there is a '/' in a snapshot or bookmark name
                 * then report an error
                 */
-               if (*end == '/' && found_snapshot) {
+               if (*end == '/' && found_delim != 0) {
                        if (why)
                                *why = NAME_ERR_TRAILING_SLASH;
                        return (-1);
                }
 
                /* Update to the next component */
-               loc = end + 1;
+               start = end + 1;
        }
 }
 
+/*
+ * Dataset is any entity, except bookmark
+ */
+int
+dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
+{
+       int ret = entity_namecheck(path, why, what);
+
+       if (ret == 0 && strchr(path, '#') != NULL) {
+               if (why != NULL) {
+                       *why = NAME_ERR_INVALCHAR;
+                       *what = '#';
+               }
+               return (-1);
+       }
+
+       return (ret);
+}
 
 /*
  * mountpoint names must be of the following form:

Modified: head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h
==============================================================================
--- head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h        Fri Apr 
21 19:41:33 2017        (r317266)
+++ head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h        Fri Apr 
21 19:53:52 2017        (r317267)
@@ -38,7 +38,7 @@ typedef enum {
        NAME_ERR_EMPTY_COMPONENT,       /* name contains an empty component */
        NAME_ERR_TRAILING_SLASH,        /* name ends with a slash */
        NAME_ERR_INVALCHAR,             /* invalid character found */
-       NAME_ERR_MULTIPLE_AT,           /* multiple '@' characters found */
+       NAME_ERR_MULTIPLE_DELIMITERS,   /* multiple '@'/'#' delimiters found */
        NAME_ERR_NOLETTER,              /* pool doesn't begin with a letter */
        NAME_ERR_RESERVED,              /* entire name is reserved */
        NAME_ERR_DISKLIKE,              /* reserved disk name (c[0-9].*) */
@@ -49,6 +49,7 @@ typedef enum {
 #define        ZFS_PERMSET_MAXLEN      64
 
 int pool_namecheck(const char *, namecheck_err_t *, char *);
+int entity_namecheck(const char *, namecheck_err_t *, char *);
 int dataset_namecheck(const char *, namecheck_err_t *, char *);
 int mountpoint_namecheck(const char *, namecheck_err_t *);
 int zfs_component_namecheck(const char *, namecheck_err_t *, char *);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to