Hi, On Wed, Dec 28, 2011 at 6:35 PM, Izumi Tsutsui <tsut...@ceres.dti.ne.jp> wrote: > Evgeniy Ivanov wrote: > >> I was trying to add bootxx_minifs3 similar to the bootxx_ext2fs. I >> failed to figure out how to use bootxx_ext2fs, because UFS and FAT are >> the only filesystems hardcoded in bootxx (pbr.S). Also it seems that a >> small ext2 boot partition is not an option for i386, because bootxx >> and boot must be on the same partition (and installing bootxx on ext2 >> kills FS, because 1Kb < ~8Kb). Though commit comment says it can be >> done. Could you please explain how to use bootxx_ext2fs? > > With the following disklabel: > --- > 16 partitions: > # size offset fstype [fsize bsize cpg/sgs] > a: 1833457 128 Linux Ext2 1024 8192 # (Cyl. 0*- > 1819*) > b: 263537 1833615 swap # (Cyl. 1819*- > 2080*) > c: 2097120 32 unused 0 0 # (Cyl. 0*- > 2080*) > d: 2097152 0 unused 0 0 # (Cyl. 0 - > 2080*) > e: 96 32 boot # (Cyl. 0*- > 0*) > --- > i.e. allocate a small explicit "boot" partition before root (a:) partition. > > installboot against /dev/wd0e, then bootxx_ext2fs is written > at top of the NetBSD partition and it loads /boot from wd0a.
Aha, it's a real disklabel, not fictitious one, and 'a' works as a fallback partition, right? With MBR it will not work, will it? Also 'e' is 0 - 0 and 'a' is 0 - 1819, so they overlap. Why 'a' is not overwritten by bootxx then? Sorry, I'm not familiar with disklabels. >> > - I don't know about MINIX FS v3, but it looks similar to Linux Ext2fs. >> > Is it difficult to share some code among them? >> >> It's also very similar to UFS (seems like Ext2fs is based on that >> code). There're some functions which have much in common, but >> different either in structure members names (like xxx_stat) or in very >> small details. I think sharing such small pieces of code will make >> things worse. Probably that's the reason why (IIRC) in Linux ext2, >> ext3 and ext4 do not share common code. > > Ok, I see. > >> > - Do you have any plan to add kernel support for the MINIX FS v3? >> >> No. > > Ok, but in that case someone might claim MINIX FS should be disabled > by default in x86 /boot. > (to avoild heap overflow etc. as ext2fs was disabled once before) I'll add flags to disable by default. >> > If not, are changes outside sys/libsa necessary? >> > (common/lib/libutil, libkern/xlat_mbr_fstype.c, sys/disk.h etc.) >> >> It's required by biosdisk modification. Biosdisk itself requires just >> one of those, but system build fails without other modifications >> (switch doesn't list everything, but adding a proper branch requires >> some another changes outside). Also it's nice to show to disklabel >> users name of MFS instead of "Unknown". > > Ok, but probably we need some approval (even by silence) > because it's exported API to userland. > (i.e. we won't be able to rename it for compatibility once it's exported) OK. >> > - What environment do you test these loaders? >> > Is there any tool to create/check these file system like >> > e2fsprogs for Ext2fs? >> >> To create MFS (sub)partitions and to format I use part and mkfs.mfs >> respectively. IIRC it's available in MINIX 3 only, but there is a life >> cd. >> To check MFS implementation I've inserted a test into boot2: it reads >> 3 files and calculates md5 sums. The biggest file had all levels of >> indirection. Ls was used to check if reads directories correctly. >> To multiboot I use an example multiboot kernel from grub's multiboot >> specification. >> >> I use VMWare and can share my test virtual disk. > > Ok. It's always good thing "what are tested or not in the patch" > in reports or logs :-) > >> > - There are some "XXX should handle LARGEFILE" comments (that I put >> > for ext2fs REV1 inode), but does MINIX FS have the similar member? >> > It doesn't seem there are extra members in struct mfs_dinode. >> >> Removed odd comments. > > BTW, can MINIFS FS handle >2GB files or not? It can't. >> > - struct mfs_sblock seems to have many uint16_t and one char members. >> > Probably it's better to put explicit padding and specify __packed >> > to avoid unexpected machine dependent alignment restrictions. >> >> It is the last member in ondisk structure, so machine dependent >> alignment after is just fine. > > Ah, ok. > >> > - in sys/lib/libsa/mfsv3.c:mfsv3_i_bswap() >> > - mdi_zone[] in is unused? (untesed on big endian?) >> >> Sorry, it's a pasto. I didn't test on big endian. > > It's better to check at least it (at least libsa) compiles > on build environments for big endian machines. I'll build cross tools and try compiling. >> > - zone_t is typedef'ed as uint32_t. Shouldn't mdi_zone[] members >> > also be swapped? >> >> Oh, they're swapped in a loop at the end of mfsv3_i_bswap(). That >> local mdi_zone[] didn't affect anything. > > : >>> + for (i = 0; i < V2_NR_TZONES; i++) >>> + new->mdi_zone[i] = (zone_t) old->mdi_zone[i]; > > Not a cast, but should be bswap32()? Sorry for confusion. Like in ext2fs swapping occurs in block_map(). >> > + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND >> > CONTRIBUTORS >> > >> > Probably not TNF, but the author or copyright holders. >> >> Fixed. > > : >>> + * THIS SOFTWARE IS PROVIDED BY THE VRIJE UNIVERSITEIT AND CONTRIBUTORS >>> ``AS > > For redistributors less variants of disclaimer make things easier, > so how about "COPYRIHGT HOLDERS" like src/sys/dev/ic/bwi.c: > >>> * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS OK. > >> Do I need to send-pr? > > Yes, so that we can search by keywords (digging ML archive is a bit annoying) > and we can also note PR number in commit log :-) Alright :-) Then I do some modifications, test email and send-pr new patches. -- Evgeniy