Change in vdsm[master]: BZ#800915 - Remove VM fails due to unnecessary join in remov... - vdsm-patches

Change in vdsm[master]: BZ#800915 - Remove VM fails due to unnecessary join in remov...

Change in vdsm[master]: Fix...

Change in vdsm[master]: BZ#802229...

Haim Ateya

Sunday, 11 March 2012 Sun, 11 Mar '12

5:44 a.m.

Haim Ateya has uploaded a new change for review. Change subject: BZ#800915 - Remove VM fails due to unnecessary join in removeVM command ...................................................................... BZ#800915 - Remove VM fails due to unnecessary join in removeVM command Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d --- M vdsm/API.py 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/2694/1 -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com>

Show replies by date

Igor Lvovsky

Tuesday, 13 March Tue, 13 Mar

8:52 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) No, ovirt-engine never send list here. It always sent string, means that this call probably was broken awhile. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Haim Ateya

Sunday, 11 March Sun, 11 Mar

8 a.m.

Haim Ateya has posted comments on this change. Change subject: BZ#800915 - Remove VM fails due to unnecessary join in removeVM command ...................................................................... Patch Set 2: Verified -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>

Dan Kenigsberg

7:47 a.m.

Dan Kenigsberg has posted comments on this change. Change subject: BZ#800915 - Remove VM fails due to unnecessary join in removeVM command ...................................................................... Patch Set 1: I would prefer that you didn't submit this (1 inline comment) .................................................... File vdsm/API.py Line 904: # This internal API deviates horribly in that it takes a apparently this comment is incorrect - it should be dropped as well. However, it seems that it was well-thought. Adam, do you care to comment? -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>

Igor Lvovsky

9:04 a.m.

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - Remove VM fails due to unnecessary join in removeVM command ...................................................................... Patch Set 2: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>

Igor Lvovsky

Wednesday, 14 March Wed, 14 Mar

5:06 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 6: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

mkublin＠redhat.com

Tuesday, 13 March Tue, 13 Mar

4:48 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Michael Kublin has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 3: Looks good to me, but someone else must approve Yes, we never send a list -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

Wednesday, 14 March Wed, 14 Mar

8:07 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has submitted this change and it was merged. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... BZ#800915 - no need to support list in removeVM command Since engine-core use string in removeVM command and never sends vdsm a list of vms, there is no need to support it in the code. change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/storage/hsm.py M vdsm/storage/sp.py M vdsm_cli/vdsClient.py 5 files changed, 18 insertions(+), 23 deletions(-) Approvals: Ewoud Kohl van Wijngaarden: Looks good to me, but someone else must approve Haim Ateya: Verified Igor Lvovsky: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

5:05 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 5: (1 inline comment) .................................................... File vdsm/BindingXMLRPC.py Line 812: (self.poolRemoveVm, 'removeVM'), As I wrote before, I rechecked it and it always was removeVM. removeVMs was introduced by Adam. So, no BC issue -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

ewoud＠kohlvanwijngaarden.nl

5:02 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 6: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Haim Ateya

4:16 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Haim Ateya has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 6: Verified -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

abaron＠redhat.com

Tuesday, 13 March Tue, 13 Mar

6:40 p.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ayal Baron has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 5: I would prefer that you didn't submit this (1 inline comment) .................................................... File vdsm/BindingXMLRPC.py Line 812: (self.poolRemoveVm, 'removeVM'), How is this backwards compatible? should be: (self.poolRemoveVm, 'removeVM'), (self.poolRemoveVm, 'removeVMs'), -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

ewoud＠kohlvanwijngaarden.nl

11:21 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 5: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

11:12 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: I would prefer that you didn't submit this (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) OK, I rechecked it and actually the removeVMs was introduced by Adam and the original one was removeVM. So, we have not compatibility issue and I will fix it. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

abaron＠redhat.com

10:55 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ayal Baron has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: I would prefer that you didn't submit this (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) No actually the old code didn't. The join was introduced by Adam in his API.py and it broke backwards compatibility. However, there is no reason to keep the plural in API.py, only in the xmlrpc bindings, so I agree that this should be changed here. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

ewoud＠kohlvanwijngaarden.nl

9:23 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: No score (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) That sounds very odd because the old code did use ','.join(vmList) which also works on a string.

...

>> ','.join('abc')

'a,b,c' But if you're sure this code works (can't test right now) adding a note should suffice Since the API will be replaced by a REST version anyway. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

ewoud＠kohlvanwijngaarden.nl

8:46 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) Isn't this already an API change? you switch from a list to string. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

8:34 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: (1 inline comment) .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) In general I agree with you, but we can't change the API now because it's not backward compatible -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

ewoud＠kohlvanwijngaarden.nl

8:23 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: I would prefer that you didn't submit this (2 inline comments) I think mixing plural and singular in functions violates the Principle of least astonishment. .................................................... File vdsm/API.py Line 904: return self._irs.removeVM(self._UUID, vmUUID, sdUUID) I think the name removeVMs is weird because it implies multiple VMs can be removed, but the arguments and code suggest it can't. .................................................... File vdsm/BindingXMLRPC.py Line 579: return pool.removeVMs(vmUUID, sdUUID) Same here -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Haim Ateya

8:12 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Haim Ateya has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: Verified -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

8:09 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 4: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Igor Lvovsky

7:26 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Igor Lvovsky has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 3: I would prefer that you didn't submit this (3 inline comments) Just minor comments .................................................... File vdsm/storage/hsm.py Line 1099: :type vmList: list Fix this one too Line 1111: pool.removeVM(vmUUID, sdUUID) please, keep the key=value style .................................................... File vdsm/storage/sp.py Line 1225: self.log.info("spUUID=%s vmUUID=%s sdUUID=%s", self.spUUID, str(vmUUID), sdUUID) you don't need str() here -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com> Gerrit-Reviewer: Michael Kublin <mkublin(a)redhat.com>

Haim Ateya

4:26 a.m.

New subject: Change in vdsm[master]: BZ#800915 - no need to support list in removeVM command

Haim Ateya has posted comments on this change. Change subject: BZ#800915 - no need to support list in removeVM command ...................................................................... Patch Set 3: Verified Tested all parts of the code - please note that engine-core never send us a list (we made sure of that), so this change will backward compatible. -- To view, visit http://gerrit.ovirt.org/2694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0e5848ece1aa147f4508d4d16842bcdb30af0d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>

4432

days inactive

4435

days old

vdsm-patches@lists.fedorahosted.org

Manage subscription

22 comments

6 participants

tags (0)

participants (6)

abaron＠redhat.com
Dan Kenigsberg
ewoud＠kohlvanwijngaarden.nl
Haim Ateya
Igor Lvovsky
mkublin＠redhat.com

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Change in vdsm[master]: BZ#800915 - Remove VM fails due to unnecessary join in remov...