[Kimchi-devel] [PATCHv2 2/3] UI: Support add guest disk
Royce Lv
lvroyce at linux.vnet.ibm.com
Wed Apr 23 09:03:49 UTC 2014
Thanks a lot for your careful review, hongliang!
On 2014年04月23日 16:46, Hongliang Wang wrote:
> User level comments:
> 1. Primary disk is allowed to be detached
> I think we should prevent user to detach the primary disk. There
> should not a "Detach" Button after the primary disk.
This is a great idea, but I went through libvirt api, I can't figure out
a way to distinguish system disk or the boot disk, anyone who has good
idea is welcome.
> 2. Two minor issues in UI
> 2.1. Be consistent to provider default selection for select menus
>
> We have default values for device type and device bus, but not storage
> pool or storage volume. Suggest to automatically select the first
> option for them.
ACK
>
>
>
> 2.2. Keep select menu text aligned
ACK
>
> Seems there are two base lines for select menus.
>
>
>
> Code level comments are inline below.
>
> On 04/22/2014 05:05 PM, lvroyce at linux.vnet.ibm.com wrote:
>> From: Royce Lv<lvroyce at linux.vnet.ibm.com>
>>
>> Extend current cdrom disk add to cdrom disk attachment.
>> Also add 'bus' for user to choose from.
>>
>> Signed-off-by: Royce Lv<lvroyce at linux.vnet.ibm.com>
>> ---
>> ui/css/theme-default/guest-storage-add.css | 16 +++++
>> ui/js/src/kimchi.guest_storage_add.main.js | 110 +++++++++++++++++++++++++++--
>> ui/pages/guest-storage-add.html.tmpl | 50 ++++++++++++-
>> 3 files changed, 168 insertions(+), 8 deletions(-)
>>
>> diff --git a/ui/css/theme-default/guest-storage-add.css b/ui/css/theme-default/guest-storage-add.css
>> index 4da8389..e0be9c0 100644
>> --- a/ui/css/theme-default/guest-storage-add.css
>> +++ b/ui/css/theme-default/guest-storage-add.css
>> @@ -52,6 +52,22 @@
>> cursor: not-allowed;
>> }
>>
>> +.guest-storage-add-wrapper-label, .guest-storage-add-wrapper-controls {
>> + display: inline-block;
>> +}
>> +
>> +.guest-storage-add-wrapper-label {
>> + height: 38px;
>> + line-height: 38px;
>> + margin-top: 5px;
>> + vertical-align: top;
>> + width: 80px;
>> +}
>> +
>> +.guest-storage-add-wrapper-controls {
>> + width: 470px;
>> +}
>> +
>> #vm-storage-button-add[disabled] {
>> background: #c0c0c0;
>> color: #ddd;
>> diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js
>> index 9e232d7..1f04e55 100644
>> --- a/ui/js/src/kimchi.guest_storage_add.main.js
>> +++ b/ui/js/src/kimchi.guest_storage_add.main.js
>> @@ -18,25 +18,123 @@
>> kimchi.guest_storage_add_main = function() {
>> var types = [{
>> label: 'cdrom',
>> - value: 'cdrom'
>> + value: 'cdrom',
>> + bus: ['ide']
>> + },
>> + {
>> + label: 'disk',
>> + value: 'disk',
>> + bus: ['virtio', 'ide']
>> }];
>> kimchi.select('guest-storage-type-list', types);
>> + kimchi.select('guest-storage-bus-list', [{label: 'ide', value: 'ide'}]);
>>
>> var storageAddForm = $('#form-guest-storage-add');
>> var submitButton = $('#guest-storage-button-add');
>> var nameTextbox = $('input[name="dev"]', storageAddForm);
>> var typeTextbox = $('input[name="type"]', storageAddForm);
>> + var busTextbox = $('input[name="bus"]', storageAddForm);
>> var pathTextbox = $('input[name="path"]', storageAddForm);
>> + var poolTextbox = $('input[name="storagepool"]', storageAddForm);
>> + var volTextbox = $('input[name="storagevol"]', storageAddForm);
>> +
>> + typeTextbox.change(function() {
>> + $('#guest-storage-bus').selectMenu();
>> + var pathObject = {'cdrom': ".path-section", 'disk': '.volume-section'}
>> + var selectType = $(this).val();
>> + $.each(pathObject, function(type, value) {
>> + if(selectType == type){
>> + $(value).removeClass('hidden');
>> + } else {
>> + $(value).addClass('hidden');
>> + }
>> + });
>> +
>> + $.each(types, function(index, elem){
>> + if (selectType == elem.value) {
>> + var buses = new Array();
>> + $.each(elem.bus, function (index, elem) {
>> + buses[index] = {label: elem, value: elem};
>> + });
>> + $('#guest-storage-bus').selectMenu("setData", buses);
>
>> + $('#guest-storage-bus-label').text(buses[0].value);
>> + $('#guest-storage-bus-type').val(buses[0].value);
> Suggest keep consistent on variable naming. e.g.
> Text box ID: #*guest-storage-bus-type*-textbox
> Label ID: #*guest-storage-bus-type*-label
ACK
>> + }
>> + });
>> + });
>> +
>> + kimchi.listStoragePools(function(result) {
>> + var options = [];
>> + if (result && result.length) {
>> + $.each(result, function(index, storagePool) {
>
>> + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) {
>> + options.push({
>> + label: storagePool.name,
>> + value: storagePool.name
>> + });
>> + }
> Seems indent is wrong here. Online code formatting tools can be used.
ACK
>> + });
>> + kimchi.select('guest-add-storage-pool-list', options);
>> + }
>> + });
>> +
>> + poolTextbox.change(function() {
>> + var options = [];
>> + kimchi.listStorageVolumes($(this).val(), function(result) {
>> + $('#guest-disk').selectMenu();
>> + if (result.length) {
>> + $.each(result, function(index, value) {
>> + // Only unused volume can be attached
>> + if (value.ref_cnt == 0) {
>> + options.push({
>> + label: value.name,
>> + value: value.path
>> + });
>> + }
>> + });
>> + }
>> + $('#guest-disk').selectMenu("setData", options);
>> + });
>> + });
>> +
>> + volTextbox.change(function () {
>> + pathTextbox.val(volTextbox.val());
>> + pathTextbox.change();
>> + });
>> +
>> + typeTextbox.change(function() {
>> + var pathObject = {'cdrom': ".path-section", 'disk': '.volume-section'}
>> + var selectType = $(this).val();
>> + $.each(pathObject, function(type, value) {
>> + if(selectType == type){
>> + $(value).removeClass('hidden');
>> + } else {
>> + $(value).addClass('hidden');
>> + }
>> + });
>> +
>> + $.each(types, function(index, elem){
>> + if (selectType == elem.value) {
>> + var buses = new Array();
>> + $.each(elem.bus, function (index, elem) {
>> + buses[index] = {label: elem, value: elem};
>> + });
>> + $('#guest-storage-bus').selectMenu("setData", buses);
>> + $('#guest-storage-bus-label').text(buses[0].value);
>> + }
>> + });
>> + });
>>
>> var submitForm = function(event) {
>> - if(submitButton.prop('disabled')) {
>> + if (submitButton.prop('disabled')) {
>> return false;
>> }
>>
>> var dev = nameTextbox.val();
>> var type = typeTextbox.val();
>> var path = pathTextbox.val();
>> - if(!path || path === '') {
>> + var bus = busTextbox.val();
>> + if (!path || path === '') {
>> return false;
>> }
>>
>> @@ -49,13 +147,13 @@ kimchi.guest_storage_add_main = function() {
>> var settings = {
>> vm: kimchi.selectedGuest,
>> type: type,
>> - path: path
>> + path: path,
>> + bus: bus
>> };
>>
>> if(dev && dev !== '') {
>> settings['dev'] = dev;
>> }
>> -
>> kimchi.addVMStorage(settings, function(result) {
>> kimchi.window.close();
>> kimchi.topic('kimchi/vmCDROMAttached').publish({
>> @@ -77,7 +175,7 @@ kimchi.guest_storage_add_main = function() {
>>
>> storageAddForm.on('submit', submitForm);
>> submitButton.on('click', submitForm);
>> - pathTextbox.on('input propertychange', function(event) {
>> + pathTextbox.on('change input propertychange', function(event) {
>> $(submitButton).prop('disabled', $(this).val() === '');
>> });
>> };
>> diff --git a/ui/pages/guest-storage-add.html.tmpl b/ui/pages/guest-storage-add.html.tmpl
>> index 71e0610..3f92c83 100644
>> --- a/ui/pages/guest-storage-add.html.tmpl
>> +++ b/ui/pages/guest-storage-add.html.tmpl
>> @@ -41,7 +41,7 @@
>> <h2>2. $_("Device Type")</h2>
>> <div class="field">
>> <p class="text-help">
>> - $_("The device type. Currently, only \"cdrom\" is supported.")
>> + $_("The device type. Currently, \"cdrom\" and \"disk\" are supported.")
>> </p>
>> <div class="btn dropdown popable">
>> <input id="guest-storage-type" name="type" value="cdrom" type="hidden" />
>> @@ -54,7 +54,51 @@
>> </div>
>> </section>
>> <section class="form-section">
>> - <h2>3. $_("File Path")</h2>
>> + <h2>3. $_("Device Bus")</h2>
>> + <div class="field">
>> + <div class="btn dropdown popable" id="guest-storage-bus">
>> + <input id="guest-storage-bus-type" name="bus" value='ide' type="hidden" />
>> + <span class="text" id="guest-storage-bus-label">ide</span>
>> + <span class="arrow"></span>
>> + <div class="popover">
>> + <ul class="select-list" id="guest-storage-bus-list" data-target="guest-storage-bus-type" data-label="guest-storage-bus-label"></ul>
>> + </div>
>> + </div>
>> + </div>
>> + </section>
>> + <div class="volume-section hidden">
>> + <section class="form-section">
>> + <h2>4. $_("Storage Pool")</h2>
>> + <div class="field storage-field">
>> + <p class="text-help">
>> + $_("Storage pool which volume located in")</p>
>> + <div class="btn dropdown popable">
>> + <input value="/storagepools/vg" id="guest-disk-pool" name="storagepool" type="hidden">
>> + <span class="text" id="guest-disk-pool-label"></span><span class="arrow"></span>
>> + <div class="popover" style="width: 100%">
>> + <ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-pool" data-label="guest-disk-pool-label"></ul>
>> + </div>
>> + </div>
>> + </div>
>> + </section>
>> + <section class="form-section">
>> + <h2>5. $_("Storage Volume")</h2>
>> + <div class="field storage-field">
>> + <p class="text-help">
>> + $_("Storage volume to be attached")</p>
>> + <div class="btn dropdown popable" id="guest-disk">
>> + <input id="guest-disk-vol" name="storagevol" type="hidden">
>> + <span class="text" id="guest-disk-vol-label"></span><span class="arrow"></span>
>> + <div class="popover" style="width: 100%">
>> + <ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-vol" data-label="guest-disk-vol-label"></ul>
>> + </div>
>> + </div>
>> + </div>
>> + </section>
>> + </div>
>> + <div class="path-section">
>> + <section class="form-section">
>> + <h2>4. $_("File Path")</h2>
>> <div class="field">
>> <p class="text-help">
>> $_("The ISO file path in the server for CDROM.")
>> @@ -62,6 +106,8 @@
>> <input type="text" class="text" name="path" />
>> </div>
>> </section>
>> + </div>
>> + </fieldset>
>> </form>
>> </div>
>> <footer>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/070403b6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 33395 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/070403b6/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 27201 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/070403b6/attachment-0001.png>
More information about the Kimchi-devel
mailing list