[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] Solaris dom0 support



On Thu, Jun 14, 2007 at 06:01:56PM -0400, Mark Johnson wrote:
> This is a little bigger of a patch.  It has a couple of things in it.
> 
> First, in xen_internal.c and xs_internal.c have general dom0 support
> for Solaris.
> Again, using #ifdef/ifndef __linux__ to separate the logic.

I'm getting a little confused about the xen_internal.c changes for the
hypercalls.

This is chunk:

> @@ -38,6 +42,8 @@
>  #include "xml.h"
>  
>  /* #define DEBUG */
> +
> +#ifdef __linux__
>  /*
>   * so far there is 2 versions of the structures usable for doing
>  * hypervisor calls.
> @@ -60,6 +66,14 @@ typedef struct v1_hypercall_struct
>      _IOC(_IOC_NONE, 'P', 0, sizeof(v1_hypercall_t))
>  
>  typedef v1_hypercall_t hypercall_t;
> +
> +#else
> +typedef struct v0_hypercall_struct {
> +    unsigned long op;
> +    unsigned long arg[5];
> +} v0_hypercall_t;
> +typedef privcmd_hypercall_t hypercall_t;
> +#endif

Seems to be redefining  v0_hypercall_t  to be the same struct in both
halves of the #ifdef.  But then later on, all references to v0_hypercall_t
in the code are #ifdef'd out for Solaris, and hypercall_t is defined in
terms of   'privcmd_hypercall_t' from the tools/include/SunOS/privcmd.h
which is identical to the v0_hypercall_t we've already got.

I think this could probably be simplified a little, but not quite sure
how just yet. Will look at how the patch applies.

The mlock -> lock_pages()  stuff all makes sense - was expecting that
from the changes you've sent to libxc previously.

The capabilities stuff is interesting - replacing the code which
reads from /sys/hypervisor/properties/capabilities  with a direct
hypercall to fetch it from Xen.  I'll have to check if that hypercall
is actually available on Linux too - if it is, then I'd probably get
rid of the reading of /sys/hypervisor/properties/capabilities completely
and make both platforms do the same hypercall.

> In xml.c and xend_internal.c, the code which requires kernel to be defined
> if ifdef __linux__'d. For our setup, If bootloader, kernel, and
> ramdisk aren't defined, pygrub will got looking for the right bits.

I'm curious as to what the changes for bootloader / kernel are for ?
Surely you always have either a bootloader, or a kenrel present in
the SEXPR ? So I'm not sure why its neccessary to disable the check
for validating presence of one of the two. I'd be interested to see 
the output of 'xm list --long' SEXPR on a Solaris system with a few 
guests running.

> There also code which puts in the the vncpasswd in xend_internal.c.

Yes that's been needed for a while, although I think we want to make
inclusion of the password conditional based on a VIR_SECURE_XML flag
passed into the virDomainDumpXML method. Its quite common for apps to
log XML in various places so rather not have password included by
default all the time unless an app explicitly needs it.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]