Project

General

Profile

Actions

Bug #5824

closed

zfs_ioc_clone() return EINVAL where EXDEV is expected

Added by Andriy Gapon over 6 years ago. Updated over 6 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
-
Category:
zfs - Zettabyte File System
Start date:
2015-04-10
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

dmu_objset_clone_check() has the following code:

        error = dsl_dir_hold(dp, doca->doca_clone, FTAG, &pdd, &tail);
...
        /* You can't clone across pools. */
        if (pdd->dd_pool != dp) {
                dsl_dir_rele(pdd, FTAG);
                return (SET_ERROR(EXDEV));
        }

But, in fact, that code is NOP, because the synctask is assigned to the clone's pool, so dp is that pool and dsl_dir_hold() is called on the clone as well.

Then there is another check:

        error = dsl_dataset_hold(dp, doca->doca_origin, FTAG, &origin);
        if (error != 0)
                return (error);

        /* You can't clone across pools. */
        if (origin->ds_dir->dd_pool != dp) {
                dsl_dataset_rele(origin, FTAG);
                return (SET_ERROR(EXDEV));
        }

But the check is unreachable in exactly the scenario it is supposed to check: dsl_dataset_hold() calls dsl_dir_hold() before doing anything else, and if it is called with a dataset name that does not belong to the pool, then the function returns EINVAL.

        err = getcomponent(name, buf, &next);
        if (err != 0)
                return (err);

        /* Make sure the name is in the specified pool. */
        spaname = spa_name(dp->dp_spa);
        if (strcmp(buf, spaname) != 0)
                return (SET_ERROR(EINVAL));

I see two possibilities:
- dmu_objset_clone_check() can convert EINVAL returned from dsl_dataset_hold() to EXDEV
- dsl_dir_hold() could return EXDEV when the name is not in the pool


Related issues

Is duplicate of illumos gate - Bug #5610: zfs clone from different source and target pools produces coredumpClosedAlexander Eremin2015-02-12

Actions
Actions #1

Updated by Alexander Eremin over 6 years ago

See #5610

Actions #2

Updated by Andriy Gapon over 6 years ago

Yes, this is it.
I would close this issue as a dup, but it looks like I can't.

Actions #3

Updated by Rich Lowe over 6 years ago

  • Status changed from New to Rejected
Actions #4

Updated by Yuri Pankov over 6 years ago

This one has far better description than #5610, I'd suggest moving Andriy's analysis to #5610.

Actions

Also available in: Atom PDF