[PATCH v3 0/4] staging: erofs: option validation for remount and some code cleanups

[PATCH v3 0/4] staging: erofs: option validation for remount and some code cleanups

[Chengguang Xu]

This patch set mainly does option validation for remount and at
the same time does related code cleanups. Currently when we call
fault injection related code we have to surround it with macro
CONFIG_EROFS_FAULT_INJECTION in every calling place, after this
patch set we don't have to do that, so the code looks clean and
more understandable.

v2->v3:
- Fold related patches to one patch.
- Fix building issue.

v1->v2:
Address Chao's comments:
- Allow to set fault_injection=0.
- Keep flag bit when setting fault_injection=0.
- Show injection info in original place.
- Rebase code on latest erofs branch in Chao's linux tree.
- Fix building issue.

Hi Greg,

You may pick up rest 2-4 patches in this patch set if there
are no more comments from Chao and Xiang.

Thanks,

Chengguang Xu (4):
  staging: erofs: code cleanup for erofs_kmalloc()
  staging: erofs: code cleanup for option parsing of fault_injection
  staging: erofs: code cleanup for erofs_show_options()
  staging: erofs: option validation in remount

 drivers/staging/erofs/internal.h | 13 ++++--
 drivers/staging/erofs/super.c    | 73 +++++++++++++++++++++++++-------
 2 files changed, 67 insertions(+), 19 deletions(-)

-- 
2.17.1

[PATCH v3 1/4] staging: erofs: code cleanup for erofs_kmalloc()

[Chengguang Xu]

Define a dummy function of time_to_inject()/erofs_show_injection_info(),
so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in
calling place.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
Reviewed-by: Chao Yu <yuchao0 at huawei.com>
Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
---
 drivers/staging/erofs/internal.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index f20c6e9b7471..0011b9d505fd 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -42,12 +42,12 @@
 #define DBG_BUGON(...)          ((void)0)
 #endif
 
-#ifdef CONFIG_EROFS_FAULT_INJECTION
 enum {
 	FAULT_KMALLOC,
 	FAULT_MAX,
 };
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
 extern char *erofs_fault_name[FAULT_MAX];
 #define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
 
@@ -143,17 +143,24 @@ static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
 	}
 	return false;
 }
+#else
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+	return false;
+}
+
+static inline void erofs_show_injection_info(int type)
+{
+}
 #endif
 
 static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
 					size_t size, gfp_t flags)
 {
-#ifdef CONFIG_EROFS_FAULT_INJECTION
 	if (time_to_inject(sbi, FAULT_KMALLOC)) {
 		erofs_show_injection_info(FAULT_KMALLOC);
 		return NULL;
 	}
-#endif
 	return kmalloc(size, flags);
 }
 
-- 
2.17.1

[PATCH v3 2/4] staging: erofs: code cleanup for option parsing of fault_injection

[Chengguang Xu]

Define a dummpy function of erofs_build_fault_attr() when macro
CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
check the macro in calling place. Based on above adjustment,
do proper code cleanup for option parsing of fault_injection.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
Reviewed-by: Chao Yu <yuchao0 at huawei.com>
---
 drivers/staging/erofs/super.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 9e421536cbdf..f496e0c1d04d 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
 };
 
-static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
-						unsigned int rate)
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					substring_t *args)
 {
 	struct erofs_fault_info *ffi = &sbi->fault_info;
+	int rate = 0;
+
+	if (args->from && match_int(args, &rate))
+		return -EINVAL;
 
 	if (rate) {
 		atomic_set(&ffi->inject_ops, 0);
@@ -157,6 +161,16 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	} else {
 		memset(ffi, 0, sizeof(struct erofs_fault_info));
 	}
+
+	set_opt(sbi, FAULT_INJECTION);
+	return 0;
+}
+#else
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					substring_t *args)
+{
+	infoln("fault_injection options not supported");
+	return 0;
 }
 #endif
 
@@ -193,7 +207,7 @@ static int parse_options(struct super_block *sb, char *options)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *p;
-	int arg = 0;
+	int err;
 
 	if (!options)
 		return 0;
@@ -238,18 +252,12 @@ static int parse_options(struct super_block *sb, char *options)
 			infoln("noacl options not supported");
 			break;
 #endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
 		case Opt_fault_injection:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
