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

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日 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

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.

>> >> >> 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

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"); }