Nir Soffer has uploaded a new change for review.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
vm: Set disk replica only if it can be prepared
We used to set drive replicating state, and then try to prepare the volume. If the preparing the volume fails, we revert the disk state.
To support LSM between mixed domain types, we need to check if the replica disk is a block device, and keep this information in Drive.diskReplicate dict. So we must prepare the volume before setting the disk in replication state. This also makes sense anyway, since if we cannot prepare the volume we cannot start the replication.
With the new order, we can simplify logging, and log only one exception in the outer try block, instead of 3 different exceptions. Since Vm._delDiskReplica() may raise, we keep now the original error and re-raise it.
Change-Id: I43814fa41c9cdc830fee9e929baa2e8249b18720 Signed-off-by: Nir Soffer nsoffer@redhat.com --- M vdsm/virt/vm.py 1 file changed, 12 insertions(+), 14 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/40022/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 4e64728..56f43a5 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -25,6 +25,7 @@ from xml.dom.minidom import parseString as _domParseStr import logging import os +import sys import tempfile import threading import time @@ -3432,27 +3433,24 @@ # prepareVolumePath dstDiskCopy['device'] = srcDrive.device
+ dstDiskCopy['path'] = self.cif.prepareVolumePath(dstDiskCopy) try: self._setDiskReplica(srcDrive, dstDisk) - except Exception: - self.log.error("Unable to set the replication for disk '%s' with " - "destination '%s'", srcDrive.name, dstDisk) - return errCode['replicaErr'] - - try: - dstDiskCopy['path'] = self.cif.prepareVolumePath(dstDiskCopy)
try: self._startDriveReplication(srcDrive, dstDiskCopy) except Exception: - self.log.exception("Unable to start the replication" - " for %s to %s", - srcDrive.name, dstDiskCopy) - self.cif.teardownVolumePath(dstDiskCopy) - raise + t, v, tb = sys.exc_info() + try: + self._delDiskReplica(srcDrive) + except Exception: + self.log.exception("Error unsetting disk replica %s", + dstDisk) + raise t, v, tb except Exception: - self.log.exception("Cannot complete the disk replication process") - self._delDiskReplica(srcDrive) + self.log.exception("Unable to start replication for %s to %s", + srcDrive.name, dstDiskCopy) + self.cif.teardownVolumePath(dstDiskCopy) return errCode['replicaErr']
if srcDrive.chunked:
automation@ovirt.org has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 1:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17837/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 1:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/18008/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/18008/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17837/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 1: Verified+1
Verify only positive flows
automation@ovirt.org has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 2:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18016/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 2:
Build Started (2/2)
0 -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1245/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18016/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1245/ : 0
automation@ovirt.org has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 3:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18121/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 3:
Build Started (2/2)
0 -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1351/
automation@ovirt.org has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 4:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18163/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 4:
Build Started (2/2)
0 -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1393/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18163/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1393/ : 0
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18121/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1351/ : 0
Adam Litke has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 4: Code-Review+2
automation@ovirt.org has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 5:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18367/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 5:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1597/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18367/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1597/ : SUCCESS
Federico Simoncelli has posted comments on this change.
Change subject: vm: Set disk replica only if it can be prepared ......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/40022/5/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 3021: # The device entry is enforced because stricly required by Line 3022: # prepareVolumePath Line 3023: dstDiskCopy['device'] = srcDrive.device Line 3024: Line 3025: dstDiskCopy['path'] = self.cif.prepareVolumePath(dstDiskCopy) You have to _setDiskReplica first and then prepare otherwise there's no track of the volume being prepared if we crash in the middle. Line 3026: try: Line 3027: self._setDiskReplica(srcDrive, dstDisk) Line 3028: Line 3029: try:
automation@ovirt.org has posted comments on this change.
Change subject: vm: Simplify error handling in diskReplicateStart ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: vm: Simplify error handling in diskReplicateStart ......................................................................
Patch Set 6: Verified+1
This version revert the order of operations, addressing Federico comments.
Federico Simoncelli has posted comments on this change.
Change subject: vm: Simplify error handling in diskReplicateStart ......................................................................
Patch Set 6: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vm: Simplify error handling in diskReplicateStart ......................................................................
vm: Simplify error handling in diskReplicateStart
Log only detailed exception in the outer try-except block, instead of detailed exception in the inner try-except block, and unhelpful exception in the outer block.
Change-Id: I43814fa41c9cdc830fee9e929baa2e8249b18720 Signed-off-by: Nir Soffer nsoffer@redhat.com Reviewed-on: https://gerrit.ovirt.org/40022 Reviewed-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/virt/vm.py 1 file changed, 2 insertions(+), 4 deletions(-)
Approvals: Nir Soffer: Verified Federico Simoncelli: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: vm: Simplify error handling in diskReplicateStart ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org