-			erofs_build_fault_attr(EROFS_SB(sb), arg);
-			set_opt(EROFS_SB(sb), FAULT_INJECTION);
+			err = erofs_build_fault_attr(EROFS_SB(sb), args);
+			if (err)
+				return err;
 			break;
-#else
-		case Opt_fault_injection:
-			infoln("fault_injection options not supported");
-			break;
-#endif
+
 		default:
 			errln("Unrecognized mount option \"%s\" "
 					"or missing value", p);
-- 
2.17.1

[PATCH v3 2/4] staging: erofs: code cleanup for option parsing of fault_injection

[Gao Xiang]

Hi Chengguang,

On 2018/9/18 23:10, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> ---
>  drivers/staging/erofs/super.c | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..f496e0c1d04d 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>  	[FAULT_KMALLOC]		= "kmalloc",
>  };
>  
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> -						unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)

minor... two checkpatch suggestions out there, it is better to fix
them before merging...

CHECK: Alignment should match open parenthesis
#143: FILE: drivers/staging/erofs/super.c:149:
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+                                       substring_t *args)

CHECK: Alignment should match open parenthesis
#163: FILE: drivers/staging/erofs/super.c:170:
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+                                       substring_t *args)


and you could add
Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks,
Gao Xiang

>  {
>  	struct erofs_fault_info *ffi = &sbi->fault_info;
> +	int rate = 0;
> +
> +	if (args->from && match_int(args, &rate))
> +		return -EINVAL;
>  
>  	if (rate) {
>  		atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,16 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>  	} else {
>  		memset(ffi, 0, sizeof(struct erofs_fault_info));
>  	}
> +
> +	set_opt(sbi, FAULT_INJECTION);
> +	return 0;
> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)
> +{
> +	infoln("fault_injection options not supported");
> +	return 0;
>  }
>  #endif
>  
> @@ -193,7 +207,7 @@ static int parse_options(struct super_block *sb, char *options)
>  {
>  	substring_t args[MAX_OPT_ARGS];
>  	char *p;
> -	int arg = 0;
> +	int err;
>  
>  	if (!options)
>  		return 0;
> @@ -238,18 +252,12 @@ static int parse_options(struct super_block *sb, char *options)
>  			infoln("noacl options not supported");
>  			break;
>  #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
>  		case Opt_fault_injection:
> -			if (args->from && match_int(args, &arg))
> -				return -EINVAL;
> -			erofs_build_fault_attr(EROFS_SB(sb), arg);
> -			set_opt(EROFS_SB(sb), FAULT_INJECTION);
> +			err = erofs_build_fault_attr(EROFS_SB(sb), args);
> +			if (err)
> +				return err;
>  			break;
> -#else
> -		case Opt_fault_injection:
> -			infoln("fault_injection options not supported");
> -			break;
> -#endif
> +
>  		default:
>  			errln("Unrecognized mount option \"%s\" "
>  					"or missing value", p);
> 

[PATCH v3 3/4] staging: erofs: code cleanup for erofs_show_options()

[Chengguang Xu]

Add new helper erofs_get_fault_rate() to get fault rate instead of
directly getting it from sbi, so we can remove the macro check
surrounding it.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
Reviewed-by: Chao Yu <yuchao0 at huawei.com>
---
 drivers/staging/erofs/super.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index f496e0c1d04d..a091b93190e1 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -165,6 +165,11 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	set_opt(sbi, FAULT_INJECTION);
 	return 0;
 }
+
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+	return sbi->fault_info.inject_rate;
+}
 #else
 static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 					substring_t *args)
@@ -172,6 +177,11 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	infoln("fault_injection options not supported");
 	return 0;
 }
+
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+	return 0;
+}
 #endif
 
 static void default_options(struct erofs_sb_info *sbi)
@@ -626,11 +636,9 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 	else
 		seq_puts(seq, ",noacl");
 #endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
 	if (test_opt(sbi, FAULT_INJECTION))
 		seq_printf(seq, ",fault_injection=%u",
-				sbi->fault_info.inject_rate);
-#endif
+			erofs_get_fault_rate(sbi));
 	return 0;
 }
 
-- 
2.17.1

[PATCH v3 3/4] staging: erofs: code cleanup for erofs_show_options()

[Gao Xiang]

On 2018/9/18 23:10, Chengguang Xu wrote:
> Add new helper erofs_get_fault_rate() to get fault rate instead of
> directly getting it from sbi, so we can remove the macro check
> surrounding it.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks,
Gao Xiang

[PATCH v3 4/4] staging: erofs: option validation in remount

[Chengguang Xu]

Add option validation in remount. After this patch, remount
can change recognized options, and for unknown options remount
will fail and report error.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
Reviewed-by: Chao Yu <yuchao0 at huawei.com>
---
 drivers/staging/erofs/super.c | 38 ++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index a091b93190e1..30b6b44dc6c4 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
 };
 
-static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
-					substring_t *args)
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+						unsigned int rate)
 {
 	struct erofs_fault_info *ffi = &sbi->fault_info;
-	int rate = 0;
-
-	if (args->from && match_int(args, &rate))
-		return -EINVAL;
 
 	if (rate) {
 		atomic_set(&ffi->inject_ops, 0);
@@ -163,14 +159,29 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	}
 
 	set_opt(sbi, FAULT_INJECTION);
-	return 0;
 }
 
 static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
 {
 	return sbi->fault_info.inject_rate;
 }
+
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi, substring_t *args)
+{
+	int rate = 0;
+
+	if (args->from && match_int(args, &rate))
+		return -EINVAL;
+
+	__erofs_build_fault_attr(sbi, rate);
+	return 0;
+}
 #else
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+						unsigned int rate)
+{
+}
+
 static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 					substring_t *args)
 {
@@ -644,10 +655,23 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 
 static int erofs_remount(struct super_block *sb, int *flags, char *data)
 {
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	unsigned int org_mnt_opt = sbi->mount_opt;
+	unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
+	int err;
+
 	BUG_ON(!sb_rdonly(sb));
+	err = parse_options(sb, data);
+	if (err)
+		goto out;
 
 	*flags |= MS_RDONLY;
 	return 0;
+out:
+	__erofs_build_fault_attr(sbi, org_inject_rate);
+	sbi->mount_opt = org_mnt_opt;
+
+	return err;
 }
 
 const struct super_operations erofs_sops = {
-- 
2.17.1

[PATCH v3 4/4] staging: erofs: option validation in remount

[Gao Xiang]

Hi Chengguang,

On 2018/9/18 23:10, Chengguang Xu wrote:
> Add option validation in remount. After this patch, remount
> can change recognized options, and for unknown options remount
> will fail and report error.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> ---
>  drivers/staging/erofs/super.c | 38 ++++++++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index a091b93190e1..30b6b44dc6c4 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
>  	[FAULT_KMALLOC]		= "kmalloc",
>  };
>  
> -static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> -					substring_t *args)
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +						unsigned int rate)
>  {
>  	struct erofs_fault_info *ffi = &sbi->fault_info;
> -	int rate = 0;
> -
> -	if (args->from && match_int(args, &rate))
> -		return -EINVAL;
>  
>  	if (rate) {
>  		atomic_set(&ffi->inject_ops, 0);
> @@ -163,14 +159,29 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>  	}
>  
>  	set_opt(sbi, FAULT_INJECTION);
> -	return 0;
>  }
>  
>  static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
>  {
>  	return sbi->fault_info.inject_rate;
>  }
> +
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi, substring_t *args)
> +{
> +	int rate = 0;
> +
> +	if (args->from && match_int(args, &rate))
> +		return -EINVAL;
> +
> +	__erofs_build_fault_attr(sbi, rate);
> +	return 0;
> +}
>  #else
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +						unsigned int rate)
> +{
> +}
> +
>  static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>  					substring_t *args)
>  {
> @@ -644,10 +655,23 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>  
>  static int erofs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	unsigned int org_mnt_opt = sbi->mount_opt;
> +	unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
> +	int err;
> +
>  	BUG_ON(!sb_rdonly(sb));
> +	err = parse_options(sb, data);
> +	if (err)
> +		goto out;
>  
>  	*flags |= MS_RDONLY;

I cannot apply this patch since the above line has been changed in
5f0abea6ab6d("staging: erofs: rename superblock flags (MS_xyz -> SB_xyz)").


And two more checkpatch CHECKs as [PATCH v3 2/4]

CHECK: Alignment should match open parenthesis
#141: FILE: drivers/staging/erofs/super.c:149:
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+                                               unsigned int rate)

