Driver Update Disk review comments (long)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Folks,

As part of reviewing Driver Update Disk support, I would like to share
the following comments, as well as some analysis. As a reminder, all
reference to branches or product names has been removed from this
posting because the anaconda list is publicly indexed. I am happy to
debate product focused issues on internal RH mail, but not publicly.

The following paragraphs contain a summary of existing loader
operations, current (aka "version 3") Driver Update Disk support, issues
with that implementation, other items that should be fixed, and various
other comments from me that you can take or leave as you please.

TODO: I will provide some test version 3 driver disks.

Anaconda control flow in stage1 loader
--------------------------------------

First, for background, here is exactly what the loader in Anaconda does
today, as part of loading the Anaconda proper runtime (with any
updates). I decided it was useful to have this reference around.

        * loader.c enters main
        * modInfo is initialized
        * read "module-info" from media and parse with readModuleInfo
        * explicitly load cramfs, squashfs, iscsi_tcp
        * explicitly load floppy, edd, pcspkr, iscsi_ibft (if not s390x)
        * explicitly load ipv6 (if using IPv6)
        * explicitly load spufs (if using PowerPC)
        * switch to modprobeDDmode
        * if ./rundepmod run depmod (initrd overlay use)
        * attempt autodetection load
        * loadDriverDisks() (if FL_MODDISK(flags) set)
        * Attempt to use /dd.img (initramfs overlaid image) via
	  getDDFromSource
        * switch to modprobeNormalmode()
        * load explicit modules, via mlInitModuleConfig,
	  earlyModuleLoad()
        * start hald up
        * disable NetworkManager interfaces and then start
	  NetworkManager
        * Start the gdbserver
        * start up telnetd
        * doLoaderMain()
        * switch to /mnt/runtime library and binary directories
        * load SELinux policy
        * spawnShell() (gdb safe now)
        * manualDeviceCheck (if FL_NOPROBE(flags) specified)
        * loadUpdatesFromRemote()
        * loadUpdates()
        * copy data over to /tmp/updates
        * add /tmp/updates to the environment
        * explicitly load 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
        * attempt to switch to libunicode-lite
        * check --stage2 for ftp source
        * check for other extra args
        * check if dlabel used
        * check if noipv4
        * check if noipv6
        * check if kickstart (and set loaderData.ksFile)
        * check if serial
        * check if rescue
        * check install type for graphical, console, noselinux,
	  virtpconsole
        * check if updates (and set loaderData.updatessrc)
        * check if dogtail (and set loaderData.dogtailurl)
        * check if lang (and set loaderData.lang)
        * check if keymap (and set loaderData.kbd)
        * check if loglevel (and set loaderData.logLevel)
        * check if repo (and set e.g. ftp repo in loaderData.instRepo)
        * check if proxy (and set loaderData.proxy)
        * exec the stage2
        * Handle any errors, or shutdown post install

Summary of Driver Update Disk code
----------------------------------

The following is an unordered summary of Driver Update Disk functions.
You might want to incorporate this into code documentation sometime.

modprobeDDmode
--------------

Called from loader.c:main and used to switch to "DD" mode. In this mode,
we explicitly blacklist the directories containing standard system
modules so that any udev triggers will only load updates first (not an
issue for explicit module dependencies, only for modaliases).

NOTE: Does NOT re-run depmod explicitly and so does not work.

	* Write out /etc/depmod.d/ddmode.conf
		- pblacklist /lib/modules/`uname -r`

modprobeNormalmode
------------------

Called from loader.c:main and used to switch back from "DD" mode. In
this mode, all system modules are once again available for use.

NOTE: Does re-run depmod explicitly even though DDmode did not.

	* rm /etc/depmod.d/ddmode.conf
	* rerun depmod -a

dlabelProvides
--------------

Called from dlabelUnpackRPMDir in order to determine whether an RPM
provides what we're looking for. If an RPM provides
"kernel-modules-`uname -r`" then we care about it.

	* strcmp two "Provides" tags

dlabelFilter
------------

Called from dlabelUnpackRPMDir in order to determine which files we
should keep from any unpacked RPM (we only care about firmware/modules).

NOTE: Should not use strcmp and should explicitly check strncmp return.
NOTE: Should preserve /etc/depmod.d and /etc/modprobe.d files.

	* keep firmware files
	* keep .ko files

moduleDescription
-----------------

This function is not yet called. There is a FIXME in the code though. It
is intended to retrieve the description of a module file with "modinfo".

