[libvirt] [PATCH V4] Add libxenlight driver

[Jim Fehlig]

Markus Gross wrote:
> Am Dienstag 08 März 2011 03:48:55 schrieb Jim Fehlig:
>   
>> 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.
>>
>> 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
>>     
>
>   
>> +static void
>> +libxlDomainObjPrivateFree(void *data)
>> +{
>> +    libxlDomainObjPrivatePtr priv = data;
>> +
>> +    if (priv->dWaiter) {
>> +        libxl_free_waiter(priv->dWaiter);
>> +        VIR_FREE(priv->dWaiter);
>> +    }
>> +    libxl_ctx_free(&priv->ctx);
>> +    VIR_FREE(priv);
>> +}
>>     
>
> When destroying a domain the event handler is called after the domain object 
> is freed and causes a segfault for me. The following patch fixes this.
>   

Nice catch, thanks!

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d137474..303c808 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -85,7 +85,13 @@ libxlDomainObjPrivateFree(void *data)
>  {
>      libxlDomainObjPrivatePtr priv = data;
>
> +    if (priv->waiterFD >= 0) {
> +        if (priv->eventHdl)
> +            virEventRemoveHandle(priv->eventHdl);
> +        VIR_FORCE_CLOSE(priv->waiterFD);
>   

I don't think we should explicitly close waiterFD.  It is closed when
the libxl context is freed.  Stefano, should libxl apps explicitly close
the fd returned by libxl_get_wait_fd()?

> +    }
>      if (priv->dWaiter) {
> +        libxl_stop_waiting(&priv->ctx, priv->dWaiter);
>   

Ah, yes - good call.  In fact, I've changed the libxlVmCleanup function
to also stop waiting, free the waiter, and remove the libvirt event
handle.  This wasn't done in the V4 patch with intention of saving some
work in case of restarting a persistent domain.  But quite a bit of time
may pass between shutting down a persistent domain and starting it
again, so I don't think it is wise to hold these resources.  I'll post a V5.

Thanks Markus!
Jim