Change in vdsm[master]: blocksd: skip broken images in getAllVolumes

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

blocksd: skip broken images in getAllVolumes

When one of the images in the storage pool was damaged some operations
were failing for other images too (e.g. deletion).
This lead, for example, to the impossibility of removing VMs (and their
disks) on storage domains containing damaged images.

Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Signed-off-by: Federico Simoncelli 
---
M vdsm/storage/blockSD.py
1 file changed, 11 insertions(+), 4 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/11859/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index cc25417..404c3ef 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -164,18 +164,25 @@
     """
     vols = _getVolsTree(sdUUID)
     res = {}
+
     for volName in vols.iterkeys():
         res[volName] = {'imgs': [], 'parent': None}

for volName, vImg, parentVol in vols.itervalues():
         res[volName]['parent'] = parentVol
+
         if vImg not in res[volName]['imgs']:
             res[volName]['imgs'].insert(0, vImg)
+
         if (parentVol != sd.BLANK_UUID and
-                not volName.startswith(
-                    sd.REMOVED_IMAGE_PREFIX) and
-                vImg not in res[parentVol]['imgs']):
-            res[parentVol]['imgs'].append(vImg)
+                not volName.startswith(sd.REMOVED_IMAGE_PREFIX)):
+            try:
+                if vImg not in res[parentVol]['imgs']:
+                    res[parentVol]['imgs'].append(vImg)
+            except KeyError:
+                log.warning("Unable to find volume %s on domain %s, image "
+                            "%s could be damaged, skipping", parentVol, vImg,
+                            sdUUID, exc_info=True)

return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent']))
                 for k, v in res.iteritems())

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1181/ (2/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1216/ (1/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/327/ (3/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1181/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1216/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/327/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1182/ (2/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1217/ (1/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/328/ (3/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2: Verified

Verified: before the patch it is impossible to delete a VM if any image in the storage pool is damaged (even if it's not used by the VM). After the patch it is possible to remove the VM.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1182/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1217/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/328/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

Eduardo has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

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

getAllVolumes() should not work in malformed storage domains from the image point of view.
This can lead to data loss.

Quoting Ayal Baron:
"Right now we don't have any plans on dealing with this scenario from within the system."

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: oVirt Jenkins CI Server

Adam Litke has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

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

Yes please!  We are actually seeing this in a production system and it's causing us a lot of grief.  Once the damage has been done, we'd at least like to move on and not have the whole environment broken.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Adam Litke has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

FYI: This may be a candidate for fixing https://bugzilla.redhat.com/show_bug.cgi?id=910013

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has abandoned this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2: Abandoned

I suppose that this can be abandoned since this has been just recently merged:

http://gerrit.ovirt.org/#/c/11859/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: blocksd: skip broken images in getAllVolumes
......................................................................

Patch Set 2:

I suppose that this can be abandoned since this has been just recently merged:
http://gerrit.ovirt.org/#/c/12546/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9bbf7eecdb11873dd5fe9bef2afaa680aecbc32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Itamar Heim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server