[OpenWrt-Devel] [PATCH v2 0/2] kernel: add kmod-ubi and kmod-fs-ubifs

[Daniel Golle]

Hi Ralph,

On Fri, Aug 26, 2016 at 04:13:53PM +0200, Ralph Sennhauser wrote:
> Hi Daniel,
> 
> On Fri, 26 Aug 2016 15:05:58 +0200
> Daniel Golle <daniel at makrotopia.org> wrote:
> 
> > Hi Ralph,
> > 
> > the use of global variable ROOT_DEV is not guarded properly in the
> > patch auto-creating a ubiblock though this code needs to be
> > prevented from running when built as a module.
> > Please try rebuilding with the patch attached, if it resolves the
> > issue I'll commit a proper fix for that on LEDE's source.git
> > 
> 
> thanks for looking into it that fast, will give the patch a try later
> today and report back.
> 
> May I ask you why you do no handle setting ROOT_DEV in init/do_mounts
> instead of ubi/block? It isn't immediately obvious to me.

Yes, it would be easy to trigger the creation of the ubiblock device
in do_mounts, and that'd be nice because it could then also be based on
the root= kernel cmdline parameter, which would make transparently
mounting ubifs or ubiblock(squashfs) possible without having to change
the cmdline (or a fixed convention like what's currently implemented).

However, ubiblock_create doesn't give the callee any information
on the created block device. The usual thing is that it hotplug-hooks
in userspace create the device node /dev/ubiblockX_Y.
However, this doesn't happen before hotplug events are processed, thus
the easiest (and without API modification only) way in terms of knowing
the minor/major numbers of the created block device was to set ROOT_DEV
inside of of ubiblock_create(). This is far from beautiful and to make
it better the next thing on my agenda is to suggest changes to
driver/mtd/ubi/block.c in the kernel which would either allow the
callee to know the major/minor number of the created device or pass
have a variant ubiblock_create_root() which also sets ROOT_DEV.


> 
> Cheers
> Ralph
> 
> > Cheers
> > 
> > Daniel
> > 
> > On Fri, Aug 26, 2016 at 11:46:05AM +0200, Ralph Sennhauser wrote:
> > > On Thu, 25 Aug 2016 12:53:41 +0200
> > > Ralph Sennhauser <ralph.sennhauser at gmail.com> wrote:
> > >   
> > > > Hi Zoltan,
> > > > 
> > > > On Thu, 25 Aug 2016 07:40:56 +0200
> > > > Zoltan HERPAI <wigyori at uid0.hu> wrote:
> > > >   
> > > > > Daniel Golle wrote:    
> > > > > > On Wed, Aug 24, 2016 at 11:28:40PM +0200, Zoltan HERPAI wrote:
> > > > > >         
> > > > > >> Ralph Sennhauser wrote:
> > > > > >>           
> > > > > >>> I use kmod-ubi for creating a block device from the squasfs
> > > > > >>> using module parameters as there is no busybox ubiblock
> > > > > >>> applet yet. If ubi is made available as module so obvioulsy
> > > > > >>> should ubifs.
> > > > > >>>
> > > > > >>> This completes the addition of kmod-fs-squashfs in commit
> > > > > >>> 5163389b9c3b302a0d53df9a70294da5cbc08ada
> > > > > >>>
> > > > > >>> --
> > > > > >>> V2: kmod-ubifs -> kmod-fs-ubifs in commit message
> > > > > >>>             
> > > > > >> Hi Ralph,
> > > > > >>
> > > > > >> With this configuration, these packages will try to get
> > > > > >> built on targets where UBI support is not available, and
> > > > > >> failing those builds due to the missing UBI config symbols.
> > > > > >> Can you try to make these packages depend on the ubifs
> > > > > >> feature ? 
> > > > > >
> > > > > > Well, I reckon on most if not all targets with 'ubifs' feature
> > > > > > set, UBI and UBIFS is built-in the kernel anyway...
> > > > > >         
> > > > > True - there still are a couple config variables need to be
> > > > > taken care of.
> > > > > 
> > > > > -w-
> > > > >     
> > > > 
> > > > Slipped my mind that there are config updates needed for targets
> > > > not yet enabling ubi/ubifs. Will send a fixed series later.
> > > > 
> > > > Thanks to you and Daniel for the review.
> > > > 
> > > > Cheers
> > > > Ralph  
> > > 
> > > While it shouldn't be an issue to build those packages for any
> > > targets this is currently broken due to
> > > CONFIG_MTD_ROOTFS_ROOT_DEV=y. Unsetting allows to build for x86 for
> > > example, i.e get past modposts ROOT_DEV unset! for ubi.ko
> > > 
> > > Haven't had a chance to look into how to tackle this issue. Fixing
> > > the openwrt patches (which are supposed to be dropped ;)) or making
> > > CONFIG_MTD_ROOTFS_ROOT_DEV opt-in or a third solution I haven't
> > > thought up yet.
> > > 
> > > Anyway just adding the missing config symbols isn't enough.
> > > 
> > > Cheers
> > > Ralph  
> 
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel