Change in vdsm[master]: BZ#844180: Change scsi scan to asynchronous

2012-08-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#844180: Change scsi scan to asynchronous
..


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

(1 inline comment)


File vdsm/storage/iscsi.py
Line 354: processes = []
Line 355: minTimeout = config.getint('irs', 'scsi_rescan_minimal_timeout')
Line 356: maxTimeout = config.getint('irs', 'scsi_rescan_maximal_timeout')
Line 357: for hba in glob.glob(SCAN_PATTERN):
Line 358: cmd = ['echo', '- - -', '|', '/usr/bin/sudo -n','/bin/dd 
of=', hba]
This method is run in supervdsm, why do we need sudo at all?

also, why use echo here? can pass it in data as follows:

cmd = [/bin/dd', 'of=', hba]

misc.execCmd(cmd, data='- - -', sync=False)
Line 359: processes.append((hba, misc.execCmd(cmd, sudo=False, 
sync=False)))
Line 360: if (minTimeout  maxTimeout or minTimeout  0):
Line 361: minTimeout = 2
Line 362: maxTimeout = 30


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7665ebaed716f75af4d03ec1b6a9ff9ff5d84853
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Daniel Paikov pai...@gmail.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: wip - vdsm: improve message when trying to attach import dom...

2012-08-20 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: wip - vdsm: improve message when trying to attach import domain 
with wrong permissions (#842146)
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I76880b1445f259431d7aa691eda676e1210308e4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali oourf...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Oved Ourfali oourf...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#846323 - Search PV's belonging to the VG in removeVG.

2012-08-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#846323 - Search PV's belonging to the VG in removeVG.
..


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

(2 inline comments)


File vdsm/storage/lvm.py
Line 859: 
Line 860: 
Line 861: def removeVG(vgName):
Line 862: pvs = tuple(pv for pv in _lvminfo._pvs.iterkeys()
Line 863: if not isinstance(pv, Stub) and pv.vg_name == vgName)
This should be done *after* call to vgremove
Line 864: # PVS needs to be reloaded anyhow: if vg is removed they are 
staled,
Line 865: # if vg remove failed, something must be wrong with devices and 
we want
Line 866: # cache updated as well
Line 867: _lvminfo._invalidatepvs(pvs)


Line 863: if not isinstance(pv, Stub) and pv.vg_name == vgName)
Line 864: # PVS needs to be reloaded anyhow: if vg is removed they are 
staled,
Line 865: # if vg remove failed, something must be wrong with devices and 
we want
Line 866: # cache updated as well
Line 867: _lvminfo._invalidatepvs(pvs)
invalidating before removing VG is raceful (can be refreshed before removal of 
vg)
Line 868: # Remove the vg from the cache
Line 869: _lvminfo._vgs.pop(vgName, None)
Line 870: # and now destroy it
Line 871: cmd = [vgremove, -f, vgName]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica565f74774fd1dcce7c18361aef5e1464c78b68
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Daniel Paikov pai...@gmail.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ829710 Get VMList with oop if necessary

2012-08-17 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ829710 Get VMList with oop if necessary
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I021e9d03b29dfc9cf1c3b7fa5ec8a302e7912cf7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Barak Azulay bazu...@redhat.com
Gerrit-Reviewer: Dafna Ron d...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Revert BZ#842631 Use domain proxies instead of actual domai...

2012-08-17 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Revert BZ#842631 Use domain proxies instead of actual domain 
references
..


Patch Set 1: (1 inline comment)

Don't get me wrong, I'm all for this patch, but you need to explain the 
incentive...


Commit Message
Line 3: AuthorDate: 2012-08-17 05:32:18 -0400
Line 4: Commit: Federico Simoncelli fsimo...@redhat.com
Line 5: CommitDate: 2012-08-17 10:48:42 -0400
Line 6: 
Line 7: Revert BZ#842631 Use domain proxies instead of actual domain 
references
why?
Line 8: 
Line 9: This reverts commit 942c2dc8d317b6180529ed7df4eee98fbf1b9836.
Line 10: 
Line 11: Signed-off-by: Federico Simoncelli fsimo...@redhat.com


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Ship the version file with the tarballs

2012-08-17 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Ship the version file with the tarballs
..


Patch Set 1:

Why is this dependent on the domain monitor patches?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b72a1740803a9401e4b5a4504a4faa07c29f2b9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: DomainMonitor should use use real domains (no proxy)

2012-08-17 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: DomainMonitor should use use real domains (no proxy)
..


Patch Set 1: (1 inline comment)


Commit Message
Line 3: AuthorDate: 2012-08-17 06:55:54 -0400
Line 4: Commit: Federico Simoncelli fsimo...@redhat.com
Line 5: CommitDate: 2012-08-17 10:48:55 -0400
Line 6: 
Line 7: DomainMonitor should use use real domains (no proxy)
s/use use/use/

Why? (again, I'm all for this, but need to explain)
Line 8: 
Line 9: In this patch:
Line 10: * produce the domain when refreshing
Line 11: * remove the remaining getRealDomain (DomainProxy) calls


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#843387 - Payload should set volPath or raise.

2012-08-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#843387 - Payload should set volPath or raise.
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d5b36a88752ad2191226191c9502c1717033bbc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Gal Hammer gham...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: Xu He Jie x...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#846323 - Search PV's belonging to the VG in removeVG.

2012-08-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#846323 - Search PV's belonging to the VG in removeVG.
..


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

(2 inline comments)


Commit Message
Line 3: AuthorDate: 2012-08-09 18:41:47 +0300
Line 4: Commit: Eduardo Warszawski ewars...@redhat.com
Line 5: CommitDate: 2012-08-09 19:00:27 +0300
Line 6: 
Line 7: BZ#846323 - Search PV's belonging to the VG in removeVG.
why?
Line 8: 
Line 9: Only one getAllPvs() call remains!
Line 10: 
Line 11: Change-Id: Ica565f74774fd1dcce7c18361aef5e1464c78b68



File vdsm/storage/lvm.py
Line 864: rc, out, err = _lvminfo.cmd(cmd)
Line 865: if rc != 0:
Line 866: log.error(Getting pvs for vg %s failed. (%s, %s, %s),
Line 867: vgName, rc, out, err)
Line 868: # _lvminfo._invalidateAllPvs() # Let's discusse this
this code is too complicated.
The function should be:
  vgremove
  invalidateAllPVs

That's it.
Line 869: pvs = tuple()
Line 870: else:
Line 871: pvs = tuple(pvName.strip() for pvName in out)
Line 872: # Remove the vg from the cache


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica565f74774fd1dcce7c18361aef5e1464c78b68
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Daniel Paikov pai...@gmail.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Provisional fix: the domain is produced in the monitor threa...

2012-08-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Provisional fix: the domain is produced in the monitor thread.
..


Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a2d332b7bc930e681ba23141978ea4f523338d5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Royce Lv lvro...@linux.vnet.ibm.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Report hsm tasks in getAllTasksStatuses and getAllTasksInfo

2012-08-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Report hsm tasks in getAllTasksStatuses and getAllTasksInfo
..


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

Doesn't this break engine?
1 scenario that pops into mind is engine stopping all tasks after spm failover.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40a42db6554e5145d05f913c742d00dc26969899
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ew...@kohlvanwijngaarden.nl
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Properly propagate pool timeout in file handler

2012-08-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Properly propagate pool timeout in file handler
..


Patch Set 6: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie32aa474d3ab9ec455c15415df20b749d54e5658
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ew...@kohlvanwijngaarden.nl
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add deathSignal options to better peopen

2012-08-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add deathSignal options to better peopen
..


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

(4 inline comments)


Commit Message
Line 7: Add deathSignal options to better peopen
s/peopen/popen/


File vdsm/betterPopen/__init__.py
Line 37: # deathDignal is the signal that will be sent to the child 
process
s/Dignal/Signal

Line 39: # popen and when using standard peopen we should make sure 
we
s/peopen/popen/

Line 40: # reimplement this.
have you suggested this in upstream python?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f987129cea112e2a75d6f02477369417cc50dc7
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ew...@kohlvanwijngaarden.nl
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
Gerrit-Reviewer: Xu He Jie x...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add missing log object to CrabRPCServer

2012-08-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add missing log object to CrabRPCServer
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I711d0d4f627a1f42886a65305f93bcd97deccd3f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Check the metadata on domain validation

2012-08-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Check the metadata on domain validation
..


Patch Set 1: Looks good to me, approved

Note that BZ# should be in commit message when merging.

Bug 847328 - VDSM domain validation should check the presence of the metadata

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35071b85da2689cd576e9fd0d97cc374db26bf5c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Support the quiesce flag during a live snapshot

2012-08-08 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Support the quiesce flag during a live snapshot
..


Patch Set 5: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61c7c89fb42afa627b3e45cd74af03e032452a7d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Royce Lv lvro...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#844656 Respawn the domain monitor when needed

2012-08-08 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#844656 Respawn the domain monitor when needed
..


Patch Set 6: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47e4fb6883d72578ee8083e46e4f660b656a5ffc
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Omri Hochman hoo...@gmail.com
Gerrit-Reviewer: Royce Lv lvro...@linux.vnet.ibm.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#846014 - Fix virtio hotplug disk fail to a VM with IDE di...

2012-08-07 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#846014 - Fix virtio hotplug disk fail to a VM with IDE disks.
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8f9a317dd07f3fb8f69ecf4c647e91e5e9064dc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Paikov pai...@gmail.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Gal Hammer gham...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#844656 Respawn the domain monitor when needed

2012-08-07 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#844656 Respawn the domain monitor when needed
..


Patch Set 5: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47e4fb6883d72578ee8083e46e4f660b656a5ffc
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ829710 Get VMList with oop if necessary

2012-08-06 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ829710 Get VMList with oop if necessary
..


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

The oop part should be in the relevant storage domain and not in sd.py.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I021e9d03b29dfc9cf1c3b7fa5ec8a302e7912cf7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Barak Azulay bazu...@redhat.com
Gerrit-Reviewer: Dafna Ron d...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#844656 Respawn the domain monitor when needed

2012-07-31 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#844656 Respawn the domain monitor when needed
..


Patch Set 3: Looks good to me, approved

(1 inline comment)


Commit Message
Line 12: for the possible exceptions and respawn the domain monitor if it exists
s/exists/exits/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47e4fb6883d72578ee8083e46e4f660b656a5ffc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#844294 Add requiresMailbox to StorageDomain

2012-07-30 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#844294 Add requiresMailbox to StorageDomain
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If26f06102b2ca54ee6fdb2cd617302b0b1ccbea7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: [WIP] Use 1Mb cluster size for the qcow volumes

2012-07-29 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: [WIP] Use 1Mb cluster size for the qcow volumes
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c69359561193e8b7b9108d5970a06aa9d1ea6cc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: [WIP] Use 1Mb cluster size for the qcow volumes

2012-07-29 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: [WIP] Use 1Mb cluster size for the qcow volumes
..


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

After reconsideration it may be preferable to limit this to live snapshots 
which are aimed for live storage migration only.  nacking until conclusion is 
reached.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c69359561193e8b7b9108d5970a06aa9d1ea6cc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#840386: vms with shared disk will pause...

2012-07-29 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#840386: vms with shared disk will pause...
..


Patch Set 4: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82714a3a4fd86184b8ed48ef33ce69d0640d96ed
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the hostId parameter to reconstructMaster

2012-07-25 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the hostId parameter to reconstructMaster
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c85d9440b467eb755d196a6548d2f7b12bcab9f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add missing import config in domainMonitor

2012-07-25 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add missing import config in domainMonitor
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I077d6e362ff3e1d09f6053dd326aafb21513a1a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Uniform the block and file volume methods

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Uniform the block and file volume methods
..


Patch Set 25: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib945853e68f6a4562668468c7a24d218c4921cef
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Unify the volume creation code in volume.create

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Unify the volume creation code in volume.create
..


Patch Set 26: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e44da32351a420f0536505985586b24ded81a2a
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the formatConverter for Storage Domain V3

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the formatConverter for Storage Domain V3
..


Patch Set 19: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74185a4521fb88444c88de628a18c2210359af29
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Xu He Jie x...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Orthogonal storage repository conversion

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Orthogonal storage repository conversion
..


Patch Set 36: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f43faaa9578ddafbae8e3aa01c02b1d42b177ab
Gerrit-PatchSet: 36
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Use domain proxies instead of actual domain references

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Use domain proxies instead of actual domain references
..


Patch Set 35: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdedd601ca25e6cac3f1f68f2917776401c0d838
Gerrit-PatchSet: 35
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ew...@kohlvanwijngaarden.nl
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add deathSignal options to better peopen

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add deathSignal options to better peopen
..


Patch Set 3: (3 inline comments)


Commit Message
Line 9: This allows VDSM to specify as signal that will be sent by the kernel to
s/as/a/


File vdsm/betterPopen/__init__.py
Line 37: # deathDignal is the signal that will be sent to the child 
process
s/Dignal/Signal/

Line 39: # popen and when using standard peopen we should make sure 
we
s/peopen/popen/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f987129cea112e2a75d6f02477369417cc50dc7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#842631 Use domain proxies instead of actual domain refere...

2012-07-24 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#842631 Use domain proxies instead of actual domain references
..


Patch Set 36: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdedd601ca25e6cac3f1f68f2917776401c0d838
Gerrit-PatchSet: 36
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ew...@kohlvanwijngaarden.nl
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Separate the Volume.share implementation

2012-07-23 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Separate the Volume.share implementation
..


Patch Set 22: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I965b56dc98fe2d6db3ef6f3d7efe2a7b9791b555
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove the unused nocache option

2012-07-23 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Remove the unused nocache option
..


Patch Set 19: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71662f8c4c717112f783e8f5d2ce3fd13f9c
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Uniform the block and file volume methods

2012-07-23 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Uniform the block and file volume methods
..


Patch Set 23: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib945853e68f6a4562668468c7a24d218c4921cef
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Unify the volume creation code in volume.create

2012-07-23 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Unify the volume creation code in volume.create
..


Patch Set 24: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e44da32351a420f0536505985586b24ded81a2a
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add stdin tests to betterPopen

2012-07-22 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add stdin tests to betterPopen
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I043907ee0346758a70c51c97f06d8249e653520a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#840407 - Create a fake template when moving to backup SD.

2012-07-22 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#840407 - Create a fake template when moving to backup SD.
..


Patch Set 1: (2 inline comments)


Commit Message
Line 12: All the related fake template code should be removed.
this sentence is redundant and out of context.


File vdsm/storage/hsm.py
Line 1346: tName = e.absentTemplateUUID
you're assuming that only missing template would throw this exception.
Although correct for current implementation, the exception is generic and might 
be reused...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbeef2480e03cc075b80880485739139e496d0b6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Use SANLock lease offset when present

2012-07-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Use SANLock lease offset when present
..


Patch Set 6: (1 inline comment)


File vdsm/storage/blockVolume.py
Line 49: # lvm tags it's used in several places to extract the values.
s/it's used.*$//

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I27195f29efa691aadda0cac5afef55b04cc01658
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Support the quiesce flag during a live snapshot

2012-07-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Support the quiesce flag during a live snapshot
..


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

(2 inline comments)


File vdsm/libvirtvm.py
Line 1794: and type(e) == libvirt.libvirtError):
can't we check the specific error in case quiesce failed?

Line 1813: break
how can management differentiate between a snapshot that was taken with quiesce 
and one that was not?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61c7c89fb42afa627b3e45cd74af03e032452a7d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: MOM Integration

2012-07-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: MOM Integration
..


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

(5 inline comments)

Minor issues


File vdsm/ksm.py
Line 114: raise Exception('Invalid KSM tuning parameter: %s=%s' % 
(k, v))
you could also make sure that values are proper (run in [0,1,2] etc)


File vdsm/libvirtvm.py
Line 2067: if self.cif.ksmMonitor:
for symmetry perhaps this should be if not self.mom


File vdsm/vm.py
Line 572: and self.cif.ksmMonitor:
again, perhaps not self.mom

Line 574: # behaviors on VM start/destroy. Because the 
tuning can be
s/. B/, b/

Line 575: # done automatically acccording its statistics 
data.
s/its statistics/to its statistical/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu wu...@linux.vnet.ibm.com
Gerrit-Reviewer: Adam Litke a...@us.ibm.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Doron Fediuck dfedi...@redhat.com
Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
Gerrit-Reviewer: Xu He Jie x...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#826921: Don't ignore nfs_mount_options in vdsm.conf

2012-07-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#826921: Don't ignore nfs_mount_options in vdsm.conf
..


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

(3 inline comments)


File vdsm/storage/storageServer.py
Line 255: 4.0 of RHEV. Please upgrade domains to V2 or 
greater and 
s/V2/V3/

Line 265: self.__dict__[optMap[parts[0]]] = parts[1]
instead of messing around with the properties:
add this at start of method:
opts = {'retrans': None, 'version': None, 'timeout': None}
then:
if parts[0] in opts:
 opts[parts[0]] = parts[1]

then move assignment to self._timeout and friends to end of method
self._timeout = opts['timeout']...

Line 269: timeout = 600;
this would change to opts['timeout']
etc

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1d13bfd98a9b41b9728bdcb7f01b3161c26bd8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett gpadg...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the hostId parameter to reconstructMaster

2012-07-20 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the hostId parameter to reconstructMaster
..


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

(1 inline comment)


File vdsm/storage/hsm.py
Line 819: if not msdUUID or not masterVersion:
these params should be retrieved under lock and only in case we're not already 
connected.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If665af4ed8be9b5b53dffc7e1fc258b818bf7f98
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Separate the Volume.share implementation

2012-07-20 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Separate the Volume.share implementation
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I965b56dc98fe2d6db3ef6f3d7efe2a7b9791b555
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove the unused nocache option

2012-07-20 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Remove the unused nocache option
..


Patch Set 18: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71662f8c4c717112f783e8f5d2ce3fd13f9c
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Uniform the block and file volume methods

2012-07-20 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Uniform the block and file volume methods
..


Patch Set 22: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib945853e68f6a4562668468c7a24d218c4921cef
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Fix detachStorageDomain for the ISO domains

2012-07-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Fix detachStorageDomain for the ISO domains
..


Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf531f99d3d7304139def10fc6fd3ee4eedda28
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Moran Goldboim mgold...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#834893: vms with shared disk will pause...

2012-07-16 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#834893: vms with shared disk will pause...
..


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

(1 inline comment)


File vdsm/libvirtvm.py
Line 1058: if hasattr(self, 'shared') and utils.tobool(self.shared):
all params are mandatory, it makes the API simpler.
No need for hasattr, previous version was fine

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82714a3a4fd86184b8ed48ef33ce69d0640d96ed
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the hostId parameter to reconstructMaster

2012-07-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the hostId parameter to reconstructMaster
..


Patch Set 13:

Not sure why my comments weren't published before.
Trying again

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If665af4ed8be9b5b53dffc7e1fc258b818bf7f98
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the hostId parameter to reconstructMaster

2012-07-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the hostId parameter to reconstructMaster
..


Patch Set 13: (3 inline comments)


File vdsm/storage/sp.py
Line 746: else:
You did not comment on this, do you agree? nack?

Line 750:   safeLease)
it should be solved in a way that if connect is modified this is automatically 
modified as well (i.e. call _connect or something)

Line 764: elif temporaryLock == False:
it would be easiest if it were with clusteredLock where you'd set 
clusteredLock to the appropriate object before.
That way you don't need 'temporaryLock' at all.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If665af4ed8be9b5b53dffc7e1fc258b818bf7f98
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#834893: vms with shared disk will pause...

2012-07-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#834893: vms with shared disk will pause...
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82714a3a4fd86184b8ed48ef33ce69d0640d96ed
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Eli Mesika emes...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the formatConverter for Storage Domain V3

2012-07-15 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the formatConverter for Storage Domain V3
..


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

(11 inline comments)


Commit Message
Line 7: Add the formatConverter for Storage Domain V3
which supports what?


File vdsm/storage/imageRepository/formatConverter.py
Line 41: newMetadata._dict.update(metadata)
why use an intermediate?

Line 93: type(vol).newVolumeLease(metaId, vol.sdUUID, vol.volUUID)
what would happen if lease already exists? (e.g. because of a partial upgrade 
before)
looking at the blockVolume implementation we call:
sanlock.init_resource(sdUUID, volUUID, [(leasePath, leaseOffset)])

but there is no try..except there or any check to see whether it finished 
successfully?

Line 97: metaVolSize = int(vol.getMetaParam(volume.SIZE))
this part should be refactored into a separate function (resetVolSize or 
something)

Line 123: # XXX: The only reason to prepare the image is to verify 
the volume
XXX?

Line 128: # The analyzed scenario are:
s/scenario/scenarios/

Line 130: # This is safe because the prepare is superflous and 
the
s/superflous/superfluous/g

Line 143: # to read the image virtual size) and it can be 
restarted
why not retry in this case? it is unlikely to fail twice in a row and would 
improve upgrade process

Line 174: newClusterLock.release()
I would've preferred this to be:
with newClusterLock:
 ...

Line 183: newClusterLock.releaseHostId(hostId)
same here (with ...)

Line 191: domain._clusterLock.release()
this was never acquired?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74185a4521fb88444c88de628a18c2210359af29
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Xu He Jie x...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Unify the volume creation code in volume.create

2012-07-14 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Unify the volume creation code in volume.create
..


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

(3 inline comments)

Please note inline comments


File vdsm/storage/volume.py
Line 405: misc.validateUUID(argValue, argName, argAllowBlank)
it would have been shorter and perhaps a bit clearer just to write 
misc.validateUUID 4 times (it's just 4 lines as opposed to 7 here with 
indirection)

Line 443: size = volParent.getSize()
although this was the previous behaviour, what if we'd like to create a derived 
disk with a different size? or have this snapshot with a different size?

Line 486: if dom.hasVolumeLeases():
here too I'm not sure this should depend on domain version

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e44da32351a420f0536505985586b24ded81a2a
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Orthogonal storage repository conversion

2012-07-14 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Orthogonal storage repository conversion
..


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

(4 inline comments)

minor issues


Commit Message
Line 10: not correct as ion the future we will allow conversion from one domain
s/ion/in/
s/will/may/

Line 22: This also seperates the conversion process from the domain logic
s'seperates/separates/


File vdsm/storage/imageRepository/formatConverter.py
Line 30: _IMAGE_REPOSITORY_CONVERION_TABLE = {
s/CONVERION/CONVERSION/

Line 39: def _getConvereter(self, sourceFormat, targetFormat):
s/getConvereter/getConverter/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f43faaa9578ddafbae8e3aa01c02b1d42b177ab
Gerrit-PatchSet: 33
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the hostId parameter to reconstructMaster

2012-07-13 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the hostId parameter to reconstructMaster
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If665af4ed8be9b5b53dffc7e1fc258b818bf7f98
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: send POSIXFS instead of SHAREDFS

2012-07-12 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: send POSIXFS instead of SHAREDFS
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I73b0d29cf39a45589d90335e88ae84c5744796e1
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak lhorn...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Laszlo Hornyak lhorn...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yair Zaslavsky yzasl...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#833099 - Removing unused exceptions.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#833099 - Removing unused exceptions.
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2ae0b73a8b27169ea7c14362a13feb6f610c1a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#833099 - spmRole is a StoragePool attribute.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#833099 - spmRole is a StoragePool attribute.
..


Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b6f0dc0824f269de548de085fa484a7b096b8a2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#833099 - Induce MSD reconstruct when getSpmStatus fail.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#833099 - Induce MSD reconstruct when getSpmStatus fail.
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I903f696bac8470048f73ac63669cc8f79e3fd66f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#833099 - Removing unused exceptions.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#833099 - Removing unused exceptions.
..


Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2ae0b73a8b27169ea7c14362a13feb6f610c1a9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#833099 - spmRole is a StoragePool attribute.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#833099 - spmRole is a StoragePool attribute.
..


Patch Set 3: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b6f0dc0824f269de548de085fa484a7b096b8a2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#784931 - Fixing race condition in deactivateSD().

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#784931 - Fixing race condition in deactivateSD().
..


Patch Set 4: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If06f9cc8da81996590c366de9ad9ea2786a5d3ea
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Fix 2nd order exception msg in lvm._initpvs().

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Fix 2nd order exception msg in lvm._initpvs().
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48766ec89b53230974048c9c8144926d1c24a82d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove unnecesary preparePaths.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Remove unnecesary preparePaths.
..


Patch Set 1: Do not submit

I agree with Federico, this code is needed unless you find a different way to 
make sure that no other operation on these images can deactivate the LVs

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing the template activation leak.

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Introducing the template activation leak.
..


Patch Set 4: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedf863ac967f34405f038201bac324c52fbbe89
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Related to BZ#784931 - Removing the unused useCache paramete...

2012-07-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Related to BZ#784931 - Removing the unused useCache parameter.
..


Patch Set 1: Looks good to me, approved

(1 inline comment)


Commit Message
Line 9: Intentionally (A.B.) ignoring when the SD.validate(useCache=True)
me neither

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4a49e572b78f0fbf70aa515f320feb575ae631d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Do not acquire leases on shared volumes

2012-07-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Do not acquire leases on shared volumes
..


Patch Set 6: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a618141d0a76da3fdc5d4bf558fd07b8dacfec5
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the async kwarg to acquireHostId

2012-07-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the async kwarg to acquireHostId
..


Patch Set 19: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1f8cf540f262a9c2da795a97acabc011b55e0dd
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Monitor the host id in domainMonitor

2012-07-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Monitor the host id in domainMonitor
..


Patch Set 21: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c33ecdc2cd21c5afb445a339f400bc990b93998
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: send POSIXFS instead of SHAREDFS

2012-07-10 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: send POSIXFS instead of SHAREDFS
..


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

all references to SHAREDFS should just be replaced with POSIXFS.
No need for translation between the 2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I73b0d29cf39a45589d90335e88ae84c5744796e1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak lhorn...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Laszlo Hornyak lhorn...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yair Zaslavsky yzasl...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Don't ignore nfs_mount_options in vdsm.conf

2012-06-27 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Don't ignore nfs_mount_options in vdsm.conf
..


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

I take it back. engine run time per domain configuration should take precedence 
over config.
Reason we need config support to begin with is backwards compatibility (user 
set config, did not set in engine, now it no longer works).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1d13bfd98a9b41b9728bdcb7f01b3161c26bd8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett gpadg...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#784931 - Fixing raise condition from deactivateSD().

2012-06-26 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#784931 - Fixing raise condition from deactivateSD().
..


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

(7 inline comments)


Commit Message
Line 7: BZ#784931 - Fixing raise condition from deactivateSD().
s/raise/race/
s/from/in/


File vdsm/storage/mount.py
Line 46: def _runcmd(self, cmd, timeout):
You changed this into a module function but did not remove self, yet invoking 
methods do not pass self  which leads me to think that this code was never 
tested as it couldn't not work.

Line 48: p = misc.execCmd(cmd, sudo=not isRoot, sync=False)
if this requires sudo it probably means it should be run from superVdsm
I also don't understand why we need a new runcmd command (and yes, I know you 
just moved this function around)

Line 60: raise MountError(rc, ;.join((out, err)))
this function gets a generic cmd so MountError has no business here (and yes, I 
know you didn't change this, but then again, you probably shouldn't have moved 
it either)

Line 149: m = getMountFromTarget(target)
the assignment is redundant, just return getMountFromTarget

Line 152: return None
should return False, not None.

Line 154: else:
else is redundant

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If06f9cc8da81996590c366de9ad9ea2786a5d3ea
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#784931 - Fixing raise condition from deactivateSD().

2012-06-26 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#784931 - Fixing raise condition from deactivateSD().
..


Patch Set 2: (1 inline comment)


File vdsm/storage/sp.py
Line 1055: if e.errno != errno.ENOENT:
as discussed, at least switch this to positive semantics

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If06f9cc8da81996590c366de9ad9ea2786a5d3ea
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Use sparse images for alignmentScanTests

2012-06-22 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Use sparse images for alignmentScanTests
..


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8086c600b08e0008c63988abb88347d396d01f92
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saša Tomić tomi...@gmail.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-06-21 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


Patch Set 17: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Make domainMonitor compliant to PEP8

2012-06-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Make domainMonitor compliant to PEP8
..


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

Please note previous comment about aligning slots (can be a different patch 
so +1ing)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie707eed5a132704378f0fd61f57e84e527fbc06d
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove the traceback from the getVSize warning

2012-06-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Remove the traceback from the getVSize warning
..


Patch Set 8: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8f12065224636e65b8ccabcbdae83b41290229f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add the async kwarg to acquireHostId

2012-06-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Add the async kwarg to acquireHostId
..


Patch Set 13: (1 inline comment)


File vdsm/storage/safelease.py
Line 81: def acquireHostId(self, hostId, async=False):
why do you have a default value at all levels? (here and sd).
Seems like one should go?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1f8cf540f262a9c2da795a97acabc011b55e0dd
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Monitor the host id in domainMonitor

2012-06-19 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Monitor the host id in domainMonitor
..


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

(4 inline comments)


File vdsm/storage/domainMonitor.py
Line 31: vgMdFreeBelowThreashold, hasHostId)
you could align the lines here to comply with pep8

Line 95: # stopMonitor doesn't stop until the thread exists.
s/exists/exits/
s/stopMonitor/stopMonitoring/?

Line 144: if nextStatus.valid and nextStatus.hasHostId is False:
why isn't this done after emiting the state change event?


File vdsm/storage/sp.py
Line 681: for sdUUID in monitoredDomains:
why isn't this done inside stopMonitoring (inside domainMonitor.py)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c33ecdc2cd21c5afb445a339f400bc990b93998
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-06-18 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


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

(1 inline comment)


File vdsm/storage/volume.py
Line 464: def _legacySetRW(self, rwMode):
shouldn't this method be annotated as deprecated instead of having 'legacy' in 
the name?

also, why not have a decorator for the version logic? (only run if x  whatever)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-06-18 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


Patch Set 13: (1 inline comment)


File vdsm/storage/volume.py
Line 464: def _legacySetRW(self, rwMode):
if you're adding the decorator then why not put it directly on setrw?
then you don't need this function at all

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-06-12 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#808874 Skip master validation on repoStat

2012-06-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#808874 Skip master validation on repoStat
..


Patch Set 4: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I837877b5431edc3cfc6648e8fb654df2291413fc
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-06-11 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


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

(4 inline comments)


File vdsm/storage/volume.py
Line 450: if not sdCache.produce(self.sdUUID).hasVolumeLeases():
This has absolutely nothing to do with volumeLeases.
This just has to do with it being incompatible with live snapshots (internal 
snapshot might still be in use by VM on non-spm host)
Also, once we no longer support v2 domains it would be difficult to know that 
this piece of code is no longer relevant.

Line 517: self.setrw(rw=True)
this line would also be irrelevant in v3 domains, no?

Line 520: # Look at setInternal for more information on this
this looks exactly like setInternal, why not call it and have only 1 place that 
does this?

Line 521: if not sdCache.produce(self.sdUUID).hasVolumeLeases():
again has nothing to do with volume leases

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Change storageServer to handle numeric connection values

2012-06-07 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Change storageServer to handle numeric connection values
..


Patch Set 5: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d1716a6f66a108d60f39fad14d92625a692d0e3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky yzasl...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yair Zaslavsky yzasl...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Change storageServer to parse some params as strings

2012-06-06 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Change storageServer to parse some params as strings
..


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

(1 inline comment)


File vdsm/storage/storageServer.py
Line 241: options.append(timeo=%s % timeout)
the alternative is to do: 
int(timeout)
which would also verify that the parameter passed is ok.
Even better, can just change the xmlrpc bindings to convert it to int

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d1716a6f66a108d60f39fad14d92625a692d0e3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky yzasl...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yair Zaslavsky yzasl...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: getDeviceList - passing includePartitioned to HSM

2012-06-04 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: getDeviceList - passing includePartitioned to HSM
..


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

(3 inline comments)


File AUTHORS
Line 29:Deepak C Shetty deepa...@linux.vnet.ibm.com   
please remove trailing spaces


File vdsm_cli/vdsClient.py
Line 549: options = self._parseDriveSpec(args[1]) if len(args)  1 else 
{}
parse*Drive*Spec?
If it does what you need and is more general then name should be updated to 
reflect what it really does

Line 553: options['includePartitioned'] = 
options.get('includePartitioned', 
Please removing trailing space

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I388442f361ea4ae00e3d8878a2497c7819ab2214
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Change oop to be a new process instead of a fork

2012-06-03 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Change oop to be a new process instead of a fork
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65e58030e31fe54b0e79c2f2a415d081f890d41f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Reuse the stored pool host id on reconstructMaster

2012-06-03 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Reuse the stored pool host id on reconstructMaster
..


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

(2 inline comments)


File vdsm/storage/sp.py
Line 203: def _acquireHostId(self, sdUUID, isValid):
Why is this function here? it is never used?

Line 496: def _acquireTemporaryClusterLock(self, msdUUID, safeLease, 
hostId=None):
Reading this now it appears to me as though this function is misnamed as it 
doesn't make the cluster lock temporary at all, it is just *used* this way by 
the calling function.
Also, since we cannot reliably determine the host id I'm starting to think that 
the annoyance here outgrows the benefit and maybe we should just get rid of 
this 'protection' altogether, or at least get rid of the call in create and 
just use this in reconstructMaster (and then inline the funcion as I have no 
idea what to name it :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44ff6d04220032ac1f6bd0a7819132103050b516
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#784931 - Restore SD.validate() semantics.

2012-05-31 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#784931 - Restore SD.validate() semantics.
..


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

(1 inline comment)

Once commit message is fixed you can view this as a +2 from me.


Commit Message
Line 7: BZ#784931 - Restore SD.validate() semantics.
6 months ago with Federico's patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423777deafc4e1d9186054ec33f2744712065512
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#788640 - Remove subChainSizeCalc() and getSubChain(). Rem...

2012-05-29 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Remove subChainSizeCalc() and getSubChain(). Remove 
getAllChildrenList().
..


Patch Set 18: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffbbf71269eddb53c032c381d10e20b771c47c6b
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: getAllVolumes() unit test.

2012-05-29 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: getAllVolumes() unit test.
..


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

even though I don't like the external files.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7cfb77f7675a58858507acf2b708317eb30a43d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#788640 - Remove subChainSizeCalc() and getSubChain(). Rem...

2012-05-28 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Remove subChainSizeCalc() and getSubChain(). Remove 
getAllChildrenList().
..


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

(1 inline comment)


File vdsm/storage/image.py
Line 1036: return min(accumulatedChainSize, imageApparentSize) * 1.1, 
chain
This function should just return accumulatedChainSize.
It is up to the calling function to determine the usage and add the extra 10%, 
it has nothing to do with the actual size calculation

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffbbf71269eddb53c032c381d10e20b771c47c6b
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Shu Ming shum...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#788640 - Template relink refactored.

2012-05-28 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
..


Patch Set 15: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Internal volumes must be RW in domain version 3

2012-05-28 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Internal volumes must be RW in domain version 3
..


Patch Set 5: Looks good to me, approved

(1 inline comment)


File vdsm/storage/volume.py
Line 450: if not sdCache.produce(self.sdUUID).hasVolumeLeases():
I don't like the placement of this logic but with the current design I find it 
hard to think of anything better.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fa99971390d3e26ff9c65cb883e1a369bd626d7
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Use SANLock lease offset when present

2012-05-28 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: Use SANLock lease offset when present
..


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

(9 inline comments)


File vdsm/storage/blockSD.py
Line 888: if not self.hasVolumeLeases():
s/hasVolumeLeases/supportVolumeLeases/

Line 889: return None, None
I don't think this logic belongs here.
getVolumeLease should just assume that this domain supports this capability or 
at least raise a NotImplemented exception if not.

Line 897: leaseSlot = int(tag[blockVolume.TAG_PREXIX_LEN:])
PREXIX?

Line 902: # searching for the metadata offset.
At the risk of regretting this in the future when we actually need this, I 
don't think we need this :)

Line 903: if tag.startswith(blockVolume.TAG_PREFIX_LEASE):
If you choose to keep this flow then:
]s/if/elif/

also add a test which goes through this code

Line 912: * self.logBlkSize * sd.LEASE_BLOCKS)
last part should be retrieved from sanlock


File vdsm/storage/blockVolume.py
Line 41: # NOTE: create prefixes that are exactly TAG_PREXIX_LEN characters
s/PREXIX/PREFIX/

Line 48: # TAG_PREXIX_LEN is the length of the prefix strings used for the
s/PREXIX/PREFIX/

Line 50: TAG_PREXIX_LEN = 3
s/PREXIX/PREFIX/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I27195f29efa691aadda0cac5afef55b04cc01658
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#788640 - Add [block|file]SD.getAllVolumes

2012-05-22 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Add [block|file]SD.getAllVolumes
..


Patch Set 14: Looks good to me, approved

I still don't like the fact that _getVolsTree doesn't return a dictionary and 
don't really believe you will use it elsewhere, but I'm too tired to argue.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7eccf5ca100bd354aa09208ca60bb112fb697063
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#788640 - Refactor Pool.deleteImage()

2012-05-22 Thread abaron
Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Refactor Pool.deleteImage()
..


Patch Set 14: Looks good to me, approved

(1 inline comment)

Please send another patch after this to take care of the if force.


File vdsm/storage/sp.py
Line 1752: elif force:
why is this here?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbc1397c69c7ffa835abe0747b6573d56bb9d74e
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Haim Ateya hat...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


<    3   4   5   6   7   8   9   10   >