Change in vdsm[master]: netwiring: [1/3] Add API definitions.

List overview All Threads
Download

newer

older

Change in vdsm[master]: netwiring:...

Change in vdsm[master]:...

asegurap＠redhat.com

28 Nov 2012 28 Nov '12

7:26 p.m.

Antoni Segura Puimedon has uploaded a new change for review.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

netwiring: [1/3] Add API definitions.

First of the Network Wiring feature patches. It adds the schema definitions as described in: http://wiki.ovirt.org/wiki/Feature/DetailedNetworkWiring#New_API

Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/BindingXMLRPC.py M vdsm_api/vdsmapi-schema.json 2 files changed, 51 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/9560/1

diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index d460910..60efbdf 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -262,6 +262,10 @@ vm = API.VM(params['vmId']) return vm.hotunplugNic(params)

+ def updateVmDevice(self, vmId, params): + vm = API.VM(vmId) + return vm.updateDevice(params) + def vmSnapshot(self, vmId, snapDrives): vm = API.VM(vmId) return vm.snapshot(snapDrives) @@ -789,7 +793,8 @@ (self.vmHotplugDisk, 'hotplugDisk'), (self.vmHotunplugDisk, 'hotunplugDisk'), (self.vmHotplugNic, 'hotplugNic'), - (self.vmHotunplugNic, 'hotunplugNic')) + (self.vmHotunplugNic, 'hotunplugNic'), + (self.updateVmDevice, 'updateVmDevice'))

def getIrsMethods(self): return ((self.domainActivate, 'activateStorageDomain'), diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index 843cbc7..3818ab7 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -4849,6 +4849,51 @@ 'data': {'hibernationVolHandle': 'HibernationVolumeHandle'}}

## +# @updateVmDeviceParams: +# +# Parameters for @VM.updateVmDevice +# +# @type: The type of device to update. For example 'interface' for network +# devices or 'disk' for disk storage devices. +# +# @alias: Alias used to identify this device in commands. +# +# @network: Name of the bridge the interface will be added to. +# +# @linkState: The state in which the link of the interface will be upon +# successful completion of the api request. The values can be 'up' or 'down'. +# +# @promisc: If present, indicates the traffic of the network which should be +# mirrored to the interface identified by alias. +# +# Since 4.11.0 +## +{'type': 'updateVmDeviceParams', + 'data': {'type': 'str', 'network': 'str', + 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}} + +## +# @VM.updateVmDevice: +# +# Update the status of a VM's device. +# +# @vmId: The UUID of the VM to modify +# +# @params: The VM UUID and device information. +# +# Returns: +# The VM definition, as updated +# +# Since: 4.11.0 +# +# Notes: +# XXX: Split 'params' into direct parameters +## +{'command': {'class': 'VM', 'name': 'updateVmDevice'}, + 'data': {'vmId': 'UUID', 'params': 'updateVmDeviceParams'}, + 'returns': 'VmDefinition'} + +## # @HotplugNicParams: # # Parameters for @VM.hotplugNic.

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

Gerrit-MessageType: newchange Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com

Show replies by date

Jenkins CI RO

28 Nov 28 Nov

7:26 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/232/ (1/2)

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

Jenkins CI RO

7:26 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/198/ (2/2)

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

Jenkins CI RO

7:28 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/198/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/232/ : SUCCESS

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

shaohef＠linux.vnet.ibm.com

7:32 p.m.

ShaoHe Feng has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

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

(2 inline comments)

.................................................... File vdsm_api/vdsmapi-schema.json Line 4869: # Since 4.11.0 Line 4870: ## Line 4871: {'type': 'updateVmDeviceParams', Line 4872: 'data': {'type': 'str', 'network': 'str', Line 4873: 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}} Please align the descriptions so they all start at the same column. See the rest of the file for how it's done. Sorry to be pedantic, but the rest of the file looks so nice :) Line 4874: Line 4875: ## Line 4876: # @VM.updateVmDevice: Line 4877: #

Line 4878: # Update the status of a VM's device. Line 4879: # Line 4880: # @vmId: The UUID of the VM to modify Line 4881: # Line 4882: # @params: The VM UUID and device information. alignment. Line 4883: # Line 4884: # Returns: Line 4885: # The VM definition, as updated Line 4886: #

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

Jenkins CI RO

