On 09/22/2014 04:14 AM, huoyuxin(a)linux.vnet.ibm.com wrote: > From: Yu Xin Huo <huoyuxin(a)linux.vnet.ibm.com&gt; > > 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(a)linux.vnet.ibm.com&gt; > --- > 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 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(a)linux.vnet.ibm.com wrote: >>> From: Yu Xin Huo <huoyuxin(a)linux.vnet.ibm.com&gt; >>> >>> 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(a)linux.vnet.ibm.com&gt; >>> --- >>> 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'); >>> } >>> }, >>> >> > > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 10/03/2014 01:34 PM, Christy Perez wrote: > > 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(a)linux.vnet.ibm.com wrote: >>>>> From: Yu Xin Huo <huoyuxin(a)linux.vnet.ibm.com&gt; >>>>> >>>>> 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(a)linux.vnet.ibm.com&gt; >>>>> --- >>>>> 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. 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(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >