Igor Lvovsky has uploaded a new change for review.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Add hotPlug/hotUnplug NIC feature
Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/define.py M vdsm/libvirtvm.py 4 files changed, 118 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/1388/1 -- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Eduardo has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
(8 inline comments)
Passed with objections. Dan, up to you.
.................................................... File vdsm/API.py Line 364: IMHO this two functions are unnecessary. There are only validates (oh my...) Better if will be done in the level using the parameters.
.................................................... File vdsm/BindingXMLRPC.py Line 245: def vmHotplugNic(self, params): All this functions but vm*plug* (4) + vmMigrate (1) receive vmId as 1st param and pass all the others. May be better to make input uniform and wrap all this in invariant code.
.................................................... File vdsm/libvirtvm.py Line 1295: 'message': e.message}} IMHO: is better to unify all the return values. In addition: get an errCode['hotplugNic'] and change the message.
Line 1303: self.saveState() I'm not sure here.
Line 1314: if dev.macAddr.lower() == nicParams['macAddr'].lower(): Why do you suspect that are UPPER mac's in devices? Enforce it when adding.
Line 1328: self._devices[vm.NIC_DEVICES].remove(nic) If you worry about thread safety, this remove can raise.
Line 1331: for dev in self.conf['devices'][:]: Do list comprehension.
Line 1353: 'message': e.message}} Same as before.
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 1: (4 inline comments)
.................................................... File vdsm/BindingXMLRPC.py Line 245: def vmHotplugNic(self, params): Separate patch
.................................................... File vdsm/libvirtvm.py Line 1314: if dev.macAddr.lower() == nicParams['macAddr'].lower(): you are right, but we need to do it in several places. So, let's fix it later on separate patch
Line 1328: self._devices[vm.NIC_DEVICES].remove(nic) It's not
Line 1331: for dev in self.conf['devices'][:]: Why? It's the same thing
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 1:
Dan, hols it please I preffered pushed it after Federico's http://gerrit.ovirt.org/#change,1389
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 1:
Ok, I've pushed http://gerrit.ovirt.org/1389 even though it is raceful.
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 2: Verified
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 2: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 3: Verified
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Patch Set 3: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Add hotPlug/hotUnplug NIC feature ......................................................................
Add hotPlug/hotUnplug NIC feature
Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/define.py M vdsm/libvirtvm.py 4 files changed, 124 insertions(+), 1 deletion(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Igor Lvovsky: Verified
-- To view, visit http://gerrit.ovirt.org/1388 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Iecd56650308f5836913f513fffc179f39fc8f6cc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
vdsm-patches@lists.fedorahosted.org