From: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; Enable users add and detach disk when vm is running. Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; --- 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"); }

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.

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.

On 10/16/2014 09:07 AM, Wen Wang wrote: > From: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; > > Enable users add and detach disk when vm is running. > > Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; > --- > 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(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

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)

> > 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. > > > On 10/16/2014 09:07 AM, Wen Wang wrote: >> From: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; >> >> Enable users add and detach disk when vm is running. >> >> Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; >> --- >> 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(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

On 2014年10月22日 14:51, 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) According to our xchat log: Mar 19 21:07:56 <royce> #info royce finished scsi interface cdrom change, patch reviewed, but aline reported error on 12.10 ubuntu, tested it worked on 13.04 and 13.10, will test on others: So do we want to move to scsi cdrom instead now?

>> >> 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. >> >> >> On 10/16/2014 09:07 AM, Wen Wang wrote: >>> From: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; >>> >>> Enable users add and detach disk when vm is running. >>> >>> Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; >>> --- >>> 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(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

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. 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. On 10/16/2014 09:07 AM, Wen Wang wrote: > From: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; > > Enable users add and detach disk when vm is running. > > Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com&gt; > --- > 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"); > } >