[PATCH] mkfs: optimization and code cleanup
Optimizations by reducing the STREQ operations and do some code cleanup. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/mkfs.c | 29 +++++++++++++---------------- 1 files changed, 13 insertions(+), 16 deletions(-) diff --git a/daemon/mkfs.c b/daemon/mkfs.c index a2c2366..7757623 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c @@ -44,13 +44,16 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, int r; char *err; char mke2fs[] = "mke2fs"; + int extfs = 0; + + if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || + STREQ (fstype, "ext4")) extfs = 1; /* For ext2/3/4 run the mke2fs program directly. This is because * the mkfs program "eats" some options, in particular the -F * option. */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) { + if (extfs) { if (e2prog (mke2fs) == -1) return -1; ADD_ARG (argv, i, mke2fs); @@ -64,9 +67,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, /* Force mke2fs to create a filesystem, even if it thinks it * shouldn't (RHBZ#690819). */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) - ADD_ARG (argv, i, "-F"); + if (extfs) ADD_ARG (argv, i, "-F"); /* mkfs.ntfs requires the -Q argument otherwise it writes zeroes * to every block and does bad block detection, neither of which @@ -77,16 +78,13 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, /* mkfs.reiserfs produces annoying interactive prompts unless you * tell it to be quiet. + * mkfs.jfs as the same + * mkfs.xfs must force to make xfs filesystem when the device already + * has a filesystem on it */ - if (STREQ (fstype, "reiserfs")) - ADD_ARG (argv, i, "-f"); - - /* Same for JFS. */ - if (STREQ (fstype, "jfs")) - ADD_ARG (argv, i, "-f"); - - if (STREQ (fstype, "xfs")) - ADD_ARG (argv, i, "-f"); + if (STREQ(fstype, "reiserfs") || STREQ(fstype, "jfs") || + STREQ(fstype, "xfs")) + ADD_ARG(argv, i, "-f"); /* For GFS, GFS2, assume a single node. */ if (STREQ (fstype, "gfs") || STREQ (fstype, "gfs2")) { @@ -147,8 +145,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, } if (optargs_bitmask & GUESTFS_MKFS_OPTS_INODE_BITMASK) { - if (!STREQ (fstype, "ext2") && !STREQ (fstype, "ext3") && - !STREQ (fstype, "ext4")) { + if (!extfs) { reply_with_error ("inode size (-I) can only be set on ext2/3/4 filesystems"); return -1; } -- 1.7.8.rc4
On 12/14/2011 12:29 AM, Wanlong Gao wrote:
Optimizations by reducing the STREQ operations and do some code cleanup.
Signed-off-by: Wanlong Gao<gaowanlong@cn.fujitsu.com> --- daemon/mkfs.c | 29 +++++++++++++---------------- 1 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/daemon/mkfs.c b/daemon/mkfs.c index a2c2366..7757623 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c
@@ -77,16 +78,13 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
/* mkfs.reiserfs produces annoying interactive prompts unless you * tell it to be quiet. + * mkfs.jfs as the same
s/as/is/
+ * mkfs.xfs must force to make xfs filesystem when the device already + * has a filesystem on it */ - if (STREQ (fstype, "reiserfs")) - ADD_ARG (argv, i, "-f"); - - /* Same for JFS. */ - if (STREQ (fstype, "jfs")) - ADD_ARG (argv, i, "-f"); - - if (STREQ (fstype, "xfs")) - ADD_ARG (argv, i, "-f"); + if (STREQ(fstype, "reiserfs") || STREQ(fstype, "jfs") || + STREQ(fstype, "xfs")) + ADD_ARG(argv, i, "-f");
/* For GFS, GFS2, assume a single node. */ if (STREQ (fstype, "gfs") || STREQ (fstype, "gfs2")) {
Seems like an obvious improvement. ACK. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
v1->v2: fix a typo pointed by Matt Optimizations by reducing the STREQ operations and do some code cleanup. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/mkfs.c | 29 +++++++++++++---------------- 1 files changed, 13 insertions(+), 16 deletions(-) diff --git a/daemon/mkfs.c b/daemon/mkfs.c index a2c2366..7757623 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c @@ -44,13 +44,16 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, int r; char *err; char mke2fs[] = "mke2fs"; + int extfs = 0; + + if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || + STREQ (fstype, "ext4")) extfs = 1; /* For ext2/3/4 run the mke2fs program directly. This is because * the mkfs program "eats" some options, in particular the -F * option. */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) { + if (extfs) { if (e2prog (mke2fs) == -1) return -1; ADD_ARG (argv, i, mke2fs); @@ -64,9 +67,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, /* Force mke2fs to create a filesystem, even if it thinks it * shouldn't (RHBZ#690819). */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) - ADD_ARG (argv, i, "-F"); + if (extfs) ADD_ARG (argv, i, "-F"); /* mkfs.ntfs requires the -Q argument otherwise it writes zeroes * to every block and does bad block detection, neither of which @@ -77,16 +78,13 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, /* mkfs.reiserfs produces annoying interactive prompts unless you * tell it to be quiet. + * mkfs.jfs is the same + * mkfs.xfs must force to make xfs filesystem when the device already + * has a filesystem on it */ - if (STREQ (fstype, "reiserfs")) - ADD_ARG (argv, i, "-f"); - - /* Same for JFS. */ - if (STREQ (fstype, "jfs")) - ADD_ARG (argv, i, "-f"); - - if (STREQ (fstype, "xfs")) - ADD_ARG (argv, i, "-f"); + if (STREQ(fstype, "reiserfs") || STREQ(fstype, "jfs") || + STREQ(fstype, "xfs")) + ADD_ARG(argv, i, "-f"); /* For GFS, GFS2, assume a single node. */ if (STREQ (fstype, "gfs") || STREQ (fstype, "gfs2")) { @@ -147,8 +145,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, } if (optargs_bitmask & GUESTFS_MKFS_OPTS_INODE_BITMASK) { - if (!STREQ (fstype, "ext2") && !STREQ (fstype, "ext3") && - !STREQ (fstype, "ext4")) { + if (!extfs) { reply_with_error ("inode size (-I) can only be set on ext2/3/4 filesystems"); return -1; } -- 1.7.8.rc4
Hi Rich: Will you review this? Thanks -Wanlong Gao
v1->v2: fix a typo pointed by Matt
Optimizations by reducing the STREQ operations and do some code cleanup.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- daemon/mkfs.c | 29 +++++++++++++---------------- 1 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/daemon/mkfs.c b/daemon/mkfs.c index a2c2366..7757623 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c @@ -44,13 +44,16 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, int r; char *err; char mke2fs[] = "mke2fs"; + int extfs = 0; + + if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || + STREQ (fstype, "ext4")) extfs = 1;
/* For ext2/3/4 run the mke2fs program directly. This is because * the mkfs program "eats" some options, in particular the -F * option. */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) { + if (extfs) { if (e2prog (mke2fs) == -1) return -1; ADD_ARG (argv, i, mke2fs); @@ -64,9 +67,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, /* Force mke2fs to create a filesystem, even if it thinks it * shouldn't (RHBZ#690819). */ - if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") || - STREQ (fstype, "ext4")) - ADD_ARG (argv, i, "-F"); + if (extfs) ADD_ARG (argv, i, "-F");
/* mkfs.ntfs requires the -Q argument otherwise it writes zeroes * to every block and does bad block detection, neither of which @@ -77,16 +78,13 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
/* mkfs.reiserfs produces annoying interactive prompts unless you * tell it to be quiet. + * mkfs.jfs is the same + * mkfs.xfs must force to make xfs filesystem when the device already + * has a filesystem on it */ - if (STREQ (fstype, "reiserfs")) - ADD_ARG (argv, i, "-f"); - - /* Same for JFS. */ - if (STREQ (fstype, "jfs")) - ADD_ARG (argv, i, "-f"); - - if (STREQ (fstype, "xfs")) - ADD_ARG (argv, i, "-f"); + if (STREQ(fstype, "reiserfs") || STREQ(fstype, "jfs") || + STREQ(fstype, "xfs")) + ADD_ARG(argv, i, "-f");
/* For GFS, GFS2, assume a single node. */ if (STREQ (fstype, "gfs") || STREQ (fstype, "gfs2")) { @@ -147,8 +145,7 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize, }
if (optargs_bitmask & GUESTFS_MKFS_OPTS_INODE_BITMASK) { - if (!STREQ (fstype, "ext2") && !STREQ (fstype, "ext3") && - !STREQ (fstype, "ext4")) { + if (!extfs) { reply_with_error ("inode size (-I) can only be set on ext2/3/4 filesystems"); return -1; }
On Fri, Dec 23, 2011 at 08:40:44AM +0800, Wanlong Gao wrote:
Hi Rich:
Will you review this?
Sorry, it dropped through the cracks. ACK. I'll push it shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
On 12/23/2011 04:16 PM, Richard W.M. Jones wrote:
On Fri, Dec 23, 2011 at 08:40:44AM +0800, Wanlong Gao wrote:
Hi Rich:
Will you review this?
Sorry, it dropped through the cracks.
ACK. I'll push it shortly.
Sorry for the whitespace.
Rich.
On Fri, Dec 23, 2011 at 04:28:35PM +0800, Wanlong Gao wrote:
On 12/23/2011 04:16 PM, Richard W.M. Jones wrote:
On Fri, Dec 23, 2011 at 08:40:44AM +0800, Wanlong Gao wrote:
Hi Rich:
Will you review this?
Sorry, it dropped through the cracks.
ACK. I'll push it shortly.
Sorry for the whitespace.
No problems! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
participants (3)
-
Matthew Booth -
Richard W.M. Jones -
Wanlong Gao