Re: [PATCH] minix: v3 super-block does not have s_state field

[Sami Kerola <kerolasa@iki.fi>]

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]

On Wed, Jul 13, 2011 at 06:05, Davidlohr Bueso <dave@gnu.org> wrote:
> I don't think we should always rely on having the kernel headers, that's
> why the fallback is provided.
[snip]
> I think with this patch we address the issue without removing our
> fallback.

Hi Dave et.al.

The preprocessor directive bellow is problematic. I don't see where,
or how, it might get satisfied so I am afraid the 'fall back' is
always in use regardless whether an user has kernel headers or not.

#ifdef KERNEL_INCLUDES_ARE_CLEAN

To fix that I modified the patch to use autoconf to check whether each
necessary header is present, and use them if possible. Notice that
Dave that I wrote your name to Reviewed-by patch line so it would be
nice to hear that you're OK with the change. See the attachment, or
pull from my repository.

The following changes since commit bfa8d39b5826b928deb6d84aee3a4a1d6557364c:

  build-sys: fix spaces versus tabs conflict (2011-07-11 15:12:06 +0200)

are available in the git repository at:
  https://github.com/kerolasa/lelux-utiliteetit minix

Sami Kerola (1):
      minix: v3 super-block does not have s_state field

 configure.ac            |    5 ++
 disk-utils/minix.h      |  129 ++++++++++++++++++++++++++++------------------
 disk-utils/mkfs.minix.c |    2 -
 3 files changed, 83 insertions(+), 53 deletions(-)

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/