7:50 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/236/ (2/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

7:50 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/202/ (1/2)

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

asegurap＠redhat.com

7:51 p.m.

Antoni Segura Puimedon has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 2:

Thanks Feng! I totally hadn't noticed about the indentation of the comments :P

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

Jenkins CI RO

7:51 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/202/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/236/ : SUCCESS

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

Dan Kenigsberg

29 Nov 29 Nov

1:09 p.m.

Dan Kenigsberg has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

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

(5 inline comments)

.................................................... File vdsm_api/vdsmapi-schema.json Line 4852: # @updateVmDeviceParams: Line 4853: # Line 4854: # Parameters for @VM.updateVmDevice Line 4855: # Line 4856: # @type: The type of device to update. For example 'interface' for network I think this can be made more similar to VmInterfaceDevice - I think this is @device. Line 4857: # devices or 'disk' for disk storage devices. Line 4858: # Line 4859: # @alias: Alias used to identify this device in commands. Line 4860: #

Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. Line 4869: # Line 4870: # Since 4.11.0 this is going to be available in 4.10.3 (I hope) Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams', Line 4873: 'data': {'type': 'str', 'network': 'str', Line 4874: 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}}

I think linkState should be optional here (default is up) , as well as in VmInterfaceDevice.

promisc should not be an empty string. Actually, it is called "portMirroring", it's optional, and is a list of network that should be mirrored into this iface. This should be put in VmInterfaceDevice as well - preferably in a preceding patch. Line 4875: Line 4876: ## Line 4877: # @VM.updateVmDevice: Line 4878: #

Maybe it simplifies the review process - I should probably first see how complex is the implementation. Line 268: Line 269: def vmSnapshot(self, vmId, snapDrives): Line 270: vm = API.VM(vmId) Line 271: return vm.snapshot(snapDrives)

Line 793: (self.vmHotplugDisk, 'hotplugDisk'), Line 794: (self.vmHotunplugDisk, 'hotunplugDisk'), Line 795: (self.vmHotplugNic, 'hotplugNic'), Line 796: (self.vmHotunplugNic, 'hotunplugNic'), Line 797: (self.updateVmDevice, 'updateVmDevice')) Please keep the name convention for vm-related commands: vmUpdateDevice seems better. Line 798: Line 799: def getIrsMethods(self): Line 800: return ((self.domainActivate, 'activateStorageDomain'), Line 801: (self.domainAttach, 'attachStorageDomain'),

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

agl＠us.ibm.com

1:42 p.m.

Adam Litke has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

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

(7 inline comments)

.................................................... File vdsm_api/vdsmapi-schema.json Line 4852: # @updateVmDeviceParams: Line 4853: # Line 4854: # Parameters for @VM.updateVmDevice Line 4855: # Line 4856: # @type: The type of device to update. For example 'interface' for network 'type' is a terrible field name. Please use 'deviceType'. The enum describing the type of this field is: @VmDeviceType Line 4857: # devices or 'disk' for disk storage devices. Line 4858: # Line 4859: # @alias: Alias used to identify this device in commands. Line 4860: #

Line 4861: # @network: Name of the bridge the interface will be added to. Line 4862: # Line 4863: # @linkState: The state in which the link of the interface will be upon Line 4864: # successful completion of the api request. The values can be 'up' Line 4865: # or 'down'. Could there ever be more states than 'up'/'down'? If so, I think a boolean value would make more sense here. You could change the name to 'linkActive'. If there are more than 2 possible states, we will have to define an enum for the possible values. Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. Line 4869: #

Line 4864: # successful completion of the api request. The values can be 'up' Line 4865: # or 'down'. Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. The above 4 fields are used to make changes to the existing device. In my opinion, all of them should be optional and the semantics of an unspecified field should be for vdsm to leave it unchanged. Line 4869: # Line 4870: # Since 4.11.0 Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams',

Line 4870: # Since 4.11.0 Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams', Line 4873: 'data': {'type': 'str', 'network': 'str', Line 4874: 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}} Agree with Dan on the type for linkState. We either need a new enum (or use boolean) because interface state is not the same thing as link state (even if the two currently have the same values). Line 4875: Line 4876: ## Line 4877: # @VM.updateVmDevice: Line 4878: #

Line 4878: # Line 4879: # Update the status of a VM's device. Line 4880: # Line 4881: # @vmId: The UUID of the VM to modify Line 4882: # vmId isn't required. It is an implicit property of the instantiated VM object that this method belongs to. Line 4883: # @params: The VM UUID and device information. Line 4884: # Line 4885: # Returns: Line 4886: # The VM definition, as updated

Line 4887: # Line 4888: # Since: 4.11.0 Line 4889: # Line 4890: # Notes: Line 4891: # XXX: Split 'params' into direct parameters I don't think you want to split them actually. See below. Line 4892: ## Line 4893: {'command': {'class': 'VM', 'name': 'updateVmDevice'}, Line 4894: 'data': {'vmId': 'UUID', 'params': 'updateVmDeviceParams'}, Line 4895: 'returns': 'VmDefinition'}

Line 4890: # Notes: Line 4891: # XXX: Split 'params' into direct parameters Line 4892: ## Line 4893: {'command': {'class': 'VM', 'name': 'updateVmDevice'}, Line 4894: 'data': {'vmId': 'UUID', 'params': 'updateVmDeviceParams'}, Will this command only be used for updating network ('interface') devices or might it be used to modify other devices in the future?

If only for network devices, then the command name should be changed to updateVmInterfaceDevice and the params type should be updateVmInterfaceDeviceParams.

If other devices are allowed, then you need to use a union because linkState won't make sense for an audio card. Your union would be named updateVmDeviceParams with a single data element 'type'. You would create one sub-type: updateVmInterfaceDeviceParams to contain all of the params (except 'type' which is stored in the union). Line 4895: 'returns': 'VmDefinition'} Line 4896: Line 4897: ## Line 4898: # @HotplugNicParams:

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

asegurap＠redhat.com

1:51 p.m.

Antoni Segura Puimedon has posted comments on this change.

Change subject: netwiring: [1/3] Add API definitions. ......................................................................

Patch Set 2: (9 inline comments)

Thanks a lot for the reviews! I think that we can rework this and the spec into something more specific to the "interface" use case.

.................................................... File vdsm_api/vdsmapi-schema.json Line 4852: # @updateVmDeviceParams: Line 4853: # Line 4854: # Parameters for @VM.updateVmDevice Line 4855: # Line 4856: # @type: The type of device to update. For example 'interface' for network I was following for all this file the specification in http://wiki.ovirt.org/wiki/Feature/DetailedNetworkWiring#New_API

Should I change it in both places and tell Alona to make the appropriate changes in the engine? Line 4857: # devices or 'disk' for disk storage devices. Line 4858: # Line 4859: # @alias: Alias used to identify this device in commands. Line 4860: #

Line 4861: # @network: Name of the bridge the interface will be added to. Line 4862: # Line 4863: # @linkState: The state in which the link of the interface will be upon Line 4864: # successful completion of the api request. The values can be 'up' Line 4865: # or 'down'. Alona already proposed such change and the fault is on me in this case for wanting to keep the up/down names we all know and love from linux. I agree though that the api is much simpler with LinkActive and it being a bool. I will change it and notify the engine side. Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. Line 4869: #

Line 4864: # successful completion of the api request. The values can be 'up' Line 4865: # or 'down'. Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. I find it reasonable to do as you propose. I will update the definition and the rest of the branch to reflect this. Line 4869: # Line 4870: # Since 4.11.0 Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams',

Line 4866: # Line 4867: # @promisc: If present, indicates the traffic of the network which should be Line 4868: # mirrored to the interface identified by alias. Line 4869: # Line 4870: # Since 4.11.0 Done Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams', Line 4873: 'data': {'type': 'str', 'network': 'str', Line 4874: 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}}

Line 4870: # Since 4.11.0 Line 4871: ## Line 4872: {'type': 'updateVmDeviceParams', Line 4873: 'data': {'type': 'str', 'network': 'str', Line 4874: 'linkState': 'NetworkInterfaceState', 'alias': 'str', 'promisc': ''}} I will move to the boolean. Line 4875: Line 4876: ## Line 4877: # @VM.updateVmDevice: Line 4878: #

Line 4878: # Line 4879: # Update the status of a VM's device. Line 4880: # Line 4881: # @vmId: The UUID of the VM to modify Line 4882: # I will then update the spec in http://wiki.ovirt.org/wiki/Feature/DetailedNetworkWiring#New_API to make it into @VM.vmupdateDevice(params) Line 4883: # @params: The VM UUID and device information. Line 4884: # Line 4885: # Returns: Line 4886: # The VM definition, as updated

Line 4890: # Notes: Line 4891: # XXX: Split 'params' into direct parameters Line 4892: ## Line 4893: {'command': {'class': 'VM', 'name': 'updateVmDevice'}, Line 4894: 'data': {'vmId': 'UUID', 'params': 'updateVmDeviceParams'}, The spec I got allowed it to be used for other devices. I would honestly prefer it only for network interfaces, as libvirt's virDomainUpdateDevice is already used for storage by other commands. Line 4895: 'returns': 'VmDefinition'} Line 4896: Line 4897: ## Line 4898: # @HotplugNicParams:

.................................................... File vdsm/BindingXMLRPC.py Line 263: return vm.hotunplugNic(params) Line 264: Line 265: def updateVmDevice(self, vmId, params): Line 266: vm = API.VM(vmId) Line 267: return vm.updateDevice(params) I made indeed to ease the review process. However, I don't have anything against squashing it into a single patch, if you want. Line 268: Line 269: def vmSnapshot(self, vmId, snapDrives): Line 270: vm = API.VM(vmId) Line 271: return vm.snapshot(snapDrives)

Line 793: (self.vmHotplugDisk, 'hotplugDisk'), Line 794: (self.vmHotunplugDisk, 'hotunplugDisk'), Line 795: (self.vmHotplugNic, 'hotplugNic'), Line 796: (self.vmHotunplugNic, 'hotunplugNic'), Line 797: (self.updateVmDevice, 'updateVmDevice')) Again, just following http://wiki.ovirt.org/wiki/Feature/DetailedNetworkWiring#New_API I can change it and tell Alona, of course. Line 798: Line 799: def getIrsMethods(self): Line 800: return ((self.domainActivate, 'activateStorageDomain'), Line 801: (self.domainAttach, 'attachStorageDomain'),

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Dan Kenigsberg

2 Dec 2 Dec

8:11 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Dan Kenigsberg has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

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

Yes, we already have changeCD and changeFloppy that use libvirt's updateDevice, but I have a feeling that exposing a generic vmUpdateDevice is more powerful. For example, changeCD assumes that there's only one CD sitting on /dev/hdc. It would be much nicer to use updateDevice infrastructure to allow multiple CDs.

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

shaohef＠linux.vnet.ibm.com

3 Dec 3 Dec

11:05 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

ShaoHe Feng has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

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

(1 inline comment)

Hopefully, my patch can be merged. http://gerrit.ovirt.org/#/c/9433/

it is easy to find the minor error.

.................................................... File vdsm_api/vdsmapi-schema.json Line 4867: 'data': {'deviceType': 'VmDeviceType'}, Line 4868: 'union': ['vmUpdateInterfaceDeviceParams']} Line 4869: Line 4870: ## Line 4871: # @vmUpdateIntefaceDeviceParams: vmUpdateInterfaceDeviceParams? Inte --> Inter ? Line 4872: # Line 4873: # Parameters for @VM.updateDevice Line 4874: # Line 4875: # @alias: Alias used to identify this device in commands.

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

asegurap＠redhat.com

2:24 p.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Antoni Segura Puimedon has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 4: (1 inline comment)

Good catch ShaoHe, thanks!

.................................................... File vdsm_api/vdsmapi-schema.json Line 4867: 'data': {'deviceType': 'VmDeviceType'}, Line 4868: 'union': ['vmUpdateInterfaceDeviceParams']} Line 4869: Line 4870: ## Line 4871: # @vmUpdateIntefaceDeviceParams: Done Line 4872: # Line 4873: # Parameters for @VM.updateDevice Line 4874: # Line 4875: # @alias: Alias used to identify this device in commands.

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

agl＠us.ibm.com

4 Dec 4 Dec

10:37 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Adam Litke has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 5: (1 inline comment)

.................................................... File vdsm_api/vdsmapi-schema.json Line 4885: # @portMirroring: #optional If present, indicates the traffic of which networks Line 4886: # should be mirrored to the interface identified by alias. Line 4887: # Line 4888: # Since 4.10.3 Line 4889: ## I am a bit concerned by the different semantics for optional parameters. I think we need to decide what omitting an optional parameter should mean and keep it the same across all fields. I think it's sensible for omit to mean leave the current configuration unchanged.

For portMirroring, an empty array can mean disable mirroring on all networks and if the param is completely omitted we don't change anything.

For network, I am not sure how to handle it. Using the empty string to indicate disconnect is a pretty dirty hack. Line 4890: {'type': 'vmUpdateInterfaceDeviceParams', Line 4891: 'data': {'*network': 'str', '*linkActive': 'bool', Line 4892: 'alias': 'str', '*portMirroring': '[str]'}} Line 4893:

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

