[Kimchi-devel] [PATCH] Guest disk hot plug UI

[Christy Perez]

On 10/22/2014 07:04 AM, Aline Manera wrote:
> 
> On 10/22/2014 04:51 AM, Royce Lv wrote:
>> On 2014年10月22日 01:27, Aline Manera wrote:
>>>
>>> Disk hot plug is supported for virtio and SCSI bus and we use IDE bus
>>> for CDROM devices.
>>> So with this patch set, the user can try to add a new CDROM but it
>>> will always fail as it is IDE bus.
>> Yes, as I can recall I tested SCSI bus for cdrom before, but under
>> some distros, SCSI bus for cdrom seems have problem.
>> (I will test again and update reason here)
> 
> Yeap. I remember that.
> Maybe we could double check to see if there was some update on libvirt
> packages that had problems.
> 
>>>
>>> We have 2 options to improve user experience in this case:
>>>
>>> 1) Do not allow CDROM hot plug, ie, remove "CDROM" option from combo
>>> box when VM is running.
>>>
>>> 2) Modify backend to allow user specifies a new CDROM when VM is
>>> running but it will be only available on next reboot.
>> We have considered option 2, but with concern of the difference
>> between definition of xml and the running vm,
>> especially when you edit the xml for several times(added several
>> delayed effective devices, and want to delete them afterwards),
>> what views to show user is a problem, the running one or the defined one.
>>
>> For example:
>> User has a vm running with a cdrom A, then he wants to add cdrom B
>> Do we show users cdrom B before next reboot?
>> 1. If we show, cdrom B exists in our definition but not in guest
>> OS--Unless backend distinguish what devices come from definition and
>> what are from running vm.
>> If user want to delete cdrom B before next reboot, we need to know it
>> is deleted from definition to prevent error.
>> 2. If we don't show, as virt manager did, user may have no chance to
>> delete B before next reboot.
>> We also need to consider user add same cdrom twice behavior if we
>> don't show.
>>
>> so we finally chose to reject all device adding in running time when
>> it is IDE for now at backend.
> 
> IMO we could allow it takes effect after reboot and show it to user with
> a warning icon (and on mouse over we display "It will be added on next
> reboot")
> With this warning icon, the user will not be able to edit/remove the
> device as it is does not exist in fact.
> 
Also, IIRC, virt-manager lets you make changes in this way.

>>>
>>>
>>> On 10/16/2014 09:07 AM, Wen Wang wrote:
>>>> From: Wen Wang <wenwang at linux.vnet.ibm.com>
>>>>
>>>> Enable users add and detach disk when vm is running.
>>>>
>>>> Signed-off-by: Wen Wang <wenwang at linux.vnet.ibm.com>
>>>> ---
>>>> ui/js/src/kimchi.guest_edit_main.js | 29 ++++++++++++-----------------
>>>> 1 files changed, 12 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/ui/js/src/kimchi.guest_edit_main.js
>>>> b/ui/js/src/kimchi.guest_edit_main.js
>>>> index 030e112..eb0254e 100644
>>>> --- a/ui/js/src/kimchi.guest_edit_main.js
>>>> +++ b/ui/js/src/kimchi.guest_edit_main.js
>>>> @@ -57,17 +57,14 @@ kimchi.guest_edit_main = function() {
>>>> text: false
>>>> });
>>>>
>>>> - if(kimchi.thisVMState != "running") {
>>>> - $('.detach', container).button({
>>>> - icons: {
>>>> - primary: 'ui-icon-trash'
>>>> - },
>>>> - text: false
>>>> - });
>>>> + $('.detach', container).button({
>>>> + icons: {
>>>> + primary: 'ui-icon-trash'
>>>> + },
>>>> + text: false
>>>> + });
>>>>
>>>> - } else {
>>>> - $('.detach', container).remove();
>>>> - }
>>>> + $('.view .detach', container).remove();
>>>>
>>>> $('.save', container).button({
>>>> icons: {
>>>> @@ -443,14 +440,8 @@ kimchi.guest_edit_main = function() {
>>>> guest['icon'] = guest['icon'] || 'images/icon-vm.png';
>>>> $('#form-guest-edit-general').fillWithObject(guest);
>>>> kimchi.thisVMState = guest['state'];
>>>> -
>>>> refreshCDROMs();
>>>> - if(kimchi.thisVMState === "running") {
>>>> - $("#form-guest-edit-general input").prop("disabled", "disabled");
>>>> - $("#guest-edit-attach-cdrom-button").remove();
>>>> - $("#form-guest-edit-interface .header button").remove();
>>>> - } else {
>>>> - $('#guest-edit-attach-cdrom-button').button({
>>>> + $('#guest-edit-attach-cdrom-button').button({
>>>> icons: {
>>>> primary: "ui-icon-plusthick"
>>>> },
>>>> @@ -459,6 +450,10 @@ kimchi.guest_edit_main = function() {
>>>> event.preventDefault();
>>>> kimchi.window.open("guest-storage-add.html");
>>>> });
>>>> + if(kimchi.thisVMState === "running") {
>>>> + $("#form-guest-edit-general input").prop("disabled", "disabled");
>>>> + $("#form-guest-edit-interface .header button").remove();
>>>> + } else {
>>>> $("#action-button-container").removeClass("hidden");
>>>> }
>>>>
>>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
>>
> 
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel