diff mbox series

[f2fs-dev,v2,2/2] resize.f2fs: add caution message for resize

Message ID 20250411100526.7939-2-chao@kernel.org (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev,v2,1/2] Revert "resize.f2fs: add option for large_nat_bitmap feature" | expand

Commit Message

Chao Yu April 11, 2025, 10:05 a.m. UTC
resize.f2fs doesn't guarantee atomically resizing f2fs image,
so that potential suddent power cut, IO error, out of memory,
SIGKILL received or program exits due to other reasons may cause
data corruption.

This patch adds caution message for resize users to notice
potential risk of using resizing tool, and remind them to backup
data before resize.

resize.f2fs <partition>

"Please notice there is high risk of data lost during resize, please backup your data before resize.
Do you want to resize this partition? [y/n] y
process resize"

If we want to force to use resize.f2fs, we can use -F option,
let's enable -F option in Android by default to not break any
usage.

Cc: Ju Hyung Park <qkrwngud825@gmail.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fsck/main.c       | 27 +++++++++++++++++++++++++--
 fsck/resize.c     |  2 +-
 include/f2fs_fs.h |  1 +
 man/resize.f2fs.8 | 12 ++++++++++++
 4 files changed, 39 insertions(+), 3 deletions(-)

Comments

Juhyung Park April 11, 2025, 10:20 a.m. UTC | #1
On Fri, Apr 11, 2025 at 3:05 AM Chao Yu <chao@kernel.org> wrote:
>
> resize.f2fs doesn't guarantee atomically resizing f2fs image,
> so that potential suddent power cut, IO error, out of memory,
> SIGKILL received or program exits due to other reasons may cause
> data corruption.
>
> This patch adds caution message for resize users to notice
> potential risk of using resizing tool, and remind them to backup
> data before resize.
>
> resize.f2fs <partition>
>
> "Please notice there is high risk of data lost during resize, please backup your data before resize.
> Do you want to resize this partition? [y/n] y
> process resize"
>
> If we want to force to use resize.f2fs, we can use -F option,
> let's enable -F option in Android by default to not break any
> usage.
>
> Cc: Ju Hyung Park <qkrwngud825@gmail.com>

Can you update this to "Juhyung Park"? That's the preferred name that
I use now ;)

> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fsck/main.c       | 27 +++++++++++++++++++++++++--
>  fsck/resize.c     |  2 +-
>  include/f2fs_fs.h |  1 +
>  man/resize.f2fs.8 | 12 ++++++++++++
>  4 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/fsck/main.c b/fsck/main.c
> index af40613..ef6b17d 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -222,6 +222,8 @@ static void add_default_options(void)
>                 if (c.func == FSCK) {
>                         /* -a */
>                         c.auto_fix = 1;
> +               } else if (c.func == RESIZE) {
> +                       c.force = 1;
>                 }
>
>                 /*
> @@ -601,7 +603,7 @@ void f2fs_parse_options(int argc, char *argv[])
>  #endif
>         } else if (!strcmp("resize.f2fs", prog)) {
>  #ifdef WITH_RESIZE
> -               const char *option_string = "d:fHst:o:V";
> +               const char *option_string = "d:fFHst:o:V";
>
>                 c.func = RESIZE;
>                 while ((option = getopt(argc, argv, option_string)) != EOF) {
> @@ -618,9 +620,12 @@ void f2fs_parse_options(int argc, char *argv[])
>                                                         c.dbg_lv);
>                                 break;
>                         case 'f':
> +                               c.ignore_error = 1;
> +                               MSG(0, "Info: Ingore error during resize\n");

s/Ingore/ignore
s/error/errors

> +                               break;
> +                       case 'F':
>                                 c.force = 1;
>                                 MSG(0, "Info: Force to resize\n");
> -                               break;

Is this fallthrough intended?

>                         case 'H':
>                                 c.need_whint = true;
>                                 c.whint = WRITE_LIFE_NOT_SET;
> @@ -1104,6 +1109,24 @@ out_range:
>  #ifdef WITH_RESIZE
>  static int do_resize(struct f2fs_sb_info *sbi)
>  {
> +       char answer[255] = {0};
> +       int ret;
> +
> +       if (!c.force) {
> +retry:
> +               printf("\nPlease notice there is high risk of data lost during resize, "
> +                       "please backup your data before resize.\n"
> +                       "Do you want to resize this partition? [y/n] ");

"high risk" is probably too aggressive. I had many successes in resize.

Maybe "Resize operation is currently experimental, please backup your
data.\nDo you want to continue? [y/n] "?

> +               ret = scanf("%s", answer);
> +               ASSERT(ret >= 0);
> +               if (!strcasecmp(answer, "y"))
> +                       printf("process resize\n");

Something like "Proceeding to resize" looks better here.

> +               else if (!strcasecmp(answer, "n"))
> +                       return 0;
> +               else
> +                       goto retry;
> +       }
> +
>         if (!c.target_sectors)
>                 c.target_sectors = c.total_sectors;
>
> diff --git a/fsck/resize.c b/fsck/resize.c
> index 2681b9f..1ab7d75 100644
> --- a/fsck/resize.c
> +++ b/fsck/resize.c
> @@ -765,7 +765,7 @@ int f2fs_resize(struct f2fs_sb_info *sbi)
>                 }
>         else if (((c.target_sectors * c.sector_size >>
>                         get_sb(log_blocksize)) > get_sb(block_count)) ||
> -                       c.force)
> +                       c.ignore_error)
>                 return f2fs_resize_grow(sbi);
>         else {
>                 MSG(0, "Nothing to resize.\n");
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 7e3f25b..928f963 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -1528,6 +1528,7 @@ struct f2fs_configuration {
>         int no_kernel_check;
>         int fix_on;
>         int force;
> +       int ignore_error;
>         int defset;
>         int bug_on;
>         unsigned int invalid_sb;
> diff --git a/man/resize.f2fs.8 b/man/resize.f2fs.8
> index 02ff245..bdda4fd 100644
> --- a/man/resize.f2fs.8
> +++ b/man/resize.f2fs.8
> @@ -18,6 +18,12 @@ resize.f2fs \- resize filesystem size
>  .I overprovision-ratio-percentage
>  ]
>  [
> +.B \-f
> +]
> +[
> +.B \-F
> +]
> +[
>  .B \-H
>  ]
>  [
> @@ -53,6 +59,12 @@ Specify the percentage of the volume that will be used as overprovision area.
>  This area is hidden to users, and utilized by F2FS cleaner. If not specified, the
>  best number will be assigned automatically according to the partition size.
>  .TP
> +.BI \-f
> +Force to fix any inconsistent data during resize.
> +.TP
> +.BI \-F
> +Skip caution dialogue and resize partition directly.
> +.TP
>  .BI \-H
>  Specify support write hint.
>  .TP
> --
> 2.40.1
>
Chao Yu April 11, 2025, 10:24 a.m. UTC | #2
On 2025/4/11 18:20, Juhyung Park wrote:
> On Fri, Apr 11, 2025 at 3:05 AM Chao Yu <chao@kernel.org> wrote:
>>
>> resize.f2fs doesn't guarantee atomically resizing f2fs image,
>> so that potential suddent power cut, IO error, out of memory,
>> SIGKILL received or program exits due to other reasons may cause
>> data corruption.
>>
>> This patch adds caution message for resize users to notice
>> potential risk of using resizing tool, and remind them to backup
>> data before resize.
>>
>> resize.f2fs <partition>
>>
>> "Please notice there is high risk of data lost during resize, please backup your data before resize.
>> Do you want to resize this partition? [y/n] y
>> process resize"
>>
>> If we want to force to use resize.f2fs, we can use -F option,
>> let's enable -F option in Android by default to not break any
>> usage.
>>
>> Cc: Ju Hyung Park <qkrwngud825@gmail.com>
> 
> Can you update this to "Juhyung Park"? That's the preferred name that
> I use now ;)

Ah, sure. ;)

Thanks,

> 
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fsck/main.c       | 27 +++++++++++++++++++++++++--
>>   fsck/resize.c     |  2 +-
>>   include/f2fs_fs.h |  1 +
>>   man/resize.f2fs.8 | 12 ++++++++++++
>>   4 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/main.c b/fsck/main.c
>> index af40613..ef6b17d 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -222,6 +222,8 @@ static void add_default_options(void)
>>                  if (c.func == FSCK) {
>>                          /* -a */
>>                          c.auto_fix = 1;
>> +               } else if (c.func == RESIZE) {
>> +                       c.force = 1;
>>                  }
>>
>>                  /*
>> @@ -601,7 +603,7 @@ void f2fs_parse_options(int argc, char *argv[])
>>   #endif
>>          } else if (!strcmp("resize.f2fs", prog)) {
>>   #ifdef WITH_RESIZE
>> -               const char *option_string = "d:fHst:o:V";
>> +               const char *option_string = "d:fFHst:o:V";
>>
>>                  c.func = RESIZE;
>>                  while ((option = getopt(argc, argv, option_string)) != EOF) {
>> @@ -618,9 +620,12 @@ void f2fs_parse_options(int argc, char *argv[])
>>                                                          c.dbg_lv);
>>                                  break;
>>                          case 'f':
>> +                               c.ignore_error = 1;
>> +                               MSG(0, "Info: Ingore error during resize\n");
> 
> s/Ingore/ignore
> s/error/errors
> 
>> +                               break;
>> +                       case 'F':
>>                                  c.force = 1;
>>                                  MSG(0, "Info: Force to resize\n");
>> -                               break;
> 
> Is this fallthrough intended?
> 
>>                          case 'H':
>>                                  c.need_whint = true;
>>                                  c.whint = WRITE_LIFE_NOT_SET;
>> @@ -1104,6 +1109,24 @@ out_range:
>>   #ifdef WITH_RESIZE
>>   static int do_resize(struct f2fs_sb_info *sbi)
>>   {
>> +       char answer[255] = {0};
>> +       int ret;
>> +
>> +       if (!c.force) {
>> +retry:
>> +               printf("\nPlease notice there is high risk of data lost during resize, "
>> +                       "please backup your data before resize.\n"
>> +                       "Do you want to resize this partition? [y/n] ");
> 
> "high risk" is probably too aggressive. I had many successes in resize.
> 
> Maybe "Resize operation is currently experimental, please backup your
> data.\nDo you want to continue? [y/n] "?
> 
>> +               ret = scanf("%s", answer);
>> +               ASSERT(ret >= 0);
>> +               if (!strcasecmp(answer, "y"))
>> +                       printf("process resize\n");
> 
> Something like "Proceeding to resize" looks better here.
> 
>> +               else if (!strcasecmp(answer, "n"))
>> +                       return 0;
>> +               else
>> +                       goto retry;
>> +       }
>> +
>>          if (!c.target_sectors)
>>                  c.target_sectors = c.total_sectors;
>>
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index 2681b9f..1ab7d75 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -765,7 +765,7 @@ int f2fs_resize(struct f2fs_sb_info *sbi)
>>                  }
>>          else if (((c.target_sectors * c.sector_size >>
>>                          get_sb(log_blocksize)) > get_sb(block_count)) ||
>> -                       c.force)
>> +                       c.ignore_error)
>>                  return f2fs_resize_grow(sbi);
>>          else {
>>                  MSG(0, "Nothing to resize.\n");
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 7e3f25b..928f963 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -1528,6 +1528,7 @@ struct f2fs_configuration {
>>          int no_kernel_check;
>>          int fix_on;
>>          int force;
>> +       int ignore_error;
>>          int defset;
>>          int bug_on;
>>          unsigned int invalid_sb;
>> diff --git a/man/resize.f2fs.8 b/man/resize.f2fs.8
>> index 02ff245..bdda4fd 100644
>> --- a/man/resize.f2fs.8
>> +++ b/man/resize.f2fs.8
>> @@ -18,6 +18,12 @@ resize.f2fs \- resize filesystem size
>>   .I overprovision-ratio-percentage
>>   ]
>>   [
>> +.B \-f
>> +]
>> +[
>> +.B \-F
>> +]
>> +[
>>   .B \-H
>>   ]
>>   [
>> @@ -53,6 +59,12 @@ Specify the percentage of the volume that will be used as overprovision area.
>>   This area is hidden to users, and utilized by F2FS cleaner. If not specified, the
>>   best number will be assigned automatically according to the partition size.
>>   .TP
>> +.BI \-f
>> +Force to fix any inconsistent data during resize.
>> +.TP
>> +.BI \-F
>> +Skip caution dialogue and resize partition directly.
>> +.TP
>>   .BI \-H
>>   Specify support write hint.
>>   .TP
>> --
>> 2.40.1
>>
Chao Yu April 11, 2025, 10:51 a.m. UTC | #3
On 2025/4/11 18:20, Juhyung Park wrote:
> On Fri, Apr 11, 2025 at 3:05 AM Chao Yu <chao@kernel.org> wrote:
>>
>> resize.f2fs doesn't guarantee atomically resizing f2fs image,
>> so that potential suddent power cut, IO error, out of memory,
>> SIGKILL received or program exits due to other reasons may cause
>> data corruption.
>>
>> This patch adds caution message for resize users to notice
>> potential risk of using resizing tool, and remind them to backup
>> data before resize.
>>
>> resize.f2fs <partition>
>>
>> "Please notice there is high risk of data lost during resize, please backup your data before resize.
>> Do you want to resize this partition? [y/n] y
>> process resize"
>>
>> If we want to force to use resize.f2fs, we can use -F option,
>> let's enable -F option in Android by default to not break any
>> usage.
>>
>> Cc: Ju Hyung Park <qkrwngud825@gmail.com>
> 
> Can you update this to "Juhyung Park"? That's the preferred name that
> I use now ;)
> 
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fsck/main.c       | 27 +++++++++++++++++++++++++--
>>   fsck/resize.c     |  2 +-
>>   include/f2fs_fs.h |  1 +
>>   man/resize.f2fs.8 | 12 ++++++++++++
>>   4 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/main.c b/fsck/main.c
>> index af40613..ef6b17d 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -222,6 +222,8 @@ static void add_default_options(void)
>>                  if (c.func == FSCK) {
>>                          /* -a */
>>                          c.auto_fix = 1;
>> +               } else if (c.func == RESIZE) {
>> +                       c.force = 1;
>>                  }
>>
>>                  /*
>> @@ -601,7 +603,7 @@ void f2fs_parse_options(int argc, char *argv[])
>>   #endif
>>          } else if (!strcmp("resize.f2fs", prog)) {
>>   #ifdef WITH_RESIZE
>> -               const char *option_string = "d:fHst:o:V";
>> +               const char *option_string = "d:fFHst:o:V";
>>
>>                  c.func = RESIZE;
>>                  while ((option = getopt(argc, argv, option_string)) != EOF) {
>> @@ -618,9 +620,12 @@ void f2fs_parse_options(int argc, char *argv[])
>>                                                          c.dbg_lv);
>>                                  break;
>>                          case 'f':
>> +                               c.ignore_error = 1;
>> +                               MSG(0, "Info: Ingore error during resize\n");
> 
> s/Ingore/ignore
> s/error/errors
> 
>> +                               break;
>> +                       case 'F':
>>                                  c.force = 1;
>>                                  MSG(0, "Info: Force to resize\n");
>> -                               break;
> 
> Is this fallthrough intended?
> 
>>                          case 'H':
>>                                  c.need_whint = true;
>>                                  c.whint = WRITE_LIFE_NOT_SET;
>> @@ -1104,6 +1109,24 @@ out_range:
>>   #ifdef WITH_RESIZE
>>   static int do_resize(struct f2fs_sb_info *sbi)
>>   {
>> +       char answer[255] = {0};
>> +       int ret;
>> +
>> +       if (!c.force) {
>> +retry:
>> +               printf("\nPlease notice there is high risk of data lost during resize, "
>> +                       "please backup your data before resize.\n"
>> +                       "Do you want to resize this partition? [y/n] ");
> 
> "high risk" is probably too aggressive. I had many successes in resize.
> 
> Maybe "Resize operation is currently experimental, please backup your
> data.\nDo you want to continue? [y/n] "?
> 
>> +               ret = scanf("%s", answer);
>> +               ASSERT(ret >= 0);
>> +               if (!strcasecmp(answer, "y"))
>> +                       printf("process resize\n");
> 
> Something like "Proceeding to resize" looks better here.

They all make sense to me, let me update in v4.

Thanks,

> 
>> +               else if (!strcasecmp(answer, "n"))
>> +                       return 0;
>> +               else
>> +                       goto retry;
>> +       }
>> +
>>          if (!c.target_sectors)
>>                  c.target_sectors = c.total_sectors;
>>
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index 2681b9f..1ab7d75 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -765,7 +765,7 @@ int f2fs_resize(struct f2fs_sb_info *sbi)
>>                  }
>>          else if (((c.target_sectors * c.sector_size >>
>>                          get_sb(log_blocksize)) > get_sb(block_count)) ||
>> -                       c.force)
>> +                       c.ignore_error)
>>                  return f2fs_resize_grow(sbi);
>>          else {
>>                  MSG(0, "Nothing to resize.\n");
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 7e3f25b..928f963 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -1528,6 +1528,7 @@ struct f2fs_configuration {
>>          int no_kernel_check;
>>          int fix_on;
>>          int force;
>> +       int ignore_error;
>>          int defset;
>>          int bug_on;
>>          unsigned int invalid_sb;
>> diff --git a/man/resize.f2fs.8 b/man/resize.f2fs.8
>> index 02ff245..bdda4fd 100644
>> --- a/man/resize.f2fs.8
>> +++ b/man/resize.f2fs.8
>> @@ -18,6 +18,12 @@ resize.f2fs \- resize filesystem size
>>   .I overprovision-ratio-percentage
>>   ]
>>   [
>> +.B \-f
>> +]
>> +[
>> +.B \-F
>> +]
>> +[
>>   .B \-H
>>   ]
>>   [
>> @@ -53,6 +59,12 @@ Specify the percentage of the volume that will be used as overprovision area.
>>   This area is hidden to users, and utilized by F2FS cleaner. If not specified, the
>>   best number will be assigned automatically according to the partition size.
>>   .TP
>> +.BI \-f
>> +Force to fix any inconsistent data during resize.
>> +.TP
>> +.BI \-F
>> +Skip caution dialogue and resize partition directly.
>> +.TP
>>   .BI \-H
>>   Specify support write hint.
>>   .TP
>> --
>> 2.40.1
>>
diff mbox series

Patch

diff --git a/fsck/main.c b/fsck/main.c
index af40613..ef6b17d 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -222,6 +222,8 @@  static void add_default_options(void)
 		if (c.func == FSCK) {
 			/* -a */
 			c.auto_fix = 1;
+		} else if (c.func == RESIZE) {
+			c.force = 1;
 		}
 
 		/*
@@ -601,7 +603,7 @@  void f2fs_parse_options(int argc, char *argv[])
 #endif
 	} else if (!strcmp("resize.f2fs", prog)) {
 #ifdef WITH_RESIZE
-		const char *option_string = "d:fHst:o:V";
+		const char *option_string = "d:fFHst:o:V";
 
 		c.func = RESIZE;
 		while ((option = getopt(argc, argv, option_string)) != EOF) {
@@ -618,9 +620,12 @@  void f2fs_parse_options(int argc, char *argv[])
 							c.dbg_lv);
 				break;
 			case 'f':
+				c.ignore_error = 1;
+				MSG(0, "Info: Ingore error during resize\n");
+				break;
+			case 'F':
 				c.force = 1;
 				MSG(0, "Info: Force to resize\n");
-				break;
 			case 'H':
 				c.need_whint = true;
 				c.whint = WRITE_LIFE_NOT_SET;
@@ -1104,6 +1109,24 @@  out_range:
 #ifdef WITH_RESIZE
 static int do_resize(struct f2fs_sb_info *sbi)
 {
+	char answer[255] = {0};
+	int ret;
+
+	if (!c.force) {
+retry:
+		printf("\nPlease notice there is high risk of data lost during resize, "
+			"please backup your data before resize.\n"
+			"Do you want to resize this partition? [y/n] ");
+		ret = scanf("%s", answer);
+		ASSERT(ret >= 0);
+		if (!strcasecmp(answer, "y"))
+			printf("process resize\n");
+		else if (!strcasecmp(answer, "n"))
+			return 0;
+		else
+			goto retry;
+	}
+
 	if (!c.target_sectors)
 		c.target_sectors = c.total_sectors;
 
diff --git a/fsck/resize.c b/fsck/resize.c
index 2681b9f..1ab7d75 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -765,7 +765,7 @@  int f2fs_resize(struct f2fs_sb_info *sbi)
 		}
 	else if (((c.target_sectors * c.sector_size >>
 			get_sb(log_blocksize)) > get_sb(block_count)) ||
-			c.force)
+			c.ignore_error)
 		return f2fs_resize_grow(sbi);
 	else {
 		MSG(0, "Nothing to resize.\n");
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 7e3f25b..928f963 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1528,6 +1528,7 @@  struct f2fs_configuration {
 	int no_kernel_check;
 	int fix_on;
 	int force;
+	int ignore_error;
 	int defset;
 	int bug_on;
 	unsigned int invalid_sb;
diff --git a/man/resize.f2fs.8 b/man/resize.f2fs.8
index 02ff245..bdda4fd 100644
--- a/man/resize.f2fs.8
+++ b/man/resize.f2fs.8
@@ -18,6 +18,12 @@  resize.f2fs \- resize filesystem size
 .I overprovision-ratio-percentage
 ]
 [
+.B \-f
+]
+[
+.B \-F
+]
+[
 .B \-H
 ]
 [
@@ -53,6 +59,12 @@  Specify the percentage of the volume that will be used as overprovision area.
 This area is hidden to users, and utilized by F2FS cleaner. If not specified, the
 best number will be assigned automatically according to the partition size.
 .TP
+.BI \-f
+Force to fix any inconsistent data during resize.
+.TP
+.BI \-F
+Skip caution dialogue and resize partition directly.
+.TP
 .BI \-H
 Specify support write hint.
 .TP