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@xxxxxxxxxxxxx> > 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@xxxxxxxxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list