Xen devel - May 2012 - [PATCH] tools/hotplug/Linux/init.d/: added other xen kernel modules on xencommons start

Fabio Fantoni writes ("[Xen-devel] [PATCH] tools/hotplug/Linux/init.d/:
added other xen kernel modules on xencommons
start"):
> # HG changeset patch
> # User Fabio Fantoni
> # Date 1338467204 -7200
> # Node ID 1f57503f10112718ecbbe424fa8fc9c55785f4c0
> # Parent  dd7319230f4ac295b6d14ce2e2a3dccf82bb87d8
> tools/hotplug/Linux/init.d/: added other xen kernel modules on 
> xencommons start
This looks at least harmless to me.

I''m surprised, however, that these things aren''t loaded
automatically.
For example, shouldn''t the xenbus driver''s enumeration
automatically
load blkback too ?

Having said that, I''m inclined to apply this unless someone 
explains that it''s a bad idea.

Ian.

On Fri, 2012-06-08 at 15:07 +0100, Ian Jackson wrote:
> Fabio Fantoni writes ("[Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/: added other xen kernel modules on xencommons
start"):
> > # HG changeset patch
> > # User Fabio Fantoni
> > # Date 1338467204 -7200
> > # Node ID 1f57503f10112718ecbbe424fa8fc9c55785f4c0
> > # Parent  dd7319230f4ac295b6d14ce2e2a3dccf82bb87d8
> > tools/hotplug/Linux/init.d/: added other xen kernel modules on 
> > xencommons start
> 
> This looks at least harmless to me.
> 
> I''m surprised, however, that these things aren''t loaded
automatically.
> For example, shouldn''t the xenbus driver''s enumeration
automatically
> load blkback too ?
Yes it should, there is autoloading stuff for all the backends.

Not sure about gntalloc. I suspect not.
> 
> Having said that, I''m inclined to apply this unless someone 
> explains that it''s a bad idea.
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH] tools/hotplug/Linux/init.d/:
added other xen kernel modules on xencommons
start"):
> On Fri, 2012-06-08 at 15:07 +0100, Ian Jackson wrote:
> > Fabio Fantoni writes ("[Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/: added other xen kernel modules on xencommons
start"):
> > > tools/hotplug/Linux/init.d/: added other xen kernel modules on 
> > > xencommons start
> > 
> > This looks at least harmless to me.
> > 
> > I''m surprised, however, that these things aren''t
loaded automatically.
> > For example, shouldn''t the xenbus driver''s
enumeration automatically
> > load blkback too ?
> 
> Yes it should, there is autoloading stuff for all the backends.
> 
> Not sure about gntalloc. I suspect not.
> 
> > Having said that, I''m inclined to apply this unless someone 
> > explains that it''s a bad idea.
I have applied it.

But we should still try to fix the upstream kernels not to need it.

Thanks,
Ian.

>>> On 03.08.12 at 13:08, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/:
> added other xen kernel modules on xencommons start"):
>> On Fri, 2012-06-08 at 15:07 +0100, Ian Jackson wrote:
>> > Fabio Fantoni writes ("[Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/:
> added other xen kernel modules on xencommons start"):
>> > > tools/hotplug/Linux/init.d/: added other xen kernel modules
on
>> > > xencommons start
>> > 
>> > This looks at least harmless to me.
>> > 
>> > I''m surprised, however, that these things aren''t
loaded automatically.
>> > For example, shouldn''t the xenbus driver''s
enumeration automatically
>> > load blkback too ?
>> 
>> Yes it should, there is autoloading stuff for all the backends.
>> 
>> Not sure about gntalloc. I suspect not.
>> 
>> > Having said that, I''m inclined to apply this unless
someone
>> > explains that it''s a bad idea.
> 
> I have applied it.
> 
> But we should still try to fix the upstream kernels not to need it.
And just like for the respective pending blktap item, it should
- be made sure this gets backed out again after 4.2, making sure
  the loading happens either automatically or when the module is
  in fact needed, and
- not exclusively try the pv-ops kernel''s module names.

Jan

Jan Beulich writes ("Re: [Xen-devel] [PATCH] tools/hotplug/Linux/init.d/:
added other xen kernel modules on xencommons
start"):
> On 03.08.12 at 13:08, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> > But we should still try to fix the upstream kernels not to need it.
> 
> And just like for the respective pending blktap item, it should
> - be made sure this gets backed out again after 4.2, making sure
>   the loading happens either automatically or when the module is
>   in fact needed, and
Please do remind us :-).
> - not exclusively try the pv-ops kernel''s module names.
Do you mean that 4.2 should try loading some bigger set of module
names ?  If so then do you have a list ? :-)

Ian.

>>> On 07.08.12 at 19:22, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
>> Jan Beulich writes ("Re: [Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/:
>> - not exclusively try the pv-ops kernel''s module names.
> 
> Do you mean that 4.2 should try loading some bigger set of module
> names ?  If so then do you have a list ? :-)
- xen-blkback''s counterpart is blkbk
- xen-netback''s counterpart is netbk

xen-evtchn/evtchn and xen-gntdev/gntdev are already taken
care of, albeit in a little strange a way (the two entries being
separated by an increasing amount of other ones, when it is
really pointless to load the second one if the first one''s load
succeeded).

To not needlessly try everything, it might additionally be worth
a thought to
- first try loading via module alias rather than module name (if
  that succeeds for a carefully chosen module that got its alias
  added late - according to our patches, the devname: aliases
  got introduced in 2.6.35, and the xen-backend: ones in 3.1 -,
  only try loading via module alias for all subsequent ones)
- second try loading a (or all) pvops named module(s) (if that/
  any of them succeed(s), there''s no need to try _any_ non-
  pvops names subsequently, i.e. including ones that don''t even
  exist in the legacy trees)
- last try loading the traditional/forward-port named ones

I notice, however, that in pvops no devname: module aliases
exist - is that an oversight, Konrad? While for misc devices with
dynamic minors these don''t help with autoloading, they do help
with abstracting away the module name as would be desirable
here (and later in the tools, when they get to load the modules).

Bottom line is - for the recently (c/s 25728:a6edbc39fc84)
added backend modules the above outlined scheme should work,
while for the infrastructure ones step 1 should be skipped.

Jan

On Wed, Aug 08, Jan Beulich wrote:
> >>> On 07.08.12 at 19:22, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> >> Jan Beulich writes ("Re: [Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/:
> >> - not exclusively try the pv-ops kernel''s module names.
> > 
> > Do you mean that 4.2 should try loading some bigger set of module
> > names ?  If so then do you have a list ? :-)
> 
> - xen-blkback''s counterpart is blkbk
> - xen-netback''s counterpart is netbk
> 
> xen-evtchn/evtchn and xen-gntdev/gntdev are already taken
> care of, albeit in a little strange a way (the two entries being
> separated by an increasing amount of other ones, when it is
> really pointless to load the second one if the first one''s load
> succeeded).
> 
> To not needlessly try everything, it might additionally be worth
> a thought to
> - first try loading via module alias rather than module name (if
>   that succeeds for a carefully chosen module that got its alias
>   added late - according to our patches, the devname: aliases
>   got introduced in 2.6.35, and the xen-backend: ones in 3.1 -,
>   only try loading via module alias for all subsequent ones)
> - second try loading a (or all) pvops named module(s) (if that/
>   any of them succeed(s), there''s no need to try _any_ non-
>   pvops names subsequently, i.e. including ones that don''t even
>   exist in the legacy trees)
> - last try loading the traditional/forward-port named ones
I think it can be done like this because I''m sure that the xenlinux
based dom0 kernels have the drivers compiled as modules. So if evtchn.ko
exists its xenlinux based, otherwise its pvops. I havent runtime tested
that patch yet.

Olaf

diff -r 7ce01c435f5a tools/hotplug/Linux/init.d/xencommons
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -50,18 +50,39 @@ if test -f /proc/xen/capabilities && \
 	exit 0
 fi
 
+# Load all drivers in a xenlinux based dom0
+do_modprobe_xenlinux() {
+	for mod in gntdev netbk blkbk xen-scsibk usbbk tpmbk pciback
+	do
+		modprobe ${mod} 2>/dev/null
+	done
+}
+
+# Load all drivers in a pvops based dom0
+do_modprobe_pvops() {
+	for mod in evtchn gntdev gntalloc acpi-processor
+	do
+		modprobe xen-${mod} 2>/dev/null
+	done
+	for be in vbd vif pci
+	do
+		modprobe xen-backend:${be} 2>/dev/null
+	done
+}
+
 do_start () {
         local time=0
 	local timeout=30
 
-	modprobe xen-evtchn 2>/dev/null
-	modprobe xen-gntdev 2>/dev/null
-	modprobe xen-gntalloc 2>/dev/null
-	modprobe xen-blkback 2>/dev/null
-	modprobe xen-netback 2>/dev/null
-	modprobe evtchn 2>/dev/null
-	modprobe gntdev 2>/dev/null
-	modprobe xen-acpi-processor 2>/dev/null
+	# Check if dom0 is based on xenlinux, its drivers are all modules
+	# If loading succeeds assume its xenlinux based, otherwise its pvops based
+	if modprobe evtchn 2>/dev/null
+	then
+		do_modprobe_xenlinux
+	else
+		do_modprobe_pvops
+	fi
+
 	mkdir -p /var/run/xen
 
 	if ! `xenstore-read -s / >/dev/null 2>&1`

>>> On 10.08.12 at 17:04, Olaf Hering <olaf@aepfle.de> wrote:
> On Wed, Aug 08, Jan Beulich wrote:
> 
>> >>> On 07.08.12 at 19:22, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
>> >> Jan Beulich writes ("Re: [Xen-devel] [PATCH]
tools/hotplug/Linux/init.d/:
>> >> - not exclusively try the pv-ops kernel''s module
names.
>> > 
>> > Do you mean that 4.2 should try loading some bigger set of module
>> > names ?  If so then do you have a list ? :-)
>> 
>> - xen-blkback''s counterpart is blkbk
>> - xen-netback''s counterpart is netbk
>> 
>> xen-evtchn/evtchn and xen-gntdev/gntdev are already taken
>> care of, albeit in a little strange a way (the two entries being
>> separated by an increasing amount of other ones, when it is
>> really pointless to load the second one if the first one''s
load
>> succeeded).
>> 
>> To not needlessly try everything, it might additionally be worth
>> a thought to
>> - first try loading via module alias rather than module name (if
>>   that succeeds for a carefully chosen module that got its alias
>>   added late - according to our patches, the devname: aliases
>>   got introduced in 2.6.35, and the xen-backend: ones in 3.1 -,
>>   only try loading via module alias for all subsequent ones)
>> - second try loading a (or all) pvops named module(s) (if that/
>>   any of them succeed(s), there''s no need to try _any_ non-
>>   pvops names subsequently, i.e. including ones that don''t
even
>>   exist in the legacy trees)
>> - last try loading the traditional/forward-port named ones
> 
> I think it can be done like this because I''m sure that the
xenlinux
> based dom0 kernels have the drivers compiled as modules. So if evtchn.ko
> exists its xenlinux based, otherwise its pvops. I havent runtime tested
> that patch yet.
That''s the case for our kernels, but doesn''t have to be for
any
derived ones (and there are still a few people cloning our patches).

Jan

On Fri, Aug 10, Jan Beulich wrote:
> That''s the case for our kernels, but doesn''t have to be
for any
> derived ones (and there are still a few people cloning our patches).
Do they build things into the kernel? 
Should some other module than evtchn be used to decide which branch to
take?

Olaf

Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/hotplug/Linux/init.d/:
added other xen kernel modules on xencommons
start"):
> I think it can be done like this because I''m sure that the
xenlinux
> based dom0 kernels have the drivers compiled as modules. So if evtchn.ko
> exists its xenlinux based, otherwise its pvops. I havent runtime tested
> that patch yet.
I was under the impression that there is no harm in attempting to load
nonexistent modules.  So it would surely be better just to expand the
list that we try.

Ian.

>>> On 10.08.12 at 17:10, Olaf Hering <olaf@aepfle.de> wrote:
> On Fri, Aug 10, Jan Beulich wrote:
> 
>> That''s the case for our kernels, but doesn''t have to
be for any
>> derived ones (and there are still a few people cloning our patches).
> 
> Do they build things into the kernel? 
> Should some other module than evtchn be used to decide which branch to
> take?
There are people who build in everything. So you would only
ever be able to derive results from being able to load some
specific module; not being able to load a certain module doesn''t
allow drawing any conclusion.

Jan

On Fri, Aug 10, Jan Beulich wrote:
> >>> On 10.08.12 at 17:10, Olaf Hering <olaf@aepfle.de>
wrote:
> > On Fri, Aug 10, Jan Beulich wrote:
> > 
> >> That''s the case for our kernels, but doesn''t
have to be for any
> >> derived ones (and there are still a few people cloning our
patches).
> > 
> > Do they build things into the kernel? 
> > Should some other module than evtchn be used to decide which branch to
> > take?
> 
> There are people who build in everything. So you would only
> ever be able to derive results from being able to load some
> specific module; not being able to load a certain module doesn''t
> allow drawing any conclusion.
If attempting to load pvops modules in a xenlinux based kernel is an
issue then there needs to be another way to tell them appart. A lame
test would be test -f /proc/xen/balloon which doesnt seem to exist in
pvops dom0.

If attempting to load non-existant modules is not an issue then I will
prepare a patch which adds "netbk blkbk xen-scsibk usbbk pciback"
to the list.

Olaf

Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/hotplug/Linux/init.d/:
added other xen kernel modules on xencommons
start"):
> If attempting to load non-existant modules is not an issue then I will
> prepare a patch which adds "netbk blkbk xen-scsibk usbbk pciback"
> to the list.
The existing scattershot list of modprobes is based on the assumption
that loading non-existent modules is harmless.  So yes please.

Thanks,
Ian.