Skip to content

Commit

Permalink
Instead of storing the filesystem just received in top_zfs,
Browse files Browse the repository at this point in the history
mount and share it directly in zfs_receive_one().

This will make a filesystem be mounted as soon as it's been received,
instead of waiting to the very end - which might take days.

Signed-off-by: Turbo Fredriksson <[email protected]>

Closes: openzfs#2673
  • Loading branch information
FransUrbo committed Sep 7, 2014
1 parent cd3939c commit 060738e
Showing 1 changed file with 13 additions and 11 deletions.
24 changes: 13 additions & 11 deletions lib/libzfs/libzfs_sendrecv.c
Original file line number Diff line number Diff line change
Expand Up @@ -3068,6 +3068,11 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
}
}

if (clp) {
err |= changelist_postfix(clp);
changelist_free(clp);
}

/*
* Mount the target filesystem (if created). Also mount any
* children of the target filesystem if we did a replication
Expand All @@ -3084,23 +3089,20 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
if (h->zfs_type == ZFS_TYPE_VOLUME) {
*cp = '@';
} else if (newfs || stream_avl) {
/*
* Track the first/top of hierarchy fs,
* for mounting and sharing later.
*/
if (top_zfs && *top_zfs == NULL)
*top_zfs = zfs_strdup(hdl, zc.zc_value);

This comment has been minimized.

Copy link
@behlendorf

behlendorf Sep 10, 2014

Regardless of whether we mount it immediately we should preserve the logic which tracks the first/top fs. I'm also not sure if this is entirely safe.

This comment has been minimized.

Copy link
@FransUrbo

FransUrbo via email Sep 10, 2014

Author Owner

This comment has been minimized.

Copy link
@behlendorf

behlendorf Sep 10, 2014

Well why in particular was it necessary to remove the assignment of *top_zfs? As for making this kind of change I'm just not familiar enough with this part of the code to make this kind of change without spending more time examining it.

This comment has been minimized.

Copy link
@FransUrbo

FransUrbo Sep 10, 2014

Author Owner

Because I was afraid that adding the filesystem to the list, mount it and then at the very end mount it again would lead to trouble. Never bothered to check it, though...

You just have a "feeling" that it might be problematic?!! :). Well, to ease your mind, I do to. Mine is just very, very faint :)

This comment has been minimized.

Copy link
@behlendorf

behlendorf Sep 10, 2014

It may, but removing it as you've done changes the expected behavior which isn't good. Any caller which depended on top_zfs is now broken.

You just have a "feeling" that it might be problematic?!!

Yes. We're talking about a filesystem here. It's important that we get it right and understand the consequences of a given change. :) I'm not saying this is totally wrong, just that I haven't personally spent enough time in this area of the code to say for certain.

/* mount and share received datasets */
clp = changelist_gather(h, ZFS_PROP_MOUNTPOINT,
CL_GATHER_MOUNT_ALWAYS,
0);
if (clp) {
err |= changelist_postfix(clp);
changelist_free(clp);
}
}
zfs_close(h);
}
*cp = '@';
}

if (clp) {
err |= changelist_postfix(clp);
changelist_free(clp);
}

if (prop_errflags & ZPROP_ERR_NOCLEAR) {
(void) fprintf(stderr, dgettext(TEXT_DOMAIN, "Warning: "
"failed to clear unreceived properties on %s"),
Expand Down

0 comments on commit 060738e

Please sign in to comment.