Change in vdsm[master]: [WIP] Add ballooning support

Mark Wu has uploaded a new change for review.

Change subject: [WIP] Add ballooning support
......................................................................

[WIP] Add ballooning support

Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
---
M vdsm/API.py
M vdsm/define.py
M vdsm/libvirtvm.py
M vdsm/vm.py
4 files changed, 27 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/3669/1
--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce a new verb setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Introduce a new verb setBalloonTarget

We need this API to achieve memory overcommitment on guest level. Here's
the definition of the new API:

setBalloonTarget:
    Dynamically change the target amount of physical memory allocated
    to a guest.

    Input parameter:
        vmId   - VM UUID
        target - target physical memory size in kibibytes
    Return value:
        success: return doneCode
        failure: return errCode including underlying libvirt error message.

And the ballooning information is added into the response of
getVmStats(). It can be collected on vdsm client by this way:

   balloon_info = s.getVmStats(vmId)['statsList'][0]['balloonInfo']

Please note that setBalloonTarget can work only when a virtio balloon
device is available for the guest. So to test it with vdsm API, you need
put the following python dictionary into the 'devices' list in VM's conf
to enable balloon device:

{'device': 'memballoon', 'type': 'balloon',
'specParams':
 {'model': 'virtio'}}

On ovirt-engine, there's checkbox 'Memory Balloon Enabled' in the
'New/Edit VM' UI. You can enable it in cluster 3.1.

Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
---
M vdsm/API.py
M vdsm/BindingXMLRPC.py
M vdsm/define.py
M vdsm/libvirtvm.py
M vdsm/vm.py
M vdsm_api/vdsmapi-schema.json
M vdsm_cli/vdsClient.py
7 files changed, 99 insertions(+), 1 deletion(-)

Approvals:
  Adam Litke: Looks good to me, but someone else must approve
  Mark Wu: Verified
  Dan Kenigsberg: Looks good to me, approved


--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 11: Looks good to me, approved

Copying Doron's ack, with an unfortunate 1 month latency.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 11: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 11: Verified

Fix a small pep8 issue reported by Hervé Leclerc in patch v11.
This patch was submit before the libvirtvm.py pep8 clean patch, so it's not found
before.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Doron Fediuck has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 10:

Mark / Royce,
can you please verify this change and set the relevant flag?

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 10: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Doron Fediuck has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 10: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 10:

Doron,
I believe we don't have unit problem now. We just use the same unit as libvirt api:
KiB (1024 bytes)

This patch also changes the output of getVmStatsm, so I update the schema for it in v10.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Doron Fediuck has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 9:

Any issues left with KB / KiB ?

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 9:

Build Successful 

http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/591/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 8: (1 inline comment)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5015: # @VM.setBalloonTarget:
Line 5016: #
Line 5017: # Dynamically change the target amount of physical memory allocated to a
guest.
Line 5018: #
Line 5019: # @target: The target of guest's physical memory size in kibibytes
Libvirt does use KB.  So if our API uses KB as well then there should be no need for
translation, right?
Line 5020: #
Line 5021: # Since: 4.10.0
Line 5022: ##
Line 5023: {'command': {'class': 'VM', 'name':
'setBalloonTarget'},


--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 8: (2 inline comments)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5015: # @VM.setBalloonTarget:
Line 5016: #
Line 5017: # Dynamically change the target amount of physical memory allocated to a
guest.
Line 5018: #
Line 5019: # @target: The target of guest's physical memory size in kibibytes
Are you sure, Adam? I see no unit translation here, and libvirt accpets KiB
http://libvirt.org/html/libvirt-libvirt.html#virDomainSetMemory
Line 5020: #
Line 5021: # Since: 4.10.0
Line 5022: ##
Line 5023: {'command': {'class': 'VM', 'name':
'setBalloonTarget'},


....................................................
File vdsm/libvirtvm.py
Line 2159:             self.log.error("Set new balloon target failed",
exc_info=True)
Line 2160:             if msg == None:
Line 2161:                 error = errCode[key]
Line 2162:             else:
Line 2163:                 error = {'status' : {'code': errCode[key] \
needless trailing backslash
Line 2164:                          ['status']['code'], 'message':
msg}}
Line 2165:             return error
Line 2166: 
Line 2167:         if self._dom == None:


--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 8: (1 inline comment)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5015: # @VM.setBalloonTarget:
Line 5016: #
Line 5017: # Dynamically change the target amount of physical memory allocated to a
guest.
Line 5018: #
Line 5019: # @target: The target of guest's physical memory size in kibibytes
Thanks. Will fix it in next patch
Line 5020: #
Line 5021: # Since: 4.10.0
Line 5022: ##
Line 5023: {'command': {'class': 'VM', 'name':
'setBalloonTarget'},


--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 8: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5015: # @VM.setBalloonTarget:
Line 5016: #
Line 5017: # Dynamically change the target amount of physical memory allocated to a
guest.
Line 5018: #
Line 5019: # @target: The target of guest's physical memory size in kibibytes
kilobytes
Line 5020: #
Line 5021: # Since: 4.10.0
Line 5022: ##
Line 5023: {'command': {'class': 'VM', 'name':
'setBalloonTarget'},


--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 8:

Build Failed 

http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/469/ : ABORTED

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Doron Fediuck has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 7:

The benefit would be a controlled environment.
ie- in specific scenarios, we'd like all VMs in a host to start with a default balloon
size, and later on adjust if needed. This actually optimizes memory over commitment ;)

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 7: No score

I don't see the benefit of setting it on vm start. The target should be changed
automatically according to the memory pressure. Why do we need assign it a fixed value on
startup?
If it turns out we really need it after discussion,  we can add it in anther patch as you
suggested.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Doron Fediuck has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 7:

One functional question, if I may.
Can we add support to an initial balloon size when starting the VM?

Current patch deals with runtime, and my question could be handled by another patch.
Still, this is something we should consider. I'm fine with accepting this patch and
getting another one later on.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce a new verb setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce a new verb setBalloonTarget
......................................................................


Patch Set 7: Verified

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 6: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 6:

Change from patch set 5:
    remove the usage of default 'memSize' because that API validates it when
creating a VM, so it's safe to use it directly.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/vm.py
Line 405:                 dev['target'] = int(self.conf.get('memSize',
256)) * 1024
Yes, I could add the default memory size to config.py.in. But I find that the parameter
'memSize' is already validated in API.VM.create(). So it should be safe to use it
directly. In next version, I am going to change it to:
self.conf.get('memSize')

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(2 inline comments)

Just some suggestions about using a hard-coded default memory value.

....................................................
File vdsm/libvirtvm.py
Line 2102:                 max_mem = self.conf.get('memSize', 256) * 1024
Why are you allowing a default value (256) here?  How do you know that it would be
correct?  Would it be better to raise an error if you are unable to access
self.conf['memSize'] ?

....................................................
File vdsm/vm.py
Line 405:                 dev['target'] = int(self.conf.get('memSize',
256)) * 1024
I see.  This might be a problem.  What is someone wants to change the default memory from
256M to 512M?  Now they will have to remember two places to change it.  I think the
default amount of memory should be defined as a single constant that can be used in
multiple places.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 5: Verified

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/vm.py
Line 405:                 dev['target'] = int(self.conf.get('memSize',
256)) * 1024
Yes, I want to set the target to the maximum amount of memory. But if 'memSize' is
not configured in vm's conf, vdsm choose 256MB as the default size for a vm. 
Please see the constructor of _DomXML:
        memSizeKB = str(int(self.conf.get('memSize', '256')) * 1024)

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/vm.py
Line 405:                 dev['target'] = int(self.conf.get('memSize',
256)) * 1024
Why are you targeting the balloons to 256MB by default?  Usually, we would set the target
to the maximum amount of memory (which is already the default behavior.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API getBalloonInfo and setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce API getBalloonInfo and setBalloonTarget
......................................................................


Patch Set 4: I would prefer that you didn't submit this

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API getBalloonInfo and setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce API getBalloonInfo and setBalloonTarget
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File vdsm/vm.py
Line 933:         self.conf['balloon'] = self.getBalloonInfo()
but we already poll cpu usage (git grep dom.info). we could pretty easily poll and cache
balloon size.

We had bad experience with verbs that may block Engine/Vdsm traffic due to a hung qemu
process. Let us try to avoid that, as well a a need for a new and costy per-vm polling
verb. If you have 200 Vm and the client polls each and every one's balloon separately,
for all hosts, your client would become pretty busy pretty quickly.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API getBalloonInfo and setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API getBalloonInfo and setBalloonTarget
......................................................................


Patch Set 4: (1 inline comment)

Yes, setBalloonTarget only works with a virtio balloon device is added to the target
guest. That's why I sent a mail do discuss reserve the balloon device created by
libvirt. I will add this note to the commit message.

....................................................
File vdsm/libvirtvm.py
Line 2104:         except libvirt.libvirtError:
oops! I will fix it in next version. Thanks!

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API getBalloonInfo and setBalloonTarget

Adam Litke has posted comments on this change.

Change subject: Introduce API getBalloonInfo and setBalloonTarget
......................................................................


Patch Set 4: Verified; I would prefer that you didn't submit this

(1 inline comment)

Once I fix the error in libvirtvm.py I can get this to work.  Note that the
setBalloonTarget verb will not work until vdsm creates vms with a virtio balloon device.

....................................................
File vdsm/libvirtvm.py
Line 2104:         except libvirt.libvirtError:
Since you are accessing the exception content below this should be:

except libvirt.libvirtError, e:

Otherwise e is undefined.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File vdsm/vm.py
Line 933:         self.conf['balloon'] = self.getBalloonInfo()
Yes, I agree with that vm.conf is not suitable for current balloon size.  Putting the
target balloon size into the conf of balloon device sounds good. It can be updated on
setting balloon target successfully. And balloon information can be retrieved from the
conf of balloon device because the current balloon size should be the same as the target
size. But the problem is that it's not convenient to find balloon device from the
'devices' list in vm.conf. I have to traverse the 'device' list and check
the 'type' to find it.

So in next version I add another API getBalloonInfo to get balloon information. It's
not cached in vdsm, and is only invoked by client.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 3:

oh, please add a vdsClient verb so this can be tested from the cli.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 2118:         return {'status': doneCode, 'vmList': self.status()}
this is a new verb, I do not think 'vmList' is useful for anybody.

....................................................
File vdsm/vm.py
Line 933:         self.conf['balloon'] = self.getBalloonInfo()
this function must return immediately, and never block on a buggy qemu monitor or a busy
libvirtd.

If you expect unilateral changes in balloon size, it should be polled. if not, it can be
cached on other occasions.

Also, self.conf is intended for static configuration (such as VM devices, its memory size,
or target balloon size) but not for volatile current balloon size.

The target balloon size should probably be part of the balloon device, not of the
top-level .conf dictionary. Otherwise, it would not be acted up in migration destination
host.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Change in vdsm[master]: Introduce API setBalloonTarget

Mark Wu has posted comments on this change.

Change subject: Introduce API setBalloonTarget
......................................................................


Patch Set 3: Verified

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Adam Litke has posted comments on this change.

Change subject: [WIP] Add ballooning support
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(2 inline comments)

Looks pretty good.  Just a few minor comments/improvements to look at.

....................................................
Commit Message
Line 10: Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Please add a more descriptive commit message.  You should fully describe the semantics of
the new API you are planning to add including parameters, return values, and semantics.

....................................................
File vdsm/libvirtvm.py
Line 2110:             return self._dom.info()[1:3]
Why oh why didn't libvirt present the domain info as a dictionary?  In vdsm, we can do
a better job.  Rather than returning an array slice, please create a dictionary and return
each of these values with meaningful key names.

--
To view, visit http://gerrit.ovirt.org/3669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36b1211dbce28bb5bb5926f2db7e2e42a698efc9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;