[libvirt] [PATCH 3/4] xen: make direct call when there is only one subdriver

Laine Stump laine at laine.org
Thu Jul 28 20:00:28 UTC 2011


On 07/21/2011 05:39 PM, Eric Blake wrote:
> No need to use a for loop if we know there is exactly one client.
>
> * src/xen/xen_driver.c (xenUnifiedClose): Call close
> unconditionally, to match xenUnifiedOpen.
> (xenUnifiedNodeGetInfo, xenUnifiedDomainCreateXML)
> (xenUnifiedDomainSave, xenUnifiedDomainRestore)
> (xenUnifiedDomainCoreDump, xenUnifiedDomainUpdateDeviceFlags):
> Make direct call to lone implementation.
> * src/xen/xend_internal.h (xenDaemonDomainCoreDump)
> (xenDaemonUpdateDeviceFlags, xenDaemonCreateXML): Add prototypes.
> * src/xen/xend_internal.c (xenDaemonDomainCoreDump)
> (xenDaemonUpdateDeviceFlags, xenDaemonCreateXML): Export.
> ---
>   src/xen/xen_driver.c    |   61 ++++++++++++----------------------------------
>   src/xen/xend_internal.c |    6 ++--
>   src/xen/xend_internal.h |    6 ++++
>   3 files changed, 25 insertions(+), 48 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 2bad8c4..b7122f0 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -414,7 +414,8 @@ fail:
>   clean:
>       VIR_DEBUG("Failed to activate a mandatory sub-driver");
>       for (i = 0 ; i<  XEN_UNIFIED_NR_DRIVERS ; i++)
> -        if (priv->opened[i]) drivers[i]->xenClose(conn);
> +        if (priv->opened[i])
> +            drivers[i]->xenClose(conn);


I only see whitespace change here. Was there supposed to be more?


>       virMutexDestroy(&priv->lock);
>       VIR_FREE(priv);
>       conn->privateData = NULL;
> @@ -434,8 +435,8 @@ xenUnifiedClose (virConnectPtr conn)
>       virDomainEventCallbackListFree(priv->domainEventCallbacks);
>
>       for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&  drivers[i]->xenClose)
> -            (void) drivers[i]->xenClose (conn);
> +        if (priv->opened[i])
> +            drivers[i]->xenClose(conn);


I guess the logic here is that if opened[i] is true, there must have 
been a xenOpen(), and if there was a xenOpen() there must be a xenClose()?


>       virMutexDestroy(&priv->lock);
>       VIR_FREE(conn->privateData);
> @@ -537,14 +538,9 @@ static int
>   xenUnifiedNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info)
>   {
>       GET_PRIVATE(conn);
> -    int i;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&
> -            drivers[i]->xenNodeGetInfo&&
> -            drivers[i]->xenNodeGetInfo (conn, info) == 0)
> -            return 0;
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonNodeGetInfo(conn, info);
>       return -1;


For all of these, is it that you've determined by examining all of the 
driver info structs that only one driver has a particular callback? Can 
we guarantee that will continue to be the case in the future?


Conditional ACK, based on satisfactory answers to these questions...


>   }
>
> @@ -621,15 +617,9 @@ xenUnifiedDomainCreateXML (virConnectPtr conn,
>                              const char *xmlDesc, unsigned int flags)
>   {
>       GET_PRIVATE(conn);
> -    int i;
> -    virDomainPtr ret;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&  drivers[i]->xenDomainCreateXML) {
> -            ret = drivers[i]->xenDomainCreateXML (conn, xmlDesc, flags);
> -            if (ret) return ret;
> -        }
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonCreateXML(conn, xmlDesc, flags);
>       return NULL;
>   }
>
> @@ -1055,14 +1045,9 @@ static int
>   xenUnifiedDomainSave (virDomainPtr dom, const char *to)
>   {
>       GET_PRIVATE(dom->conn);
> -    int i;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&
> -            drivers[i]->xenDomainSave&&
> -            drivers[i]->xenDomainSave (dom, to) == 0)
> -            return 0;
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonDomainSave(dom, to);
>       return -1;
>   }
>
> @@ -1070,14 +1055,9 @@ static int
>   xenUnifiedDomainRestore (virConnectPtr conn, const char *from)
>   {
>       GET_PRIVATE(conn);
> -    int i;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&
> -            drivers[i]->xenDomainRestore&&
> -            drivers[i]->xenDomainRestore (conn, from) == 0)
> -            return 0;
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonDomainRestore(conn, from);
>       return -1;
>   }
>
> @@ -1085,14 +1065,9 @@ static int
>   xenUnifiedDomainCoreDump (virDomainPtr dom, const char *to, unsigned int flags)
>   {
>       GET_PRIVATE(dom->conn);
> -    int i;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&
> -            drivers[i]->xenDomainCoreDump&&
> -            drivers[i]->xenDomainCoreDump (dom, to, flags) == 0)
> -            return 0;
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonDomainCoreDump(dom, to, flags);
>       return -1;
>   }
>
> @@ -1630,13 +1605,9 @@ xenUnifiedDomainUpdateDeviceFlags (virDomainPtr dom, const char *xml,
>                                      unsigned int flags)
>   {
>       GET_PRIVATE(dom->conn);
> -    int i;
> -
> -    for (i = 0; i<  XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i]&&  drivers[i]->xenDomainUpdateDeviceFlags&&
> -            drivers[i]->xenDomainUpdateDeviceFlags(dom, xml, flags) == 0)
> -            return 0;
>
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
> +        return xenDaemonUpdateDeviceFlags(dom, xml, flags);
>       return -1;
>   }
>
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index bd64167..f56beb9 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1638,7 +1638,7 @@ xenDaemonDomainSave(virDomainPtr domain, const char *filename)
>    *
>    * Returns 0 in case of success, -1 in case of error.
>    */
> -static int
> +int
>   xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename,
>                           unsigned int flags)
>   {
> @@ -2608,7 +2608,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
>    *
>    * Returns a new domain object or NULL in case of failure
>    */
> -static virDomainPtr
> +virDomainPtr
>   xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc,
>                        unsigned int flags)
>   {
> @@ -2841,7 +2841,7 @@ cleanup:
>    *
>    * Returns 0 in case of success, -1 in case of failure.
>    */
> -static int
> +int
>   xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
>                              unsigned int flags)
>   {
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index a5dd359..eee67b5 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -107,6 +107,8 @@ int xenDaemonDomainShutdown(virDomainPtr domain);
>   int xenDaemonDomainReboot(virDomainPtr domain, unsigned int flags);
>   int xenDaemonDomainDestroyFlags(virDomainPtr domain, unsigned int flags);
>   int xenDaemonDomainSave(virDomainPtr domain, const char *filename);
> +int xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename,
> +                            unsigned int flags);
>   int xenDaemonDomainRestore(virConnectPtr conn, const char *filename);
>   int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory);
>   int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory);
> @@ -140,6 +142,8 @@ int	xenDaemonDomainGetVcpus		(virDomainPtr domain,
>                                            int maxinfo,
>                                            unsigned char *cpumaps,
>                                            int maplen);
> +int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
> +                               unsigned int flags);
>   int xenDaemonDomainGetAutostart          (virDomainPtr dom,
>                                             int *autostart);
>   int xenDaemonDomainSetAutostart          (virDomainPtr domain,
> @@ -149,6 +153,8 @@ int xenDaemonDomainSetAutostart          (virDomainPtr domain,
>   extern struct xenUnifiedDriver xenDaemonDriver;
>   int xenDaemonInit (void);
>
> +virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc,
> +                                unsigned int flags);
>   virDomainPtr xenDaemonLookupByID(virConnectPtr conn, int id);
>   virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid);
>   virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname);




More information about the libvir-list mailing list