diff mbox

[LEDE-DEV] fstools: added xfs and f2fs to block-mount

Message ID AM3PR08MB06426A668C5969EF6B960F0792DE0@AM3PR08MB0642.eurprd08.prod.outlook.com
State Changes Requested
Headers show

Commit Message

Alberto Bursi Oct. 15, 2016, 5:25 p.m. UTC 
added the code to recognize the filesystem checkers for xfs and f2fs
added xfs and f2fs to the filesystem whitelist of block.

Signed-off-by: Alberto Bursi <alberto.bursi@outlook.it>
-----

  		}

Comments

Alberto Bursi Oct. 16, 2016, 2:23 p.m. UTC | #1 
Damn, while this patch adds only partial xfs support to fstools (it 
allows to mount xfs by block mount).

Most of fstools have code to use f2fs too already so the fact that block 
mount couldn't mount f2fs is 100% a bug and this patch fixes that.

I'm going to add xfs support to other fstools components in a following 
patch.

-Alberto

On 10/15/2016 07:25 PM, Alberto Bursi wrote:
> added the code to recognize the filesystem checkers for xfs and f2fs
> added xfs and f2fs to the filesystem whitelist of block.
>
> Signed-off-by: Alberto Bursi <alberto.bursi@outlook.it>
> -----
>
> --- fstools/block.c	2016-10-15 19:15:31.916714011 +0200
> +++ fstools/block.c	2016-10-15 19:22:44.164702448 +0200
> @@ -702,6 +702,8 @@ static void check_filesystem(struct blki
>   {
>   	pid_t pid;
>   	struct stat statbuf;
> +    	const char *xfsck = "/sbin/xfs_repair";
> +    	const char *f2fsck = "/usr/sbin/fsck.f2fs";
>   	const char *e2fsck = "/usr/sbin/e2fsck";
>   	const char *dosfsck = "/usr/sbin/dosfsck";
>   	const char *ckfs;
> @@ -712,6 +714,10 @@ static void check_filesystem(struct blki
>
>   	if (!strncmp(pr->id->name, "vfat", 4)) {
>   		ckfs = dosfsck;
> +    	} else if (!strncmp(pr->id->name, "f2fs", 4)) {
> +        	ckfs = f2fsck;
> +    	} else if (!strncmp(pr->id->name, "xfs", 3)) {
> +        	ckfs = xfsck;
>   	} else if (!strncmp(pr->id->name, "ext", 3)) {
>   		ckfs = e2fsck;
>   	} else {
> @@ -1170,8 +1176,10 @@ static int mount_extroot(char *cfg)
>   	}
>   	if (pr) {
>   		if (strncmp(pr->id->name, "ext", 3) &&
> -		    strncmp(pr->id->name, "ubifs", 5)) {
> -			ULOG_ERR("extroot: unsupported filesystem %s, try ext4\n",
> pr->id->name);
> +            strncmp(pr->id->name, "xfs", 3) &&
> +            strncmp(pr->id->name, "f2fs", 4) &&
> +            strncmp(pr->id->name, "ubifs", 5)) {
> +            ULOG_ERR("extroot: unsupported filesystem %s, try ext4,
> xfs, f2fs, ubifs\n", pr->id->name);
>   			return -1;
>   		}
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
>
John Crispin Oct. 17, 2016, 2:31 p.m. UTC | #2 
On 16/10/2016 16:23, Alberto Bursi wrote:
> Damn, while this patch adds only partial xfs support to fstools (it 
> allows to mount xfs by block mount).
> 

why only partial ?

> Most of fstools have code to use f2fs too already so the fact that block 
> mount couldn't mount f2fs is 100% a bug and this patch fixes that.

it is not a bug but a missing feature. a bug is if a piece of code is
incorrect. if a piece of code was never implemented then we call it
missing feature.

> I'm going to add xfs support to other fstools components in a following 
> patch.

please explain what this would consist of. i was about to merge your
patch but your comments make me think it is incomplete.

	John




> -Alberto
> 
> On 10/15/2016 07:25 PM, Alberto Bursi wrote:
>> added the code to recognize the filesystem checkers for xfs and f2fs
>> added xfs and f2fs to the filesystem whitelist of block.
>>
>> Signed-off-by: Alberto Bursi <alberto.bursi@outlook.it>
>> -----
>>
>> --- fstools/block.c	2016-10-15 19:15:31.916714011 +0200
>> +++ fstools/block.c	2016-10-15 19:22:44.164702448 +0200
>> @@ -702,6 +702,8 @@ static void check_filesystem(struct blki
>>   {
>>   	pid_t pid;
>>   	struct stat statbuf;
>> +    	const char *xfsck = "/sbin/xfs_repair";
>> +    	const char *f2fsck = "/usr/sbin/fsck.f2fs";
>>   	const char *e2fsck = "/usr/sbin/e2fsck";
>>   	const char *dosfsck = "/usr/sbin/dosfsck";
>>   	const char *ckfs;
>> @@ -712,6 +714,10 @@ static void check_filesystem(struct blki
>>
>>   	if (!strncmp(pr->id->name, "vfat", 4)) {
>>   		ckfs = dosfsck;
>> +    	} else if (!strncmp(pr->id->name, "f2fs", 4)) {
>> +        	ckfs = f2fsck;
>> +    	} else if (!strncmp(pr->id->name, "xfs", 3)) {
>> +        	ckfs = xfsck;
>>   	} else if (!strncmp(pr->id->name, "ext", 3)) {
>>   		ckfs = e2fsck;
>>   	} else {
>> @@ -1170,8 +1176,10 @@ static int mount_extroot(char *cfg)
>>   	}
>>   	if (pr) {
>>   		if (strncmp(pr->id->name, "ext", 3) &&
>> -		    strncmp(pr->id->name, "ubifs", 5)) {
>> -			ULOG_ERR("extroot: unsupported filesystem %s, try ext4\n",
>> pr->id->name);
>> +            strncmp(pr->id->name, "xfs", 3) &&
>> +            strncmp(pr->id->name, "f2fs", 4) &&
>> +            strncmp(pr->id->name, "ubifs", 5)) {
>> +            ULOG_ERR("extroot: unsupported filesystem %s, try ext4,
>> xfs, f2fs, ubifs\n", pr->id->name);
>>   			return -1;
>>   		}
>>
>> _______________________________________________
>> Lede-dev mailing list
>> Lede-dev@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
>>
> 
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
>
Alberto Bursi Oct. 17, 2016, 3:37 p.m. UTC | #3 
On 10/17/2016 04:31 PM, John Crispin wrote:
>> I'm going to add xfs support to other fstools components in a following
>> patch.
>
> please explain what this would consist of. i was about to merge your
> patch but your comments make me think it is incomplete.
>

Sorry for the confusion.

This patch allows block-mount to mount xfs and f2fs on boot, I tested 
the patch and it works on my tp-link test router.

But afterwards I searched for similar checks for code in the same 
package/source folder, and yes, in fstools there are more 
filesystem-specific checks and operations.
Code for f2fs is already there, but code for xfs is not.

I didn't yet have the time to look more closely what that does exactly.

A list of files that must be changed to add xfs:

/mount_root.c there are filesystem type checks

/libfstools/find.c line 88 there is a filesystem type check.

/libfstools/libfstools.h line 25 there is a list of filesystem-specific 
variables

/libfstools/overlay.c line 198, another filesystem type check

/libfstools/rootdisk.c line 264, there is code to automatically format 
the overlay as either f2fs or ext4 if it isn't formatted correctly.

So while with my patch block mount will work with both f2fs and xfs, 
some other operations I didn't know about are not going to work on xfs.

-Alberto
John Crispin Oct. 17, 2016, 3:43 p.m. UTC | #4 
On 17/10/2016 17:37, Alberto Bursi wrote:
> 
> 
> On 10/17/2016 04:31 PM, John Crispin wrote:
>>> I'm going to add xfs support to other fstools components in a following
>>> patch.
>>
>> please explain what this would consist of. i was about to merge your
>> patch but your comments make me think it is incomplete.
>>
> 
> Sorry for the confusion.
> 
> This patch allows block-mount to mount xfs and f2fs on boot, I tested 
> the patch and it works on my tp-link test router.
> 
> But afterwards I searched for similar checks for code in the same 
> package/source folder, and yes, in fstools there are more 
> filesystem-specific checks and operations.
> Code for f2fs is already there, but code for xfs is not.
> 
> I didn't yet have the time to look more closely what that does exactly.
> 
> A list of files that must be changed to add xfs:
> 
> /mount_root.c there are filesystem type checks
> 
> /libfstools/find.c line 88 there is a filesystem type check.
> 
> /libfstools/libfstools.h line 25 there is a list of filesystem-specific 
> variables
> 
> /libfstools/overlay.c line 198, another filesystem type check
> 
> /libfstools/rootdisk.c line 264, there is code to automatically format 
> the overlay as either f2fs or ext4 if it isn't formatted correctly.
> 
> So while with my patch block mount will work with both f2fs and xfs, 
> some other operations I didn't know about are not going to work on xfs.

ok, please resend the whole patch once xfs support has been properly
added to fstools. this current "some works some may not" sounds quirky.
lets just drop the patch until; you had time to fully verify and add the
missing pieces.

thanks for the explaination.

	John
diff mbox

Patch

--- fstools/block.c	2016-10-15 19:15:31.916714011 +0200
+++ fstools/block.c	2016-10-15 19:22:44.164702448 +0200
@@ -702,6 +702,8 @@  static void check_filesystem(struct blki
  {
  	pid_t pid;
  	struct stat statbuf;
+    	const char *xfsck = "/sbin/xfs_repair";
+    	const char *f2fsck = "/usr/sbin/fsck.f2fs";
  	const char *e2fsck = "/usr/sbin/e2fsck";
  	const char *dosfsck = "/usr/sbin/dosfsck";
  	const char *ckfs;
@@ -712,6 +714,10 @@  static void check_filesystem(struct blki

  	if (!strncmp(pr->id->name, "vfat", 4)) {
  		ckfs = dosfsck;
+    	} else if (!strncmp(pr->id->name, "f2fs", 4)) {
+        	ckfs = f2fsck;
+    	} else if (!strncmp(pr->id->name, "xfs", 3)) {
+        	ckfs = xfsck;
  	} else if (!strncmp(pr->id->name, "ext", 3)) {
  		ckfs = e2fsck;
  	} else {
@@ -1170,8 +1176,10 @@  static int mount_extroot(char *cfg)
  	}
  	if (pr) {
  		if (strncmp(pr->id->name, "ext", 3) &&
-		    strncmp(pr->id->name, "ubifs", 5)) {
-			ULOG_ERR("extroot: unsupported filesystem %s, try ext4\n", 
pr->id->name);
+            strncmp(pr->id->name, "xfs", 3) &&
+            strncmp(pr->id->name, "f2fs", 4) &&
+            strncmp(pr->id->name, "ubifs", 5)) {
+            ULOG_ERR("extroot: unsupported filesystem %s, try ext4, 
xfs, f2fs, ubifs\n", pr->id->name);
  			return -1;