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 -=|