CHECK: Alignment should match open parenthesis
#175: FILE: drivers/staging/erofs/super.c:181:
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+                                               unsigned int rate)

And I think I need to test the basic erofs remount function tomorrow
before merging... it is too late and I have to go to sleep now...

Thanks,
Gao Xiang

>  	return 0;
> +out:
> +	__erofs_build_fault_attr(sbi, org_inject_rate);
> +	sbi->mount_opt = org_mnt_opt;
> +
> +	return err;
>  }
>  
>  const struct super_operations erofs_sops = {
> 

[PATCH v3 4/4] staging: erofs: option validation in remount

[Gao Xiang]

Hi Chengguang,

On 2018/9/19 0:58, Gao Xiang wrote:
> Hi Chengguang,
> 
> On 2018/9/18 23:10, Chengguang Xu wrote:
>> Add option validation in remount. After this patch, remount
>> can change recognized options, and for unknown options remount
>> will fail and report error.
>>
>> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
>> Reviewed-by: Chao Yu <yuchao0 at huawei.com>
>> ---
>>  drivers/staging/erofs/super.c | 38 ++++++++++++++++++++++++++++-------
>>  1 file changed, 31 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>> index a091b93190e1..30b6b44dc6c4 100644
>> --- a/drivers/staging/erofs/super.c
>> +++ b/drivers/staging/erofs/super.c
>> @@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
>>  	[FAULT_KMALLOC]		= "kmalloc",
>>  };
>>  
>> -static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>> -					substring_t *args)
>> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
>> +						unsigned int rate)
>>  {
>>  	struct erofs_fault_info *ffi = &sbi->fault_info;
>> -	int rate = 0;
>> -
>> -	if (args->from && match_int(args, &rate))
>> -		return -EINVAL;
>>  
>>  	if (rate) {
>>  		atomic_set(&ffi->inject_ops, 0);
>> @@ -163,14 +159,29 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>  	}
>>  
>>  	set_opt(sbi, FAULT_INJECTION);
>> -	return 0;
>>  }
>>  
>>  static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
>>  {
>>  	return sbi->fault_info.inject_rate;
>>  }
>> +
>> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi, substring_t *args)
>> +{
>> +	int rate = 0;
>> +
>> +	if (args->from && match_int(args, &rate))
>> +		return -EINVAL;
>> +
>> +	__erofs_build_fault_attr(sbi, rate);
>> +	return 0;
>> +}
>>  #else
>> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
>> +						unsigned int rate)
>> +{
>> +}
>> +
>>  static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>  					substring_t *args)
>>  {
>> @@ -644,10 +655,23 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>>  
>>  static int erofs_remount(struct super_block *sb, int *flags, char *data)
>>  {
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	unsigned int org_mnt_opt = sbi->mount_opt;
>> +	unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
>> +	int err;
>> +
>>  	BUG_ON(!sb_rdonly(sb));
>> +	err = parse_options(sb, data);
>> +	if (err)
>> +		goto out;
>>  
>>  	*flags |= MS_RDONLY;
> I cannot apply this patch since the above line has been changed in
> 5f0abea6ab6d("staging: erofs: rename superblock flags (MS_xyz -> SB_xyz)").
> 
> 
> And two more checkpatch CHECKs as [PATCH v3 2/4]
> 
> CHECK: Alignment should match open parenthesis
> #141: FILE: drivers/staging/erofs/super.c:149:
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +                                               unsigned int rate)
> 
> CHECK: Alignment should match open parenthesis
> #175: FILE: drivers/staging/erofs/super.c:181:
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +                                               unsigned int rate)
> 
> And I think I need to test the basic erofs remount function tomorrow
> before merging... it is too late and I have to go to sleep now.
I tested just now, and seems good. :)

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

and look forward to your final patchset.

Thanks,
Gao Xiang