[libvirt] [PATCH V5] Add libxenlight driver

[Jim Fehlig]

Daniel Veillard wrote:
> On Wed, Mar 09, 2011 at 11:45:49PM -0700, Jim Fehlig wrote:
>   
>> Add a new xen driver based on libxenlight [1], which is the primary
>> toolstack starting with Xen 4.1.0.  The driver is stateful, runs
>> privileged only, and is accessed with libxl:/// URI.
>>
>> V5:
>>  - Ensure events are unregistered when domain private data
>>    is destroyed.  Discovered and fixed by Markus Gross.
>>
>> V4:
>>  - Handle restart of libvirtd, reconnecting to previously
>>    started domains
>>  - Rebased to current master
>>  - Tested against Xen 4.1 RC7-pre (c/s 22961:c5d121fd35c0)
>>
>> V3:
>>   - Reserve vnc port within driver when autoport=yes
>>
>> V2:
>>   - Update to Xen 4.1 RC6-pre (c/s 22940:5a4710640f81)
>>   - Rebased to current master
>>   - Plug memory leaks found by Stefano Stabellini and valgrind
>>   - Handle SHUTDOWN_crash domain death event
>>
>> [1] http://lists.xensource.com/archives/html/xen-devel/2009-11/msg00436.html
>>     
> [...]
>   
>
>
>   It's great to have documentation, but ...
>   I just regret that we are unable to hide how we connect to the Xen
> server, after all libvirt was precisely designed to try to minimize
> the change on the application stack as the lower layers of
> virtualization evolves, and here we fail.

Yes, well said - and I agree. I mentioned this concern early on, but
unfortunately it was on IRC instead of the list where it could get more
discussion.

>  Sure the URI is a very minimal
> part compared to the actual XML description and code but the fact we are
> using a different driver internally could possibly be masked to the user.
>
>   Can we make an attempt at hiding how we connect to Xen here like we
> did with the "unified" driver but while keeping with different
> subdirectories and drivers.

I'm experimenting with an idea that seems to be quite fruitful. In
daemon/libvirtd.c, I moved the registration of libxl driver before qemu
to prevent qemu from being default if libxenlight is available but xend
is not. And in startup of libxl driver, I try to connect to xen:/// URI
(legacy toolstack will be tried first) and silently disable the driver
if successful.

I've tested this approach with xend disabled, which essentially disables
xen_unified, and libxl driver is selected by default and when specifying
xen:///. With xend enabled, libxl driver is not loaded and xen_unified
is selected by default and when using xen:///. I hope I'm not
overlooking a flaw in this approach :-). Unless there are objections to
this solution, I can provide a much nicer V6! The documentation will
simply be a few comments in the existing xen hypervisor doc.

>  Ideally if virt-manager could connect though
> the new stack without knowing, then that mean we suceeded and really
> adding value here.

I've tested virt-manager and it behaves as you wish :-).

>> +                            (uint8_t *)dom_xml, strlen(dom_xml) + 1)) {
>> +        libxlError(VIR_ERR_INTERNAL_ERROR,
>> +                   _("libxenlight failed to store userdata"));
>> +        goto error;
>> +    }
>>     
>
>   Bonus point for the Xen guys here, the per-domain data storage is a
> great idea, suits us well and simplify data handling a lot ! I assume
> it follows on migrations etc... Maybe we should register "libvirt-xml"
> in their small registry in tools/libxl/libxl.h
>   

Yep, I'll send a patch to xen-devel.

>   It would be nice to grab in domains created by other means if we have
> a chance.
>   

I'm not sure we want to be interfering with domains created by other
libxl clients, which will be waiting for events, domain death, etc. for
domains under their control.

> [...]
>   
>> +static int
>> +libxlStartup(int privileged) {
>> +    const libxl_version_info *ver_info;
>> +    char *log_file = NULL;
>> +
>> +    /* Check that the user is root, silently disable if not */
>> +    if (!privileged) {
>> +        VIR_INFO0("Not running privileged, disabling libxenlight driver");
>> +        return 0;
>> +    }
>>     
>
>   okay so fo local management we will always go though remote to talk to
> the libvirtd daemon, right ?
>   

Yes.

>
>   Okay, I tried t make sure libxl_domain_shutdown() is really
>   asynchronous in all cases, I somehow failed, can you confirm ?
>   

Yes, AFAIK that is the case. Stefano, is that true?

I'll incorporate the rest of your comments in V6 after we have agreed on
a solution to the connection issue.

Thanks!
Jim