Please review: Bug: https://www.illumos.org/issues/5515 Webrev: http://31bits.net/illumos/cr/5515-dataset-user-hold-doesnt-reject-empty-tags/ Patch: http://repo.or.cz/w/illumos-gate/jeffpc.git/patch/86e8d34e994c9f247014b92a0f88de261a170a46
This patch causes the hold ioctl handler to return EINVAL in one of two cases: (1) the hold tag is empty (new) (2) user wanted to put a hold on something that's not a snapshot (existing) Sadly, the ioctl return value makes its way to libzfs (zfs_hold_nvl), which does what libzfs does - prints an error message. http://src.illumos.org/source/xref/illumos-gate/usr/src/lib/libzfs/common/libzfs_dataset.c#4305 It of course assumes that we got an EINVAL because of case #2 and so the user sees: "operation not applicable to datasets of this type" This is quite misleading. My best idea is to add a empty-tag check to libzfs. Any better ideas? Since the error message is a rather minor cosmetic issue, I'm hoping to not delay the assertion failure preventing fix in the ioctl handler because of it. These changes can be pulled via: $ git pull git://repo.or.cz/illumos-gate/jeffpc.git zfs-hold Thanks, Jeff.
>From 86e8d34e994c9f247014b92a0f88de261a170a46 Mon Sep 17 00:00:00 2001 From: Josef 'Jeff' Sipek <[email protected]> Date: Thu, 8 Jan 2015 13:01:10 -0500 Subject: [PATCH] 5515 dataset user hold doesn't reject empty tags --- usr/src/uts/common/fs/zfs/zfs_ioctl.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/usr/src/uts/common/fs/zfs/zfs_ioctl.c b/usr/src/uts/common/fs/zfs/zfs_ioctl.c index 20a6466..9acefb4 100644 --- a/usr/src/uts/common/fs/zfs/zfs_ioctl.c +++ b/usr/src/uts/common/fs/zfs/zfs_ioctl.c @@ -27,7 +27,7 @@ * Copyright (c) 2011, 2014 by Delphix. All rights reserved. * Copyright (c) 2013 by Saso Kiselkov. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved. - * Copyright (c) 2014, Nexenta Systems, Inc. All rights reserved. + * Copyright (c) 2015, Nexenta Systems, Inc. All rights reserved. */ /* @@ -5110,6 +5110,7 @@ zfs_ioc_smb_acl(zfs_cmd_t *zc) static int zfs_ioc_hold(const char *pool, nvlist_t *args, nvlist_t *errlist) { + nvpair_t *pair; nvlist_t *holds; int cleanup_fd = -1; int error; @@ -5119,6 +5120,19 @@ zfs_ioc_hold(const char *pool, nvlist_t *args, nvlist_t *errlist) if (error != 0) return (SET_ERROR(EINVAL)); + /* make sure the user didn't pass us any invalid (empty) tags */ + for (pair = nvlist_next_nvpair(holds, NULL); pair != NULL; + pair = nvlist_next_nvpair(holds, pair)) { + char *htag; + + error = nvpair_value_string(pair, &htag); + if (error != 0) + return (SET_ERROR(error)); + + if (strlen(htag) == 0) + return (SET_ERROR(EINVAL)); + } + if (nvlist_lookup_int32(args, "cleanup_fd", &cleanup_fd) == 0) { error = zfs_onexit_fd_hold(cleanup_fd, &minor); if (error != 0) -- 1.7.9.2
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
