Re: [PATCH] Btrfs-progs: receive: fix the case that we can not find subvolume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi dave,

> On Tue, Dec 17, 2013 at 05:13:49PM +0800, Wang Shilong wrote:
>> If we change our default subvolume, btrfs receive will fail to find
>> subvolume. To fix this problem, i have two ideas.
>> 
>> 1.make btrfs snapshot ioctl support passing source subvolume's objectid
> 
>> 2.when we want to using interval subvolume path, we mount it other place
>> that use subvolume 5 as its default subvolume.
> 
> 3. Tell the user to mount the toplevel subvol by himself and run receive
>   again

If we really don't want to bother kernel change, i think we can add a option for btrfs receive(for example -f)
to force tool to resolve such ENOENT and at the same time, we output something like:

fprintf(stderr, "Default subvolume is changed,……….")

if -f is not assigned, we will fail here.

> 
>> We'd better use the second approach because it won't bother kernel change.
> 
> I don't think that the silent mount is the right way to fix it, that way
> the btrfs tool tooks responsibility not to break anything.  Like the
> unhandled umount failure below. I think admins and power users do not
> like to see some random tool mess with the system like this.

> 
>> @@ -199,6 +200,10 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
>> 	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
>> 	struct btrfs_ioctl_vol_args_v2 args_v2;
>> 	struct subvol_info *parent_subvol = NULL;
>> +	char *dev = NULL;
>> +	char tmp_name[15] = "btrfs-XXXXXX";
>> +	char tmp_dir[30] = "/tmp";
> 
> Mounting valuable data under /tmp is dangerous, what if some /tmp
> cleaner starts to remove old files. I've seen that happen in practice.

Agree with  this.

> 
>> @@ -269,10 +308,14 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
>> 		fprintf(stderr, "ERROR: creating snapshot %s -> %s "
>> 				"failed. %s\n", parent_subvol->path,
>> 				path, strerror(-ret));
>> -		goto out;
>> 	}
>> 
>> +out_umount:
>> +	umount(tmp_dir);
> 
> umount fails for whatever reason,

will fix it.

> 
>> +	rmdir(tmp_dir);
> 
> at least this does not delete the files recursively.

Why we need delete the files recursively here,
I only create dir ,something like /tmp/btrfs-XXXXX, and i only want to delete the temp dir
btrfs-XXXX here…

Thanks,
Wang

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux