[libvirt] [PATCH 7/8] Implement basic virDomainGetState in all drivers

Eric Blake eblake at redhat.com
Thu May 5 21:57:08 UTC 2011


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110505/e389bfd6/attachment-0001.sig>


More information about the libvir-list mailing list