[Kimchi-devel] [PATCH v3] shutdown and reboot actions on linux vs linux with KVM

[Aline Manera]

On 05/11/2015 15:42, chandra at linux.vnet.ibm.com wrote:
> From: chandrureddy <chandra at linux.vnet.ibm.com>
>
> Review Comments taken care
> ---
>   src/wok/plugins/gingerbase/model/host.py           | 47 +++++++++++++++-------
>   .../gingerbase/ui/js/src/gingerbase.host.js        | 25 ++++++------
>   src/wok/plugins/gingerbase/ui/pages/i18n.json.tmpl |  3 ++
>   3 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/src/wok/plugins/gingerbase/model/host.py b/src/wok/plugins/gingerbase/model/host.py
> index 670fec5..9cbef47 100644
> --- a/src/wok/plugins/gingerbase/model/host.py
> +++ b/src/wok/plugins/gingerbase/model/host.py
> @@ -39,6 +39,14 @@ from wok.plugins.gingerbase.repositories import Repositories
>   from wok.plugins.gingerbase.swupdate import SoftwareUpdate
>
>   HOST_STATS_INTERVAL = 1
> +DOM_STATE_MAP = {0: 'nostate',
> +                 1: 'running',
> +                 2: 'blocked',
> +                 3: 'paused',
> +                 4: 'shutdown',
> +                 5: 'shutoff',
> +                 6: 'crashed',
> +                 7: 'pmsuspended'}
>
>
>   class HostModel(object):
> @@ -142,29 +150,40 @@ class HostModel(object):
>
>       def shutdown(self, args=None):
>           # Check for running vms before shutdown
> -        # FIXME : Find alternative way to figure out if any vms running
> -        # running_vms = self._get_vms_list_by_state('running')
> -        # if len(running_vms) > 0:
> -        #     raise OperationFailed("GGBHOST0001E")
> +        running_vms = self.get_vmlist_bystate('running')
> +        if len(running_vms) > 0:
> +            raise OperationFailed("GGBHOST0001E")
>
>           wok_log.info('Host is going to shutdown.')
>           os.system('shutdown -h now')
>
>       def reboot(self, args=None):
> -        # Find running VMs
> -        # FIXME : Find alternative way to figure out if any vms running
> -        # running_vms = self._get_vms_list_by_state('running')
> -        # if len(running_vms) > 0:
> -        #     raise OperationFailed("GGBHOST0002E")
> +        # Check for running vms before reboot
> +        running_vms = self.get_vmlist_bystate('running')
> +        if len(running_vms) > 0:
> +            raise OperationFailed("GGBHOST0002E")
>
>           wok_log.info('Host is going to reboot.')
>           os.system('reboot')
>
> -    # def _get_vms_list_by_state(self, state):
> -    #     conn = self.conn.get()
> -    #     return [dom.name().decode('utf-8')
> -    #             for dom in conn.listAllDomains(0)
> -    #             if (DOM_STATE_MAP[dom.info()[0]]) == state]
> +    def get_vmlist_bystate(self, state='running'):
> +        try:
> +            libvirt_mod = __import__('libvirt')
> +        except Exception, e:
> +            wok_log.info("Unable to import libvirt module. Details:",
> +                         e.message)
> +            # Ignore any error and assume there is no vm running in the host
> +            return []
> +
> +        try:
> +            conn = libvirt_mod.open(None)
> +            return [dom.name().decode('utf-8')
> +                    for dom in conn.listAllDomains(0)
> +                    if (DOM_STATE_MAP[dom.info()[0]] == state)]
> +        except Exception, e:
> +            wok_log.info("Unable to get virtual machines information. "
> +                         "Details:", e.message)

> +            raise OperationFailed("There may be vms running on the host")
>

Sorry, I haven't noticed that before, but to raise an exception we need 
to use the error message code.
Add the error message to i18n.py and use the code to raise the exception.

>   class SoftwareUpdateProgressModel(object):
> diff --git a/src/wok/plugins/gingerbase/ui/js/src/gingerbase.host.js b/src/wok/plugins/gingerbase/ui/js/src/gingerbase.host.js
> index 0d52b92..77ee736 100644
> --- a/src/wok/plugins/gingerbase/ui/js/src/gingerbase.host.js
> +++ b/src/wok/plugins/gingerbase/ui/js/src/gingerbase.host.js
> @@ -496,22 +496,21 @@ kimchi.host_main = function() {
>           };
>
>           wok.confirm(settings, function() {
> -            kimchi.shutdown(params);
>               $(shutdownButtonID).prop('disabled', true);
>               $(restartButtonID).prop('disabled', true);
>               // Check if there is any VM is running.
> -            // FIXME : Find alternative way to figure out if any vms running
> -            // kimchi.listVMs(function(vms) {
> -            //     for(var i = 0; i < vms.length; i++) {
> -            //         if(vms[i]['state'] === 'running') {
> -            //             wok.message.error.code('GGBHOST6001E');
> -            //             $(shutdownButtonID).prop('disabled', false);
> -            //             $(restartButtonID).prop('disabled', false);
> -            //             return;
> -            //         }
> -            //     }
> -            //
> -            // });
> +            // Based on the success will shutdown/reboot
> +            kimchi.shutdown(params, function(success) {
> +                wok.message.success(i18n['GGBHOST6009M'])
> +                $(shutdownButtonID).prop('disabled', false);
> +                $(restartButtonID).prop('disabled', false);
> +                return;
> +            }, function(error) {
> +            // Looks like VMs are running.
> +            wok.message.error.code('GGBHOST6001E');
> +            $(shutdownButtonID).prop('disabled', false);
> +            $(restartButtonID).prop('disabled', false);
> +        });
>           }, function() {
>           });
>       };
> diff --git a/src/wok/plugins/gingerbase/ui/pages/i18n.json.tmpl b/src/wok/plugins/gingerbase/ui/pages/i18n.json.tmpl
> index f6228ab..3f3b662 100644
> --- a/src/wok/plugins/gingerbase/ui/pages/i18n.json.tmpl
> +++ b/src/wok/plugins/gingerbase/ui/pages/i18n.json.tmpl
> @@ -69,6 +69,7 @@
>       "GGBHOST6006M": "$_("Received")",
>       "GGBHOST6007M": "$_("Sent")",
>       "GGBHOST6008M": "$_("Shutting down or restarting host will cause unsaved work lost. Continue to shut down/restarting?")",
> +    "GGBHOST6009M": "$_("The system is going down")",
>
>
>       "GGBREPO6001M": "$_("Confirm")",
> @@ -113,6 +114,8 @@
>       "GGBDR6012M": "$_("Pending...")",
>       "GGBDR6013M": "$_("Report name is the same as the original one.")",
>
> +    "GGBHOST6001E": "$_("Unable to shut down system as there are some virtual machines running!")",
> +
>       "GGBVM6001M": "$_("This will delete the virtual machine and its virtual disks. This operation cannot be undone. Would you like to continue?")",
>       "GGBVM6002M": "$_("Power off Confirmation")",
>       "GGBVM6003M": "$_("This action may produce undesirable results, "