Re: [Xen-devel] [PATCH v6 11/13] xencommons: move module list into a generic place

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
> 
> This will allows us to share the same module list with
> sysemd, and lets us upkeep it in one place. Document this
> while at it on the top level README and expand on the wiki:
> 
> http://wiki.xen.org/wiki/Category:Host_Configuration#Linux_kernel_modules
> 
> In order to upkeep parallelism builds be explicit about the
> requirement to complete all actions before any installation
> targets.
> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxx>
> ---
>  README                                             | 18 +++++++++++
>  config/modules                                     | 16 ++++++++++

This list is Linux specific. If tools/hotplug/Linux/config or something
doesn't work for the upcoming system patches then perhaps
config/Linux.modules, where Linux is chosen to match $(XEN_OS) aka
"uname -s".

>  tools/hotplug/Linux/Makefile                       | 36 
> +++++++++++++++++++---
>  .../Linux/init.d/{xencommons => xencommons.in}     | 16 +---------
>  4 files changed, 67 insertions(+), 19 deletions(-)
>  create mode 100644 config/modules
>  rename tools/hotplug/Linux/init.d/{xencommons => xencommons.in} (89%)
> 
> diff --git a/README b/README
> index 9bbe734..34d7c0b 100644
> --- a/README
> +++ b/README
> @@ -129,6 +129,24 @@ performed with root privileges.]
>     versions of those scripts, so that you can copy the dist directory
>     to another machine and install from that distribution.
>  
> +Required Linux modules
> +======================
> +
> +Xen has a set of Linux modules which the init scripts ensure to load before
> +enablement of xen guests. The list of modules are upkept in one place:

enablement isn't a word. I think "before starting Xen guests" is OK.

upkept also isn't, "maintained" perhaps.

Also, end users don't care a jot about any of this. Move it right to the
end of the README? At the moment it comes before the Python stuff which
is end user focused.

TBH if it were me I'd just put the "The file supports a simple.." as a
comment in config/modules (wherever it ends up) and leave it at that.

> +  * config/modules
> +
> +The file supports a simple language, comments are ignored, and if you there
> +are module replacements this can be listed by using a pipe to show preference
> +for the first module, followed by the older module.
> +
> +We should strive to not require statically loading modules but it seems some
> +systems have had issues with automatically loading some Linux kernel modules.
> +For more details refer to:

This last paragraph is just meta commentary, IMHO it is commit log
material (at best) rather than README material.

> +
> +http://wiki.xen.org/wiki/Category:Host_Configuration#Linux_kernel_modules
> +
>  Python Runtime Libraries
>  ========================
>  
> diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
> index d5de9e6..90b0b0c 100644
> --- a/tools/hotplug/Linux/Makefile
> +++ b/tools/hotplug/Linux/Makefile
> @@ -33,17 +33,45 @@ UDEV_RULES_DIR = $(CONFIG_DIR)/udev
>  UDEV_RULES = xen-backend.rules $(UDEV_RULES-y)
>  
>  .PHONY: all
> -all:
> +all: $(XENCOMMONS_INITD)
> +
> +$(XENCOMMONS_INITD): $(XEN_ROOT)/config/modules $(XENCOMMONS_INITD).in
> +     @set -e ;                                                       \
> +     IFS=''                                                          ;\
> +     cat  $(XEN_ROOT)/config/modules | (                             \
> +             while read l ; do                                       \
> +                     if echo $${l} | egrep -q "^#" ; then            \
> +                             continue                                ;\
> +                     fi                                              ;\
> +                     if echo "$${l}" | egrep -q "\|" ; then          \
> +                             m1=$${l%%|*}                            ;\
> +                             m2=$${l#*|}                             ;\
> +                             echo "        modprobe $$m1 2>/dev/null || 
> modprobe $$m2 2>/dev/null" ;\
> +                     else                                            \
> +                             echo "        modprobe $$l 2>/dev/null"         
> ;\
> +                     fi                                              ;\
> +             done                                                    \
> +     ) > $(XENCOMMONS_INITD).modules                                 ;\
> +     cat  $(XENCOMMONS_INITD).in     | (                             \
> +             while read l ; do                                       \
> +                     if echo "$${l}" | egrep -q "@LOAD_MODULES@" ; then \
> +                             cat $(XENCOMMONS_INITD).modules         ;\
> +                     else                                            \
> +                             echo $$l                                ;\
> +                     fi                                              ;\

Sed with something like "/@LOAD_MODULES@/r$(...).modules" can replace
this second loop.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel