On thu, 28 Nov 2013 17:59:07 +0100, David Sterba wrote:
> Subvolume deletion does not do a full transaction commit. This can lead
> to an unexpected result when the system crashes between deletion and
> commit, the subvolume directory will appear again. Add options to request
> filesystem sync after each deleted subvolume or after the last one.
>
> If the command with sync option finishes succesfully, the subvolume(s)
> deletion status is safely stored on the media.
>
> Userspace approach is more flexible than in-kernel. Related discussions:
> http://www.spinics.net/lists/linux-btrfs/msg22088.html
> http://www.spinics.net/lists/linux-btrfs/msg27240.html
>
> CC: Alex Lyakas <alex.bt...@zadarastorage.com>
> Signed-off-by: David Sterba <dste...@suse.cz>
> ---
>
> The option names may not be ideal, feel free to suggest better.
>
> cmds-subvolume.c | 77
> +++++++++++++++++++++++++++++++++++++++++++++++++++-----
> man/btrfs.8.in | 23 ++++++++++++++---
> 2 files changed, 90 insertions(+), 10 deletions(-)
>
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index c6a5284a02a4..c124c3185252 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -202,24 +202,67 @@ int test_issubvolume(char *path)
> }
>
> static const char * const cmd_subvol_delete_usage[] = {
> - "btrfs subvolume delete <subvolume> [<subvolume>...]",
> + "btrfs subvolume delete [options] <subvolume> [<subvolume>...]",
> "Delete subvolume(s)",
> + "Delete subvolumes from the filesystem. The corresponding directory",
> + "is removed instantly but the data blocks are removed later.",
> + "The deletion does not involve full commit by default due to",
> + "performance reasons (as a consequence, the subvolume may appear again",
> + "after a crash). Use one of the --sync options to sync the filesystem",
> + "via a btrfs specific ioctl.",
> + "",
> + "-s|--sync-after call sync at the end of the operation",
> + "-S|--sync-each sync the filesystem after deleting each
> subvolume",
> NULL
> };
>
> static int cmd_subvol_delete(int argc, char **argv)
> {
> - int res, fd, len, e, cnt = 1, ret = 0;
> + int res, len, e, ret = 0;
> + int cnt;
> + int fd = -1;
> struct btrfs_ioctl_vol_args args;
> char *dname, *vname, *cpath;
> char *dupdname = NULL;
> char *dupvname = NULL;
> char *path;
> DIR *dirstream = NULL;
> + int sync_mode = 0;
> + struct option long_options[] = {
> + {"sync-after", no_argument, NULL, 's'}, /* sync mode 1 */
> + {"sync-each", no_argument, NULL, 'S'}, /* sync mode 2 */
> + {NULL, 0, NULL, 0}
> + };
Generally, we put it out of the functions.
> - if (argc < 2)
> + optind = 1;
> + while (1) {
> + int c;
> +
> + c = getopt_long(argc, argv, "sS", long_options, NULL);
> + if (c < 0)
> + break;
> +
> + switch(c) {
> + case 's':
> + sync_mode = 1;
> + break;
> + case 'S':
> + sync_mode = 2;
> + break;
> + default:
> + usage(cmd_subvol_delete_usage);
> + }
> + }
> +
> + if (check_argc_min(argc - optind, 1))
> usage(cmd_subvol_delete_usage);
>
> + printf("Sync filesystem mode: %s\n",
> + !sync_mode ? "none (default)" :
> + sync_mode == 1 ? "at the end" : "after each");
> +
> + cnt = optind;
> +
> again:
> path = argv[cnt];
>
> @@ -276,8 +319,6 @@ again:
> res = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args);
> e = errno;
>
> - close_file_or_dir(fd, dirstream);
> -
> if(res < 0 ){
> fprintf( stderr, "ERROR: cannot delete '%s/%s' - %s\n",
> dname, vname, strerror(e));
> @@ -285,14 +326,38 @@ again:
> goto out;
> }
>
> + if (sync_mode == 1) {
> + res = ioctl(fd, BTRFS_IOC_SYNC);
It will flush all the dirty pages, it is unnecessary. I think BTRFS_IOC_{START,
WAIT}_SYNC is better.
Thanks
Miao
> + if (res < 0) {
> + fprintf(stderr,
> + "ERROR: unable to sync after '%s': %s\n",
> + path, strerror(e));
> + ret = 1;
> + }
> + }
> +
> out:
> free(dupdname);
> free(dupvname);
> dupdname = NULL;
> dupvname = NULL;
> cnt++;
> - if (cnt < argc)
> + if (cnt < argc) {
> + close_file_or_dir(fd, dirstream);
> goto again;
> + }
> +
> + if (sync_mode == 2 && fd != -1) {
> + res = ioctl(fd, BTRFS_IOC_SYNC);
> + e = errno;
> + if (res < 0) {
> + fprintf(stderr,
> + "ERROR: unable to do final sync: %s\n",
> + strerror(e));
> + ret = 1;
> + }
> + }
> + close_file_or_dir(fd, dirstream);
>
> return ret;
> }
> diff --git a/man/btrfs.8.in b/man/btrfs.8.in
> index b6203483296e..2d5c12e08640 100644
> --- a/man/btrfs.8.in
> +++ b/man/btrfs.8.in
> @@ -8,7 +8,7 @@ btrfs \- control a btrfs filesystem
> .SH SYNOPSIS
> \fBbtrfs\fP \fBsubvolume create\fP [-i \fI<qgroupid>\fP]
> [\fI<dest>\fP/]\fI<name>\fP
> .PP
> -\fBbtrfs\fP \fBsubvolume delete\fP \fI<subvolume>\fP [\fI<subvolume>...\fP]
> +\fBbtrfs\fP \fBsubvolume delete\fP [\fIoptions\fP] \fI<subvolume>\fP
> [\fI<subvolume>...\fP]
> .PP
> \fBbtrfs\fP \fBsubvolume list\fP [\fIoptions\fP] [-G [+|-]\fIvalue\fP] [-C
> [+|-]\fIvalue\fP] [--sort=rootid,gen,ogen,path] \fI<path>\fP
> .PP
> @@ -168,9 +168,24 @@ times.
> .RE
> .TP
>
> -\fBsubvolume delete\fR\fI <subvolume> \fP[\fI<subvolume>...\fP]\fR
> -Delete the subvolume \fI<subvolume>\fR. If \fI<subvolume>\fR is not a
> -subvolume, \fBbtrfs\fR returns an error.
> +\fBsubvolume delete\fR [\fIoptions\fP] \fI<subvolume>\fP
> [\fI<subvolume>...\fP]\fR
> +Delete the subvolume(s) from the filesystem. If \fI<subvolume>\fR is not a
> +subvolume, \fBbtrfs\fR returns an error but continues if there are more
> arguments
> +to process.
> +
> +The corresponding directory is removed instantly but the data blocks are
> +removed later. The deletion does not involve full commit by default due to
> +performance reasons (as a consequence, the subvolume may appear again after a
> +crash). Use one of the --sync options to sync the filesystem via a btrfs
> +specific ioctl.
> +.RS
> +
> +\fIOptions\fP
> +.IP "\fB-s|--sync-after\fP" 5
> +call sync at the end of the operation
> +.IP "\fB-S|--sync-each\fP" 5
> +sync the filesystem after deleting each subvolume
> +.RE
> .TP
>
> \fBsubvolume list\fR [\fIoptions\fP] [-G [+|-]\fIvalue\fP] [-C
> [+|-]\fIvalue\fP] [--sort=rootid,gen,ogen,path] \fI<path>\fR
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
