On Sat, Jul 20, 2019 at 3:34 PM Omar Sandoval <osan...@osandov.com> wrote:
>
> From: Omar Sandoval <osan...@fb.com>
>
> When we process a clone request, we look up the source subvolume by
> UUID, even if the source is the subvolume that we're currently
> receiving. Usually, this is fine. However, if for some reason we
> previously received the same subvolume, then this will use paths
> relative to the previously received subvolume instead of the current
> one. This is incorrect, since the send stream may use temporary names
> for the clone source. This can be reproduced as follows:
>
> btrfs subvol create subvol
> dd if=/dev/urandom of=subvol/foo bs=1M count=1
> cp --reflink subvol/foo subvol/bar
> mkdir subvol/dir
> mv subvol/foo subvol/dir/
> btrfs property set subvol ro true
> btrfs send -f stream subvol
> mkdir first second
> btrfs receive -f stream first
> btrfs receive -f stream second
>
> The second receive results in this error:
>
> ERROR: cannot open first/subvol/o259-7-0/foo: No such file or directory
>
> Fix it by always cloning from the current subvolume if its UUID matches.
> This has the nice side effect of avoiding unnecessary UUID tree lookups
> in that case. Also, while we're here, get rid of some code that has been
> commented out since it was added.
>
> Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> Signed-off-by: Omar Sandoval <osan...@fb.com>
> ---
> cmds/receive.c | 34 ++++++++--------------------------
> 1 file changed, 8 insertions(+), 26 deletions(-)
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index a3e62985..ed089af2 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -753,15 +753,14 @@ static int process_clone(const char *path, u64 offset,
> u64 len,
> if (ret < 0)
> goto out;
>
> - si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
> - NULL,
> - subvol_search_by_received_uuid);
> - if (IS_ERR_OR_NULL(si)) {
> - if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> - BTRFS_UUID_SIZE) == 0) {
> - /* TODO check generation of extent */
> - subvol_path = rctx->cur_subvol_path;
> - } else {
> + if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> + BTRFS_UUID_SIZE) == 0) {
> + subvol_path = rctx->cur_subvol_path;
> + } else {
> + si = subvol_uuid_search(&rctx->sus, 0, clone_uuid,
> clone_ctransid,
> + NULL,
> + subvol_search_by_received_uuid);
> + if (IS_ERR_OR_NULL(si)) {
Hi Omar,
This, and the change log, look good.
> if (!si)
> ret = -ENOENT;
> else
> @@ -769,23 +768,6 @@ static int process_clone(const char *path, u64 offset,
> u64 len,
> error("clone: did not find source subvol");
> goto out;
> }
> - } else {
> - /*if (rs_args.ctransid > rs_args.rtransid) {
> - if (!r->force) {
> - ret = -EINVAL;
> - fprintf(stderr, "ERROR: subvolume %s was "
> - "modified after it was "
> - "received.\n",
> - r->subvol_parent_name);
> - goto out;
> - } else {
> - fprintf(stderr, "WARNING: subvolume %s was "
> - "modified after it was "
> - "received.\n",
> - r->subvol_parent_name);
> - }
> - }*/
> -
Isn't this unrelated? Shouldn't go to a separate patch?
Also, would you please create a test case as well?
Thanks.
> /* strip the subvolume that we are receiving to from the
> start of subvol_path */
> if (rctx->full_root_path) {
> size_t root_len = strlen(rctx->full_root_path);
> --
> 2.22.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
