
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> 2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume. Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) { options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } }, -- 1.7.1

On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) {
It is not enough to determine if a pool has or not free volumes. For each pool, you need to call: GET /storagepools/<pool>/storagevolumes?ref_count=0 If the response is a list > 0, the pool should be listed as an option, otherwise not.
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },

On 9/22/2014 10:57 PM, Aline Manera wrote:
On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) {
It is not enough to determine if a pool has or not free volumes. For each pool, you need to call:
GET /storagepools/<pool>/storagevolumes?ref_count=0
If the response is a list > 0, the pool should be listed as an option, otherwise not.
If sending a request for each pool, then how many requests will need to be sent? If there are many pools defined, this will greatly lose performance. So backend API need to be enhanced either of below: 1. Add an attribute in each pool to indicate whether there is free volumes when /storagepools. 2. Add a filter parameter to /storagepools to response pools with free volumes directly. I prefer the 2nd.
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },

On 09/23/2014 03:56 AM, Yu Xin Huo wrote:
On 9/22/2014 10:57 PM, Aline Manera wrote:
On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) {
It is not enough to determine if a pool has or not free volumes. For each pool, you need to call:
GET /storagepools/<pool>/storagevolumes?ref_count=0
If the response is a list > 0, the pool should be listed as an option, otherwise not.
If sending a request for each pool, then how many requests will need to be sent? If there are many pools defined, this will greatly lose performance.
So backend API need to be enhanced either of below: 1. Add an attribute in each pool to indicate whether there is free volumes when /storagepools. 2. Add a filter parameter to /storagepools to response pools with free volumes directly.
I prefer the 2nd.
Agree. Second option will have better performance.
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },

On 10/03/2014 10:34 AM, Aline Manera wrote:
On 09/23/2014 03:56 AM, Yu Xin Huo wrote:
On 9/22/2014 10:57 PM, Aline Manera wrote:
On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) {
It is not enough to determine if a pool has or not free volumes. For each pool, you need to call:
GET /storagepools/<pool>/storagevolumes?ref_count=0
If the response is a list > 0, the pool should be listed as an option, otherwise not.
If sending a request for each pool, then how many requests will need to be sent? If there are many pools defined, this will greatly lose performance.
So backend API need to be enhanced either of below: 1. Add an attribute in each pool to indicate whether there is free volumes when /storagepools. 2. Add a filter parameter to /storagepools to response pools with free volumes directly.
I prefer the 2nd.
Agree. Second option will have better performance.
Are we ever going to allow shared disks? I feel like we should plan for that possibility when redesigning what we show for a storage pool.
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 10/03/2014 01:34 PM, Christy Perez wrote:
On 09/23/2014 03:56 AM, Yu Xin Huo wrote:
On 9/22/2014 10:57 PM, Aline Manera wrote:
On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) { It is not enough to determine if a pool has or not free volumes. For each pool, you need to call:
GET /storagepools/<pool>/storagevolumes?ref_count=0
If the response is a list > 0, the pool should be listed as an option, otherwise not. If sending a request for each pool, then how many requests will need to be sent? If there are many pools defined, this will greatly lose performance.
So backend API need to be enhanced either of below: 1. Add an attribute in each pool to indicate whether there is free volumes when /storagepools. 2. Add a filter parameter to /storagepools to response pools with free volumes directly.
I prefer the 2nd. Agree. Second option will have better performance. Are we ever going to allow shared disks? I feel like we should plan for
On 10/03/2014 10:34 AM, Aline Manera wrote: that possibility when redesigning what we show for a storage pool.
Christy, do you mean from a UI perspective or API? Anyway, I agree when listing the storage volumes we need to point if it is shareable or not
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 10/03/2014 01:34 PM, Aline Manera wrote:
On 10/03/2014 01:34 PM, Christy Perez wrote:
On 09/23/2014 03:56 AM, Yu Xin Huo wrote:
On 9/22/2014 10:57 PM, Aline Manera wrote:
On 09/22/2014 04:14 AM, huoyuxin@linux.vnet.ibm.com wrote:
From: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com>
2 Issues: 1. In menu widget, it handle the passin params incorrectly. 2. Filter out storage pools without any available volume.
Signed-off-by: Yu Xin Huo <huoyuxin@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- ui/js/widgets/select-menu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index a8c5acb..6e01d3e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -61,7 +61,7 @@ kimchi.guest_storage_add_main = function() { var options = []; if (result && result.length) { $.each(result, function(index, storagePool) { - if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) { + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso') && storagePool.nr_volumes) { It is not enough to determine if a pool has or not free volumes. For each pool, you need to call:
GET /storagepools/<pool>/storagevolumes?ref_count=0
If the response is a list > 0, the pool should be listed as an option, otherwise not. If sending a request for each pool, then how many requests will need to be sent? If there are many pools defined, this will greatly lose performance.
So backend API need to be enhanced either of below: 1. Add an attribute in each pool to indicate whether there is free volumes when /storagepools. 2. Add a filter parameter to /storagepools to response pools with free volumes directly.
I prefer the 2nd. Agree. Second option will have better performance. Are we ever going to allow shared disks? I feel like we should plan for
On 10/03/2014 10:34 AM, Aline Manera wrote: that possibility when redesigning what we show for a storage pool.
Christy, do you mean from a UI perspective or API? Anyway, I agree when listing the storage volumes we need to point if it is shareable or not
I suppose I mean API, which really would end up affecting API and UI.
options.push({ label: storagePool.name, value: storagePool.name diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js index ad53200..c4b0209 100644 --- a/ui/js/widgets/select-menu.js +++ b/ui/js/widgets/select-menu.js @@ -36,8 +36,10 @@ var selectedClass = 'active'; var itemTag = 'li'; var item; + that.listControl.find('li').remove(); + that.label.text(""); + that.target.val(""); if (options.length > 0) { - that.listControl.find('li').remove(); $.each(options, function(index, option) { item = $('<' + itemTag + '>' + option.label +'</' + itemTag + '>'); item.data('value', option.value); @@ -58,8 +60,6 @@ that.target.change(); } }); - } else { - kimchi.message.error.code('KCHAPI6006E'); } },
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (4)
-
Aline Manera
-
Christy Perez
-
huoyuxin@linux.vnet.ibm.com
-
Yu Xin Huo