Change in vdsm[master]: vm: return diskMapping on status()

danken at redhat.com danken at redhat.com
Tue Aug 26 17:13:28 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: vm: return diskMapping on status()
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/31700/5//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: vm: return diskMapping on status()
Line 8: 
Line 9: status() is used by API.Global.getVMList and should return also the disk
Line 10: mapping retrieved by the guest agent.
I'm surprised to see new stuff added to the list() verb. It should report only the information needed for recreating the VM. guestIPs and displayIP is a glitch from the past. guestInfo (such as appsList) is projected in getVdsStats since it can pop up when guest agent is installed, much after list() is called.

Could you explain in the commit message the motivation for this choice? What is the motivation of this new interface?
Line 11: 
Line 12: Change-Id: Ia68ff3c36ba91cde893876a16fc6702419a60d49


http://gerrit.ovirt.org/#/c/31700/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 3576: # @displayPort:           The port in use for unencrypted display data
Line 3577: #
Line 3578: # @guestIPs:              A space separated string of assigned IPv4 addresses
Line 3579: #
Line 3580: # @guestDiskMapping:      A dictionary containing information about the disk mapping
short lines look much better on side-by-side review.

more importantly, it's optional (as it's missing from former versions, and can disappear if the guest agent is uninstalled).
Line 3581: #                         within the guest. The key is the device serial and the value is
Line 3582: #                         the mapping information.
Line 3583: #
Line 3584: # @smartcardEnable:       Info whether smartcard is enabled.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia68ff3c36ba91cde893876a16fc6702419a60d49
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list