NOTE: This function should be used in the code.

	* store "modinfo -- description thepath" in "command"
	* popen that "command"
	* return description pointer

globErrFunc
-----------

Called from dlabelUnpackRPMDir but it does not do anything (yet).

	* TODO (return 0)

dlabelUnpackRPMDir
------------------

Called from loadDriverDisk, and used to unpack RPM files found in a
directory (for example, "repodata"), into a directory. The filter
function dlabelFilter is used to determine which files are copied.

	* change the directory to "destination"
	* store kernel-modules-`uname -r` in "kernelver"
	* iterate over rpm files in "rpmdir", and call explodeRPM

driverDiskFiles
---------------

This is a struct containing the locations of files to examine in order
to determine a valid Driver Update Disk. It only contains "repodata",
and not "rhdd3", presumably because it was intended that this be used
elsewhere in cases "rhdd3" might not exist, but this is not the case.

NOTE: Probably want to add "rhdd3" or re-consider this logic.

        * "repodata"

verifyDriverDisk
----------------

Called from loadDriverFromMedia, loadFromLocation, and also
loadDriverDiskFomPartition. Not called every possible time.

NOTE: This should be called every time a disk is validated, for example
in loadDriverDisk, where this function is re-implemented.

	* checks for "/rhdd3"
	* checks for "/<arch>/repodata"

loadDriverDisk
--------------

Called from loadDriverFromMedia, loadFromLocation, and also
loadDriverdiskFromPartition. Copies over modules.

NOTE: Should call verifyDriverDisk.
NOTE: "repodata" and "rpms" not in their consistent locations.
NOTE: Should copy over depmod.d and modprobe.d for update.

	* re-verifies "/rhdd3" without using verifyDriverDisk
	* creates "/tmp/DD/lib/modules"
	* creates "/tmp/DD/lib/firmware"
	* copies "/rpms/<arch>"
	* call depmod -a
	* add in firmware search directory
	* stop and start fw_loader

getRemovableDevices
-------------------

Called from loadDriverFromMedia, and from loadUpdates in loader.c. It is
intended to obtain all removable devices.

NOTE: What about removable flash devices?

	* iterate over getDevices(DEVICE_DISK | DEVICE_CDROM)
	* return list of devices

loadDriverFromMedia
-------------------

Called from loadDriverDisks, chooseManualDriver in driverselect.c,
mountHardDrive in hdinstall.c, and from doLoaderMain in loader.c. It is
intended to provide a state machine that finds driver disks partitioned
or unpartitioned media, mounts that media, extracts driver updates,
loads those drivers, and performs probe events as are required.

NOTE: "/tmp/dpart", "/tmp/drivers", and "tmp/loop6" should be
"#define"s.

	* Simple state machine has the following valid states:
		- DEV_DEVICE, DEV_PART, DEV_CHOOSEFILE, DEV_LOADFILE
		- DEV_INSERT, DEV_LOAD, DEV_PROBE, DEV_DONE (end)
	* DEV_DEVICE: call getRemovableDevices()
	* DEV_PART: call getPartitionsList()
	* DEV_PART: unmount "/tmp/dpart", then mount "/tmp/dpart"
	* DEV_LOADFILE: umount then mount "/tmp/drivers" from/on
	  "/dev/loop6"
	* DEV_LOAD: using "/tmp/drivers", call loadDriverDisk()
	* DEV_LOAD: unmount "/tmp/drivers", and also "/tmp/dpart"
	* DEV_PROBE: call busProbe()
	* DEV_PROBE: call getDevices() and check we found something
	* DEV_PROBE: if not, offer user choice to load something
	* DEV_DONE: end of loop (should be reached)

loadDriverDisks
---------------

Called from getDDFromSource, and from main in loader.c. Intended to
actually perform the necessary loading process for any update disks.

	* call loadDriverFromMedia()
	* optionally call again, and repeat as needed until we're done

loadFromLocation
----------------

Called from getDDFromSource, and from getDDFromDev. Intended to ensure
that we load any updates from external sources or devices.

	* call verifyDriverDisk()
	* call loadDriverDisk()

getDDFromSource
---------------

Called from useKickstartDD, and from main in loader.c. Intended to
ensure that we load any updates from kickstart sources (nfs, ftp, or an
explicitly available path).

NOTE: "/tmp/dd.img" should be a #define.

	* path is "/tmp/dd.img" unless it is set explicitly
	* if nfs specified, call getFileFromNfs
	* if ftp specified, call getFileFromUrl
	* if path specified, use that instead of "/tmp/dd.img"

useKickstartDD
--------------

Called from kickstart.c by way of processing the ksCommandNames ksTable
entries in ksReadCommands, when encountering "driverdisk".

	* call either getDDFromDev or getDDFromSource according to ks

getDDFromDev
------------

Called from useKickstartDD, and used to load a driver disk by calling
loadFromLocation.

	* load a driver disk using loadFromLocation

findDriverDiskByLabel
---------------------

Called from main in loader.c, and used to match use of volume labels
matching "OEMDRV" on removable devices. In this case, those devices are
intended to be treated as automatic driver update sources, unless it is
specified that they should not be used as such in the environment.

NOTE: "OEMDRV" should be a #define.

	* look for "OEMDRV" label on all block devices

loadDriverDiskFromPartition
---------------------------

Called from main in loader.c, and used to load a driver disk from a
specified partition in the result returned by findDriverDiskByLabel.

	* call verifyDriverDisk()
	* call loadDriverDisk()

sortModuleList
--------------

A struct containing sorted (according to description) module lists.
These are present to the user when a manual load is performed.

	* Module info stored

sortDrivers
-----------

This function returns the correct alphabetical order for two entries in
the specified sortModuleList, but it also finds those descriptions.

NOTE: This function should perform a sort. chooseManualDriver should
actually set the entries for descriptions correctly, rather than relying
upon a hacky and horrible side effect of calling out to the sort.

	* alphabetical re-order two driver descriptions
	* update description entries

getManualModuleArgs
-------------------

This function updates the explicit module parameters that will be passed
to modules from the boot line.

NOTE: modprobe can read this information now. This may not be necessary.

	* modify moduleArgs with specified arguments

chooseManualDriver
------------------

Called from loadDriverFromMedia in driverdisk.c, doLoaderMain in
loader.c, and also manualDeviceCheck in loader.c. Intended as a failsafe
in the event that no automated modules are probed for installed devices
as a result of automated loading processes, or in the case that probing
intentionally has been disabled in the environment.

NOTE: This could be expanded to use an environment variable that allowed
explicit modules to be used, even if probing is enabled and drivers have
been loaded normally. This would really aid testing, and has uses.

	* with loaderData->modInfo, create sortModuleList
	* with sortDrivers (which pulls out description), call a qsort
	* call modules.c:mlLoadModule()

------------------------------- End of overview ------------------------

SKIP: You can skip to these questions if you are short of time. Included
here to encourage reading the above, and to prevent "top posting".

Questions about the current driver disk implementation
------------------------------------------------------

	* Why doesn't modprobeDDmode re-run depmod explicitly?
	* Are flash disks covered by the partition code check?
	* What about findDriverDiskbyLabel from whole devices?

Concerns for items that should be fixed in the implementation
-------------------------------------------------------------

	* Fix use of strcmp and strncmp.
	* Never do !strcmp, explicitly check against <,=,> 0 in
	  dlabelFilter. That is not as easy to read as possible
	* Retain RPM /etc/depmod.d and /etc/modprobe.d files in
	  dlabelFilter. This allows updates to provide options
	* Call moduleDescription correctly (currently not used)
	* Use verifyDriverDisk throughout (do not reimplement in
	  loadDriverDisk, and scan for "rhdd3" consistently)
	* Correct location for repodata and rpms on driverdisk
	* Ensure flash devices included in getRemovableDevices
	* Lots of #define's are needed for the following:
		- "/tmp/dpart"
		- "/tmp/drivers"
		- "/tmp/loop6"
		- "/tmp/dd.img"
		- "OEMDRV"

Future items that should or could be improved in the implementation
-------------------------------------------------------------------

	* Fix "Red Hat" comparison for autoDD load in loader.c
	* Check that pblacklist blacklisting support is working (jcm)
	* Consider http load of driver disks in addition to ftp

Items that should be tested
---------------------------

	* CDROM/DVD drivers (whole disk partition)
	* USB Flash drivers (whole device or a partition)
	* "OEMDRV" automated driver disk load support (including from
	  removable, and whole device as well as partition)
	* initramfs drivers (supplied via TFTP)
	* Driver fallback in case none found (manual driver load)
	* Driver load in kickstart via nfs, ftp, path

Thanks for reading,

Jon.


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux