On Mon, Sep 29, 2014 at 12:49 PM, Steven Hartland via illumos-zfs < [email protected]> wrote:
> I've just spotted what looks like a possible miss-merge of: > 4369 implement zfs bookmarks > This is not a mis-merge, at least not from DelphixOS, where the patch is the same. > > It seems we've lost the snap must be in the specified pool > check as well as the check for error on unmount which was > specifically added by: > 3744 zfs shouldn't ignore errors unmounting snapshots > I believe that the semantics should be the same as if that code had been left in place. dsl_destroy_snapshot_check() will fail if a snapshot is in a different pool. It will also fail if a snapshot is mounted. Do you have a test case which shows that the current behavior is undesirable? --matt > > Both seem a little odd. > > https://github.com/illumos/illumos-gate/commit/ > 78f171005391b928aaf1642b3206c534ed644332#diff- > 6296d5738a0663ee611fea974d51e0ca > > Prior to this commit this we had:- > > static int > zfs_ioc_destroy_snaps(const char *poolname, nvlist_t *innvl, nvlist_t > *outnvl) > { > int error, poollen; > nvlist_t *snaps; > nvpair_t *pair; > boolean_t defer; > > if (nvlist_lookup_nvlist(innvl, "snaps", &snaps) != 0) > return (SET_ERROR(EINVAL)); > defer = nvlist_exists(innvl, "defer"); > > poollen = strlen(poolname); > for (pair = nvlist_next_nvpair(snaps, NULL); pair != NULL; > pair = nvlist_next_nvpair(snaps, pair)) { > const char *name = nvpair_name(pair); > > /* > * The snap must be in the specified pool. > */ > if (strncmp(name, poolname, poollen) != 0 || > (name[poollen] != '/' && name[poollen] != '@')) > return (SET_ERROR(EXDEV)); > > error = zfs_unmount_snap(name); > if (error != 0) > return (error); > } > > return (dsl_destroy_snapshots_nvl(snaps, defer, outnvl)); > } > > After it we have: > zfs_ioc_destroy_snaps(const char *poolname, nvlist_t *innvl, nvlist_t > *outnvl) > { > nvlist_t *snaps; > nvpair_t *pair; > boolean_t defer; > > if (nvlist_lookup_nvlist(innvl, "snaps", &snaps) != 0) > return (SET_ERROR(EINVAL)); > defer = nvlist_exists(innvl, "defer"); > > for (pair = nvlist_next_nvpair(snaps, NULL); pair != NULL; > pair = nvlist_next_nvpair(snaps, pair)) { > (void) zfs_unmount_snap(nvpair_name(pair)); > } > > return (dsl_destroy_snapshots_nvl(snaps, defer, outnvl)); > } > > > ------------------------------------------- > illumos-zfs > Archives: https://www.listbox.com/member/archive/182191/=now > RSS Feed: https://www.listbox.com/member/archive/rss/182191/ > 21635000-ebd1d460 > Modify Your Subscription: https://www.listbox.com/ > member/?member_id=21635000&id_secret=21635000-73dc201a > Powered by Listbox: http://www.listbox.com >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
