Re: [PATCHv3] a new filesystem module for nilfs2

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

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

Jiro SEKIBA wrote:
> Hi,
>
> This is a revised patch to support nilfs2, a log file system.
> The patch is basically just a retrofit of the one I sent before
> against current tree.
>
> I've checked it both on qemu and qemu-system-ppc with grub-fstest.
>   
Thanks for your effort.
I recommend running indent on all new files

+ * Copyright (C) 2010 Jiro SEKIBA <jir@unicus.jp> 
+ */
This should go into 
+ *  Copyright (C) 2003,2004,2005,2007,2008,2010  Free Software Foundation, Inc.

Because of legal reason.
You can of course add a notion like:
/* Wrtitten by Jiro SEKIBA <jir@unicus.jp>. */

+/* nilfs btree node flag  */
Please add a full stop. Some of th

+} __attribute__ ((packed));
Most structures in nilfs like in most modern FS need no __attribute__ ((packed)). And adding it inflicts performance penalty on RISC.
+/** nilfs_fs.h **/
Your code doesn't look derived from any other nilfs implementation. I think these comments are only confusing.
+	{
+	  s = 0;
+	  goto out;
+	}
I think it would be slightly more clear by putting *indexp = index; return 1; here.

+  /* assume sizeof(struct grub_nilfs2_cpfile_header) < 
+                     sizeof(struct grub_nilfs2_checkpoint)
+  */
Capitalize and add a full stop please.
+      if(grub_errno)
+	{
+	  grub_error(GRUB_ERR_BAD_FS,"disk read error\n");
+	  return -1;
No need to run grub_error if grub_errno is already set
+	{
+	  grub_error(GRUB_ERR_BAD_FS,"btree corruption\n");
+	  return -1;
+	}
What do you think about possible fallback to iterate over all nodes in case of fs corruption?
+    grub_error(GRUB_ERR_BAD_FS,"btree lookup failure");
+    return GRUB_ERR_BAD_FS;
can be just done with:
return grub_error(GRUB_ERR_BAD_FS, "btree lookup failure");

> Thanks,
>
> Regards,
>   
> ------------------------------------------------------------------------
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]