Igor Lvovsky has posted comments on this change.

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


Patch Set 14: (3 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 1522: 
Line 1523:         return {'status': doneCode, 'vmList': self.status()}
Line 1524: 
Line 1525:     def _updateInterfaceDevice(self, params):
Line 1526:         if 'alias' not in params:
I saw your answer for this but I still think that this verification should be 
in API.py same as deviceType.
The only way to be here is via vmUpdateDevice in API.py
According to your code there is no valid flow to get 'deviceType' in 
vmUpdateDevice but not to get 'alias'.
Line 1527:             self.log.error('Missing the required alias parameters.')
Line 1528:             return {'status': {'code': 
errCode['MissParam']['status']['code'],
Line 1529:                                'message': 'Missing the required 
alias '
Line 1530:                                'parameter'}}


Line 1626:                         {'code': 
errCode['updateDevice']['status']['code'],
Line 1627:                          'message': e.message}}
Line 1628: 
Line 1629:         # TODO: Update the VM conf and Nic instance.
Line 1630:         self._getUnderlyingNetworkInterfaceInfo()
Probably it's will work but don't forget that this one run over *all* network 
interfaces.
Maybe this is too much?
But on other hand better to gather updated info from libvirt instead setting it 
according to given parameters
Line 1631: 
Line 1632:         return {'status': doneCode, 'vmList': self.status()}
Line 1633: 
Line 1634:     def updateDevice(self, params):


....................................................
File vdsm/vdsmd.init.in
Line 486:     then
Line 487:         log_failure_msg "$prog: Failed to create ephemeral dummy 
bridge."
Line 488:         return $ret_val
Line 489:     fi
Line 490: 
I am not sure, but...
Should we clean up the bridge at vdsm stop?
Line 491:     @VDSMDIR@/vdsm-restore-net-config
Line 492:     /usr/bin/vdsm-tool load-needed-modules
Line 493:     mk_data_center
Line 494:     mk_core_path


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b9b4f49f80466a83e3e13f1042ac2a8866c6bcd
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to