On 05/04/2011 08:45 AM, Jiri Denemark wrote:
> Reason is currently always set to 0 (i.e., *_UNKNOWN).
> ---
> +++ b/src/xen/xen_driver.c
> @@ -1010,6 +1010,22 @@ xenUnifiedDomainGetInfo (virDomainPtr dom, virDomainInfoPtr info)
> }
>
> static int
> +xenUnifiedDomainGetState(virDomainPtr dom, int *state, int *reason)
> +{
> + GET_PRIVATE(dom->conn);
> + int i;
> +
> + for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) {
> + if (priv->opened[i] &&
> + drivers[i]->domainGetState &&
> + drivers[i]->domainGetState(dom, state, reason) == 0)
> + return 0;
> + }
...
> +int
> +xenHypervisorGetDomainState(virDomainPtr domain, int *state, int *reason)
You definitely need this function in xen_hypervisor.c, but I still
wonder if the for loop across callbacks in xen_driver.c is appropriate,
given that we are trying to get rid of the secondary callback structure,
or if we should just inline direct calls to the appropriate helper (I
brought up this same point in my review of 2/8). After all, not all
possible xen helpers implement the callback (for example, xen_inotify.c
does not).
> +++ b/src/xen/xen_hypervisor.h
> @@ -66,6 +66,10 @@ int xenHypervisorPauseDomain (virDomainPtr domain)
> int xenHypervisorGetDomainInfo (virDomainPtr domain,
> virDomainInfoPtr info)
> ATTRIBUTE_NONNULL (1);
> +int xenHypervisorGetDomainState (virDomainPtr domain,
> + int *state,
> + int *reason)
> + ATTRIBUTE_NONNULL (1);
That is, either we use the callback structure, and this function could
be private, or we declare this function and call it directly, then we
don't need an extra row in the secondary callback structure.
> +++ b/src/xen/xs_internal.h
> @@ -23,6 +23,9 @@ virDrvOpenStatus xenStoreOpen (virConnectPtr conn,
> int xenStoreClose (virConnectPtr conn);
> int xenStoreGetDomainInfo (virDomainPtr domain,
> virDomainInfoPtr info);
> +int xenStoreDomainGetState (virDomainPtr domain,
Spacing looks awkward here (space vs. tab).
You'll have tweaks to fold in a flag parameter, but overall this patch
looked sane.
ACK.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list