[-- Attachment #2: 0001-minix-v3-super-block-does-not-have-s_state-field.txt --]
[-- Type: text/plain, Size: 6807 bytes --]

From 3e60f8617e349aea3a1d3e70e89695dc7c8afda9 Mon Sep 17 00:00:00 2001
From: Sami Kerola <kerolasa@iki.fi>
Date: Tue, 12 Jul 2011 16:45:10 +0200
Subject: [PATCH] minix: v3 super-block does not have s_state field

Originally attempt was to use minix definitions and file system
structures from linux/minix_fs.h, but that failed at first try.

mkfs.minix.c:164:10: error: no member named 's_state' in 'struct
      minix3_super_block'
                Super3.s_state |= MINIX_VALID_FS;
                ~~~~~~ ^
mkfs.minix.c:165:10: error: no member named 's_state' in 'struct
      minix3_super_block'
                Super3.s_state &= ~MINIX_ERROR_FS;

Primary reason seems to be that the minix3 super-block does not
have s_state field. And it looks to me that it has never had it.
Further details about s_state can be found from minix v3 file
system support kernel patch.

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=939b00df0306bc4b5cd25c3c3c78e89b91e72fc8

Former minix disk s_state is now in kernel memory super-block
info structure as a s_mount_state field, if someone wonders what
happen to it.

Issue appeared commit a2657ae3ffb56616ac9c921886bcca8ef242499f
(two weeks ago), and hopefully not too many users where affected.
The mismatch in this case is not completely fatal because it is
at the end of the structure.

The patch fixes also #ifdef KERNEL_INCLUDES_ARE_CLEAN
preprocessor directive, which never is set. Autoconf is set to
inspect that necessary kernel headers are present. For fallback
there are local definitions.

Reviewed-by: Davidlohr Bueso <dave@gnu.org>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 configure.ac            |    5 ++
 disk-utils/minix.h      |  129 ++++++++++++++++++++++++++++------------------
 disk-utils/mkfs.minix.c |    2 -
 3 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/configure.ac b/configure.ac
index a02b5e3..c2c74f5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,6 +123,11 @@ AC_CHECK_HEADERS(
 	linux/version.h \
 	linux/falloc.h \
 	linux/cdrom.h \
+	linux/fs.h \
+	linux/limits.h \
+	linux/magic.h \
+	linux/minix_fs.h \
+	linux/types.h \
 	fcntl.h \
 	locale.h \
 	stdint.h \
diff --git a/disk-utils/minix.h b/disk-utils/minix.h
index fc1d1c0..6b2ee10 100644
--- a/disk-utils/minix.h
+++ b/disk-utils/minix.h
@@ -1,53 +1,92 @@
 #ifndef __MINIX_H__
 #define __MINIX_H__
 
-#ifdef KERNEL_INCLUDES_ARE_CLEAN
-
-#include <linux/fs.h>
-#include <linux/minix_fs.h>
-
+#ifdef HAVE_LINUX_TYPES_H
+#include <linux/types.h>
 #else
-
 typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned int u32;
+#endif
+
+#ifdef HAVE_LINUX_LIMITS_H
+#include <linux/limits.h>
+#else
+#define NAME_MAX  255		/* # chars in a file name */
+#endif
+
+#ifdef HAVE_LINUX_FS_H
+#include <linux/fs.h>
+#else
+#define BLOCK_SIZE_BITS 10
+#define BLOCK_SIZE (1<<BLOCK_SIZE_BITS)
+#endif
+
+#ifdef HAVE_LINUX_MAGIC_H
+#include <linux/magic.h>
+#else
+#define MINIX_SUPER_MAGIC	0x137F	/* minix v1 fs, 14 char names */
+#define MINIX_SUPER_MAGIC2	0x138F	/* minix v1 fs, 30 char names */
+#define MINIX2_SUPER_MAGIC	0x2468	/* minix v2 fs, 14 char names */
+#define MINIX2_SUPER_MAGIC2	0x2478	/* minix v2 fs, 30 char names */
+#define MINIX3_SUPER_MAGIC	0x4d5a	/* minix v3 fs, 60 char names */
+#endif
+
+#ifdef HAVE_LINUX_MINIX_FS_H
+#include <linux/minix_fs.h>
+#else
 
+/*
+ * This is the original minix inode layout on disk.
+ * Note the 8-bit gid and atime and ctime.
+ */
 struct minix_inode {
-        u16 i_mode;
-        u16 i_uid;
-        u32 i_size;
-        u32 i_time;
-        u8  i_gid;
-        u8  i_nlinks;
-        u16 i_zone[9];
+	u16 i_mode;
+	u16 i_uid;
+	u32 i_size;
+	u32 i_time;
+	u8 i_gid;
+	u8 i_nlinks;
+	u16 i_zone[9];
 };
 
+/*
+ * The new minix inode has all the time entries, as well as
+ * long block numbers and a third indirect block (7+1+1+1
+ * instead of 7+1+1). Also, some previously 8-bit values are
+ * now 16-bit. The inode is now 64 bytes instead of 32.
+ */
 struct minix2_inode {
-        u16 i_mode;
-        u16 i_nlinks;
-        u16 i_uid;
-        u16 i_gid;
-        u32 i_size;
-        u32 i_atime;
-        u32 i_mtime;
-        u32 i_ctime;
-        u32 i_zone[10];
+	u16 i_mode;
+	u16 i_nlinks;
+	u16 i_uid;
+	u16 i_gid;
+	u32 i_size;
+	u32 i_atime;
+	u32 i_mtime;
+	u32 i_ctime;
+	u32 i_zone[10];
 };
 
+/*
+ * minix super-block data on disk
+ */
 struct minix_super_block {
-        u16 s_ninodes;
-        u16 s_nzones;
-        u16 s_imap_blocks;
-        u16 s_zmap_blocks;
-        u16 s_firstdatazone;
-        u16 s_log_zone_size;
-        u32 s_max_size;
-        u16 s_magic;
-        u16 s_state;
-        u32 s_zones;
+	u16 s_ninodes;
+	u16 s_nzones;
+	u16 s_imap_blocks;
+	u16 s_zmap_blocks;
+	u16 s_firstdatazone;
+	u16 s_log_zone_size;
+	u32 s_max_size;
+	u16 s_magic;
+	u16 s_state;
+	u32 s_zones;
 };
 
-/* V3 minix super-block data on disk */
+/*
+ * V3 minix super-block data on disk
+ */
 struct minix3_super_block {
 	u32 s_ninodes;
 	u16 s_pad0;
@@ -61,29 +100,17 @@ struct minix3_super_block {
 	u16 s_magic;
 	u16 s_pad2;
 	u16 s_blocksize;
-	u8  s_disk_version;
-        u16 s_state;
+	u8 s_disk_version;
 };
 
-#define BLOCK_SIZE_BITS 10
-#define BLOCK_SIZE (1<<BLOCK_SIZE_BITS)
-
-#define NAME_MAX   255   /* # chars in a file name */
-#define MAX_INODES 65535
-
 #define MINIX_INODES_PER_BLOCK ((BLOCK_SIZE)/(sizeof (struct minix_inode)))
-#define MINIX2_INODES_PER_BLOCK ((BLOCK_SIZE)/(sizeof (struct minix2_inode)))
-
-#define MINIX_VALID_FS               0x0001          /* Clean fs. */
-#define MINIX_ERROR_FS               0x0002          /* fs has errors. */
+#define MINIX_VALID_FS		0x0001	/* Clean fs. */
+#define MINIX_ERROR_FS		0x0002	/* fs has errors. */
 
-#define MINIX_SUPER_MAGIC    0x137F          /* original minix fs */
-#define MINIX_SUPER_MAGIC2   0x138F          /* minix fs, 30 char names */
-#define MINIX2_SUPER_MAGIC   0x2468	     /* minix V2 fs */
-#define MINIX2_SUPER_MAGIC2  0x2478	     /* minix V2 fs, 30 char names */
-#define MINIX3_SUPER_MAGIC   0x4d5a          /* minix V3 fs (60 char names) */
+#endif				/* HAVE_LINUX_MINIX_FS_H */
 
-#endif /* KERNEL_INCLUDES_ARE_CLEAN */
+#define MAX_INODES 65535
+#define MINIX2_INODES_PER_BLOCK ((BLOCK_SIZE)/(sizeof (struct minix2_inode)))
 
 #define Inode (((struct minix_inode *) inode_buffer)-1)
 #define Inode2 (((struct minix2_inode *) inode_buffer)-1)
diff --git a/disk-utils/mkfs.minix.c b/disk-utils/mkfs.minix.c
index 916dd17..322c023 100644
--- a/disk-utils/mkfs.minix.c
+++ b/disk-utils/mkfs.minix.c
@@ -161,8 +161,6 @@ static void super_set_state(void)
 {
 	switch (fs_version) {
 	case 3:
-		Super3.s_state |= MINIX_VALID_FS;
-		Super3.s_state &= ~MINIX_ERROR_FS;
 		break;
 	default:
 		Super.s_state |= MINIX_VALID_FS;
-- 
1.7.6