Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 04/10/2014 06:57 AM, shaohef@linux.vnet.ibm.com wrote:

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>

"Stop" is obscure for user.

Currently, stopping guests in Kimchi is not graceful in the sense that the guest becomes 'shut down' immediately. This could be be problematic because guest file systems could become corrupted. Perhaps a warning should be added before a guest is immediately stopped or reset.

So "power off" is more exact than "stop" in literal meaning.

The aciton power off a VM forcefully is same that we cut the physical machine power. This action may produce undesirable results, for example unflushed disk cache in the guest.

update API.md, backend source code and test case code.

Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- docs/API.md | 3 ++- src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 +- src/kimchi/mockmodel.py | 2 +- src/kimchi/model/vms.py | 4 ++-- tests/test_model.py | 12 ++++++------ tests/test_rest.py | 12 ++++++------ 7 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/docs/API.md b/docs/API.md index 630015e..143c70c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -102,7 +102,8 @@ the following general conventions: **Actions (POST):**

* start: Power on a VM -* stop: Power off forcefully +* poweroff: Power off a VM forcefully. Note this action may produce undesirable + results, for example unflushed disk cache in the guest. * connect: Prepare the connection for spice or vnc

### Sub-resource: Virtual Machine Screenshot diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 81ad3a6..e75b27f 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -38,7 +38,7 @@ class VM(Resource): for ident, node in sub_nodes.items(): setattr(self, ident, node(model, self.ident)) self.start = self.generate_action_handler('start') - self.stop = self.generate_action_handler('stop') + self.poweroff = self.generate_action_handler('poweroff') self.connect = self.generate_action_handler('connect')

@property diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 0eb721a..7cf0970 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -76,7 +76,7 @@ messages = { "KCHVM0015E": _("Graphics address to listen on must be IPv4 or IPv6"), "KCHVM0016E": _("Specify a template to create a virtual machine from"), "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), - "KCHVM0020E": _("Unable to stop virtual machine %(name)s. Details: %(err)s"), + "KCHVM0020E": _("Unable to poweroff virtual machine %(name)s. Details: %(err)s"), "KCHVM0021E": _("Unable to delete virtual machine %(name)s. Details: %(err)s"),

"KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index dbdd57e..98fb080 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -129,7 +129,7 @@ class MockModel(object): def vm_start(self, name): self._get_vm(name).info['state'] = 'running'

- def vm_stop(self, name): + def vm_poweroff(self, name): self._get_vm(name).info['state'] = 'shutoff'

def vm_connect(self, name): diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index c8dfc90..8334d9e 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -335,7 +335,7 @@ class VMModel(object): info = self.lookup(name)

if info['state'] == 'running': - self.stop(name) + self.poweroff(name)

try: dom.undefine() @@ -372,7 +372,7 @@ class VMModel(object): raise OperationFailed("KCHVM0019E", {'name': name, 'err': e.get_error_message()})

- def stop(self, name): + def poweroff(self, name): dom = self.get_vm(name, self.conn) try: dom.destroy() diff --git a/tests/test_model.py b/tests/test_model.py index b8e6d47..3041196 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -91,7 +91,7 @@ class ModelTests(unittest.TestCase): self.assertTrue('kimchi-vm' in vms)

inst.vm_start('kimchi-vm') - rollback.prependDefer(inst.vm_stop, 'kimchi-vm') + rollback.prependDefer(inst.vm_poweroff, 'kimchi-vm')

info = inst.vm_lookup('kimchi-vm') self.assertEquals('running', info['state']) @@ -222,7 +222,7 @@ class ModelTests(unittest.TestCase): inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) self.assertEquals(iso_path, cdrom_info['path']) - inst.vm_stop(vm_name) + inst.vm_poweroff(vm_name)

# removing non existent cdrom self.assertRaises(NotFoundError, inst.vmstorage_delete, vm_name, @@ -587,7 +587,7 @@ class ModelTests(unittest.TestCase): self.assertTrue('kimchi-vm1' in vms)

inst.vm_start('kimchi-vm1') - rollback.prependDefer(self._rollback_wrapper, inst.vm_stop, + rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff, 'kimchi-vm1')

info = inst.vm_lookup('kimchi-vm1') @@ -597,7 +597,7 @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidParameter, inst.vm_update, 'kimchi-vm1', params)

- inst.vm_stop('kimchi-vm1') + inst.vm_poweroff('kimchi-vm1') params = {'name': u'пeω-∨м'} self.assertRaises(OperationFailed, inst.vm_update, 'kimchi-vm1', {'name': 'kimchi-vm2'}) @@ -795,7 +795,7 @@ class ModelTests(unittest.TestCase): self.assertEquals('failed', inst.task_lookup(taskid)['status'])

# This wrapper function is needed due to the new backend messaging in - # vm model. vm_stop and vm_delete raise exception if vm is not found. + # vm model. vm_poweroff and vm_delete raise exception if vm is not found. # These functions are called after vm has been deleted if test finishes # correctly, then NofFoundError exception is raised and rollback breaks def _rollback_wrapper(self, func, vmname): @@ -820,7 +820,7 @@ class ModelTests(unittest.TestCase): u'kīмсhī-∨м')

inst.vm_start(u'kīмсhī-∨м') - rollback.prependDefer(self._rollback_wrapper, inst.vm_stop, + rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff, u'kīмсhī-∨м')

inst.vm_delete(u'kīмсhī-∨м') diff --git a/tests/test_rest.py b/tests/test_rest.py index 8f4573e..cd40d55 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -201,7 +201,7 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(400, resp.status)

- resp = self.request('/vms/vm-1/stop', '{}', 'POST') + resp = self.request('/vms/vm-1/poweroff', '{}', 'POST') self.assertEquals(200, resp.status)

req = json.dumps({'name': 12}) @@ -257,8 +257,8 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) self.assertTrue(resp.getheader('Content-type').startswith('image'))

- # Force stop the VM - resp = self.request('/vms/test-vm/stop', '{}', 'POST') + # Force poweroff the VM + resp = self.request('/vms/test-vm/poweroff', '{}', 'POST') vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('shutoff', vm['state'])

@@ -572,8 +572,8 @@ class RestTests(unittest.TestCase): vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('running', vm['state'])

- # Force stop the VM - resp = self.request('/vms/test-vm/stop', '{}', 'POST') + # Force poweroff the VM + resp = self.request('/vms/test-vm/poweroff', '{}', 'POST') vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('shutoff', vm['state'])

@@ -1222,7 +1222,7 @@ class RestTests(unittest.TestCase): self.assertEquals(405, resp.status)

# No screenshot after stopped the VM - self.request('/vms/test-vm/stop', '{}', 'POST') + self.request('/vms/test-vm/poweroff', '{}', 'POST') resp = self.request('/vms/test-vm/screenshot') self.assertEquals(404, resp.status)

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 04/10/2014 06:57 AM, shaohef@linux.vnet.ibm.com wrote:

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>

"Stop" is obscure for user. "power off" is more exact than "stop" in literal meaning.

Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- ui/js/src/kimchi.api.js | 6 +++--- ui/js/src/kimchi.guest_main.js | 6 +++--- ui/pages/guest.html.tmpl | 4 ++-- ui/pages/help/guests.dita | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js index 0c5c790..84405b3 100644 --- a/ui/js/src/kimchi.api.js +++ b/ui/js/src/kimchi.api.js @@ -208,9 +208,9 @@ var kimchi = { }); },

