Change in vdsm[master]: CannotRemoveLogicalVolume is raised with arguments.

Eduardo has uploaded a new change for review.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................

CannotRemoveLogicalVolume is raised with arguments.

Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Signed-off-by: Eduardo <ewarszaw(a)redhat.com&gt;
---
M vdsm/storage/lvm.py
M vdsm/storage/storage_exception.py
2 files changed, 7 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/8808/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 78c846a..88d818b 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -1015,7 +1015,7 @@
         if _isLVActive(vgName, lvName):
             #Fix me
             #Should not remove active LVs
-            #raise se.CannotRemoveLogicalVolume(vgName, lvName)
+            #raise se.CannotRemoveLogicalVolume(vgName, (lvName,))
             log.warning("Removing active volume %s/%s" % (vgName, lvName))
 
     #LV exists or not in cache, attempting to remove it.
@@ -1036,7 +1036,7 @@
     else:
         # Otherwise LV info needs to be refreshed
         _lvminfo._invalidatelvs(vgName, lvName)
-        raise se.CannotRemoveLogicalVolume(vgName, str(lvNames))
+        raise se.CannotRemoveLogicalVolume(vgName, lvNames)
 
 
 def extendLV(vgName, lvName, size):
diff --git a/vdsm/storage/storage_exception.py b/vdsm/storage/storage_exception.py
index 54e64e6..b5f63da 100644
--- a/vdsm/storage/storage_exception.py
+++ b/vdsm/storage/storage_exception.py
@@ -1305,8 +1305,12 @@
 
 
 class CannotRemoveLogicalVolume(StorageException):
+    def __init__(self, vgName, lvNames):
+        self.value("vg: %s lvs: %s", vgName, ", ".join(lvNames))
+        self.vg = vgName
+        self.lvs = lvNames
     code = 551
-    message = "Cannot remove Logical Volume"
+    message = "Failed to remove some LVs."
 
 
 class CannotDeactivateLogicalVolume(StorageException):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;

ShaoHe Feng has posted comments on this change.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................


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

(1 inline comment)

....................................................
File vdsm/storage/storage_exception.py
Line 1305: 
Line 1306: 
Line 1307: class CannotRemoveLogicalVolume(StorageException):
Line 1308:     def __init__(self, vgName, lvNames):
Line 1309:         self.value("vg: %s lvs: %s", vgName, ",
".join(lvNames))
self.value("vg: %s lvs: %s" % (vgName, ", ".join(lvNames)))?
Line 1310:         self.vg = vgName
Line 1311:         self.lvs = lvNames
Line 1312:     code = 551
Line 1313:     message = "Failed to remove some LVs."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;

ShaoHe Feng has posted comments on this change.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/storage_exception.py
Line 1305: 
Line 1306: 
Line 1307: class CannotRemoveLogicalVolume(StorageException):
Line 1308:     def __init__(self, vgName, lvNames):
Line 1309:         self.value("vg: %s lvs: %s", vgName, ",
".join(lvNames))
do you means:
 self.value = "vg: %s lvs: %s" % (vgName, ", ".join(lvNames)) 
 ?
Line 1310:         self.vg = vgName
Line 1311:         self.lvs = lvNames
Line 1312:     code = 551
Line 1313:     message = "Failed to remove some LVs."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;

ShaoHe Feng has posted comments on this change.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File vdsm/storage/storage_exception.py
Line 1305: 
Line 1306: 
Line 1307: class CannotRemoveLogicalVolume(StorageException):
Line 1308:     def __init__(self, vgName, lvNames):
Line 1309:         self.value("vg: %s lvs: %s", vgName, ",
".join(lvNames))
also, I have check the parent class:
 there is no attribute  value.

class GeneralException(Exception):
    code = 100
    message = "General Exception"

    def __init__(self, *value):
        self.value = value

    def __str__(self):
        return "%s: %s" % (self.message, repr(self.value))

    def response(self):
        return {'status': {'code': self.code, 'message':
str(self)}}


class StorageException(GeneralException):
    code = 200
    message = "General Storage Exception"
Line 1310:         self.vg = vgName
Line 1311:         self.lvs = lvNames
Line 1312:     code = 551
Line 1313:     message = "Failed to remove some LVs."


Line 1307: class CannotRemoveLogicalVolume(StorageException):
Line 1308:     def __init__(self, vgName, lvNames):
Line 1309:         self.value("vg: %s lvs: %s", vgName, ",
".join(lvNames))
Line 1310:         self.vg = vgName
Line 1311:         self.lvs = lvNames
and no other reference self.lvs and self.vg. 
 if you want  these value in the Exception, 
 why not inherit from the GeneralException directly and
 call call the GeneralException constructor __init__(self, *value).
Line 1312:     code = 551
Line 1313:     message = "Failed to remove some LVs."
Line 1314: 
Line 1315: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;

Ayal Baron has posted comments on this change.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;

Eduardo has abandoned this change.

Change subject: CannotRemoveLogicalVolume is raised with arguments.
......................................................................


Patch Set 1: Abandoned

We need a more sophisticated method to list the devices that failed only.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ied29c1dddf4e9d6cf890cb66d362acf9eed99ba6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;