[PATCH 1/5] Ubuntu: Add the LVM dependency package to README

The README file contains instructions to build Kimchi from the source tree, including the exact command to install the dependencies on Ubuntu. But that command does not contain the LVM package which is needed to probe LVM devices when creating a logical storage pool. Add the LVM package ("lvm2") to the apt-get install command in the README file. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index d80486b..a1f02ee 100644 --- a/docs/README.md +++ b/docs/README.md @@ -60,7 +60,7 @@ for more information on how to configure your system to access this repository. python-pam python-m2crypto python-jsonschema \ qemu-kvm libtool python-psutil python-ethtool \ sosreport python-ipaddr python-lxml nfs-common \ - open-iscsi + open-iscsi lvm2 Packages version requirement: python-jsonschema >= 1.3.0 -- 1.8.4.2

When creating a new logical storage pool, the user must select at least one logical device. If they do not, the error message "msg.validate.pool.edit.logical.device" is displayed. That ID has no corresponding translation, though. Add English and Brazilian Portuguese translations for the message ID "msg.validate.pool.edit.logical.device". This fixes the message "Undefined" mentioned in the issue #310. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- po/en_US.po | 5 ++++- po/kimchi.pot | 5 ++++- po/pt_BR.po | 7 +++++-- po/zh_CN.po | 5 ++++- ui/pages/i18n.html.tmpl | 1 + 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/po/en_US.po b/po/en_US.po index 2893946..77e0677 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 0.1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-07-11 17:32-0400\n" "Last-Translator: Crístian Viana <vianac@linux.vnet.ibm.com>\n" "Language-Team: English\n" @@ -282,6 +282,9 @@ msgstr "This is not a real linux path." msgid "Invalid nfs mount path." msgstr "Invalid nfs mount path." +msgid "No logical device selected." +msgstr "No logical device selected." + msgid "This storage pool is empty." msgstr "This storage pool is empty." diff --git a/po/kimchi.pot b/po/kimchi.pot index 738b070..836b013 100755 --- a/po/kimchi.pot +++ b/po/kimchi.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" @@ -271,6 +271,9 @@ msgstr "" msgid "Invalid nfs mount path." msgstr "" +msgid "No logical device selected." +msgstr "" + msgid "This storage pool is empty." msgstr "" diff --git a/po/pt_BR.po b/po/pt_BR.po index d12564f..724e339 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -20,9 +20,9 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 1.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-06-27 10:48+0000\n" -"Last-Translator: Alexandre Hirata <hirata@linux.vnet.ibm.com>\n" +"Last-Translator: Crístian Viana <vianac@linux.vnet.ibm.com>\n" "Language-Team: Aline Manera <alinefm@br.ibm.com>\n" "Language: pt_BR\n" "MIME-Version: 1.0\n" @@ -299,6 +299,9 @@ msgstr "Esse não é um caminho real no linux." msgid "Invalid nfs mount path." msgstr "Caminho de montagem NFS inválido." +msgid "No logical device selected." +msgstr "Nenhum dispositivo lógico selecionado." + msgid "This storage pool is empty." msgstr "Esse storage pool está vazio." diff --git a/po/zh_CN.po b/po/zh_CN.po index 758af1d..34f7aca 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20,7 +20,7 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 0.1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-06-27 10:48+0000\n" "Last-Translator: ShaoHe Feng <shaohef@linux.vnet.ibm.com>\n" "Language-Team: ShaoHe Feng <shaohef@linux.vnet.ibm.com>\n" @@ -287,6 +287,9 @@ msgstr "这不是一个符合要求的LINUX路径" msgid "Invalid nfs mount path." msgstr "无效的nfs挂载路径" +msgid "No logical device selected." +msgstr "" + msgid "This storage pool is empty." msgstr "这个存储池为空" diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index e684f2e..2217203 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -99,6 +99,7 @@ var i18n = { 'msg.validate.pool.edit.name':"$_("Invalid Storage Pool name. It may only contain letters, numbers, underscores, and hyphens.")", 'msg.validate.pool.edit.path':"$_("This is not a real linux path.")", 'msg.validate.pool.edit.nfspath':"$_("Invalid nfs mount path.")", + 'msg.validate.pool.edit.logical.device':"$_("No logical device selected.")", 'msg.kimchi.storage.pool.empty':"$_("This storage pool is empty.")", 'msg.kimchi.list.volume.fail':"$_("Failed to list the storage pool.")", 'msg.kimchi.storage.pool.not.active':"$_("The storage pool is not active now.")", -- 1.8.4.2

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/20/2014 08:09 PM, Crístian Viana wrote:
When creating a new logical storage pool, the user must select at least one logical device. If they do not, the error message "msg.validate.pool.edit.logical.device" is displayed. That ID has no corresponding translation, though.
Add English and Brazilian Portuguese translations for the message ID "msg.validate.pool.edit.logical.device". This fixes the message "Undefined" mentioned in the issue #310.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- po/en_US.po | 5 ++++- po/kimchi.pot | 5 ++++- po/pt_BR.po | 7 +++++-- po/zh_CN.po | 5 ++++- ui/pages/i18n.html.tmpl | 1 + 5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/po/en_US.po b/po/en_US.po index 2893946..77e0677 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 0.1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-07-11 17:32-0400\n" "Last-Translator: Crístian Viana <vianac@linux.vnet.ibm.com>\n" "Language-Team: English\n" @@ -282,6 +282,9 @@ msgstr "This is not a real linux path." msgid "Invalid nfs mount path." msgstr "Invalid nfs mount path."
+msgid "No logical device selected." +msgstr "No logical device selected." + msgid "This storage pool is empty." msgstr "This storage pool is empty."
diff --git a/po/kimchi.pot b/po/kimchi.pot index 738b070..836b013 100755 --- a/po/kimchi.pot +++ b/po/kimchi.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" @@ -271,6 +271,9 @@ msgstr "" msgid "Invalid nfs mount path." msgstr ""
+msgid "No logical device selected." +msgstr "" + msgid "This storage pool is empty." msgstr ""
diff --git a/po/pt_BR.po b/po/pt_BR.po index d12564f..724e339 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -20,9 +20,9 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 1.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-06-27 10:48+0000\n" -"Last-Translator: Alexandre Hirata <hirata@linux.vnet.ibm.com>\n" +"Last-Translator: Crístian Viana <vianac@linux.vnet.ibm.com>\n" "Language-Team: Aline Manera <alinefm@br.ibm.com>\n" "Language: pt_BR\n" "MIME-Version: 1.0\n" @@ -299,6 +299,9 @@ msgstr "Esse não é um caminho real no linux." msgid "Invalid nfs mount path." msgstr "Caminho de montagem NFS inválido."
+msgid "No logical device selected." +msgstr "Nenhum dispositivo lógico selecionado." + msgid "This storage pool is empty." msgstr "Esse storage pool está vazio."
diff --git a/po/zh_CN.po b/po/zh_CN.po index 758af1d..34f7aca 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20,7 +20,7 @@ msgid "" msgstr "" "Project-Id-Version: kimchi 0.1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-01-17 12:24-0200\n" +"POT-Creation-Date: 2014-01-20 18:46-0200\n" "PO-Revision-Date: 2013-06-27 10:48+0000\n" "Last-Translator: ShaoHe Feng <shaohef@linux.vnet.ibm.com>\n" "Language-Team: ShaoHe Feng <shaohef@linux.vnet.ibm.com>\n" @@ -287,6 +287,9 @@ msgstr "这不是一个符合要求的LINUX路径" msgid "Invalid nfs mount path." msgstr "无效的nfs挂载路径"
+msgid "No logical device selected." +msgstr "" + msgid "This storage pool is empty." msgstr "这个存储池为空"
diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index e684f2e..2217203 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -99,6 +99,7 @@ var i18n = { 'msg.validate.pool.edit.name':"$_("Invalid Storage Pool name. It may only contain letters, numbers, underscores, and hyphens.")", 'msg.validate.pool.edit.path':"$_("This is not a real linux path.")", 'msg.validate.pool.edit.nfspath':"$_("Invalid nfs mount path.")", + 'msg.validate.pool.edit.logical.device':"$_("No logical device selected.")", 'msg.kimchi.storage.pool.empty':"$_("This storage pool is empty.")", 'msg.kimchi.list.volume.fail':"$_("Failed to list the storage pool.")", 'msg.kimchi.storage.pool.not.active':"$_("The storage pool is not active now.")",

The dialog to create a new logical storage pool must validate the user input by checking if they selected at least one of the available devices. However, the code is looking for checkboxes with a non-existing element name and it is always reporting that the user did not select any of them - even when they do. Therefore, it is impossible to create a logical storage pool. Change the HTML element name of the devices' checkboxes on the storage pool page to match the validation algorithm. Instead of "source.devices", use the name "devices" on those elements. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/pages/storagepool-add.html.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/pages/storagepool-add.html.tmpl b/ui/pages/storagepool-add.html.tmpl index e8dac43..03c063d 100644 --- a/ui/pages/storagepool-add.html.tmpl +++ b/ui/pages/storagepool-add.html.tmpl @@ -143,7 +143,7 @@ </script> <script id="partitionTmpl" type="html/text"> <div> - <input type="checkbox" value="{path}" name="source.devices"> + <input type="checkbox" value="{path}" name="devices"> <label>{path}</label> </div> </script> -- 1.8.4.2

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/20/2014 08:09 PM, Crístian Viana wrote:
The dialog to create a new logical storage pool must validate the user input by checking if they selected at least one of the available devices. However, the code is looking for checkboxes with a non-existing element name and it is always reporting that the user did not select any of them - even when they do. Therefore, it is impossible to create a logical storage pool.
Change the HTML element name of the devices' checkboxes on the storage pool page to match the validation algorithm. Instead of "source.devices", use the name "devices" on those elements.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/pages/storagepool-add.html.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/pages/storagepool-add.html.tmpl b/ui/pages/storagepool-add.html.tmpl index e8dac43..03c063d 100644 --- a/ui/pages/storagepool-add.html.tmpl +++ b/ui/pages/storagepool-add.html.tmpl @@ -143,7 +143,7 @@ </script> <script id="partitionTmpl" type="html/text"> <div> - <input type="checkbox" value="{path}" name="source.devices"> + <input type="checkbox" value="{path}" name="devices"> <label>{path}</label> </div> </script>

Currently, there are four different storage pool types. The JavaScript code that handles that logic assumes different values in the 'else' blocks of different functions. It is a good practice to be explicit on what we are checking for and not to depend on the possible last value out of a set. This can easily lead to bugs in the future if additional storage pool types are added. Also, the code is harder to follow because the 'else' assumptions on similar comparisons are sometimes one value and sometimes another value. Use explicit and consistent values when checking the different pool types in 'if/else' clauses. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/js/src/kimchi.storagepool_add_main.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ui/js/src/kimchi.storagepool_add_main.js b/ui/js/src/kimchi.storagepool_add_main.js index 24de979..2f54db9 100644 --- a/ui/js/src/kimchi.storagepool_add_main.js +++ b/ui/js/src/kimchi.storagepool_add_main.js @@ -71,7 +71,7 @@ kimchi.initStorageAddPage = function() { $('.logical-section').addClass('tmpl-html'); $('.nfs-section').addClass('tmpl-html'); $('.iscsi-section').removeClass('tmpl-html'); - } else { + } else if ($(this).val() === 'logical') { $('.path-section').addClass('tmpl-html'); $('.logical-section').removeClass('tmpl-html'); $('.nfs-section').addClass('tmpl-html'); @@ -108,10 +108,11 @@ kimchi.validateForm = function() { return kimchi.validateNfsForm(); } else if (poolType === "iscsi") { return kimchi.validateIscsiForm(); - } else { + } else if (poolType === "logical") { return kimchi.validateLogicalForm(); + } else { + return false; } - }; kimchi.validateDirForm = function () { @@ -205,7 +206,7 @@ kimchi.addPool = function(event) { source.path = $('#nfspathId').val(); source.host = $('#nfsserverId').val(); formData.source = source; - } else { + } else if (poolType === 'iscsi') { var source = {}; source.target = $('#iscsiTargetId').val(); source.host = $('#iscsiserverId').val(); -- 1.8.4.2

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/20/2014 08:09 PM, Crístian Viana wrote:
Currently, there are four different storage pool types. The JavaScript code that handles that logic assumes different values in the 'else' blocks of different functions. It is a good practice to be explicit on what we are checking for and not to depend on the possible last value out of a set. This can easily lead to bugs in the future if additional storage pool types are added. Also, the code is harder to follow because the 'else' assumptions on similar comparisons are sometimes one value and sometimes another value.
Use explicit and consistent values when checking the different pool types in 'if/else' clauses.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/js/src/kimchi.storagepool_add_main.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.storagepool_add_main.js b/ui/js/src/kimchi.storagepool_add_main.js index 24de979..2f54db9 100644 --- a/ui/js/src/kimchi.storagepool_add_main.js +++ b/ui/js/src/kimchi.storagepool_add_main.js @@ -71,7 +71,7 @@ kimchi.initStorageAddPage = function() { $('.logical-section').addClass('tmpl-html'); $('.nfs-section').addClass('tmpl-html'); $('.iscsi-section').removeClass('tmpl-html'); - } else { + } else if ($(this).val() === 'logical') { $('.path-section').addClass('tmpl-html'); $('.logical-section').removeClass('tmpl-html'); $('.nfs-section').addClass('tmpl-html'); @@ -108,10 +108,11 @@ kimchi.validateForm = function() { return kimchi.validateNfsForm(); } else if (poolType === "iscsi") { return kimchi.validateIscsiForm(); - } else { + } else if (poolType === "logical") { return kimchi.validateLogicalForm(); + } else { + return false; } - };
kimchi.validateDirForm = function () { @@ -205,7 +206,7 @@ kimchi.addPool = function(event) { source.path = $('#nfspathId').val(); source.host = $('#nfsserverId').val(); formData.source = source; - } else { + } else if (poolType === 'iscsi') { var source = {}; source.target = $('#iscsiTargetId').val(); source.host = $('#iscsiserverId').val();

After deleting a storage pool, the page may update the pool list even if the operation has not been finished. That results in inconsistent information in the UI, like duplicate or missing entries. Also, if there is an error during this operation, nothing is displayed to the user. Make sure the storage pool list is updated only after the delete operation finishes successfully, and display an error message if something goes wrong. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/js/src/kimchi.storage_main.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ui/js/src/kimchi.storage_main.js b/ui/js/src/kimchi.storage_main.js index 169e32a..5605f4b 100644 --- a/ui/js/src/kimchi.storage_main.js +++ b/ui/js/src/kimchi.storage_main.js @@ -86,9 +86,12 @@ kimchi.storageBindClick = function() { }; kimchi.confirm(settings, function() { var poolName = $pool.data('name'); - kimchi.deleteStoragePool(poolName); - kimchi.doListStoragePools(); - }, null); + kimchi.deleteStoragePool(poolName, function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + }); }); $('.pool-activate').on('click', function(event) { -- 1.8.4.2

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/20/2014 08:09 PM, Crístian Viana wrote:
After deleting a storage pool, the page may update the pool list even if the operation has not been finished. That results in inconsistent information in the UI, like duplicate or missing entries. Also, if there is an error during this operation, nothing is displayed to the user.
Make sure the storage pool list is updated only after the delete operation finishes successfully, and display an error message if something goes wrong.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- ui/js/src/kimchi.storage_main.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/ui/js/src/kimchi.storage_main.js b/ui/js/src/kimchi.storage_main.js index 169e32a..5605f4b 100644 --- a/ui/js/src/kimchi.storage_main.js +++ b/ui/js/src/kimchi.storage_main.js @@ -86,9 +86,12 @@ kimchi.storageBindClick = function() { }; kimchi.confirm(settings, function() { var poolName = $pool.data('name'); - kimchi.deleteStoragePool(poolName); - kimchi.doListStoragePools(); - }, null); + kimchi.deleteStoragePool(poolName, function() { + kimchi.doListStoragePools(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + }); });
$('.pool-activate').on('click', function(event) {

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/20/2014 08:09 PM, Crístian Viana wrote:
The README file contains instructions to build Kimchi from the source tree, including the exact command to install the dependencies on Ubuntu. But that command does not contain the LVM package which is needed to probe LVM devices when creating a logical storage pool.
Add the LVM package ("lvm2") to the apt-get install command in the README file.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/README.md b/docs/README.md index d80486b..a1f02ee 100644 --- a/docs/README.md +++ b/docs/README.md @@ -60,7 +60,7 @@ for more information on how to configure your system to access this repository. python-pam python-m2crypto python-jsonschema \ qemu-kvm libtool python-psutil python-ethtool \ sosreport python-ipaddr python-lxml nfs-common \ - open-iscsi + open-iscsi lvm2
Packages version requirement: python-jsonschema >= 1.3.0
participants (2)
-
Aline Manera
-
Crístian Viana