- stopVM : function(vm, suc, err) { + poweroffVM : function(vm, suc, err) { kimchi.requestJSON({ - url : kimchi.url + 'vms/' + encodeURIComponent(vm) + '/stop', + url : kimchi.url + 'vms/' + encodeURIComponent(vm) + '/poweroff', type : 'POST', contentType : 'application/json', dataType : 'json', @@ -221,7 +221,7 @@ var kimchi = {

resetVM : function(vm, suc, err) { kimchi.requestJSON({ - url : kimchi.url + 'vms/' + encodeURIComponent(vm) + '/stop', + url : kimchi.url + 'vms/' + encodeURIComponent(vm) + '/poweroff', type : 'POST', contentType : 'application/json', dataType : 'json', diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 8a20d07..58e348b 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -37,13 +37,13 @@ kimchi.vmstart = function(event) { } };

-kimchi.vmstop = function(event) { +kimchi.vmpoweroff = function(event) { var button=$(this); if (!button.hasClass('loading')) { button.addClass('loading'); var vm=button.closest('li[name=guest]'); var vm_id=vm.attr("id"); - kimchi.stopVM(vm_id, function(result) { + kimchi.poweroffVM(vm_id, function(result) { button.removeClass('loading'); kimchi.listVmsAuto(); }, function(err) { @@ -248,7 +248,7 @@ kimchi.createGuestLi = function(vmObject, prevScreenImage, openMenu) {

//Setup action event handlers guestActions.find("[name=vm-start]").on({click : kimchi.vmstart}); - guestActions.find("[name=vm-stop]").on({click : kimchi.vmstop}); + guestActions.find("[name=vm-poweroff]").on({click : kimchi.vmpoweroff}); if (vmRunningBool) { //If the guest is not running, do not enable reset guestActions.find("[name=vm-reset]").on({click : kimchi.vmreset}); } diff --git a/ui/pages/guest.html.tmpl b/ui/pages/guest.html.tmpl index f8edf11..588b7b0 100644 --- a/ui/pages/guest.html.tmpl +++ b/ui/pages/guest.html.tmpl @@ -49,7 +49,7 @@ <div class="top"> <a class="btn shutoff-disabled" name="vm-reset" href="javascript:void(0);" title="$_("Reset")"><span class="icon reset"></span></a> <a class="btn running-hidden" name="vm-start" href="javascript:void(0);" title="$_("Start")"><span class="icon power-down"></span></a> - <a class="btn shutoff-hidden" name="vm-stop" href="javascript:void(0);" title="$_("Stop")"><span class="icon power-up"></span></a> + <a class="btn shutoff-hidden" name="vm-poweroff" href="javascript:void(0);" title="$_("Power Off")"><span class="icon power-up"></span></a> </div> <div class="bottom"> <div name="actionmenu" class="btn dropdown popable vm-action" style="width: 70px"> @@ -60,7 +60,7 @@ <button class="button-big running-disabled" name="vm-edit"><span class="text">$_("Edit")</span></button> <button class="button-big shutoff-hidden" name="vm-reset"><span class="text">$_("Reset")</span></button> <button class="button-big running-hidden" name="vm-start"><span class="text">$_("Start")</span></button> - <button class="button-big shutoff-hidden" name="vm-stop"><span class="text">$_("Stop")</span></button> + <button class="button-big shutoff-hidden" name="vm-poweroff"><span class="text">$_("Power Off")</span></button> <a class="button-big red " name="vm-delete">$_("Delete")</a> </div> </div> diff --git a/ui/pages/help/guests.dita b/ui/pages/help/guests.dita index f6e2c6c..61f3dcc 100644 --- a/ui/pages/help/guests.dita +++ b/ui/pages/help/guests.dita @@ -35,7 +35,7 @@ guest is not active.</dd> <dt>Reset</dt> <dd>Click to reset the guest. </dd> </dlentry><dlentry> -<dt>Power (Start or Stop)</dt> +<dt>Power (Start or Power off)</dt> <dd>Click to power on or power off the guest. If the icon is red, the power is off; if the icon is green, the power is on.</dd> </dlentry></dl> </p>

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 04/10/2014 06:57 AM, shaohef@linux.vnet.ibm.com wrote:

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>

"power off" action may produce undesirable results, for example unflushed disk cache in the guest.

We should let user confirm it.

Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_main.js | 19 ++++++++++++++----- ui/pages/i18n.html.tmpl | 4 ++++ 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 58e348b..ba93b1e 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -43,11 +43,20 @@ kimchi.vmpoweroff = function(event) { button.addClass('loading'); var vm=button.closest('li[name=guest]'); var vm_id=vm.attr("id"); - kimchi.poweroffVM(vm_id, function(result) { - button.removeClass('loading'); - kimchi.listVmsAuto(); - }, function(err) { - kimchi.message.error(err.responseJSON.reason); + var settings = { + title : i18n['KCHVM6002M'], + content : i18n['KCHVM6003M'], + confirm : i18n['KCHAPI6002M'], + cancel : i18n['KCHAPI6003M'] + }; + kimchi.confirm(settings, function() { + kimchi.poweroffVM(vm_id, function(result) { + button.removeClass('loading'); + kimchi.listVmsAuto(); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); + }); + }, function() { }); } else { event.preventDefault(); diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index caf5104..6f655e2 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -125,6 +125,10 @@ var i18n = { 'KCHDR6011M': "$_("Report name should contain only letters, digits and/or hyphen ('-').")",

'KCHVM6001M': "$_("This will delete the virtual machine and its virtual disks. This operation cannot be undone. Would you like to continue?")", + 'KCHVM6002M': "$_("Power off Confirmation")", + 'KCHVM6003M': "$_("This action may produce undesirable results, " + "for example unflushed disk cache in the guest. " + "Would you like to continue?")",

'KCHVMCD6001M': "$_("This CDROM will be detached permanently and you can re-attach it. Continue to detach it?")", 'KCHVMCD6002M': "$_("Attach")",