[libvirt] [PATCH] xen: Use internal interfaces in xenDomainUsedCpus

[Jim Fehlig]

Stefan Bader wrote:
> On 05.08.2013 19:52, Jim Fehlig wrote:
>   
>> libvirt typically uses a '*Internal' naming pattern for these types of
>> internal functions, e.g. xenUnifiedDomainGetVcpusFlagsInternal.  Also as
>> we touch this code we should strive to use the libvirt pattern of
>> putting each parameter after the first on a new line when the list of
>> parameters exceeds 80 columns.  Finally, since you added a line after
>> the xenUnifiedNodeGetInfo declaration, we should add one here too.
>>
>>     
> Ok, changed.
>
>   
>> I don't think this comment is necessary.  Instead, just send a follow-up
>> patch :).
>>     
>
> Oh well, it was a kind of reminder, but beside of the "doing it correct" part,
> the current usage is ok as there is no special checking between public and
> private usage which could lock up... :)
>   

Nod.

[...]

> From 47ce666f6a4d91832883341c56f0a55181698e76 Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader at canonical.com>
> Date: Mon, 15 Jul 2013 16:03:58 +0200
> Subject: [PATCH] xen: Use internal interfaces in xenDomainUsedCpus
>
> Since commit 95e18efd most public interfaces (xenUnified...) obtain
> a virDomainDefPtr via xenGetDomainDefFor...() which take the unified
> lock.
> This is already taken before calling xenDomainUsedCpus(), so we get
> a deadlock for active guests. Avoid this by splitting up
> xenUnifiedDomainGetVcpusFlags() and xenUnifiedDomainGetVcpus() into
> public and private function calls (which get the virDomainDefPtr passed)
> and use those in xenDomainUsedCpus().
>
>     xenDomainUsedCpus
>       ...
>       nb_vcpu = xenUnifiedDomainGetMaxVcpus(dom);
>         return xenUnifiedDomainGetVcpusFlags(...)
>           ...
>           if (!(def = xenGetDomainDefForDom(dom)))
>             return xenGetDomainDefForUUID(dom->conn, dom->uuid);
>               ...
>               ret = xenHypervisorLookupDomainByUUID(conn, uuid);
>                 ...
>                 xenUnifiedLock(priv);
>                 name = xenStoreDomainGetName(conn, id);
>                 xenUnifiedUnlock(priv);
>       ...
>       if ((ncpus = xenUnifiedDomainGetVcpus(dom, cpuinfo, nb_vcpu,
>         ...
>         if (!(def = xenGetDomainDefForDom(dom)))
>           [again like above]
>
> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  src/xen/xen_driver.c | 100 +++++++++++++++++++++++++++++++++++----------------
>  src/xen/xen_driver.h |   2 +-
>  2 files changed, 70 insertions(+), 32 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 4ae38d3..e1fcbcc 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -73,12 +73,18 @@
>  
>  static int
>  xenUnifiedNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
> +
>  static int
> -xenUnifiedDomainGetMaxVcpus(virDomainPtr dom);
> +xenUnifiedDomainGetVcpusFlagsInternal(virDomainPtr dom,
> +                                      virDomainDefPtr def,
> +                                      unsigned int flags);
>   

Super nit: I added a line between these function declarations for
consistency.  ACK to the patch and now pushed.  Thanks Stefan!

Regards,
Jim