[libvirt] [PATCH 1/2] Introduce a lock for libxl long-running api

[Jim Fehlig]

Bamvor Jian Zhang wrote:
>>> +static int 
>>> +libxlDomainAbortJob(virDomainPtr dom) 
>>> +{ 
>>> +    libxlDriverPrivatePtr driver = dom->conn->privateData; 
>>> +    virDomainObjPtr vm; 
>>> +    int ret = -1; 
>>> +    libxlDomainObjPrivatePtr priv; 
>>> + 
>>> +    libxlDriverLock(driver); 
>>> +    vm = virDomainFindByUUID(&driver->domains, dom->uuid); 
>>> +    libxlDriverUnlock(driver); 
>>> +    if (!vm) { 
>>> +        char uuidstr[VIR_UUID_STRING_BUFLEN]; 
>>> +        virUUIDFormat(dom->uuid, uuidstr); 
>>> +        virReportError(VIR_ERR_NO_DOMAIN, 
>>> +                       _("no domain with matching uuid '%s'"), uuidstr); 
>>> +        goto cleanup; 
>>> +    } 
>>> + 
>>> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_ABORT) < 0) 
>>> +        goto cleanup; 
>>> + 
>>> +    if (!virDomainObjIsActive(vm)) { 
>>> +        virReportError(VIR_ERR_OPERATION_INVALID, 
>>> +                       "%s", _("domain is not running")); 
>>> +        goto endjob; 
>>> +    } 
>>> + 
>>> +    priv = vm->privateData; 
>>> + 
>>> +    if (!priv->job.asyncJob) { 
>>> +        virReportError(VIR_ERR_OPERATION_INVALID, 
>>> +                       "%s", _("no job is active on the domain")); 
>>> +        goto endjob; 
>>> +    } else { 
>>> +        virReportError(VIR_ERR_OPERATION_INVALID, 
>>> +                       _("cannot abort %s; use virDomainDestroy instead"), 
>>> +                       libxlDomainAsyncJobTypeToString(priv->job.asyncJob)); 
>>> +        goto endjob; 
>>> +    } 
>>>    
>>>       
>>  
>> This function will always fail with the above logic.  ret is initialized 
>> to -1 and is never changed. 
>>  
>> Is it even possible to safely abort a libxl operation?  If not, this 
>> function should probably remain unimplemented.  Maybe it will be useful 
>> when the libxl driver supports migration. 
>>     
> return error because of the there is no cancelation opeartion in libvirt libxl
> driver with xen 4.1. according to xen4.2 release document, maybe the
> cancelation of long-running jobs is supported.

I finally got some time to take a closer look at Xen 4.2 libxl and
notice that the "long running" operations (create, save, dump, restore,
etc.) now support a 'libxl_asyncop_how *ao_how' parameter to control
their concurrency.  If ao_how->callback is NULL, a libxl_event is
generated when the operation completes.  We'll just need to handle these
events in the existing libxlEventHandler.  Some of the async operations
support reporting intermediate progress (e.g. for
libxlDomainGetJobInfo), but at this time none of them support
cancellation AFAICT.

With the new asynchronous support in Xen 4.2 libxl, IMO we should delay
this patchset until converting the driver to support 4.2.  I didn't
think this patch would be affected by Xen 4.1 vs 4.2 libxl, but it is
and I don't see any reason to add code that further complicates the
conversion.

BTW, Ondrej was working on a patch to convert the driver to 4.2.  Now
that I have some free time, I'll pick up some of this work too.

> but it is still useful for save, dump and migration(in future), because libvirt
> should block the user abort operation othervise xenlight might crash
>   

If it is left unimplemented, libvirt would block the operation anyhow,
failing with "not supported"

Regards,
Jim