Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
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(a)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(a)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
@@ -664,11 +664,21 @@ xenUnifiedDomainIsActive(virDomainPtr dom)
int ret = -1;
/* ID field in dom may be outdated, so re-lookup */
- currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid);
+ currdom = xenHypervisorLookupDomainByUUID(conn, uuid);
+
+ /* Try XM for inactive domains. */
+ if (!currdom) {
+ if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
+ currdom = xenXMDomainLookupByUUID(conn, uuid);
+ else
+ currdom = xenDaemonLookupByUUID(conn, uuid);
+ }
if (currdom) {
ret = currdom->id == -1 ? 0 : 1;
virDomainFree(currdom);
+ } else if (virGetLastError() == NULL) {
+ virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
}
return ret;
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|