[PATCH 0 of 3] Patches when Linux Xen drivers are modules.

[PATCH 0 of 3] Patches when Linux Xen drivers are modules.

[Konrad Rzeszutek Wilk]

Keir, and Xen-devel mailing list,

Attached are three patches that help Dom0 and DomU respectivly when
both Xen backend and Xen-frontend drivers are modules.

a) The first two are to make 'xen-detect' be more useful in initrd
/initramfs situations. It can return 1 when running in PV context,
2 in HVM context, and 0 if Xen has not been detected. Also there are
three arguments: -P, -H, and -N, which will print a string value
if we are in either PV, HVM or no Xen context, respectivly.

An example user of this patch to Fedora's dracut: 
http://article.gmane.org/gmane.linux.kernel.initramfs/1252

b) The xend init script is to load backend drivers if they
are modules.

[PATCH 1 of 3] If Xen backend modules are not loaded, load them before starting Xend

[Konrad Rzeszutek Wilk]

# HG changeset patch
# User konrad@phenom.dumpdata.com
# Date 1261439503 18000
# Node ID 7bd805a543da685a3dba7b8b0c4e3e7052c863b3
# Parent  fc27db3a2ddb3f0843841748684547f6e2ed1f02
If Xen backend modules are not loaded, load them before starting Xend.

The privileged domain can now have more Xen drivers be modules.
Lets take advantage of that and load them if they had not been
loaded yet.

diff -r fc27db3a2ddb -r 7bd805a543da tools/hotplug/Linux/init.d/xend
--- a/tools/hotplug/Linux/init.d/xend	Mon Dec 21 18:32:22 2009 -0500
+++ b/tools/hotplug/Linux/init.d/xend	Mon Dec 21 18:51:43 2009 -0500
@@ -28,6 +28,7 @@
      grep '	xenfs$' /proc/filesystems >/dev/null && \
    ! grep '^xenfs ' /proc/mounts >/dev/null;
 then
+	modprobe -q xenfs
 	mount -t xenfs xenfs /proc/xen
 fi
 
@@ -35,6 +36,14 @@
 	exit 0
 fi
 
+# Load the proper modules
+function load_modules
+{
+	test -d /sys/module/xen_blkback || modprobe -q xen_blkback
+	test -d /sys/module/xen_netback || modprobe -q xen_netback
+	test -d /sys/module/xen_evtchn || modprobe -q xen_evtchn
+	test -d /sys/module/pciback || modprobe -q xen_pciback
+}
 # Wait for Xend to be up
 function await_daemons_up
 {
@@ -57,6 +66,7 @@
 	test -z "$XENCONSOLED_TRACE" || export XENCONSOLED_TRACE
 	[[ "$XENSTORED_TRACE" == @(yes|on|1) ]] && export XENSTORED_TRACE
 	[[ "$XENBACKENDD_DEBUG" == @(yes|on|1) ]] && export XENBACKENDD_DEBUG
+	load_modules
 	xend start
 	await_daemons_up
 	;;

Re: [PATCH 1 of 3] If Xen backend modules are not loaded, load them before starting Xend

[Ian Campbell]

On Tue, 2009-12-22 at 16:48 +0000, Konrad Rzeszutek Wilk wrote:
> @@ -28,6 +28,7 @@
>       grep '    xenfs$' /proc/filesystems >/dev/null && \
>     ! grep '^xenfs ' /proc/mounts >/dev/null;
>  then
> +       modprobe -q xenfs
>         mount -t xenfs xenfs /proc/xen
>  fi
>   

I think this one gets requested automatically if the filesystem isn't
currently present, see the call to request_module in
fs/filesystems.c:get_fs_type()

Ian.

Re: [PATCH 1 of 3] If Xen backend modules are not loaded, load them before starting Xend

[Konrad Rzeszutek Wilk]

On Tue, Dec 22, 2009 at 07:41:17PM +0000, Ian Campbell wrote:
> On Tue, 2009-12-22 at 16:48 +0000, Konrad Rzeszutek Wilk wrote:
> > @@ -28,6 +28,7 @@
> >       grep '    xenfs$' /proc/filesystems >/dev/null && \
> >     ! grep '^xenfs ' /proc/mounts >/dev/null;
> >  then
> > +       modprobe -q xenfs
> >         mount -t xenfs xenfs /proc/xen
> >  fi
> >   
> 
> I think this one gets requested automatically if the filesystem isn't
> currently present, see the call to request_module in
> fs/filesystems.c:get_fs_type()

Hmm, in 2.6.32 it looks to be usuable only once the filesystem has
loaded. This patch is for the case where the filesystem (xenfs) is compiled
as a module - and had not been loaded. Hence hadn't had a chance to call
fs/filesystem.c:register_filesystem().

Thought maybe I messing up this up. The "grep 'xenfs$' /proc/filesystem" checks
for xenfs in /proc/filesystem so it should not even get to the "then" statement.

Something is fishy here. Why don't we skip this patch and I will revist it
after holidays.

Re: [PATCH 1 of 3] If Xen backend modules are not loaded, load them before starting Xend

[Ian Campbell]

On Tue, 2009-12-22 at 19:57 +0000, Konrad Rzeszutek Wilk wrote: 
> On Tue, Dec 22, 2009 at 07:41:17PM +0000, Ian Campbell wrote:
> > On Tue, 2009-12-22 at 16:48 +0000, Konrad Rzeszutek Wilk wrote:
> > > @@ -28,6 +28,7 @@
> > >       grep '    xenfs$' /proc/filesystems >/dev/null && \
> > >     ! grep '^xenfs ' /proc/mounts >/dev/null;
> > >  then
> > > +       modprobe -q xenfs
> > >         mount -t xenfs xenfs /proc/xen
> > >  fi
> > >   
> > 
> > I think this one gets requested automatically if the filesystem isn't
> > currently present, see the call to request_module in
> > fs/filesystems.c:get_fs_type()
> 
> Hmm, in 2.6.32 it looks to be usuable only once the filesystem has
> loaded. This patch is for the case where the filesystem (xenfs) is compiled
> as a module - and had not been loaded. Hence hadn't had a chance to call
> fs/filesystem.c:register_filesystem().

Are you looking at __get_fs_type() and not get_fs_type()? The later is: 
        struct file_system_type *get_fs_type(const char *name)
        {
                struct file_system_type *fs;
                const char *dot = strchr(name, '.');
                int len = dot ? dot - name : strlen(name);
        
                fs = __get_fs_type(name, len);
                if (!fs && (request_module("%.*s", len, name) == 0))
                        fs = __get_fs_type(name, len);
        ....

So either the module is already loaded (or its statically compiled) or
it calls request_module to cause it to become loaded and the second
__get_fs_type succeeds. 

> Thought maybe I messing up this up. The "grep 'xenfs$' /proc/filesystem" checks
> for xenfs in /proc/filesystem so it should not even get to the "then" statement.

Yes, the script looks wrong to me before your patch as well, the check
of /proc/filesystem should be removed.

> Something is fishy here. Why don't we skip this patch and I will revist it
> after holidays.
> 

OK

Ian.

[PATCH 2 of 3] [xen-detect] Return 0 if no Xen detected; 1 if running in PV context; and 2 if in HVM context

[Konrad Rzeszutek Wilk]

# HG changeset patch
# User konrad@phenom.dumpdata.com
# Date 1261438342 18000
# Node ID fc27db3a2ddb3f0843841748684547f6e2ed1f02
# Parent  402aa73665eddb3f7b3ede0a8894acd24397218d
[xen-detect] Return 0 if no Xen detected; 1 if running in PV context; and 2 if in HVM context.

On top of printing whether we are in HVM or PV context (or none
at all), also return a value.

diff -r 402aa73665ed -r fc27db3a2ddb tools/misc/xen-detect.c
--- a/tools/misc/xen-detect.c	Mon Dec 21 17:50:08 2009 -0500
+++ b/tools/misc/xen-detect.c	Mon Dec 21 18:32:22 2009 -0500
@@ -68,7 +68,8 @@
     cpuid(base + 1, &eax, &ebx, &ecx, &edx, pv_context);
     printf("Running in %s context on Xen v%d.%d.\n",
            pv_context ? "PV" : "HVM", (uint16_t)(eax >> 16), (uint16_t)eax);
-    return 1;
+
+    return pv_context ? 1 : 2;
 }
 
 static jmp_buf sigill_jmp;
@@ -79,9 +80,12 @@
 
 int main(void)
 {
+    /* 0 - no Xen, 1 - PV Xen, 2 - HVM Xen */
+    int rc = 0;
+
     /* Check for execution in HVM context. */
-    if ( check_for_xen(0) )
-        return 0;
+    if ( (rc = check_for_xen(0)) )
+        return rc;
 
     /*
      * Set up a signal handler to test the paravirtualised CPUID instruction.
@@ -90,8 +94,8 @@
      */
     if ( !setjmp(sigill_jmp)
          && (signal(SIGILL, sigill_handler) != SIG_ERR)
-         && check_for_xen(1) )
-        return 0;
+         && (rc = check_for_xen(1)) )
+        return rc;
 
     printf("Not running on Xen.\n");
     return 0;

Re: [PATCH 2 of 3] [xen-detect] Return 0 if no Xen detected; 1 if running in PV context; and 2 if in HVM context

[Keir Fraser]

On 22/12/2009 16:48, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:

> [xen-detect] Return 0 if no Xen detected; 1 if running in PV context; and 2 if
> in HVM context.
> 
> On top of printing whether we are in HVM or PV context (or none
> at all), also return a value.

Returning an enumeration seems moderately hard to use and also non-zero
return values conventionally mean error or at least 'exceptional case'
(e.g., grep finding nothing to print).

Se my response to patch 3/3 for my alternative suggestion.

 -- Keir

[PATCH 3 of 3] [xen-detect] Add arguments to print out only outputs we are interested in

[Konrad Rzeszutek Wilk]

# HG changeset patch
# User konrad@phenom.dumpdata.com
# Date 1261498156 18000
# Node ID 04c067941b17bdc679ffdc2f6c81f8e2f78e9cc5
# Parent  7bd805a543da685a3dba7b8b0c4e3e7052c863b3
[xen-detect] Add arguments to print out only outputs we are interested in.

This is quite usefull in scripts where you can do
if xen-detect -P || modprobe xen-<some module>

And do not have to parse the output - instead you will get
the output only if the condition exists.

diff -r 7bd805a543da -r 04c067941b17 tools/misc/xen-detect.c
--- a/tools/misc/xen-detect.c	Mon Dec 21 18:51:43 2009 -0500
+++ b/tools/misc/xen-detect.c	Tue Dec 22 11:09:16 2009 -0500
@@ -29,6 +29,12 @@
 #include <string.h>
 #include <setjmp.h>
 #include <signal.h>
+#include <unistd.h>
+
+/* Which outputs should be displayed. */
+int disp_HVM = 1;
+int disp_PV = 1;
+int disp_NOXEN = 1;
 
 static void cpuid(uint32_t idx,
                   uint32_t *eax,
@@ -66,8 +72,9 @@
 
  found:
     cpuid(base + 1, &eax, &ebx, &ecx, &edx, pv_context);
-    printf("Running in %s context on Xen v%d.%d.\n",
-           pv_context ? "PV" : "HVM", (uint16_t)(eax >> 16), (uint16_t)eax);
+    if ((pv_context &&  disp_PV) || (!pv_context && disp_HVM))
+        printf("Running in %s context on Xen v%d.%d.\n",
+                pv_context ? "PV" : "HVM", (uint16_t)(eax >> 16), (uint16_t)eax);
 
     return pv_context ? 1 : 2;
 }
@@ -78,10 +85,42 @@
     longjmp(sigill_jmp, 1);
 }
 
-int main(void)
+static void usage(void)
+{
+    printf("usage:\n\n");
+    printf("  xen_detect [options]\n\n");
+    printf("options:\n");
+    printf("   -P,  print output if running in PV context.\n");
+    printf("   -H,  print output if running in HVM context.\n");
+    printf("   -N,  print output if not running in any Xen context.\n");
+};
+
+int main(int argc, char **argv)
 {
     /* 0 - no Xen, 1 - PV Xen, 2 - HVM Xen */
     int rc = 0;
+    int ch;
+
+    while ((ch = getopt(argc, argv, "PHN?h")) != -1 ) {
+        switch (ch) {
+        case 'h':
+        case '?':
+                usage();
+                return(-1);
+        case 'H':
+                disp_HVM = 1;
+                disp_PV = disp_NOXEN = 0;
+                break;
+        case 'P':
+                disp_PV = 1;
+                disp_HVM = disp_NOXEN = 0;
+                break;
+        case 'N':
+                disp_NOXEN = 1;
+                disp_PV = disp_HVM = 0;
+                break;
+        }
+    }
 
     /* Check for execution in HVM context. */
     if ( (rc = check_for_xen(0)) )
@@ -97,6 +136,7 @@
          && (rc = check_for_xen(1)) )
         return rc;
 
