Xen devel - Jun 2010 - [PATCH 1 of 1] stubdom hanging at creation

The following patch is a first fix for qemu to rectify stubdom hanging 
at boot time.

It leads to another issue when the device model tries to attach console-1 
(I''m using linux-2.6-pvops.git as dom0):
- tools/ioemu-remote/hw/xen_console.c: ''con_connect'' calls 
''xc_gnttab_map_grant_ref'' which never returns,
- tools/libxc/xc_linux.c: ''xc_gnttab_map_grant_ref'' hangs
calling ''mmap'',
- linux-2.6-pvops.git/drivers/xen/gntdev.c: ''gntdev_mmap''
calls
''apply_to_page_range'' and never returns.

A short gntdev debug log:
[  495.734675] gntdev_ioctl_map_grant_ref: priv ffff88002c6a8f40, add 1
[  495.735544] gntdev_print_maps: maps list (priv ffff88002c6a8f40, usage 
1/1024)
[  495.735544]   index  0, count  1 [new]
[  495.757925] gntdev_mmap: map 0+1 at 7fc6fa16a000 (pgoff 0)

Aftermath, either xl/xm or any xen tool hang without any output. Still 
dom0 seems to run ''almost'' normaly. Any idea about this ?

diff --git a/vl.c b/vl.c
index 404d67a..e86d05d 100644
--- a/vl.c
+++ b/vl.c
@@ -5967,21 +5967,21 @@ int main(int argc, char **argv, char **envp)
      /* just use the first displaystate for the moment */
      ds = display_state;
      /* terminal init */
-#ifdef CONFIG_STUBDOM
-    if (xenfb_pv_display_init(ds) == 0) {
-    } else
-#endif
      if (nographic) {
          if (curses) {
              fprintf(stderr, "fatal: -nographic can''t be used
with -curses\n");
              exit(1);
          }
      } else { 
+#if defined(CONFIG_STUBDOM)
+        if (xenfb_pv_display_init(ds) == 0) {
+        } else
+#endif
  #if defined(CONFIG_CURSES)
-            if (curses) {
-                /* At the moment curses cannot be used with other displays */
-                curses_display_init(ds, full_screen);
-            } else
+        if (curses) {
+            /* At the moment curses cannot be used with other displays */
+            curses_display_init(ds, full_screen);
+        } else
  #endif
              {
                  if (vnc_display != NULL || vncunused != 0) {

-- 
Eric Chanudet

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Eric Chanudet writes ("[Xen-devel] [PATCH 1 of 1] stubdom hanging at
creation"):
> The following patch is a first fix for qemu to rectify stubdom hanging 
> at boot time.
...
> +#if defined(CONFIG_STUBDOM)
> +        if (xenfb_pv_display_init(ds) == 0) {
> +        } else
> +#endif
This part looks plausible
>   #if defined(CONFIG_CURSES)
> -            if (curses) {
> -                /* At the moment curses cannot be used with other displays
*/
> -                curses_display_init(ds, full_screen);
> -            } else
> +        if (curses) {
> +            /* At the moment curses cannot be used with other displays */
> +            curses_display_init(ds, full_screen);
> +        } else
>   #endif
Isn''t that just an unintended whitespace change ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Isn''t that just an unintended whitespace change ?
It is re-indentation actually, sorry about that. Here is the refreshed 
patch without it.

diff --git a/vl.c b/vl.c
index 404d67a..0d9a3b4 100644
--- a/vl.c
+++ b/vl.c
@@ -5967,16 +5967,16 @@ int main(int argc, char **argv, char **envp)
      /* just use the first displaystate for the moment */
      ds = display_state;
      /* terminal init */
-#ifdef CONFIG_STUBDOM
-    if (xenfb_pv_display_init(ds) == 0) {
-    } else
-#endif
      if (nographic) {
          if (curses) {
              fprintf(stderr, "fatal: -nographic can''t be used
with -curses\n");
              exit(1);
          }
      } else { 
+#if defined(CONFIG_STUBDOM)
+        if (xenfb_pv_display_init(ds) == 0) {
+        } else
+#endif
  #if defined(CONFIG_CURSES)
              if (curses) {
                  /* At the moment curses cannot be used with other displays */

-- 
Eric Chanudet


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Eric Chanudet writes ("Re: [Xen-devel] [PATCH 1 of 1] stubdom hanging at
creation"):
> It is re-indentation actually, sorry about that. Here is the refreshed 
> patch without it.
Thanks.  I have applied this.  I had to do it manually because your
patch still didn''t apply properly; the indentation in the original
code in your patch did not match that in qemu-xen-unstable.

Also you did this:
> -#ifdef CONFIG_STUBDOM
> +#if defined(CONFIG_STUBDOM)
Deliberately ?

And finally you should submit your next patch with a Signed-off-by
line like this
    Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
except with your name and email address.  This indicates that you are
certifying the code as suitable (copyright-wise and so on) for
inclusion in Xen.  You can find the precise meaning in the Linux
upstream kernel tree (Documentation/SubmittingPatches, copy below).

In this case, and since your patch was so small, I took your messages
to grant the relevant permissions.

Thanks,
Ian.
>From Documentation/SubmittingPatches:
        Developer''s Certificate of Origin 1.1

        By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.

-- 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 29 Jun 2010, Ian Jackson wrote:
> Also you did this:
>> -#ifdef CONFIG_STUBDOM
>> +#if defined(CONFIG_STUBDOM)
> Deliberately ?
Yes, to remain consistent with this piece of code, which uses this syntax 
for CONFIG_CURSES, CONFIG_SDL, etc, tested few line further. It is 
probably the same thing that goes through my mind when I changed the 
indentation.
> And finally you should submit your next patch with a Signed-off-by
I will, thank you for the documentation.

-- 
Eric Chanudet


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel