> > For filesystems that we officially support, there is no change here. For
> > those that require a cmdline option for support, module loading has now
> > moved from within the loader to inside the __init__ methods for the formats
> > themselves. The intention here is to avoid kernel errors in modules that
> > the user never even wants to have involved.
>
> The commit message needs some updating...
Can do.
> > diff --git a/loader/loader.c b/loader/loader.c
> > index 98ea24c..7b3eb0c 100644
> > --- a/loader/loader.c
> > +++ b/loader/loader.c
> > @@ -1919,7 +1919,7 @@ int main(int argc, char ** argv) {
> > if (isVioConsole())
> > setenv("TERM", "vt100", 1);
> >
> > - mlLoadModuleSet("cramfs:vfat:nfs:floppy:edd:pcspkr:squashfs:ext4:ext2:iscsi_tcp:iscsi_ibft");
> > + mlLoadModuleSet("cramfs:floppy:edd:pcspkr:squashfs:iscsi_tcp:iscsi_ibft");
>
> cramfs and squashfs shouldn't need explicit loads. We should then check
> some of the others and see if they're being loaded by udev also (pcspkr
> and floppy are the two that immediately spring to mind)
Okay, I might do this as round two, once this original stuff goes in.
> > @@ -2071,7 +2071,7 @@ int main(int argc, char ** argv) {
> > stop_fw_loader(&loaderData);
> > start_fw_loader(&loaderData);
> >
> > - mlLoadModuleSet("raid0:raid1:raid5:raid6:raid456:raid10:linear:fat:msdos:gfs2:reiserfs:jfs:xfs:btrfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-crypt:cbc:sha256:lrw:xts");
> > + mlLoadModuleSet("raid0:raid1:raid5:raid6:raid456:raid10:linear:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-crypt:cbc:sha256:lrw:xts");
>
> Mostly an aside, but maybe we should implement similar for raid
> personalities, dm stuff, etc...
>
> If we could lose all mlLoadModuleSet() calls like this, it would be
> super-fly
Agreed. I'll look into that as part of a later patch as well.
> > diff --git a/storage/formats/fs.py b/storage/formats/fs.py
> > @@ -427,6 +431,23 @@ class FS(DeviceFormat):
> > if rc >= 4:
> > raise FSError("filesystem check failed: %s" % rc)
> >
> > + def loadModule(self):
> > + """Load whatever kernel module is required to support this filesystem."""
> > + if not self._module or self._module in kernel_filesystems:
> > + return
> > +
> > + try:
> > + rc = iutil.execWithRedirect("modprobe", [self._module],
> > + stdout="/dev/tty5", stderr="/dev/tty5",
> > + searchPath=1)
> > + except Exception as e:
> > + log.error("Could not load kernel module %s: %s" % (self._module, e))
> > + self._supported = False
> > +
> > + if rc:
> > + log.error("Could not load kernel module %s" % self._module)
> > + self._supported = False
> > +
>
> I still think we need a check against /proc/filesystems here to see if
> support already exists as built-in vs modular.
That's what the "self._module in kernel_filesystems" bit is about.
> Also, I wonder if we're
> going to have any filesystems that need multiple modules (so a list, not
> just a string)
I debated this myself too and chose a string because I didn't see any
existing cases of needing more than one module. But that's easy enough
to fix.
> > @@ -764,6 +786,7 @@ class Ext3FS(Ext2FS):
> > _type = "ext3"
> > _defaultFormatOptions = ["-t", "ext3"]
> > _migrationTarget = "ext4"
> > + _module = "ext2"
>
> The module here should be ext3. But it's built-in instead of modular
Okay, I can change it to ext3 just to be absolutely correct.
- Chris
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