-    printf("Not running on Xen.\n");
+    if (disp_NOXEN)
+        printf("Not running on Xen.\n");
     return 0;
 }

Re: [PATCH 3 of 3] [xen-detect] Add arguments to print out only outputs we are interested in

[Keir Fraser]

On 22/12/2009 16:48, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:

> # HG changeset patch
> # User konrad@phenom.dumpdata.com
> # Date 1261498156 18000
> # Node ID 04c067941b17bdc679ffdc2f6c81f8e2f78e9cc5
> # Parent  7bd805a543da685a3dba7b8b0c4e3e7052c863b3
> [xen-detect] Add arguments to print out only outputs we are interested in.
> 
> This is quite usefull in scripts where you can do
> if xen-detect -P || modprobe xen-<some module>
> 
> And do not have to parse the output - instead you will get
> the output only if the condition exists.

Er, what? The above shell fragment is affected by the value returned by
xen-detect (0 versus non-0), not by whether it prints something!

Here is my suggestion: keep your new command-line options, but instead of
affecting printing, have them affect the return value. E.g., -P causes
return 0 if running PV, else return 1. Further, we can add a -q option to
quiesce normal output from xen-detect, to make it quieter for use in shell
scripts.

E.g, xen-detect -Pq && modprobe some-xen-pv-specific-module

What do you think of that?

 -- Keir

Re: [PATCH 3 of 3] [xen-detect] Add arguments to print out only outputs we are interested in

[Konrad Rzeszutek Wilk]

On Tue, Dec 22, 2009 at 08:21:14PM +0000, Keir Fraser wrote:
> On 22/12/2009 16:48, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
> 
> > # HG changeset patch
> > # User konrad@phenom.dumpdata.com
> > # Date 1261498156 18000
> > # Node ID 04c067941b17bdc679ffdc2f6c81f8e2f78e9cc5
> > # Parent  7bd805a543da685a3dba7b8b0c4e3e7052c863b3
> > [xen-detect] Add arguments to print out only outputs we are interested in.
> > 
> > This is quite usefull in scripts where you can do
> > if xen-detect -P || modprobe xen-<some module>
> > 
> > And do not have to parse the output - instead you will get
> > the output only if the condition exists.
> 
> Er, what? The above shell fragment is affected by the value returned by
> xen-detect (0 versus non-0), not by whether it prints something!

Wrong combination. Here is what the Dracut maintainer suggested:

[konrad@phenom ~]$ if xen-detect; then echo blah; fi
Not running on Xen.
blah

Which is not what I wanted it to do. Having the -N, -P, -H arguments
fixed the issue.

> 
> Here is my suggestion: keep your new command-line options, but instead of
> affecting printing, have them affect the return value. E.g., -P causes
> return 0 if running PV, else return 1. Further, we can add a -q option to
> quiesce normal output from xen-detect, to make it quieter for use in shell
> scripts.
> 
> E.g, xen-detect -Pq && modprobe some-xen-pv-specific-module
> 
> What do you think of that?

That should do it. Will post a rework after the X-mas holidays.

Re: [PATCH 3 of 3] [xen-detect] Add arguments to print out only outputs we are interested in

[Keir Fraser]

On 22/12/2009 20:26, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:

>> Er, what? The above shell fragment is affected by the value returned by
>> xen-detect (0 versus non-0), not by whether it prints something!
> 
> Wrong combination. Here is what the Dracut maintainer suggested:
> That should do it. Will post a rework after the X-mas holidays.

I'll sort this one myself, based on your patch for the getopt() stuff. Will
only take a few minutes.

 Thanks,
 Keir