xl: Enable 'xl console' in 'x86_64'

xl: Enable 'xl console' in 'x86_64'

[Yu Zhiguo]

'xl console' cannot be used on arch 'x86_64',
because the path of 'xenconsole' is wrong.
Fix this bug.

Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>

diff -r c87ec146229a -r bb537e15d23a tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Apr 23 15:04:26 2010 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 29 02:23:21 2010 +0800
@@ -28,6 +28,7 @@
 #include <stdint.h>
 #include <inttypes.h>
 #include <assert.h>
+#include <sys/utsname.h>
 
 #include "libxl.h"
 #include "libxl_utils.h"
@@ -741,8 +742,13 @@
 {
     struct stat st;
     const char *XENCONSOLE = "/usr/lib/xen/bin/xenconsole";
+    struct utsname utsbuf;
     char *cmd;
 
+    if (uname(&utsbuf) != -1) {
+        if (!strcmp(utsbuf.machine, "x86_64") || !strcmp(utsbuf.machine, "ia64"))
+            XENCONSOLE = "/usr/lib64/xen/bin/xenconsole";
+    }
     if (stat(XENCONSOLE, &st) != 0) {
         XL_LOG(ctx, XL_LOG_ERROR, "could not access %s", XENCONSOLE);
         return ERROR_FAIL;

Re: xl: Enable 'xl console' in 'x86_64'

[Jeremy Fitzhardinge]

On 04/28/2010 03:29 AM, Yu Zhiguo wrote:
> 'xl console' cannot be used on arch 'x86_64',
> because the path of 'xenconsole' is wrong.
> Fix this bug.
>
> Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>
>
> diff -r c87ec146229a -r bb537e15d23a tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Fri Apr 23 15:04:26 2010 +0100
> +++ b/tools/libxl/libxl.c	Thu Apr 29 02:23:21 2010 +0800
> @@ -28,6 +28,7 @@
>  #include <stdint.h>
>  #include <inttypes.h>
>  #include <assert.h>
> +#include <sys/utsname.h>
>  
>  #include "libxl.h"
>  #include "libxl_utils.h"
> @@ -741,8 +742,13 @@
>  {
>      struct stat st;
>      const char *XENCONSOLE = "/usr/lib/xen/bin/xenconsole";
> +    struct utsname utsbuf;
>      char *cmd;
>  
> +    if (uname(&utsbuf) != -1) {
> +        if (!strcmp(utsbuf.machine, "x86_64") || !strcmp(utsbuf.machine, "ia64"))
> +            XENCONSOLE = "/usr/lib64/xen/bin/xenconsole";
> +    }
>   

Won't this fail if the tools were build 32bit, but being run on a 64bit
machine?  Why not just look in both places?  Or fix the xenconsole to
install in /usr/lib regardless of architecture (it isn't a library, so
nothing will care about its architecture).

    J

Re: xl: Enable 'xl console' in 'x86_64'

[Yu Zhiguo]

Hi Jeremy

Jeremy Fitzhardinge wrote:
>>  
>> +    if (uname(&utsbuf) != -1) {
>> +        if (!strcmp(utsbuf.machine, "x86_64") || !strcmp(utsbuf.machine, "ia64"))
>> +            XENCONSOLE = "/usr/lib64/xen/bin/xenconsole";
>> +    }
>>   
> 
> Won't this fail if the tools were build 32bit, but being run on a 64bit
> machine?  Why not just look in both places?  Or fix the xenconsole to
> install in /usr/lib regardless of architecture (it isn't a library, so
> nothing will care about its architecture).

Thanks for you reply. I think just look up both location is better.
Please refer to the following.

-------------

'xl console' cannot be used on arch 'x86_64',
because the path of 'xenconsole' is wrong.
Fix this bug.

Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>

diff -r 9a1d7caa2024 -r 49176e864ca7 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Apr 26 12:13:23 2010 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 29 18:36:28 2010 +0800
@@ -741,15 +741,19 @@
 {
     struct stat st;
     const char *XENCONSOLE = "/usr/lib/xen/bin/xenconsole";
+    const char *XENCONSOLE64 = "/usr/lib64/xen/bin/xenconsole";
     char *cmd;
 
-    if (stat(XENCONSOLE, &st) != 0) {
-        XL_LOG(ctx, XL_LOG_ERROR, "could not access %s", XENCONSOLE);
+    if (!stat(XENCONSOLE, &st)) {
+        cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE, domid, cons_num);
+        return (system(cmd) != 0) ? ERROR_FAIL : 0;
+    } else if (!stat(XENCONSOLE64, &st)) {
+        cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE64, domid, cons_num);
+        return (system(cmd) != 0) ? ERROR_FAIL : 0;
+    } else {
+        XL_LOG(ctx, XL_LOG_ERROR, "could not access %s and %s", XENCONSOLE, XENCONSOLE64);
         return ERROR_FAIL;
     }
-
-    cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE, domid, cons_num);
-    return (system(cmd) != 0) ? ERROR_FAIL : 0;
 }
 
 static char ** libxl_build_device_model_args(struct libxl_ctx *ctx,

Re: xl: Enable 'xl console' in 'x86_64'

[Jeremy Fitzhardinge]

On 04/28/2010 07:38 PM, Yu Zhiguo wrote:
> Hi Jeremy
>
> Jeremy Fitzhardinge wrote:
>   
>>>  
>>> +    if (uname(&utsbuf) != -1) {
>>> +        if (!strcmp(utsbuf.machine, "x86_64") || !strcmp(utsbuf.machine, "ia64"))
>>> +            XENCONSOLE = "/usr/lib64/xen/bin/xenconsole";
>>> +    }
>>>   
>>>       
>> Won't this fail if the tools were build 32bit, but being run on a 64bit
>> machine?  Why not just look in both places?  Or fix the xenconsole to
>> install in /usr/lib regardless of architecture (it isn't a library, so
>> nothing will care about its architecture).
>>     
> Thanks for you reply. I think just look up both location is better.
>   

Hm, I think just installing xenconsole (and the other executables) in a
consistent (architecture-independent) place is a better long-term fix. 
That said, hard-coding paths like this seems like it would be awkward
for anyone packaging this stuff (I think distros would prefer to put
this kind of thing in /usr/libexec?).

    J

> Please refer to the following.
>
> -------------
>
> 'xl console' cannot be used on arch 'x86_64',
> because the path of 'xenconsole' is wrong.
> Fix this bug.
>
> Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>
>
> diff -r 9a1d7caa2024 -r 49176e864ca7 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Mon Apr 26 12:13:23 2010 +0100
> +++ b/tools/libxl/libxl.c	Thu Apr 29 18:36:28 2010 +0800
> @@ -741,15 +741,19 @@
>  {
>      struct stat st;
>      const char *XENCONSOLE = "/usr/lib/xen/bin/xenconsole";
> +    const char *XENCONSOLE64 = "/usr/lib64/xen/bin/xenconsole";
>      char *cmd;
>  
> -    if (stat(XENCONSOLE, &st) != 0) {
> -        XL_LOG(ctx, XL_LOG_ERROR, "could not access %s", XENCONSOLE);
> +    if (!stat(XENCONSOLE, &st)) {
> +        cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE, domid, cons_num);
> +        return (system(cmd) != 0) ? ERROR_FAIL : 0;
> +    } else if (!stat(XENCONSOLE64, &st)) {
> +        cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE64, domid, cons_num);
> +        return (system(cmd) != 0) ? ERROR_FAIL : 0;
> +    } else {
> +        XL_LOG(ctx, XL_LOG_ERROR, "could not access %s and %s", XENCONSOLE, XENCONSOLE64);
>          return ERROR_FAIL;
>      }
> -
> -    cmd = libxl_sprintf(ctx, "%s %d --num %d", XENCONSOLE, domid, cons_num);
> -    return (system(cmd) != 0) ? ERROR_FAIL : 0;
>  }
>  
>  static char ** libxl_build_device_model_args(struct libxl_ctx *ctx,
>
>   

Re: xl: Enable 'xl console' in 'x86_64'

[Keir Fraser]

On 29/04/2010 09:38, "Jeremy Fitzhardinge" <jeremy@goop.org> wrote:

>> Thanks for you reply. I think just look up both location is better.
> 
> Hm, I think just installing xenconsole (and the other executables) in a
> consistent (architecture-independent) place is a better long-term fix.
> That said, hard-coding paths like this seems like it would be awkward
> for anyone packaging this stuff (I think distros would prefer to put
> this kind of thing in /usr/libexec?).

The build system knows where it is installing stuff. The answer here is to
push the paths from the build system into a header file, then we will also
work with non-standard install prefixes and the like. Kind of like what xend
does with util/auxbin.py, but we don't need something as complex as that. I
will pick up this work item myself, since it is very simple. ;-)

 -- Keir

Re: xl: Enable 'xl console' in 'x86_64'

[Keir Fraser]

On 29/04/2010 10:54, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

>> Hm, I think just installing xenconsole (and the other executables) in a
>> consistent (architecture-independent) place is a better long-term fix.
>> That said, hard-coding paths like this seems like it would be awkward
>> for anyone packaging this stuff (I think distros would prefer to put
>> this kind of thing in /usr/libexec?).
> 
> The build system knows where it is installing stuff. The answer here is to
> push the paths from the build system into a header file, then we will also
> work with non-standard install prefixes and the like. Kind of like what xend
> does with util/auxbin.py, but we don't need something as complex as that. I
> will pick up this work item myself, since it is very simple. ;-)

Now applied as xen-unstable:21237:a167ea374f26

 -- Keir