4:47 p.m.
A few minor fixes for patches 1 and 2 of Joao's series adding stats
support to the libxl driver
https://www.redhat.com/archives/libvir-list/2015-November/msg00489.html
I only noticed the minor flaws while reviewing patches later in the series,
after already committing patches 1 and 2.
Jim Fehlig (2):
libxl: unref libxlDriverConfig object
libxl: don't unlock virDomainObj if refcnt is 0
src/libxl/libxl_driver.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--
2.1.4
4:47 p.m.
New subject: [libvirt] [PATCH 1/2] libxl: unref libxlDriverConfig object
Commits b6e19cf4 and 6472e54a missed unref'ing the
libxlDriverConfig object. Add missing calls to virObjectUnref.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fd4bae1..4609c00 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4648,18 +4648,21 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
virTypedParameterPtr params,
unsigned int nparams)
{
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
libxl_dominfo d_info;
int ret = -1;
if (nparams == 0)
return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
+ libxl_dominfo_init(&d_info);
+ cfg = libxlDriverConfigGet(driver);
+
if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxl_domain_info failed for domain '%d'"),
vm->def->id);
- return ret;
+ goto cleanup;
}
if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
@@ -4670,6 +4673,7 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
cleanup:
libxl_dominfo_dispose(&d_info);
+ virObjectUnref(cfg);
return ret;
}
@@ -4684,7 +4688,7 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
libxl_vcpuinfo *vcpuinfo;
int maxcpu, hostcpus;
size_t i;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
int ret = -1;
if (nparams == 0 && ncpus != 0)
@@ -4692,12 +4696,13 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
else if (nparams == 0)
return vm->def->maxvcpus;
+ cfg = libxlDriverConfigGet(driver);
if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu,
&hostcpus)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to list vcpus for domain '%d' with
libxenlight"),
vm->def->id);
- return ret;
+ goto cleanup;
}
for (i = start_cpu; i < maxcpu && i < ncpus; ++i) {
@@ -4710,7 +4715,9 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
ret = nparams;
cleanup:
- libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
+ if (vcpuinfo)
+ libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
+ virObjectUnref(cfg);
return ret;
}
@@ -4766,7 +4773,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
virDomainObjPtr vm;
libxl_dominfo d_info;
unsigned mem, maxmem;
@@ -4775,6 +4782,8 @@ libxlDomainMemoryStats(virDomainPtr dom,
virCheckFlags(0, -1);
+ cfg = libxlDriverConfigGet(driver);
+
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -4815,6 +4824,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
cleanup:
if (vm)
virObjectUnlock(vm);
+ virObjectUnref(cfg);
return ret;
}
--
2.1.4
3:14 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: unref libxlDriverConfig object
On Wed, Nov 18, 2015 at 03:47:24PM -0700, Jim Fehlig wrote:
Commits b6e19cf4 and 6472e54a missed unref'ing the
libxlDriverConfig object. Add missing calls to virObjectUnref.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
ACK
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:57 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: unref libxlDriverConfig object
Daniel P. Berrange wrote:
On Wed, Nov 18, 2015 at 03:47:24PM -0700, Jim Fehlig wrote:
> Commits b6e19cf4 and 6472e54a missed unref'ing the
> libxlDriverConfig object. Add missing calls to virObjectUnref.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_driver.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
ACK
Thanks. I've pushed this and 2/2.
Regards,
Jim
6:54 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: unref libxlDriverConfig object
On 11/18/2015 10:47 PM, Jim Fehlig wrote:
Commits b6e19cf4 and 6472e54a missed unref'ing the
libxlDriverConfig object. Add missing calls to virObjectUnref.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fd4bae1..4609c00 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4648,18 +4648,21 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
virTypedParameterPtr params,
unsigned int nparams)
{
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
libxl_dominfo d_info;
int ret = -1;
if (nparams == 0)
return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
+ libxl_dominfo_init(&d_info);
+ cfg = libxlDriverConfigGet(driver);
+
if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxl_domain_info failed for domain '%d'"),
vm->def->id);
- return ret;
+ goto cleanup;
}
if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
@@ -4670,6 +4673,7 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
cleanup:
libxl_dominfo_dispose(&d_info);
+ virObjectUnref(cfg);
return ret;
}
@@ -4684,7 +4688,7 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
libxl_vcpuinfo *vcpuinfo;
int maxcpu, hostcpus;
size_t i;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
int ret = -1;
if (nparams == 0 && ncpus != 0)
@@ -4692,12 +4696,13 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
else if (nparams == 0)
return vm->def->maxvcpus;
+ cfg = libxlDriverConfigGet(driver);
if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu,
&hostcpus)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to list vcpus for domain '%d' with
libxenlight"),
vm->def->id);
- return ret;
+ goto cleanup;
}
for (i = start_cpu; i < maxcpu && i < ncpus; ++i) {
@@ -4710,7 +4715,9 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
ret = nparams;
cleanup:
- libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
+ if (vcpuinfo)
+ libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
+ virObjectUnref(cfg);
return ret;
}
@@ -4766,7 +4773,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ libxlDriverConfigPtr cfg;
virDomainObjPtr vm;
libxl_dominfo d_info;
unsigned mem, maxmem;
@@ -4775,6 +4782,8 @@ libxlDomainMemoryStats(virDomainPtr dom,
virCheckFlags(0, -1);
+ cfg = libxlDriverConfigGet(driver);
+
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -4815,6 +4824,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
cleanup:
if (vm)
virObjectUnlock(vm);
+ virObjectUnref(cfg);
return ret;
}
Looks great and also tested this series. I see you also moved cfg init further
down which avoids unnecessary initialization for some cases.
Regards,
Joao
4:47 p.m.
New subject: [libvirt] [PATCH 2/2] libxl: don't unlock virDomainObj if refcnt is 0
Commit 6472e54a unlocks the virDomainObj even if libxlDomainObjEndJob
returns false, indicating that its refcnt has dropped to 0.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4609c00..d77a0e4 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4816,10 +4816,8 @@ libxlDomainMemoryStats(virDomainPtr dom,
libxl_dominfo_dispose(&d_info);
endjob:
- if (!libxlDomainObjEndJob(driver, vm)) {
- virObjectUnlock(vm);
+ if (!libxlDomainObjEndJob(driver, vm))
vm = NULL;
- }
cleanup:
if (vm)
--
2.1.4
3:14 a.m.
New subject: [libvirt] [PATCH 2/2] libxl: don't unlock virDomainObj if refcnt is 0
On Wed, Nov 18, 2015 at 03:47:25PM -0700, Jim Fehlig wrote:
Commit 6472e54a unlocks the virDomainObj even if
libxlDomainObjEndJob
returns false, indicating that its refcnt has dropped to 0.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
ACK
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:54 a.m.
New subject: [libvirt] [PATCH 2/2] libxl: don't unlock virDomainObj if refcnt is 0
On 11/18/2015 10:47 PM, Jim Fehlig wrote:
Commit 6472e54a unlocks the virDomainObj even if
libxlDomainObjEndJob
returns false, indicating that its refcnt has dropped to 0.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4609c00..d77a0e4 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4816,10 +4816,8 @@ libxlDomainMemoryStats(virDomainPtr dom,
libxl_dominfo_dispose(&d_info);
endjob:
- if (!libxlDomainObjEndJob(driver, vm)) {
- virObjectUnlock(vm);
+ if (!libxlDomainObjEndJob(driver, vm))
vm = NULL;
- }
cleanup:
if (vm)
And it looks this mistake is also on all patches that use libxlDomainObjEndJob
so will fix that on the next version (interface, block, getalldomainstats.
Regards,
Joao
3291
days inactive
3292
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Jim Fehlig -
Joao Martins