Re: [PATCHv3] a new filesystem module for nilfs2

[Vladimir 'φ-coder/phcoder' Serbinenko]

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 <address@hidden> 
+ */
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 <address@hidden>. */

+/* 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
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


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

signature.asc
Description: OpenPGP digital signature