<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    User level comments:<br>
    1. Primary disk is allowed to be detached<br>
    I think we should prevent user to detach the primary disk. There
    should not a "Detach" Button after the primary disk.<br>
    2. Two minor issues in UI<br>
    2.1. Be consistent to provider default selection for select menus<br>
    <br>
    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.<br>
    <br>
    <img alt="" src="cid:part1.08040905.07090501@linux.vnet.ibm.com"
      height="476" width="662"><br>
    <br>
    2.2. Keep select menu text aligned<br>
    <br>
    Seems there are two base lines for select menus.<br>
    <br>
    <img alt="" src="cid:part2.01000900.08080601@linux.vnet.ibm.com"
      height="487" width="482"><br>
    <br>
    Code level comments are inline below.<br>
    <br>
    <div class="moz-cite-prefix">On 04/22/2014 05:05 PM,
      <a class="moz-txt-link-abbreviated" href="mailto:lvroyce@linux.vnet.ibm.com">lvroyce@linux.vnet.ibm.com</a> wrote:<br>
    </div>
    <blockquote
      cite="mid:1398157542-3538-3-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">From: Royce Lv <a class="moz-txt-link-rfc2396E" href="mailto:lvroyce@linux.vnet.ibm.com">&lt;lvroyce@linux.vnet.ibm.com&gt;</a>

Extend current cdrom disk add to cdrom disk attachment.
Also add 'bus' for user to choose from.

Signed-off-by: Royce Lv <a class="moz-txt-link-rfc2396E" href="mailto:lvroyce@linux.vnet.ibm.com">&lt;lvroyce@linux.vnet.ibm.com&gt;</a>
---
 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);</pre>
    </blockquote>
    <br>
    <blockquote
      cite="mid:1398157542-3538-3-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+                $('#guest-storage-bus-label').text(buses[0].value);
+                $('#guest-storage-bus-type').val(buses[0].value);</pre>
    </blockquote>
    Suggest keep consistent on variable naming. e.g.<br>
    Text box ID:   #<b>guest-storage-bus-type</b>-textbox<br>
    Label ID:        #<b>guest-storage-bus-type</b>-label<br>
    <blockquote
      cite="mid:1398157542-3538-3-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+            }
+        });
+    });
+
+    kimchi.listStoragePools(function(result) {
+        var options = [];
+        if (result &amp;&amp; result.length) {
+            $.each(result, function(index, storagePool) {</pre>
    </blockquote>
    <br>
    <blockquote
      cite="mid:1398157542-3538-3-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+                if ((storagePool.state=="active") &amp;&amp; (storagePool.type !== 'kimchi-iso')) {
+                    options.push({
+                        label: storagePool.name,
+                        value: storagePool.name
+                        });
+                    }</pre>
    </blockquote>
    Seems indent is wrong here. Online code formatting tools can be
    used.<br>
    <blockquote
      cite="mid:1398157542-3538-3-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+                });
+                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 &amp;&amp; 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 @@
                 &lt;h2&gt;2. $_("Device Type")&lt;/h2&gt;
                 &lt;div class="field"&gt;
                     &lt;p class="text-help"&gt;
-                        $_("The device type. Currently, only \"cdrom\" is supported.")
+                        $_("The device type. Currently,  \"cdrom\" and \"disk\" are supported.")
                     &lt;/p&gt;
                     &lt;div class="btn dropdown popable"&gt;
                         &lt;input id="guest-storage-type" name="type" value="cdrom" type="hidden" /&gt;
@@ -54,7 +54,51 @@
                 &lt;/div&gt;
             &lt;/section&gt;
             &lt;section class="form-section"&gt;
-                &lt;h2&gt;3. $_("File Path")&lt;/h2&gt;
+                &lt;h2&gt;3. $_("Device Bus")&lt;/h2&gt;
+                &lt;div class="field"&gt;
+                    &lt;div class="btn dropdown popable" id="guest-storage-bus"&gt;
+                        &lt;input id="guest-storage-bus-type" name="bus" value='ide' type="hidden" /&gt;
+                        &lt;span class="text" id="guest-storage-bus-label"&gt;ide&lt;/span&gt;
+                        &lt;span class="arrow"&gt;&lt;/span&gt;
+                        &lt;div class="popover"&gt;
+                            &lt;ul class="select-list" id="guest-storage-bus-list" data-target="guest-storage-bus-type" data-label="guest-storage-bus-label"&gt;&lt;/ul&gt;
+                        &lt;/div&gt;
+                    &lt;/div&gt;
+                &lt;/div&gt;
+            &lt;/section&gt;
+                &lt;div class="volume-section hidden"&gt;
+                    &lt;section class="form-section"&gt;
+                        &lt;h2&gt;4. $_("Storage Pool")&lt;/h2&gt;
+                        &lt;div class="field storage-field"&gt;
+                            &lt;p class="text-help"&gt;
+                                $_("Storage pool which volume located in")&lt;/p&gt;
+                                &lt;div class="btn dropdown popable"&gt;
+                                    &lt;input value="/storagepools/vg" id="guest-disk-pool" name="storagepool" type="hidden"&gt;
+                                        &lt;span class="text" id="guest-disk-pool-label"&gt;&lt;/span&gt;&lt;span class="arrow"&gt;&lt;/span&gt;
+                                            &lt;div class="popover" style="width: 100%"&gt;
+                                                &lt;ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-pool" data-label="guest-disk-pool-label"&gt;&lt;/ul&gt;
+                                             &lt;/div&gt;
+                                &lt;/div&gt;
+                        &lt;/div&gt;
+                    &lt;/section&gt;
+                    &lt;section class="form-section"&gt;
+                        &lt;h2&gt;5. $_("Storage Volume")&lt;/h2&gt;
+                        &lt;div class="field storage-field"&gt;
+                            &lt;p class="text-help"&gt;
+                                $_("Storage volume to be attached")&lt;/p&gt;
+                                &lt;div class="btn dropdown popable" id="guest-disk"&gt;
+                                    &lt;input id="guest-disk-vol" name="storagevol" type="hidden"&gt;
+                                        &lt;span class="text" id="guest-disk-vol-label"&gt;&lt;/span&gt;&lt;span class="arrow"&gt;&lt;/span&gt;
+                                            &lt;div class="popover" style="width: 100%"&gt;
+                                                &lt;ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-vol" data-label="guest-disk-vol-label"&gt;&lt;/ul&gt;
+                                             &lt;/div&gt;
+                                &lt;/div&gt;
+                        &lt;/div&gt;
+                    &lt;/section&gt;
+                &lt;/div&gt;
+                &lt;div class="path-section"&gt;
+                &lt;section class="form-section"&gt;
+                &lt;h2&gt;4. $_("File Path")&lt;/h2&gt;
                 &lt;div class="field"&gt;
                     &lt;p class="text-help"&gt;
                         $_("The ISO file path in the server for CDROM.")
@@ -62,6 +106,8 @@
                     &lt;input type="text" class="text" name="path" /&gt;
                 &lt;/div&gt;
             &lt;/section&gt;
+            &lt;/div&gt;
+                &lt;/fieldset&gt;
         &lt;/form&gt;
     &lt;/div&gt;
     &lt;footer&gt;
</pre>
    </blockquote>
    <br>
  </body>
</html>