[libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods

[Jim Fehlig]

Daniel P. Berrange wrote:
> On Wed, May 08, 2013 at 11:37:44AM +0100, Daniel P. Berrange wrote:
>   
>> On Mon, May 06, 2013 at 09:40:43PM -0600, Jim Fehlig wrote:
>>     
>>> Jim Fehlig wrote:
>>>       
>>>> Daniel P. Berrange wrote:
>>>>   
>>>>         
>>>>> From: "Daniel P. Berrange" <berrange at redhat.com>
>>>>>
>>>>> Unconditionally invoke the xenHypervisorLookupDomainByID,
>>>>> xenHypervisorLookupDomainByUUID or xenDaemonLookupByName
>>>>> for looking up domains. Fallback to xenXMDomainLookupByUUID
>>>>> and xenXMDomainLookupByName for legacy XenD without inactive
>>>>> domain support
>>>>>   
>>>>>     
>>>>>           
>>>> Do you think there are any Xen installations running such an old xend
>>>> toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the
>>>> XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code.
>>>>
>>>>
>>>>   
>>>>         
>>>>> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
>>>>> ---
>>>>>  src/xen/xen_driver.c    | 99 +++++++++++--------------------------------------
>>>>>  src/xen/xend_internal.c | 89 --------------------------------------------
>>>>>  src/xen/xend_internal.h | 14 -------
>>>>>  src/xen/xs_internal.c   | 62 -------------------------------
>>>>>  src/xen/xs_internal.h   |  2 -
>>>>>  5 files changed, 22 insertions(+), 244 deletions(-)
>>>>>   
>>>>>     
>>>>>           
>>>> I spent some time testing this one and didn't notice any problems.
>>>>   
>>>>         
>>> Apparently "some" time was not enough time. With this patch, I noticed
>>> 'virsh undefine dom' failing because the tri-state virDomainIsActive()
>>> is returning -1.
>>>       
>> Opps, I made a mistake only checking the hypervisor, which of course
>> will not know about the domain if it is shutoff :-)  Adding the following
>> extra hunk fixes it
>>     
>
> Sigh, helps if i paste the right patch
>
> @@ -660,15 +660,26 @@ xenUnifiedDomainLookupByName(virConnectPtr conn,
>  static int
>  xenUnifiedDomainIsActive(virDomainPtr dom)
>  {
> +    xenUnifiedPrivatePtr priv = dom->conn->privateData;
>      virDomainPtr currdom;
>      int ret = -1;
>  
>      /* ID field in dom may be outdated, so re-lookup */
>      currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid);
>  
> +    /* Try XM for inactive domains. */
> +    if (!currdom) {
> +        if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
> +            currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid);
> +        else
> +            currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid);
> +    }
> +
>      if (currdom) {
>          ret = currdom->id == -1 ? 0 : 1;
>          virDomainFree(currdom);
> +    } else if (virGetLastError() == NULL) {
> +        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
>      }
>  
>      return ret;
>   

Yep, this fixes the issue.  ACK with it squashed in.

Regards,
Jim