[PATCH 1/3] Fix issue #339: Enable backend to handle not persistent pools
This patch changes the error message when storage pool is not found and adds the isPersistent checkings, in order to return correctly from deactivate function. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/storagepools.py | 3 ++- src/kimchi/model/storagepools.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 62f9525..b75bca0 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -90,7 +90,8 @@ class StoragePool(Resource): 'source': self.info['source'], 'type': self.info['type'], 'nr_volumes': self.info['nr_volumes'], - 'autostart': self.info['autostart']} + 'autostart': self.info['autostart'], + 'persistent': self.info['persistent']} val = self.info.get('task_id') if val: diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index fe70fa1..fea19f6 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -175,7 +175,7 @@ class StoragePoolModel(object): return conn.storagePoolLookupByName(name.encode("utf-8")) except libvirt.libvirtError as e: if e.get_error_code() == libvirt.VIR_ERR_NO_STORAGE_POOL: - raise NotFoundError("KCHTMPL0002E", {'name': name}) + raise NotFoundError("KCHPOOL0002E", {'name': name}) else: raise @@ -229,6 +229,7 @@ class StoragePoolModel(object): pool = self.get_storagepool(name, self.conn) info = pool.info() autostart = True if pool.autostart() else False + persistent = True if pool.isPersistent() else False xml = pool.XMLDesc(0) path = xmlutils.xpath_get_text(xml, "/pool/target/path")[0] pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] @@ -254,7 +255,8 @@ class StoragePoolModel(object): 'capacity': info[1], 'allocated': info[2], 'available': info[3], - 'nr_volumes': nr_volumes} + 'nr_volumes': nr_volumes, + 'persistent': persistent} if not pool.isPersistent(): # Deal with deep scan generated pool @@ -354,10 +356,15 @@ class StoragePoolModel(object): {'name': name, 'server': source['addr']}) return try: + persistent = pool.isPersistent() pool.destroy() except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0010E", {'name': name, 'err': e.get_error_message()}) + # If pool was not persistent, then it was erased by destroy() and + # must return nothing here, to trigger _redirect() and avoid errors + if not persistent: + return "" def delete(self, name): if self._pool_used_by_template(name): -- 1.8.5.3
This patch implements a checking in the UI in order to warn the user that if pool is not persistent it will be removed when deactivated. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- ui/js/src/kimchi.storage_main.js | 30 +++++++++++++++++++++++++----- ui/pages/i18n.html.tmpl | 3 ++- ui/pages/tabs/storage.html.tmpl | 2 +- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ui/js/src/kimchi.storage_main.js b/ui/js/src/kimchi.storage_main.js index 886feef..c2c721d 100644 --- a/ui/js/src/kimchi.storage_main.js +++ b/ui/js/src/kimchi.storage_main.js @@ -99,13 +99,33 @@ kimchi.storageBindClick = function() { kimchi.message.error(err.responseJSON.reason); }); }); + $('.pool-deactivate').on('click', function(event) { var poolName = $(this).data('name'); - kimchi.changePoolState(poolName, 'deactivate', function() { - kimchi.doListStoragePools(); - }, function(err) { - kimchi.message.error(err.responseJSON.reason); - }); + var settings = { + title : i18n['KCHAPI6001M'], + content : i18n['KCHPOOL6012M'], + confirm : i18n['KCHAPI6002M'], + cancel : i18n['KCHAPI6003M'] + }; + if (!$(this).data('persistent')) { + kimchi.confirm(settings, function() { + kimchi.changePoolState(poolName, 'deactivate', function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + }, function() { + return false; + }); + } + else { + kimchi.changePoolState(poolName, 'deactivate', function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + } }); $('.storage-action').on('click', function() { diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index a7dddf4..08429c5 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -111,7 +111,8 @@ var i18n = { 'KCHPOOL6008E': "$_("Server name can not be blank.")", 'KCHPOOL6009E': "$_("This is not a valid Server Name or IP. please, modify it.")", 'KCHPOOL6010M': "$_("Looking for available partitions ...")", - 'KCHPOOL6011M': "$_("No available partitions found.")" + 'KCHPOOL6011M': "$_("No available partitions found.")", + 'KCHPOOL6012M': "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" }; </script> </body> diff --git a/ui/pages/tabs/storage.html.tmpl b/ui/pages/tabs/storage.html.tmpl index 4fbdce5..c20c954 100644 --- a/ui/pages/tabs/storage.html.tmpl +++ b/ui/pages/tabs/storage.html.tmpl @@ -72,7 +72,7 @@ <div class="btn dropdown popable storage-action" data-state="{state}" data-type="{type}" data-name="{name}"> <span class="text">$_("Actions")</span><span class="arrow"></span> <div class="popover actionsheet right-side" style="width: 250px"> - <button class="button-big pool-deactivate" data-stat="{state}" data-name="{name}"><span class="text">$_("Deactivate")</span></button> + <button class="button-big pool-deactivate" data-stat="{state}" data-name="{name}" data-persistent="{persistent}"><span class="text">$_("Deactivate")</span></button> <button class="button-big pool-activate" data-stat="{state}" data-name="{name}"><span class="text">$_("Activate")</span></button> <button class="button-big red pool-delete" data-stat="{state}" data-name="{name}"><span class="text">$_("Undefine")</span></button> </div> -- 1.8.5.3
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年03月07日 08:15, Rodrigo Trujillo wrote:
This patch implements a checking in the UI in order to warn the user that if pool is not persistent it will be removed when deactivated.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- ui/js/src/kimchi.storage_main.js | 30 +++++++++++++++++++++++++----- ui/pages/i18n.html.tmpl | 3 ++- ui/pages/tabs/storage.html.tmpl | 2 +- 3 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/ui/js/src/kimchi.storage_main.js b/ui/js/src/kimchi.storage_main.js index 886feef..c2c721d 100644 --- a/ui/js/src/kimchi.storage_main.js +++ b/ui/js/src/kimchi.storage_main.js @@ -99,13 +99,33 @@ kimchi.storageBindClick = function() { kimchi.message.error(err.responseJSON.reason); }); }); + $('.pool-deactivate').on('click', function(event) { var poolName = $(this).data('name'); - kimchi.changePoolState(poolName, 'deactivate', function() { - kimchi.doListStoragePools(); - }, function(err) { - kimchi.message.error(err.responseJSON.reason); - }); + var settings = { + title : i18n['KCHAPI6001M'], + content : i18n['KCHPOOL6012M'], + confirm : i18n['KCHAPI6002M'], + cancel : i18n['KCHAPI6003M'] + }; + if (!$(this).data('persistent')) { + kimchi.confirm(settings, function() { + kimchi.changePoolState(poolName, 'deactivate', function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + }, function() { + return false; + }); + } + else { + kimchi.changePoolState(poolName, 'deactivate', function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + } });
$('.storage-action').on('click', function() { diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index a7dddf4..08429c5 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -111,7 +111,8 @@ var i18n = { 'KCHPOOL6008E': "$_("Server name can not be blank.")", 'KCHPOOL6009E': "$_("This is not a valid Server Name or IP. please, modify it.")", 'KCHPOOL6010M': "$_("Looking for available partitions ...")", - 'KCHPOOL6011M': "$_("No available partitions found.")" + 'KCHPOOL6011M': "$_("No available partitions found.")", + 'KCHPOOL6012M': "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" }; </script> </body> diff --git a/ui/pages/tabs/storage.html.tmpl b/ui/pages/tabs/storage.html.tmpl index 4fbdce5..c20c954 100644 --- a/ui/pages/tabs/storage.html.tmpl +++ b/ui/pages/tabs/storage.html.tmpl @@ -72,7 +72,7 @@ <div class="btn dropdown popable storage-action" data-state="{state}" data-type="{type}" data-name="{name}"> <span class="text">$_("Actions")</span><span class="arrow"></span> <div class="popover actionsheet right-side" style="width: 250px"> - <button class="button-big pool-deactivate" data-stat="{state}" data-name="{name}"><span class="text">$_("Deactivate")</span></button> + <button class="button-big pool-deactivate" data-stat="{state}" data-name="{name}" data-persistent="{persistent}"><span class="text">$_("Deactivate")</span></button> <button class="button-big pool-activate" data-stat="{state}" data-name="{name}"><span class="text">$_("Activate")</span></button> <button class="button-big red pool-delete" data-stat="{state}" data-name="{name}"><span class="text">$_("Undefine")</span></button> </div>
This patch fixes the problems in the tests due to the inclusion of new persistent variable in pools information. It also adds a minor test. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 ++- tests/test_rest.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 4ad6a30..4ead84b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -959,7 +959,8 @@ class MockStoragePool(object): 'source': {}, 'type': 'dir', 'nr_volumes': 0, - 'autostart': 0} + 'autostart': 0, + 'persistent': True} self._volumes = {} def refresh(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 863fe80..17bd253 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -783,6 +783,9 @@ class RestTests(unittest.TestCase): else: self.assertEquals(False, storagepool['autostart']) + # Test if storage pool is persistent + self.assertEquals(True, storagepool['persistent']) + # activate the storage pool resp = self.request('/storagepools/test-pool/activate', '{}', 'POST') storagepool = json.loads( -- 1.8.5.3
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年03月07日 08:15, Rodrigo Trujillo wrote:
This patch fixes the problems in the tests due to the inclusion of new persistent variable in pools information. It also adds a minor test.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 ++- tests/test_rest.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 4ad6a30..4ead84b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -959,7 +959,8 @@ class MockStoragePool(object): 'source': {}, 'type': 'dir', 'nr_volumes': 0, - 'autostart': 0} + 'autostart': 0, + 'persistent': True} self._volumes = {}
def refresh(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 863fe80..17bd253 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -783,6 +783,9 @@ class RestTests(unittest.TestCase): else: self.assertEquals(False, storagepool['autostart'])
+ # Test if storage pool is persistent + self.assertEquals(True, storagepool['persistent']) + # activate the storage pool resp = self.request('/storagepools/test-pool/activate', '{}', 'POST') storagepool = json.loads(
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> Good catch! On 2014年03月07日 08:15, Rodrigo Trujillo wrote:
This patch changes the error message when storage pool is not found and adds the isPersistent checkings, in order to return correctly from deactivate function.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/storagepools.py | 3 ++- src/kimchi/model/storagepools.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 62f9525..b75bca0 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -90,7 +90,8 @@ class StoragePool(Resource): 'source': self.info['source'], 'type': self.info['type'], 'nr_volumes': self.info['nr_volumes'], - 'autostart': self.info['autostart']} + 'autostart': self.info['autostart'], + 'persistent': self.info['persistent']}
val = self.info.get('task_id') if val: diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index fe70fa1..fea19f6 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -175,7 +175,7 @@ class StoragePoolModel(object): return conn.storagePoolLookupByName(name.encode("utf-8")) except libvirt.libvirtError as e: if e.get_error_code() == libvirt.VIR_ERR_NO_STORAGE_POOL: - raise NotFoundError("KCHTMPL0002E", {'name': name}) + raise NotFoundError("KCHPOOL0002E", {'name': name}) else: raise
@@ -229,6 +229,7 @@ class StoragePoolModel(object): pool = self.get_storagepool(name, self.conn) info = pool.info() autostart = True if pool.autostart() else False + persistent = True if pool.isPersistent() else False xml = pool.XMLDesc(0) path = xmlutils.xpath_get_text(xml, "/pool/target/path")[0] pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] @@ -254,7 +255,8 @@ class StoragePoolModel(object): 'capacity': info[1], 'allocated': info[2], 'available': info[3], - 'nr_volumes': nr_volumes} + 'nr_volumes': nr_volumes, + 'persistent': persistent}
if not pool.isPersistent(): # Deal with deep scan generated pool @@ -354,10 +356,15 @@ class StoragePoolModel(object): {'name': name, 'server': source['addr']}) return try: + persistent = pool.isPersistent() pool.destroy() except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0010E", {'name': name, 'err': e.get_error_message()}) + # If pool was not persistent, then it was erased by destroy() and + # must return nothing here, to trigger _redirect() and avoid errors + if not persistent: + return ""
def delete(self, name): if self._pool_used_by_template(name):
participants (3)
-
Aline Manera -
Rodrigo Trujillo -
Royce Lv