5 Dec 5 Dec

9:53 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/208/ (1/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

9:53 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/242/ (2/2)

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

Jenkins CI RO

9:55 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 6:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/208/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/242/ : SUCCESS

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

agl＠us.ibm.com

11:51 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Adam Litke has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

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

Waiting on others who participated in the discussion to approve as well.

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

Dan Kenigsberg

1:12 p.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Dan Kenigsberg has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 6: Looks good to me, approved

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

shaohef＠linux.vnet.ibm.com

9:01 p.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

ShaoHe Feng has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

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

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

shaohef＠linux.vnet.ibm.com

9:45 p.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

ShaoHe Feng has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

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

(1 inline comment)

sorry, -1 for I did not find "updateDevice" define.

when I reviewed the depended patch, I find it was discarded

.................................................... File vdsm/BindingXMLRPC.py Line 263: return vm.hotunplugNic(params) Line 264: Line 265: def vmUpdateDevice(self, vmId, params): Line 266: vm = API.VM(vmId) Line 267: return vm.updateDevice(params) O, in you depended patch set-14 you define updateDevice for libvirtvm class : http://gerrit.ovirt.org/#/c/9562/14/vdsm/libvirtvm.py

but you have discarded updateDevice http://gerrit.ovirt.org/#/c/9562/17/vdsm/libvirtvm.py.

I also grep the whole vdsm project. did not find it. git grep "updateDevice" Line 268: Line 269: def vmSnapshot(self, vmId, snapDrives): Line 270: vm = API.VM(vmId) Line 271: return vm.snapshot(snapDrives)

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

asegurap＠redhat.com

6 Dec 6 Dec

3:26 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Antoni Segura Puimedon has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 6: (1 inline comment)

.................................................... File vdsm/BindingXMLRPC.py Line 263: return vm.hotunplugNic(params) Line 264: Line 265: def vmUpdateDevice(self, vmId, params): Line 266: vm = API.VM(vmId) Line 267: return vm.updateDevice(params) You're right. I will reinstate updateDevice in patch set-14 now. So this and the following patch will be right. Line 268: Line 269: def vmSnapshot(self, vmId, snapDrives): Line 270: vm = API.VM(vmId) Line 271: return vm.snapshot(snapDrives)

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

Jenkins CI RO

7 Dec 7 Dec

7:35 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 7:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/279/ (2/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

7:35 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 7:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/245/ (1/2)

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

Jenkins CI RO

7:36 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 7:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/245/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/279/ : SUCCESS

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

Dan Kenigsberg

8:06 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Dan Kenigsberg has posted comments on this change.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

Patch Set 7: Verified; Looks good to me, approved

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

Gerrit-MessageType: comment Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Dan Kenigsberg

8:06 a.m.

New subject: Change in vdsm[master]: netwiring: [2/4] Add API definitions.

Dan Kenigsberg has submitted this change and it was merged.

Change subject: netwiring: [2/4] Add API definitions. ......................................................................

netwiring: [2/4] Add API definitions.

Adds the schema definitions as for vmUpdateDevice described in: http://wiki.ovirt.org/wiki/Feature/DetailedNetworkWiring#New_API

Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm_api/vdsmapi-schema.json 3 files changed, 67 insertions(+), 1 deletion(-)

Approvals: Dan Kenigsberg: Verified; Looks good to me, approved

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

Gerrit-MessageType: merged Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Alona Kaplan alkaplan@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

4809

Age (days ago)

4817

Last active (days ago)

vdsm-patches@lists.fedorahosted.org

28 comments

5 participants

tags (0)

participants (5)

agl＠us.ibm.com
asegurap＠redhat.com
Dan Kenigsberg
Jenkins CI RO
shaohef＠linux.vnet.ibm.com