1:49 p.m.
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(a)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
3:18 a.m.
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(a)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 :|
3:25 a.m.
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(a)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
3:31 a.m.
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(a)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 :|
5:38 p.m.
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(a)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
5:47 p.m.
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
3:34 a.m.
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 :|
12:30 p.m.
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
3374
days inactive
3392
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jim Fehlig -
Peter Krempa