Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 6: Verified+1

Verified with unit tests.

-- 
To view, visit https://gerrit.ovirt.org/65102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65102/5/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 237: """
Line 238: raise NotImplementedError()
Line 239: 
Line 240: def _autodelete_if_required(self):
Line 241: if self.autodelete:
> We need a log here:
Done
Line 242: timeout = config.getint("jobs", "autodelete_delay")
Line 243: if timeout >= 0:
Line 244: _scheduler.schedule(timeout, self._delete)
Line 245: 


-- 
To view, visit https://gerrit.ovirt.org/65102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 4:

(6 comments)

https://gerrit.ovirt.org/#/c/65102/4/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 146
Line 147
Line 148
Line 149
Line 150
> We can log here that we are starting the job.
Done


Line 132: if self.status == STATUS.PENDING:
Line 133: # Autodelete should only be handled here for pending 
state.
Line 134: # There is no operation running so we can go straight 
to
Line 135: # aborted state.  In all other cases, autodelete is 
handled as
Line 136: # the _run method finishes.
> +1 (for consistency purposes from my POV)
Done
Line 137: self._status = STATUS.ABORTED
Line 138: if self.autodelete:
Line 139: self._autodelete()
Line 140: elif self.status == STATUS.RUNNING:


Line 148: raise JobNotActive()
Line 149: 
Line 150: def run(self):
Line 151: if not self._may_run():
Line 152: return
> We need log info for starting the job - maybe inside _may_run?
Done
Line 153: try:
Line 154: self._run()
Line 155: except exception.ActionStopped:
Line 156: self._abort_completed()


Line 155: except exception.ActionStopped:
Line 156: self._abort_completed()
Line 157: except Exception as e:
Line 158: self._run_failed(e)
Line 159: else:
> For extra consistency, we can introduce _run_completed() for these lines.
Done
Line 160: with self._status_lock:
Line 161: self._status = STATUS.DONE
Line 162: finally:
Line 163: if self.autodelete:


Line 157: except Exception as e:
Line 158: self._run_failed(e)
Line 159: else:
Line 160: with self._status_lock:
Line 161: self._status = STATUS.DONE
> What we miss in the successful case ia log info about job completing.
Done
Line 162: finally:
Line 163: if self.autodelete:
Line 164: self._autodelete()
Line 165: 


PS4, Line 177: _abort_completed
> perhaps _run_aborted(self) could be a bit clearer?
I prefer the current name but I'll add some docstrings to these methods to make 
their scope and function more clear.


-- 
To view, visit https://gerrit.ovirt.org/65102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Do not use rm.acquireResource return value

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Do not use rm.acquireResource return value
..


Patch Set 8: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65042
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff5653f0b15861d2034011f7fccc76bf25788f6
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Streamline calls to volume methods

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Streamline calls to volume methods
..


Patch Set 9: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65047
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbdfbd5cf6a4a7345c350359e76c2156720c683
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Streamline usage of image.Image class

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Streamline usage of image.Image class
..


Patch Set 10: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc9de180542cd81a6d563d52b4b30b9520046469
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Simplify long lines using continuation \

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Simplify long lines using continuation \
..


Patch Set 8: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65045
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I51fc21b452f2f1d920ad1353d2b14067d432a837
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Streamline building domains list for upgrade

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Streamline building domains list for upgrade
..


Patch Set 8: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65043
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2be0a4816d733fdae13bcb933201b1ede795ca68
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Fix domain.produceVolume calling convention

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Fix domain.produceVolume calling convention
..


Patch Set 9: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65046
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id804b4754507f1c6bf4e85f006f6db6fdda2e330
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Remove useless continuation to a commented line

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Remove useless continuation to a commented line
..


Patch Set 8: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65044
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b8a0f989fbd55280163c54481f1748bcfff7b6a
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Fix positional args calling convention

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Fix positional args calling convention
..


Patch Set 9: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65048
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d73df3daa59efb512bfdc9f6e400bacb3c08c49
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Remove vol_extend_policy option

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Remove vol_extend_policy option
..


Patch Set 10: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65053
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib067c09a985a633a452c476e21d8c2c073e6ca50
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Sort dict literal by key

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Sort dict literal by key
..


Patch Set 10: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65052
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I128ed804fcdbdf7b6b562f5cef662ae2ba2c9014
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Remove double dict key

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Remove double dict key
..


Patch Set 9: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65051
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I05f6aeda1858e85b8660d972593e82bcb7e3bff3
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: pep8 1.6.2 fix

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: pep8 1.6.2 fix
..


Patch Set 9: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65050
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8e561da219c31aa26d32dcb615eacf223050f5c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Streamline acquiring of multiple images locks

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Streamline acquiring of multiple images locks
..


Patch Set 8: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65041
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I20bcebeb11f3ccbe10b029a424f4c27a890eea8c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: fix abort to raise

[alitke]

Adam Litke has abandoned this change.

Change subject: fix abort to raise
..


Abandoned

-- 
To view, visit https://gerrit.ovirt.org/65147
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I5efe2319d023bd813777c76a0bbaa0b9576281f6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/65102/4/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 162: finally:
Line 163: if self.autodelete:
Line 164: self._autodelete()
Line 165: 
Line 166: def _may_run(self):
Add docstrings for each of these helpers.
Line 167: with self._status_lock:
Line 168: if self.status == STATUS.ABORTED:
Line 169: logging.debug('Refusing to run aborted job %r', 
self._id)
Line 170: return False


-- 
To view, visit https://gerrit.ovirt.org/65102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 3:

(7 comments)

https://gerrit.ovirt.org/#/c/65102/3/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 168
Line 169
Line 170
Line 171
Line 172
> Should raise?
Done


Line 169
Line 170
Line 171
Line 172
Line 173
> We must remove this line, _abort cannot know if the job was aborted, it is 
Done


Line 152: return
Line 153: try:
Line 154: self._run()
Line 155: except exception.ActionStopped:
Line 156: self._handle_completed_abort()
> Maybe _abort_completed?
Done
Line 157: except Exception as e:
Line 158: self._handle_execution_error(e)
Line 159: else:
Line 160: # We always mark the job done.  Even if we were in 
aborting state


Line 154: self._run()
Line 155: except exception.ActionStopped:
Line 156: self._handle_completed_abort()
Line 157: except Exception as e:
Line 158: self._handle_execution_error(e)
> Maybe _run_failed?
Done
Line 159: else:
Line 160: # We always mark the job done.  Even if we were in 
aborting state
Line 161: # _run might finish normally before the abort action 
executes.
Line 162: with self._status_lock:


Line 157: except Exception as e:
Line 158: self._handle_execution_error(e)
Line 159: else:
Line 160: # We always mark the job done.  Even if we were in 
aborting state
Line 161: # _run might finish normally before the abort action 
executes.
> I think this comment is confusing, setting status to DONE in the else is pr
Done
Line 162: with self._status_lock:
Line 163: self._status = STATUS.DONE
Line 164: finally:
Line 165: if self.autodelete:


Line 162: with self._status_lock:
Line 163: self._status = STATUS.DONE
Line 164: finally:
Line 165: if self.autodelete:
Line 166: self._autodelete()
> Cannot be more elegant!
Thanks!
Line 167: 
Line 168: def _prepare_to_run(self):
Line 169: with self._status_lock:
Line 170: if self.status == STATUS.ABORTED:


Line 164: finally:
Line 165: if self.autodelete:
Line 166: self._autodelete()
Line 167: 
Line 168: def _prepare_to_run(self):
> May be _may_run?
Done
Line 169: with self._status_lock:
Line 170: if self.status == STATUS.ABORTED:
Line 171: logging.debug('Refusing to run aborted job %r', 
self._id)
Line 172: return False


-- 
To view, visit https://gerrit.ovirt.org/65102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: volume_artifacts: Add qcow2_compat on create.

[alitke]

Adam Litke has posted comments on this change.

Change subject: volume_artifacts: Add qcow2_compat on create.
..


Patch Set 20:

(1 comment)

https://gerrit.ovirt.org/#/c/64372/20/vdsm/storage/sdm/volume_artifacts.py
File vdsm/storage/sdm/volume_artifacts.py:

PS20, Line 148: qcow2Compat
qcow2_compat?


-- 
To view, visit https://gerrit.ovirt.org/64372
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd2d53fba0dd69cdb4f60e152cf6d254cfb863a
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: core: Expose API for qemuimg commit

[alitke]

Adam Litke has posted comments on this change.

Change subject: core: Expose API for qemuimg commit
..


Patch Set 12: Code-Review+2

(2 comments)

https://gerrit.ovirt.org/#/c/64222/12/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 205: cmd.append(top)
Line 206: 
Line 207: # For simplicity, we always run commit in the image directory.
Line 208: workdir = os.path.dirname(top)
Line 209: return QemuImgOperation(cmd, cwd=workdir)
Have you tested that progress output is the same also for this command?  If so, 
cool!
Line 210: 
Line 211: 
Line 212: class QemuImgOperation(object):
Line 213: REGEXPR = re.compile(r'\s*\(([\d.]+)/100%\)\s*')


https://gerrit.ovirt.org/#/c/64222/12/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 433: make_image(top, size, qemuimg.FORMAT.QCOW2, 1, "1.1", 
base)
Line 434: 
Line 435: op = qemuimg.commit(top, topFormat=qemuimg.FORMAT.QCOW2)
Line 436: op.wait_for_completion()
Line 437: self.assertEquals(100, op.progress)
Cool!
Line 438: 
Line 439: 
Line 440: def make_image(path, size, format, index, qcow2_compat, backing=None):
Line 441: qemuimg.create(path, size=size, format=format, 
qcow2Compat=qcow2_compat,


-- 
To view, visit https://gerrit.ovirt.org/64222
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If7a13be40541fb268541bd8614a642263b96b487
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Fix test to not wait forever on failure

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Fix test to not wait forever on failure
..


Patch Set 2: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/65149
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I57d49895ff0c222b5cdda537e722248d1f2681b7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Fix abort race in SDM.copy_data

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: storage: Fix abort race in SDM.copy_data
..

storage: Fix abort race in SDM.copy_data

The copy_data job supports aborting the qemuimg process.  If such a
process has been started we kill it.  If a process hasn't yet been
created we do nothing but place the job in aborting state.  Later before
creating the qemuimg command we want to check if we have been asked to
abort.  If so, raise ActionStopped instead of continuing.

Change-Id: I0c7d9aadb981e8b851a70c7ad0fdc0d75b46dc51
Signed-off-by: Adam Litke 
---
M tests/storage_sdm_copy_data_test.py
M vdsm/storage/sdm/api/copy_data.py
2 files changed, 44 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/65150/1

diff --git a/tests/storage_sdm_copy_data_test.py 
b/tests/storage_sdm_copy_data_test.py
index 6b116d5..0c6589d 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -21,6 +21,7 @@
 
 import threading
 from contextlib import contextmanager
+from functools import partial
 
 from fakelib import FakeScheduler
 from monkeypatch import MonkeyPatchScope
@@ -285,6 +286,35 @@
 self.assertEqual(sc.ILLEGAL_VOL, dst_vol.getLegality())
 self.assertEqual(gen_id, dst_vol.getMetaParam(sc.GENERATION))
 
+@permutations((('file',), ('block',)))
+def test_abort_before_copy(self, env_type):
+def _fake_run(job_instance, real_run):
+job_instance._status = jobs.STATUS.ABORTING
+real_run()
+
+fmt = sc.RAW_FORMAT
+with self.get_vols(env_type, fmt, fmt) as (src_chain, dst_chain):
+src_vol = src_chain[0]
+dst_vol = dst_chain[0]
+gen_id = dst_vol.getMetaParam(sc.GENERATION)
+source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
+  img_id=src_vol.imgUUID, vol_id=src_vol.volUUID,
+  generation=0)
+dest = dict(endpoint_type='div', sd_id=dst_vol.sdUUID,
+img_id=dst_vol.imgUUID, vol_id=dst_vol.volUUID,
+generation=gen_id)
+fake_convert = FakeQemuConvertChecker(src_vol, dst_vol,
+  error=RuntimeError)
+with MonkeyPatchScope([(qemuimg, 'convert', fake_convert)]):
+job_id = make_uuid()
+job = storage.sdm.api.copy_data.Job(job_id, 0, source, dest)
+# Simulate status changing to aborting right after _run called
+job._run = partial(_fake_run, job, job._run)
+job.run()
+self.assertEqual(jobs.STATUS.ABORTED, job.status)
+self.assertEqual(sc.LEGAL_VOL, dst_vol.getLegality())
+self.assertEqual(gen_id, dst_vol.getMetaParam(sc.GENERATION))
+
 def test_wrong_generation(self):
 fmt = sc.RAW_FORMAT
 with self.get_vols('block', fmt, fmt) as (src_chain, dst_chain):
@@ -318,20 +348,23 @@
 self.ready_event = threading.Event()
 
 def __call__(self, *args, **kwargs):
-assert sc.LEGAL_VOL == self.src_vol.getLegality()
-assert sc.ILLEGAL_VOL == self.dst_vol.getLegality()
-return FakeQemuImgOperation(self.ready_event, self.wait_for_abort,
+return FakeQemuImgOperation(self.src_vol, self.dst_vol,
+self.ready_event, self.wait_for_abort,
 self.error)
 
 
 class FakeQemuImgOperation(object):
-def __init__(self, ready_event, wait_for_abort, error):
+def __init__(self, src_vol, dst_vol, ready_event, wait_for_abort, error):
+self.src_vol = src_vol
+self.dst_vol = dst_vol
 self.ready_event = ready_event
 self.wait_for_abort = wait_for_abort
 self.error = error
 self.abort_event = threading.Event()
 
 def start(self):
+assert sc.LEGAL_VOL == self.src_vol.getLegality()
+assert sc.ILLEGAL_VOL == self.dst_vol.getLegality()
 self.ready_event.set()
 
 def abort(self):
diff --git a/vdsm/storage/sdm/api/copy_data.py 
b/vdsm/storage/sdm/api/copy_data.py
index 3dc6fff..7182f49 100644
--- a/vdsm/storage/sdm/api/copy_data.py
+++ b/vdsm/storage/sdm/api/copy_data.py
@@ -26,6 +26,7 @@
 from vdsm import jobs
 from vdsm import properties
 from vdsm import qemuimg
+from vdsm.common import exception
 from vdsm.storage import constants as sc
 from vdsm.storage import guarded
 from vdsm.storage import workarounds
@@ -60,12 +61,9 @@
 self._operation.abort()
 
 def _run(self):
+print "called: status=%r" % self.status
 with guarded.context(self._source.locks + self._dest.locks):
 with self._source.prepare(), self._dest.prepare():
-# Do not start copying if we have already been aborted
-

Change in vdsm[master]: qemuimg: Require explicit start of QemuImgOperation

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: qemuimg: Require explicit start of QemuImgOperation
..

qemuimg: Require explicit start of QemuImgOperation

Currently the underlying qemuimg command is started by
QemuImgOperation.__init__.  In general this is a bad practice because it
eliminates control and can make testing more difficult.  Add a start()
method to this class and require that it be called to actually launch
the underlying command.

This will be used by a subsequent patch which creates the operation
under one lock and starts it under a different lock.

Change-Id: I8e427c909a8dffc3ba2a5c838c7d1e7ce7cce55d
Signed-off-by: Adam Litke 
---
M lib/vdsm/qemuimg.py
M tests/qemuimg_test.py
M tests/storage_sdm_copy_data_test.py
M vdsm/storage/image.py
M vdsm/storage/sdm/api/copy_data.py
5 files changed, 30 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/65148/1

diff --git a/lib/vdsm/qemuimg.py b/lib/vdsm/qemuimg.py
index 8eef8fa..486882b 100644
--- a/lib/vdsm/qemuimg.py
+++ b/lib/vdsm/qemuimg.py
@@ -192,18 +192,24 @@
 REGEXPR = re.compile(r'\s*\(([\d.]+)/100%\)\s*')
 
 def __init__(self, cmd, cwd=None):
+self.base_cmd = cmd
+self.cwd = cwd
 self._aborted = False
 self._progress = 0.0
 
 self._stdout = bytearray()
 self._stderr = bytearray()
+self._proc = None
+self._stream = None
 
-cmd = cmdutils.wrap_command(cmd, with_nice=utils.NICENESS.HIGH,
+def start(self):
+cmd = cmdutils.wrap_command(self.base_cmd,
+with_nice=utils.NICENESS.HIGH,
 with_ioclass=utils.IOCLASS.IDLE)
-_log.debug(cmdutils.command_log_line(cmd, cwd=cwd))
-self._command = CPopen(cmd, cwd=cwd, deathSignal=signal.SIGKILL)
-self._stream = utils.CommandStream(
-self._command, self._recvstdout, self._recvstderr)
+_log.debug(cmdutils.command_log_line(cmd, cwd=self.cwd))
+self._proc = CPopen(cmd, cwd=self.cwd, deathSignal=signal.SIGKILL)
+self._stream = utils.CommandStream(self._proc,
+   self._recvstdout, self._recvstderr)
 
 def _recvstderr(self, buffer):
 self._stderr += buffer
@@ -243,7 +249,7 @@
 
 @property
 def finished(self):
-return self._command.poll() is not None
+return self._proc.poll() is not None
 
 def poll(self, timeout=None):
 self._stream.receive(timeout=timeout)
@@ -251,14 +257,14 @@
 if not self._stream.closed:
 return
 
-self._command.wait()
+self._proc.wait()
 
 if self._aborted:
 raise exception.ActionStopped()
 
-cmdutils.retcode_log_line(self._command.returncode, self.error)
-if self._command.returncode != 0:
-raise QImgError(self._command.returncode, "", self.error)
+cmdutils.retcode_log_line(self._proc.returncode, self.error)
+if self._proc.returncode != 0:
+raise QImgError(self._proc.returncode, "", self.error)
 
 def wait_for_completion(self):
 timeout = config.getint("irs", "progress_interval")
@@ -267,9 +273,9 @@
 _log.debug('qemu-img operation progress: %s%%', self.progress)
 
 def abort(self):
-if self._command.poll() is None:
+if self._proc.poll() is None:
 self._aborted = True
-self._command.terminate()
+self._proc.terminate()
 
 
 def resize(image, newSize, format=None):
diff --git a/tests/qemuimg_test.py b/tests/qemuimg_test.py
index 0642932..5afda1d 100644
--- a/tests/qemuimg_test.py
+++ b/tests/qemuimg_test.py
@@ -313,10 +313,12 @@
 
 def test_failure(self):
 p = qemuimg.QemuImgOperation(['false'])
+p.start()
 self.assertRaises(qemuimg.QImgError, p.poll)
 
 def test_progress_simple(self):
 p = qemuimg.QemuImgOperation(['true'])
+p.start()
 
 for progress in self._progress_iterator():
 p._recvstdout(self.PROGRESS_FORMAT % progress)
@@ -332,6 +334,7 @@
 ])
 def test_partial(self, output_list, progress_list):
 p = qemuimg.QemuImgOperation(['true'])
+p.start()
 
 for output, progress in zip(output_list, progress_list):
 p._recvstdout(output)
@@ -341,6 +344,7 @@
 
 def test_progress_batch(self):
 p = qemuimg.QemuImgOperation(['true'])
+p.start()
 
 p._recvstdout(
 (self.PROGRESS_FORMAT % 10.00) +
@@ -354,6 +358,7 @@
 
 def test_unexpected_output(self):
 p = qemuimg.QemuImgOperation(['true'])
+p.start()
 
 self.assertRaises(ValueError, p._recvstdout, 'Hello World\r')
 
diff --git a/tests/storage_sdm_copy_data_test.py 
b/tests/storage_sdm_copy_data_test.py
index d60936a..948d170 100644
--- 

Change in vdsm[master]: fix abort to raise

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: fix abort to raise
..

fix abort to raise

Change-Id: I5efe2319d023bd813777c76a0bbaa0b9576281f6
Signed-off-by: Adam Litke 
---
M lib/vdsm/jobs.py
M tests/jobsTests.py
2 files changed, 21 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/65147/1

diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index 2c6c27c..42f43bc 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -155,13 +155,24 @@
 self._status = STATUS.RUNNING
 try:
 self._run()
+except exception.ActionStopped:
+# Once a job has been aborted we expect _run to raise ActionStopped
+with self._status_lock:
+if self.status != STATUS.ABORTING:
+logging.warning("Unexpected ActionStopped exception in "
+"job %r from state %r",
+self.id, self.status)
+self._status = STATUS.ABORTED
+if self.autodelete:
+self._autodelete()
 except Exception as e:
 with self._status_lock:
 if self.status == STATUS.ABORTING:
+# Other exceptions while in aborting state mean that abort
+# was not successful.  We should not autodelete because
+# there may still be an ongoing operation.
 logging.exception("Failed to abort job (id=%s desc=%s)",
   self.id, self.description)
-# We do not autodelete failed aborting jobs because it is
-# likely that something is still running on the system.
 else:
 self._status = STATUS.FAILED
 logging.exception("Job (id=%s desc=%s) failed",
@@ -172,13 +183,12 @@
 e = exception.GeneralException(str(e))
 self._error = e
 else:
+# We always mark the job done.  Even if we were in aborting state
+# _run might finish normally before the abort action executes.
 with self._status_lock:
-if self.status == STATUS.ABORTING:
-self._status = STATUS.ABORTED
-else:
-self._status = STATUS.DONE
-if self.autodelete:
-self._autodelete()
+self._status = STATUS.DONE
+if self.autodelete:
+self._autodelete()
 
 def _abort(self):
 """
@@ -188,6 +198,8 @@
 status to change to aborted.
 - Must raise if the job could not be aborted
 - Must not raise if the job was aborted
+- A successful abort must cause the job's _run method to raise an
+  ActionStopped exception.
 """
 raise AbortNotSupported()
 
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index 6c6d49a..4c83810 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -86,6 +86,7 @@
 def _run(self):
 self.event_running.set()
 self.event_aborted.wait(1)
+raise exception.ActionStopped()
 
 def _abort(self):
 self.event_aborted.set()


-- 
To view, visit https://gerrit.ovirt.org/65147
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5efe2319d023bd813777c76a0bbaa0b9576281f6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Fix test to not wait forever on failure

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: tests: Fix test to not wait forever on failure
..

tests: Fix test to not wait forever on failure

Change-Id: I57d49895ff0c222b5cdda537e722248d1f2681b7
Signed-off-by: Adam Litke 
---
M tests/storage_sdm_copy_data_test.py
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/65149/1

diff --git a/tests/storage_sdm_copy_data_test.py 
b/tests/storage_sdm_copy_data_test.py
index 948d170..6b116d5 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -275,7 +275,8 @@
 job_id = make_uuid()
 job = storage.sdm.api.copy_data.Job(job_id, 0, source, dest)
 t = start_thread(job.run)
-fake_convert.ready_event.wait()
+if not fake_convert.ready_event.wait(1):
+raise RuntimeError("Timeout waiting for thread")
 job.abort()
 t.join(1)
 if t.isAlive():


-- 
To view, visit https://gerrit.ovirt.org/65149
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I57d49895ff0c222b5cdda537e722248d1f2681b7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: jobs: Fix abort semantics
..

jobs: Fix abort semantics

Prior to this commit jobs.abort was treated as an synchronous operation
and if it returned successfully the caller could assume that no
operations related to the job were still running.  Unforatunately this
assumption is wrong since most underlying operations are aborted
asynchronously (ie. by sending a signal to a process).

To fix this we must adopt async abort sementics in the jobs API.  If a
job is pending and abort is called we can simply move it to aborted
state.  If the job is running we move it to aborting state, call the
abort helper (_abort) and return.  When run() finishes it will move an
aborting job to aborted.  We must not autodelete or otherwise change the
state of jobs with aborting status because it could mask the fact that a
process is still changing the host or storage.

Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Signed-off-by: Adam Litke 
---
M lib/vdsm/jobs.py
M tests/jobsTests.py
2 files changed, 53 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/65102/1

diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index b92779d..2c6c27c 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -33,11 +33,12 @@
 
 
 class STATUS:
-PENDING = 'pending'  # Job has not started yet
-RUNNING = 'running'  # Job is running
-DONE = 'done'# Job has finished successfully
-ABORTED = 'aborted'  # Job was aborted by user request
-FAILED = 'failed'# Job has failed
+PENDING = 'pending'# Job has not started yet
+RUNNING = 'running'# Job is running
+DONE = 'done'  # Job has finished successfully
+ABORTING = 'aborting'  # Job is running but abort is in progress
+ABORTED = 'aborted'# Job was aborted by user request
+FAILED = 'failed'  # Job has failed
 
 
 class ClientError(Exception):
@@ -128,16 +129,20 @@
 
 def abort(self):
 with self._status_lock:
-if not self.active:
+if self.status == STATUS.PENDING:
+self._status = STATUS.ABORTED
+if self.autodelete:
+self._autodelete()
+elif self.status == STATUS.RUNNING:
+self._status = STATUS.ABORTING
+logging.info("Aborting job %r...", self.id)
+self._abort()
+# Autodelete is handled by run when the operation is terminated
+elif self.status == STATUS.ABORTING:
+logging.info("Retrying abort job %r...", self.id)
+self._abort()
+else:
 raise JobNotActive()
-logging.info('Job %r aborting...', self._id)
-self._abort()
-self._status = STATUS.ABORTED
-
-# We MUST NOT autodelete a job if abort failed.  Otherwise there could
-# still be ongoing operations on storage without any associated job.
-if self.autodelete:
-self._autodelete()
 
 def run(self):
 with self._status_lock:
@@ -150,25 +155,37 @@
 self._status = STATUS.RUNNING
 try:
 self._run()
-status = STATUS.DONE
 except Exception as e:
-status = STATUS.FAILED
-logging.exception("Job (id=%s desc=%s) failed",
-  self.id, self.description)
-if not isinstance(e, exception.VdsmException):
-e = exception.GeneralException(str(e))
-self._error = e
-finally:
 with self._status_lock:
-if self.status == STATUS.ABORTED:
-return
-self._status = status
-if self.autodelete:
-self._autodelete()
+if self.status == STATUS.ABORTING:
+logging.exception("Failed to abort job (id=%s desc=%s)",
+  self.id, self.description)
+# We do not autodelete failed aborting jobs because it is
+# likely that something is still running on the system.
+else:
+self._status = STATUS.FAILED
+logging.exception("Job (id=%s desc=%s) failed",
+  self.id, self.description)
+if self.autodelete:
+self._autodelete()
+if not isinstance(e, exception.VdsmException):
+e = exception.GeneralException(str(e))
+self._error = e
+else:
+with self._status_lock:
+if self.status == STATUS.ABORTING:
+self._status = STATUS.ABORTED
+else:
+self._status = STATUS.DONE
+if self.autodelete:
+  

Change in vdsm[master]: drop dead constant

[alitke]

Adam Litke has posted comments on this change.

Change subject: drop dead constant
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65097
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c3651d296ee757752bee659fa08835bd385e91a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: cleanup: Remove dead code from storageServer.py

[alitke]

Adam Litke has posted comments on this change.

Change subject: cleanup: Remove dead code from storageServer.py
..


Patch Set 2: Verified+1

Unit tests.

-- 
To view, visit https://gerrit.ovirt.org/65071
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c442e73c6eca16384c3cdd53478b182affe1097
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: cleanup: Remove dead code from storageServer.py

[alitke]

Adam Litke has posted comments on this change.

Change subject: cleanup: Remove dead code from storageServer.py
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/65071/1//COMMIT_MSG
Commit Message:

Line 7: cleanup: Remove dead code from storageServer.py
Line 8: 
Line 9: While reviewing some schema fixes I noticed that we have a lot of dead
Line 10: code in storageServer.py which was misleading how parameters are
Line 11: structured when connecting storage.  Remove this unused code.
> Seems that this code should have been part of commit 7b7c5b7bbfffef5393cdcc
Done
Line 12: 
Line 13: Change-Id: I7c442e73c6eca16384c3cdd53478b182affe1097


-- 
To view, visit https://gerrit.ovirt.org/65071
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c442e73c6eca16384c3cdd53478b182affe1097
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sp: Streamline image namespace locking

[alitke]

Adam Litke has posted comments on this change.

Change subject: sp: Streamline image namespace locking
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65040
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21fe6224059f1624facd95c3a6e1ba671c7deda6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Make ResourceMananger private

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Make ResourceMananger private
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65038
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35c3cffb4e1af8aa0f238c3501a82b975d56c2c3
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Get rid of getInstance

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Get rid of getInstance
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65037
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I136ab52f15332f43f2b0ecd9e9a2ff7b558df87a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: rsourceFactories: Use new resourceManager module api

[alitke]

Adam Litke has posted comments on this change.

Change subject: rsourceFactories: Use new resourceManager module api
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65036
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icdb30f34257f31622a87d1f496b3f516803eeda8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourcemanager: Use new module api

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourcemanager: Use new module api
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65035
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33d9fad90551a0c2e9b1e8a89c5f9f920681c6b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Add module api

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Add module api
..


Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65034
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie315f10bacdf49f1023d1991811afec28ed0d7f9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: cleanup: Remove dead code from storageServer.py

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: cleanup: Remove dead code from storageServer.py
..

cleanup: Remove dead code from storageServer.py

While reviewing some schema fixes I noticed that we have a lot of dead
code in storageServer.py which was misleading how parameters are
structured when connecting storage.  Remove this unused code.

Change-Id: I7c442e73c6eca16384c3cdd53478b182affe1097
Signed-off-by: Adam Litke 
---
M vdsm/storage/storageServer.py
1 file changed, 0 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/65071/1

diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py
index 1c2bc11..4bc5de9 100644
--- a/vdsm/storage/storageServer.py
+++ b/vdsm/storage/storageServer.py
@@ -24,7 +24,6 @@
 import os
 import socket
 from collections import namedtuple
-from functools import partial
 import six
 import sys
 
@@ -44,10 +43,6 @@
 import gluster.cli
 
 
-class UnsupportedAuthenticationMethod(RuntimeError):
-pass
-
-
 IscsiConnectionParameters = namedtuple("IscsiConnectionParameters",
"target, iface, credentials")
 
@@ -65,65 +60,6 @@
 FcpConnectionParameters = namedtuple("FcpConnectionParameters", "")
 
 ConnectionInfo = namedtuple("ConnectionInfo", "type, params")
-
-
-def _credentialAssembly(credInfo):
-authMethod = credInfo.get('type', 'chap').lower()
-if authMethod != 'chap':
-raise UnsupportedAuthenticationMethod(authMethod)
-
-params = credInfo.get('params', {})
-username = params.get('username', None)
-password = params.get('password', None)
-return iscsi.ChapCredentials(username, password)
-
-
-def _iscsiParameterAssembly(d):
-port = d['portal'].get('port', iscsi.ISCSI_DEFAULT_PORT)
-host = d['portal']['host']
-portal = iscsi.IscsiPortal(host, port)
-iqn = d['iqn']
-tpgt = d.get('tpgt', 1)
-target = iscsi.IscsiTarget(portal, tpgt, iqn)
-iface = iscsi.IscsiInterface(d.get('iface', 'default'))
-credInfo = d.get('credentials', None)
-cred = None
-if credInfo:
-cred = _credentialAssembly(credInfo)
-
-return IscsiConnectionParameters(target, iface, cred)
-
-
-def _namedtupleAssembly(nt, d):
-d = d.copy()
-for field in nt._fields:
-if field not in d:
-d[field] = None
-
-return nt(**d)
-
-_posixFsParameterAssembly = partial(_namedtupleAssembly,
-PosixFsConnectionParameters)
-_glusterFsParameterAssembly = partial(_namedtupleAssembly,
-  GlusterFsConnectionParameters)
-_nfsParamerterAssembly = partial(_namedtupleAssembly, NfsConnectionParameters)
-_localFsParameterAssembly = partial(_namedtupleAssembly,
-LocaFsConnectionParameters)
-
-
-_TYPE_NT_MAPPING = {
-'iscsi': _iscsiParameterAssembly,
-'sharedfs': _posixFsParameterAssembly,
-'posixfs': _posixFsParameterAssembly,
-'glusterfs': _glusterFsParameterAssembly,
-'nfs': _nfsParamerterAssembly,
-'localfs': _localFsParameterAssembly}
-
-
-def dict2conInfo(d):
-conType = d['type']
-params = _TYPE_NT_MAPPING[conType](d.get('params', {}))
-return ConnectionInfo(conType, params)
 
 
 class ExampleConnection(object):


-- 
To view, visit https://gerrit.ovirt.org/65071
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c442e73c6eca16384c3cdd53478b182affe1097
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: return type fixes for LVMVolumeGroup.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: return type fixes for LVMVolumeGroup.getInfo
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/59708/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS2, Line 6851:  -   description: Phisical volume list
  : name: pvlist
  : type:
  : - *PhysicalVolumeInfo
This key is optional.  It will be present when calling the 
LVMVolumeGroup.getInfo API but absent when calling Host.getLVMVolumeGroups.


-- 
To view, visit https://gerrit.ovirt.org/59708
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc2b2a3d1e8b4563b24c60601c44e851f2603e6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: return type fixes for StoragePool.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: return type fixes for StoragePool.getInfo
..


Patch Set 2:

(2 comments)

-1 for visibility of comments.

https://gerrit.ovirt.org/#/c/59707/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5690: Attached: The domain is attached to a Storage Pool but is
Line 5691: deactivated
Line 5692: Unattached: The domain is not attached to a Storage Pool
Line 5693: Unknown: The status of the Storage Domain is not known
Line 5694: 
Does engine have two separate enums (one capitalized on one not)?
Line 5695: StorageDomainStatusMap: 
Line 5696: added: '3.1'
Line 5697: description: A mapping of Storage Domain statuses indexed by 
Storage
Line 5698: Domain UUID.


Line 5739: datatype: int
Line 5740: 
Line 5741: -   description: Current Storage Domain status
Line 5742: name: status
Line 5743: type: *StorageStatus
This is confusing.  The commit message needs to have some information about 
when the capitalized statuses are used and when the lowercase ones are used.
Line 5744: 
Line 5745: -   description: Indicates the Storage Domain version
Line 5746: name: version
Line 5747: type: int


-- 
To view, visit https://gerrit.ovirt.org/59707
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifeadf2323d2a3535a5777d0cc16027cfb9e42f0e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: return type fixes for StoragePool.getSpmStatus

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: return type fixes for StoragePool.getSpmStatus
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/59706
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9fbae250507d4cf02ef9b73eee6403a06cd9d2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: return type fixes for Task.getStatus

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: return type fixes for Task.getStatus
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/59705
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ce3e3ab2e21e5d1f6da6bdf5dc89a3db40e2160
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: parameter type fixes for StoragePool.spmStart

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: parameter type fixes for StoragePool.spmStart
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/59704
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49072827b8ac04f720d50aca8e5a24b4be7582b7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: parameter type fixes for StoragePool.connect

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: parameter type fixes for StoragePool.connect
..


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/59702/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS2, Line 5616: active: The domain is attached to a Storage Pool and is 
activated
  : attached: The domain is attached to a Storage Pool 
but is
  : deactivated
  : unattached: The domain is not attached to a Storage 
Pool
  : unknown: The status of the Storage Domain is not 
known
Did you verify that these are passed and returned as lowercase values?  From my 
reading of the code these values are capitalized :(  Very unfortunate.


-- 
To view, visit https://gerrit.ovirt.org/59702
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I19b6f25c17e697702ec61eba6b11f256c1df4d83
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: parameter type fixes for StoragePool.connectStorageServer

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: parameter type fixes for StoragePool.connectStorageServer
..


Patch Set 2: Code-Review-1

-1 for visibility

-- 
To view, visit https://gerrit.ovirt.org/59701
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If9fe2ffc3bc2327eefaae794b7b366e8202d2f2a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: yml: parameter type fixes for StoragePool.connectStorageServer

[alitke]

Adam Litke has posted comments on this change.

Change subject: yml: parameter type fixes for StoragePool.connectStorageServer
..


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/59701/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS2, Line 381: Id
Can we do better?  How about: A UUID that will be used to identify this 
connection in subsequent API calls.


Line 380: 
Line 381: -   description: Id
Line 382: name: id
Line 383: type: *UUID
Line 384: type: object
Are you missing: ifaceName, initiatorName, and netIfaceName?
Line 385: 
Line 386: LocalFsConnectionParameters: 
Line 387: added: '3.1'
Line 388: description: Parameters for initiating a connection to


Line : 0: The type is not known
Line 5556: 1: The Storage Domain uses Network File System based 
storage
Line 5557: 2: The Storage Domain uses FibreChannel based storage
Line 5558: 3: The Storage Domain uses iSCSI based storage
Line 5559: 4: The Storage Domain uses storage on the local file 
system
5 is CIFS (aka smbfs).  If it's deprecated we can say so here but I don't think 
it should be omitted.
Line 5560: 6: The Storage Domain uses posix file system
Line 5561: 7: The Storage Domain uses glusterfs
Line 5562: 
Line 5563: StorageDomainInfo: 


-- 
To view, visit https://gerrit.ovirt.org/59701
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If9fe2ffc3bc2327eefaae794b7b366e8202d2f2a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Streamline resourceManager import

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Streamline resourceManager import
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65003
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie06a26c4fcf6a3f4d81339df1633d28d87b99520
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Remove unused listNamespaces

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Remove unused listNamespaces
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65002
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie284d49eaf63ce767375288bcab67f10001e9f14
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Move ResourceInfo class to module

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Move ResourceInfo class to module
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65001
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21170d863d19c981995aca6794a1a346e1fbe31b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use fresh ResourceManager for every test

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Use fresh ResourceManager for every test
..


Patch Set 2: Code-Review+2

Nice!

-- 
To view, visit https://gerrit.ovirt.org/65004
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a68ce670aab78b15d11dafa155b58beb6265ec
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Remove unused listNamespaces

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Remove unused listNamespaces
..


Patch Set 2:

CI failing on network tests again.

-- 
To view, visit https://gerrit.ovirt.org/65002
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie284d49eaf63ce767375288bcab67f10001e9f14
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Move Namespace class to module

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Move Namespace class to module
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/65000
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I82240985fe156345fee0c08a46de5251e9d65be8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: resourceManager: Flatten LockType constants

[alitke]

Adam Litke has posted comments on this change.

Change subject: resourceManager: Flatten LockType constants
..


Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/63628
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id78e07814f21a1dcf33efa2afe400eff041e3001
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 9: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Remove volume run link after live merge

[alitke]

Adam Litke has posted comments on this change.

Change subject: Live Merge: Remove volume run link after live merge
..


Patch Set 13: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

[alitke]

Adam Litke has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 7:

(2 comments)

https://gerrit.ovirt.org/#/c/64479/7/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

PS7, Line 281: abort_wait
wait_for_abort


PS7, Line 286: if abort_wait:
 : self.abort_event = threading.Event()
 : else:
 : self.abort_event = None
move this into the operation class and just pass the boolean value through.


-- 
To view, visit https://gerrit.ovirt.org/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: guarded: Raise if attempt to lock will deadlock

[alitke]

Adam Litke has posted comments on this change.

Change subject: guarded: Raise if attempt to lock will deadlock
..


Patch Set 1: Code-Review+2

(1 comment)

https://gerrit.ovirt.org/#/c/64977/1/lib/vdsm/storage/guarded.py
File lib/vdsm/storage/guarded.py:

PS1, Line 83: by_ns_name = operator.attrgetter("ns", "name")
: for _, group in itertools.groupby(locks, by_ns_name):
Interesting.  I've never run into operator.attrgetter and itertools.groupby 
before.  Neat.


-- 
To view, visit https://gerrit.ovirt.org/64977
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cd539548eda04ac7b9faf0ba1be49f29bfa2ed0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: guarded: Implement __repr__ for easier debugging

[alitke]

Adam Litke has posted comments on this change.

Change subject: guarded: Implement __repr__ for easier debugging
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64976
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I708cf5be83790a85792ae1cddd564db80521e8df
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Rename resourceManager tests to new convention

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Rename resourceManager tests to new convention
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64975
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I756cfe23f051c7a7dda07c2578c4a4cfba14d7e0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 7:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation when completing operation
..


Patch Set 6:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation in volume.getInfo
..


Patch Set 6:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Support generation in sdm.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Support generation in sdm.copy_data
..


Patch Set 6:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation in volume.operation context

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Validate generation in volume.operation context
..


Patch Set 6:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64486
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 6:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


Patch Set 5:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64543
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Remove TODO for SDM.copy_data abort before copy test

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Remove TODO for SDM.copy_data abort before copy test
..


Patch Set 7:

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64480
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I26e724560e9cb485fd4766ebcc04f644ad6431e4
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Use Volume.operation in SDM.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: Use Volume.operation in SDM.copy_data
..


Patch Set 6: Verified+1

Verified with unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64478
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I574cea3387ab5b99368e0317aed73683d398a596
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation in volume.operation context

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Validate generation in volume.operation context
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64486
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Support generation in sdm.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Support generation in sdm.copy_data
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation in volume.getInfo
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation when completing operation
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Use Volume.operation in SDM.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: Use Volume.operation in SDM.copy_data
..


Patch Set 5: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64478
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I574cea3387ab5b99368e0317aed73683d398a596
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


Patch Set 4: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64543
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Remove TODO for SDM.copy_data abort before copy test

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Remove TODO for SDM.copy_data abort before copy test
..


Patch Set 6:

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64480
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I26e724560e9cb485fd4766ebcc04f644ad6431e4
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 6: Verified+1

Verified by unit tests and SDM.copy_data functional testing.

-- 
To view, visit https://gerrit.ovirt.org/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation when completing operation
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64487/4/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 512
Line 513
Line 514
Line 515
Line 516
> It would be helpful to comment here that we intentionally do not use try-fi
Good point.  I'll add this comment in the patch where this code is created.


-- 
To view, visit https://gerrit.ovirt.org/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation in volume.getInfo
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64485/4/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 6964: type: *VolumeLeaseStatus
Line 6965: 
Line 6966: -   description: A monotonically increasing number, 
incremented each
Line 6967:  time a volume operation is completed 
successfully.
Line 6968:  The maximum value is 2^63-1 and the next 
increment
> 999
Done
Line 6969:  will reset the value to 0.
Line 6970: name: generation
Line 6971: type: uint
Line 6972: type: object


-- 
To view, visit https://gerrit.ovirt.org/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 5:

(4 comments)

https://gerrit.ovirt.org/#/c/64479/5/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 274
Line 275
Line 276
Line 277
Line 278
> Lets use here the started event owned by the FakeQemuConvertChecker.
Just did the event for now.


Line 278
Line 279
Line 280
Line 281
Line 282
> So we remove this...
Done


Line 296: 
Line 297: class FakeQemuImgOperation(object):
Line 298: def __init__(self, error, wait_for_abort):
Line 299: self.error = error
Line 300: self.wait_for_abort = wait_for_abort
> Lets have an event here...
Done
Line 301: self.running = True
Line 302: 
Line 303: def _wait(self):
Line 304: while self.running:


Line 310: def wait_for_completion(self):
Line 311: if self.error:
Line 312: raise self.error()
Line 313: if self.wait_for_abort:
Line 314: self._wait()
> And we can wait on the event here:
Done


-- 
To view, visit https://gerrit.ovirt.org/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Use Volume.operation in SDM.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: Use Volume.operation in SDM.copy_data
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64478/4/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 268: assert sc.ILLEGAL_VOL == self.dst_vol.getLegality()
Line 269: try:
Line 270: return FakeQemuImgOperation(self.error)
Line 271: finally:
Line 272: self.started.set()
> We talked today about fixing the race here - do you plan to address this in
Fixed.
Line 273: 
Line 274: 
Line 275: class FakeQemuImgOperation(object):
Line 276: def __init__(self, error):


-- 
To view, visit https://gerrit.ovirt.org/64478
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I574cea3387ab5b99368e0317aed73683d398a596
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[alitke]

Adam Litke has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12:

(2 comments)

https://gerrit.ovirt.org/#/c/63027/12/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS12, Line 5658: metadataDevice
I am wondering if this is too much detail for this API.  What engine really 
wants to know is "Does this VG have any immutable devices?".  Immutable devices 
must be excluded from VG modifications.  If we do it this way, then block 
domains can add here the first PV of the metadata LV and file domains can 
return an empty list.  If we need to restrict other PVs later we can easily add 
them to the list.


https://gerrit.ovirt.org/#/c/63027/12/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

PS12, Line 448: getMetadataLVDevice
This can be an internal function.


-- 
To view, visit https://gerrit.ovirt.org/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockSD: Storage domain life cycle management

[alitke]

Adam Litke has posted comments on this change.

Change subject: blockSD: Storage domain life cycle management
..


Patch Set 6:

(1 comment)

Seems straightforward.  Why did you put this in the monitor instead of 
activateStorageDomain and deactivateStorageDomain?

https://gerrit.ovirt.org/#/c/56876/6/vdsm/storage/monitor.py
File vdsm/storage/monitor.py:

PS6, Line 368: domain.setup()
Why not put this in _setupMonitor()?  It seems kind of hidden in here.


-- 
To view, visit https://gerrit.ovirt.org/56876
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7227bb43c2e1ee67a6239956aae48173a27f566e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: tests: Use make_uuid in storage tests
..

tests: Use make_uuid in storage tests

Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Signed-off-by: Adam Litke 
---
M tests/storageMailboxTests.py
M tests/storage_hsm_test.py
M tests/storage_sdm_copy_data_test.py
M tests/storage_sdm_create_volume_test.py
M tests/storage_volume_artifacts_test.py
M tests/storage_volume_metadata_test.py
M tests/storage_workarounds_test.py
M tests/storagetestlib.py
M tests/storagetestlibTests.py
9 files changed, 49 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/64943/1

diff --git a/tests/storageMailboxTests.py b/tests/storageMailboxTests.py
index 1d5b45a..5de9652 100644
--- a/tests/storageMailboxTests.py
+++ b/tests/storageMailboxTests.py
@@ -18,11 +18,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-from uuid import uuid4
 import threading
 import os
 import shutil
 
+from testlib import make_uuid
 from testlib import VdsmTestCase as TestCaseBase
 
 import storage.storage_mailbox as sm
@@ -33,7 +33,7 @@
 
 class StoragePoolStub(object):
 def __init__(self):
-self.spUUID = str(uuid4())
+self.spUUID = make_uuid()
 self.storage_repository = tempfile.mkdtemp(dir='/var/tmp')
 self.__masterDir = os.path.join(self.storage_repository, self.spUUID,
 "mastersd", DOMAIN_META_DATA)
diff --git a/tests/storage_hsm_test.py b/tests/storage_hsm_test.py
index e9e0448..b7e0e28 100644
--- a/tests/storage_hsm_test.py
+++ b/tests/storage_hsm_test.py
@@ -18,11 +18,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-import uuid
 from contextlib import contextmanager
 
 from monkeypatch import MonkeyPatchScope
 from testlib import make_config
+from testlib import make_uuid
 from testlib import VdsmTestCase
 from testlib import permutations, expandPermutations
 from storagetestlib import fake_file_env
@@ -112,8 +112,8 @@
 @contextmanager
 def fake_volume(self, vol_fmt):
 with fake_file_env() as env:
-img_id = str(uuid.uuid4())
-vol_id = str(uuid.uuid4())
+img_id = make_uuid()
+vol_id = make_uuid()
 make_file_volume(env.sd_manifest, self.SIZE, img_id, vol_id,
  vol_format=vol_fmt)
 yield env.sd_manifest.produceVolume(img_id, vol_id)
diff --git a/tests/storage_sdm_copy_data_test.py 
b/tests/storage_sdm_copy_data_test.py
index c0ec533..e770b7f 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -19,7 +19,6 @@
 #
 from __future__ import absolute_import
 
-import uuid
 from contextlib import contextmanager
 
 from fakelib import FakeScheduler
@@ -28,6 +27,7 @@
 from storagetestlib import fake_env
 from storagetestlib import make_qemu_chain, write_qemu_chain, verify_qemu_chain
 from storagetestlib import ChainVerificationError
+from testlib import make_uuid
 from testlib import VdsmTestCase, expandPermutations, permutations
 from testlib import wait_for_job
 
@@ -130,7 +130,7 @@
 def test_intra_domain_copy(self, env_type, src_fmt, dst_fmt):
 src_fmt = sc.name2type(src_fmt)
 dst_fmt = sc.name2type(dst_fmt)
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 
 with self.get_vols(env_type, src_fmt, dst_fmt) as (src_chain,
dst_chain):
@@ -173,7 +173,7 @@
  dst_chain):
 write_qemu_chain(src_chain)
 for index in copy_seq:
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 src_vol = src_chain[index]
 dst_vol = dst_chain[index]
 source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
@@ -193,7 +193,7 @@
 volume format may be set incorrectly due to an old bug.  Check that the
 workaround we have in place allows the copy to proceed without error.
 """
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 vm_conf_size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
 vm_conf_data = "VM Configuration"
 
diff --git a/tests/storage_sdm_create_volume_test.py 
b/tests/storage_sdm_create_volume_test.py
index ebc2db0..f62f325 100644
--- a/tests/storage_sdm_create_volume_test.py
+++ b/tests/storage_sdm_create_volume_test.py
@@ -20,10 +20,10 @@
 from __future__ import absolute_import
 
 from contextlib import contextmanager
-import uuid
 
 from monkeypatch import MonkeyPatchScope
 from storagefakelib import FakeResourceManager
+from testlib import make_uuid
 from testlib import VdsmTestCase, recorded, expandPermutations, permutations
 from testlib import wait_for_job
 
@@ -84,17 

Change in vdsm[master]: storage: Increment generation id when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation id when completing operation
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64487/2/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 170: vol.setMetaParam(sc.GENERATION, generation)
Line 171: self.assertEqual(generation, vol.getInfo()['generation'])
Line 172: 
Line 173: @permutations((
Line 174: (sc.MAX_GENERATION, 0),
> I think we need only this test for wrapping, previous test verified increas
Keeping first two as they catch off by one errors.
Line 175: (sc.MAX_GENERATION - 1, sc.MAX_GENERATION),
Line 176: (0, 1),
Line 177: (1, 2),
Line 178: ))


https://gerrit.ovirt.org/#/c/64487/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 55: # domains) we do not need to use the image dir reference anymore.
Line 56: return volUUID
Line 57: 
Line 58: 
Line 59: def next_generation(current_generation):
> Do we need it public?
no.  Making private.
Line 60: # Increment a generation value and wrap to 0 after MAX_GENERATION
Line 61: return (current_generation + 1) % (sc.MAX_GENERATION + 1)
Line 62: 
Line 63: 


-- 
To view, visit https://gerrit.ovirt.org/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation id in volume.operation context

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Validate generation id in volume.operation context
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64486/1/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 134: vol_id = gen_uuid()
Line 135: generation = 100
Line 136: 
Line 137: with fake_env('file') as env:
Line 138: env.make_volume(MB, img_id, vol_id)
> Sure, most test do not care about generation, but they may need other metad
Let's keep it as-is for now.
Line 139: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 140: vol.setMetaParam(sc.GENERATION, 100)
Line 141: with vol.operation(generation):
Line 142: pass


https://gerrit.ovirt.org/#/c/64486/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 507: marked ILLEGAL prior to the first modification of data and 
subsequently
Line 508: marked LEGAL again once the operation has completed.  Thus, 
if an
Line 509: interruption occurs the volume will remain in an ILLEGAL 
state.
Line 510: 
Line 511: If generation is provided we check that the volume's 
generation matches
> Missing period at the end.
Done
Line 512: """
Line 513: actual_gen = self.getMetaParam(sc.GENERATION)
Line 514: if requested_gen is not None and actual_gen != requested_gen:
Line 515: raise se.GenerationMismatch(requested_gen, actual_gen)


-- 
To view, visit https://gerrit.ovirt.org/64486
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation id in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation id in volume.getInfo
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64485/2/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 138: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 139: vol.getLeaseStatus = lambda: 'unused'
Line 140: vol.setMetaParam(sc.GENERATION, generation)
Line 141: self.assertEqual(generation, vol.getInfo()['generation'])
Line 142: 
> Do we need a test for volume without generation?
I'll permutate this test so we can check that case explicitly.
Line 143: 
Line 144: class CountedInstanceMethod(object):
Line 145: def __init__(self, method):
Line 146: self._method = method


https://gerrit.ovirt.org/#/c/64485/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 215
Line 216
Line 217
Line 218
Line 219
> This always uses VolumeMetadata, right?
correct.


-- 
To view, visit https://gerrit.ovirt.org/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/3/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 165: # ascii values, but limit non-ascii values, which are encoded by 
engine
Line 166: # using 4 bytes per character.
Line 167: DESCRIPTION_SIZE = 210
Line 168: DEFAULT_GENERATION = 0
Line 169: MAX_GENERATION = 2 ** 63 - 1
> Yes, better - don't forget to update the worst case example and the number 
Done
Line 170: 
Line 171: # Block volume metadata tags
Line 172: TAG_PREFIX_MD = "MD_"
Line 173: TAG_PREFIX_MDNUMBLKS = "MS_"


-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 2:

(4 comments)

https://gerrit.ovirt.org/#/c/64484/2//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2016-09-27 15:19:01 -0400
Line 6: 
Line 7: storage: Add support for generation to VolumeMetadata
Line 8: 
Line 9: We would like to add support for a new generation id key in the volume
> remove id.
Done
Line 10: metadata.  GEN is a monotonically increasing integer that can be used
Line 11: to determine whether certain operations have been completed on a volume
Line 12: and to prevent two hosts from sequentially performing the same 
operation
Line 13: in the event of double scheduling.


https://gerrit.ovirt.org/#/c/64484/2/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 161
Line 162
Line 163
Line 164
Line 165
> Lets separate DESCRIPTION_SIZE from the generation constants, and maybe add
Done


Line 124: IMAGE = "IMAGE"
Line 125: DESCRIPTION = "DESCRIPTION"
Line 126: LEGALITY = "LEGALITY"
Line 127: MTIME = "MTIME"
Line 128: GENERATION = "GEN"
> Lets add a comment:
Done
Line 129: POOL = MDK_POOLS  # Deprecated
Line 130: 
Line 131: # In block storage, metadata size is limited to BLOCK_SIZE (512), to
Line 132: # ensure that metadata is written atomically. This is big enough for 
the


https://gerrit.ovirt.org/#/c/64484/2/tests/storage_volume_metadata_test.py
File tests/storage_volume_metadata_test.py:

Line 188: 
Line 189: def test_generation_default(self):
Line 190: data = make_md_dict()
Line 191: lines = make_lines(generation=sc.DEFAULT_GENERATION + 1, 
**data)
Line 192: lines.remove("{}={}".format(sc.GENERATION, 
sc.DEFAULT_GENERATION + 1))
> Why not:
Simplified.
Line 193: md = volume.VolumeMetadata.from_lines(lines)


-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/1/lib/vdsm/storage/volumemetadata.py
File lib/vdsm/storage/volumemetadata.py:

Line 148: constants.DESCRIPTION: self.description,
Line 149: constants.PUUID: self.puuid,
Line 150: constants.MTIME: str(self.mtime),
Line 151: constants.LEGALITY: self.legality,
Line 152: constants.GENERATION: self.generation,
> If you think it is less work to check old code, why not.
Just checked 3.6 code.  We don't try to filter out unknown keys in getMetadata. 
 We just read lines, split each line around '=' and populate a dict with the 
pairs.  This is verified to my satisfaction.


-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add gen_uuid test helper

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Add gen_uuid test helper
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/64542/1/tests/testlib.py
File tests/testlib.py:

Line 590: while job.active:
Line 591: time.sleep(1)
Line 592: 
Line 593: 
Line 594: def gen_uuid():
> uuid_string? uuid_str? make_uuid?
make_uuid
Line 595: """
Line 596: Return a new UUID in the format used for all vdsm APIs that 
accept UUIDs.
Line 597: """


Line 592: 
Line 593: 
Line 594: def gen_uuid():
Line 595: """
Line 596: Return a new UUID in the format used for all vdsm APIs that 
accept UUIDs.
> Return a new UUID version 4 string...
Done
Line 597: """


-- 
To view, visit https://gerrit.ovirt.org/64542
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/3/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 165: # ascii values, but limit non-ascii values, which are encoded by 
engine
Line 166: # using 4 bytes per character.
Line 167: DESCRIPTION_SIZE = 210
Line 168: DEFAULT_GENERATION = 0
Line 169: MAX_GENERATION = 2 ** 63 - 1
Let's reduce this to 999
Line 170: 
Line 171: # Block volume metadata tags
Line 172: TAG_PREFIX_MD = "MD_"
Line 173: TAG_PREFIX_MDNUMBLKS = "MS_"


-- 
To view, visit https://gerrit.ovirt.org/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64543/1/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1789: 
Line 1790: 
Line 1791: class GenerationMismatch(StorageException):
Line 1792: code = 911
Line 1793: message = "The provided generation does not match the actual 
generation."
> Actual number is 6 messages ending with period, 284 without.
Ok.  Removed.
Line 1794: 
Line 1795: def __init__(self, requested, actual):


-- 
To view, visit https://gerrit.ovirt.org/64543
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Support generation id in sdm.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Support generation id in sdm.copy_data
..


Patch Set 2: Verified-1

Still investigating a test failure.

-- 
To view, visit https://gerrit.ovirt.org/64488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add gen_uuid test helper

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: tests: Add gen_uuid test helper
..

tests: Add gen_uuid test helper

The test code is sprinkled with lots of boilerplate code to generate
UUIDs in the proper format.  Add a helper to testlib so we can reduce
this duplicated logic in tests.

Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
Signed-off-by: Adam Litke 
---
M tests/testlib.py
1 file changed, 8 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/64542/1

diff --git a/tests/testlib.py b/tests/testlib.py
index c341c24..b75e7a2 100644
--- a/tests/testlib.py
+++ b/tests/testlib.py
@@ -28,6 +28,7 @@
 import pickle
 import platform
 import unittest
+import uuid
 from functools import wraps
 import shutil
 import sys
@@ -588,3 +589,10 @@
 """
 while job.active:
 time.sleep(1)
+
+
+def gen_uuid():
+"""
+Return a new UUID in the format used for all vdsm APIs that accept UUIDs.
+"""
+return str(uuid.uuid4())


-- 
To view, visit https://gerrit.ovirt.org/64542
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: storage: GenerationMismatch exception
..

storage: GenerationMismatch exception

Introduce a new exception with a public error code for generation
mismatch errors.  SDM verbs involving datapath operations will accept a
generation parameter which will be compared against the operative
entity's (eg. Volume) current generation.  If the generation values
match then the operation can proceed.  Otherwise GenerationMismatch will
be raised and the caller must resync against the actual storage state to
determine how to proceed.

Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
Signed-off-by: Adam Litke 
---
M lib/vdsm/storage/exception.py
1 file changed, 9 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/64543/1

diff --git a/lib/vdsm/storage/exception.py b/lib/vdsm/storage/exception.py
index dcd2922..ee56673 100644
--- a/lib/vdsm/storage/exception.py
+++ b/lib/vdsm/storage/exception.py
@@ -1777,7 +1777,7 @@
 
 #
 #  SDM Errors
-#  Range: 909-910
+#  Range: 910-919
 #
 
 class DomainHasGarbage(StorageException):
@@ -1786,3 +1786,11 @@
 
 def __init__(self, reason):
 self.value = reason
+
+
+class GenerationMismatch(StorageException):
+code = 911
+message = "The provided generation does not match the actual generation."
+
+def __init__(self, requested, actual):
+self.value = "requested=%s, actual=%s" % (requested, actual)


-- 
To view, visit https://gerrit.ovirt.org/64543
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Support generation id in sdm.copy_data

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Support generation id in sdm.copy_data
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/64488/1/vdsm/storage/sdm/api/copy_data.py
File vdsm/storage/sdm/api/copy_data.py:

Line 96: class CopyDataDivEndpoint(properties.Owner):
Line 97: sd_id = properties.UUID(required=True)
Line 98: img_id = properties.UUID(required=True)
Line 99: vol_id = properties.UUID(required=True)
Line 100: generation_id = properties.Integer(required=False, minval=0)
> - generation
Done
Line 101: 
Line 102: def __init__(self, params, host_id, writable):
Line 103: self.sd_id = params.get('sd_id')
Line 104: self.img_id = params.get('img_id')


Line 146: return sc.fmt2str(parent_vol.getFormat())
Line 147: 
Line 148: @property
Line 149: def volume_operation(self):
Line 150: return partial(self._vol.operation, self.generation_id)
> Can we simplify to:
Done
Line 151: 
Line 152: @contextmanager
Line 153: def prepare(self):
Line 154: dom = sdCache.produce_manifest(self.sd_id)


-- 
To view, visit https://gerrit.ovirt.org/64488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation id when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation id when completing operation
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/64487/1/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 510: the volume is legal we want to call volume.getInfo to 
determine if this
Line 511: operation has not been started or has finished successfully.  
We enable
Line 512: this by incrementing the generation id after the operation 
completes.
Line 513: Note that legality and generation id must be updated in the 
same write
Line 514: operation.
> The last note is not interesting to the users of this api. It should be rig
Done
Line 515: 
Line 516: If generation is provided we check that the volume's 
generation matches
Line 517: """
Line 518: real_gen_id = self.getMetaParam(sc.GENERATION)


Line 520: raise se.InvalidGeneration(requested_gen_id, real_gen_id)
Line 521: self.setLegality(sc.ILLEGAL_VOL)
Line 522: yield
Line 523: 
Line 524: # Update the generation id and legality in one write
> Lets replace this with the sentence from the docstring, with a IMPORTANT: p
Done
Line 525: metadata = self.getMetadata()
Line 526: metadata[sc.LEGALITY] = sc.LEGAL_VOL
Line 527: metadata[sc.GENERATION] = real_gen_id + 1
Line 528: self.setMetadata(metadata)


Line 523: 
Line 524: # Update the generation id and legality in one write
Line 525: metadata = self.getMetadata()
Line 526: metadata[sc.LEGALITY] = sc.LEGAL_VOL
Line 527: metadata[sc.GENERATION] = real_gen_id + 1
> We need to wrap around, or assert, we cannot use unlimited value:
Done
Line 528: self.setMetadata(metadata)
Line 529: 
Line 530: 
Line 531: class Volume(object):


-- 
To view, visit https://gerrit.ovirt.org/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation id in volume.operation context

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Validate generation id in volume.operation context
..


Patch Set 1:

(11 comments)

https://gerrit.ovirt.org/#/c/64486/1/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1776: 
Line 1777: 
Line 1778: #
Line 1779: #  SDM Errors
Line 1780: #  Range: 910-919
> Can you submit this separately? (we can merge this quickly)
sure.
Line 1781: #
Line 1782: 
Line 1783: class DomainHasGarbage(StorageException):
Line 1784: code = 910


Line 1787: def __init__(self, reason):
Line 1788: self.value = reason
Line 1789: 
Line 1790: 
Line 1791: class InvalidGeneration(StorageException):
> GenerationMismatch?
Done
Line 1792: code = 911
Line 1793: message = "The provided Generation ID is not valid for this 
Volume"
Line 1794: 
Line 1795: def __init__(self, requested_id, real_id):


Line 1789: 
Line 1790: 
Line 1791: class InvalidGeneration(StorageException):
Line 1792: code = 911
Line 1793: message = "The provided Generation ID is not valid for this 
Volume"
> The provided generation does match the actual volume generation.
Done
Line 1794: 
Line 1795: def __init__(self, requested_id, real_id):


Line 1791: class InvalidGeneration(StorageException):
Line 1792: code = 911
Line 1793: message = "The provided Generation ID is not valid for this 
Volume"
Line 1794: 
Line 1795: def __init__(self, requested_id, real_id):
> requested_generation, real_generation
Done


Line 1792: code = 911
Line 1793: message = "The provided Generation ID is not valid for this 
Volume"
Line 1794: 
Line 1795: def __init__(self, requested_id, real_id):
Line 1796: self.value = "requested id:%i, actual_id:%i" % 
(requested_id, real_id)
> requested=%s, actual=%s?
Done


https://gerrit.ovirt.org/#/c/64486/1/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 134: vol_id = str(uuid.uuid4())
Line 135: generation = 100
Line 136: 
Line 137: with fake_env('file') as env:
Line 138: env.make_volume(MB, img_id, vol_id)
> Add generation parameter here? This would make lot of tests easier later.
Most tests don't care about the generation.  Let's leave it for now as we can 
always change it later.
Line 139: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 140: vol.setMetaParam(sc.GENERATION, 100)
Line 141: with vol.operation(generation):
Line 142: pass


Line 151: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 152: vol.setMetaParam(sc.GENERATION, generation)
Line 153: with self.assertRaises(se.InvalidGeneration):
Line 154: with vol.operation(generation + 1):
Line 155: pass
> Lets test also genration - 1
Done
Line 156: 
Line 157: def test_get_info_generation_id(self):
Line 158: img_id = str(uuid.uuid4())
Line 159: vol_id = str(uuid.uuid4())


https://gerrit.ovirt.org/#/c/64486/1/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 498: """
Line 499: pass
Line 500: 
Line 501: @contextmanager
Line 502: def operation(self, requested_gen_id=None):
> requested_generation
Done
Line 503: """
Line 504: Must be called with the Volume Lease held.
Line 505: 
Line 506: In order to detect interrupted datapath operations a volume 
should be


Line 509: interruption occurs the volume will remain in an ILLEGAL 
state.
Line 510: 
Line 511: If generation is provided we check that the volume's 
generation matches
Line 512: """
Line 513: real_gen_id = self.getMetaParam(sc.GENERATION)
> real_generation
actual_gen
Line 514: if requested_gen_id is not None and real_gen_id != 
requested_gen_id:
Line 515: raise se.InvalidGeneration(requested_gen_id, real_gen_id)
Line 516: self.setLegality(sc.ILLEGAL_VOL)
Line 517: yield


Line 511: If generation is provided we check that the volume's 
generation matches
Line 512: """
Line 513: real_gen_id = self.getMetaParam(sc.GENERATION)
Line 514: if requested_gen_id is not None and real_gen_id != 
requested_gen_id:
Line 515: raise se.InvalidGeneration(requested_gen_id, real_gen_id)
> The _id suffix not only incorrect but make the code harder to read.
Done
Line 516: self.setLegality(sc.ILLEGAL_VOL)
Line 517: yield
Line 518: self.setLegality(sc.LEGAL_VOL)
Line 519: 


Line 517: yield
Line 518: self.setLegality(sc.LEGAL_VOL)
Line 519: 
Line 520: 
Line 521: 
> Unintended I guess?
Done
Line 522: class Volume(object):
Line 523: log = logging.getLogger('storage.Volume')
Line 524: manifestClass = VolumeManifest
Line 

Change in vdsm[master]: storage: Report generation id in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation id in volume.getInfo
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/64485/1/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 6962: -   description: The status of the Volume lease
Line 6963: name: lease
Line 6964: type: *VolumeLeaseStatus
Line 6965: 
Line 6966: -   description: A monotonically increasing generation id
> How about:
Done
Line 6967: name: generation
Line 6968: type: uint
Line 6969: type: object
Line 6970: 


Line 6964: type: *VolumeLeaseStatus
Line 6965: 
Line 6966: -   description: A monotonically increasing generation id
Line 6967: name: generation
Line 6968: type: uint
> long long? int64_t?
these are keywords in the schema.  I don't think we have anything more specific 
than uint.  Java can use int64_t if it wants to use it.
Line 6969: type: object
Line 6970: 
Line 6971: VolumeSizeInfo: 
Line 6972: added: '3.1'


https://gerrit.ovirt.org/#/c/64485/1/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 205: "image": self.getImage(),
Line 206: "ctime": meta.get(sc.CTIME, ""),
Line 207: "mtime": "0",
Line 208: "legality": meta.get(sc.LEGALITY, ""),
Line 209: "generation": meta.get(sc.GENERATION, 0)
> We have this in several places, maybe we need a DEFAULT_GENERATION constant
ok.  Will add to an earlier patch.
Line 210: }
Line 211: 
Line 212: def getInfo(self):
Line 213: """


-- 
To view, visit https://gerrit.ovirt.org/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org