[libvirt] [PATCH] Revert "LXC: show used memory as 0 when domain is not active"
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d8d5119..5b0a757 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -597,7 +597,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; - info->memory = 0; + info->memory = vm->def->mem.cur_balloon; } else { if (virCgroupGetCpuacctUsage(priv->cgroup, &(info->cpuTime)) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6998e12..48cc534 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2641,13 +2641,13 @@ qemuDomainGetInfo(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - if (VIR_ASSIGN_IS_OVERFLOW(info->memory, vm->def->mem.cur_balloon)) { - virReportError(VIR_ERR_OVERFLOW, "%s", - _("Current memory size too large")); - goto cleanup; - } + if (VIR_ASSIGN_IS_OVERFLOW(info->memory, vm->def->mem.cur_balloon)) { + virReportError(VIR_ERR_OVERFLOW, "%s", + _("Current memory size too large")); + goto cleanup; + } + if (virDomainObjIsActive(vm)) { if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); -- 2.1.4
On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
Conflicts: src/qemu/qemu_driver.c
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. 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 :|
On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote:
On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
Conflicts: src/qemu/qemu_driver.c
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
ACK
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Peter
On Tue, Aug 11, 2015 at 10:25:19AM +0200, Peter Krempa wrote:
On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote:
On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
Conflicts: src/qemu/qemu_driver.c
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
ACK
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now:
git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d
Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info->memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. 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 :|
Daniel P. Berrange wrote:
On Tue, Aug 11, 2015 at 10:25:19AM +0200, Peter Krempa wrote:
On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote:
On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
Conflicts: src/qemu/qemu_driver.c
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
ACK
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now:
git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d
Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info->memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too.
Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Regards, Jim
On 08/27/2015 04:38 PM, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now:
git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d
Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info->memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too.
Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches?
Yes, that sounds like the best way to handle it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Aug 27, 2015 at 04:47:24PM -0600, Eric Blake wrote:
On 08/27/2015 04:38 PM, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now:
git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d
Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info->memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too.
Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches?
Yes, that sounds like the best way to handle it.
Agreed 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 :|
Eric Blake wrote:
On 08/27/2015 04:38 PM, Jim Fehlig wrote:
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers.
I guess we should probably do this in the stable branch too, since I think it made it into the most recent release.
Erm, the change is out for a while now:
git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d
Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info->memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too.
Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches?
Yes, that sounds like the best way to handle it.
Ok, pushed to master and v1.2.7-maint through v1.2.18-maint. Regards, Jim
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jim Fehlig -
Peter Krempa