[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: patch: MFSv3 support (libsa) for boot2 (i386)
On Wed, Dec 28, 2011 at 6:35 PM, Izumi Tsutsui
> 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*-
> b: 263537 1833615 swap # (Cyl. 1819*-
> c: 2097120 32 unused 0 0 # (Cyl. 0*-
> d: 2097152 0 unused 0 0 # (Cyl. 0 -
> e: 96 32 boot # (Cyl. 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?
> 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)
>> > - 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
>> 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
>> 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?
>> > - 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.
>>> + * THIS SOFTWARE IS PROVIDED BY THE VRIJE UNIVERSITEIT AND CONTRIBUTORS
> 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
>> 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.
Main Index |
Thread Index |