[Kimchi-devel] [PATCH 3/3] snapshot: Allow creating a snapshot of a domain in any state

[Aline Manera]

Reviewed-by: Aline Manera <alinefm at linux.vnet.ibm.com>

On 20/04/2015 15:58, Crístian Deives wrote:
> Currently, Kimchi only allows snapshots to be taken of shutoff VMs. Now
> that the infrastructure for taking snapshots of running VMs is done, we
> can allow that feature as well.
>
> Do not restrict the snapshot creation based on the VM's state, and allow
> it to be created when the VM is running as well.
>
> Signed-off-by: Crístian Deives <cristiandeives at gmail.com>
> ---
>   src/kimchi/i18n.py              | 1 -
>   src/kimchi/model/vmsnapshots.py | 8 ++------
>   tests/test_model.py             | 9 +++++++--
>   3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 18e84bc..91e543e 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -320,7 +320,6 @@ messages = {
>       "KCHREPOS0029E": _("Repository metalink must be an http://, ftp:// or file:// URL."),
>       "KCHREPOS0030E": _("Cannot specify mirrorlist and metalink at the same time."),
>   
> -    "KCHSNAP0001E": _("Virtual machine '%(vm)s' must be stopped before creating a snapshot of it."),
>       "KCHSNAP0002E": _("Unable to create snapshot '%(name)s' on virtual machine '%(vm)s'. Details: %(err)s"),
>       "KCHSNAP0003E": _("Snapshot '%(name)s' does not exist on virtual machine '%(vm)s'."),
>       "KCHSNAP0004E": _("Unable to retrieve snapshot '%(name)s' on virtual machine '%(vm)s'. Details: %(err)s"),
> diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py
> index 331f532..85ecb24 100644
> --- a/src/kimchi/model/vmsnapshots.py
> +++ b/src/kimchi/model/vmsnapshots.py
> @@ -26,7 +26,7 @@ from lxml.builder import E
>   
>   from kimchi.exception import InvalidOperation, NotFoundError, OperationFailed
>   from kimchi.model.tasks import TaskModel
> -from kimchi.model.vms import DOM_STATE_MAP, VMModel
> +from kimchi.model.vms import VMModel
>   from kimchi.model.vmstorages import VMStorageModel, VMStoragesModel
>   from kimchi.utils import add_task
>   
> @@ -42,7 +42,7 @@ class VMSnapshotsModel(object):
>       def create(self, vm_name, params={}):
>           """Create a snapshot with the current domain state.
>   
> -        The VM must be stopped and contain only disks with format 'qcow2';
> +        The VM must contain only disks with format 'qcow2';
>           otherwise an exception will be raised.
>   
>           Parameters:
> @@ -54,10 +54,6 @@ class VMSnapshotsModel(object):
>           Return:
>           A Task running the operation.
>           """
> -        vir_dom = VMModel.get_vm(vm_name, self.conn)
> -        if DOM_STATE_MAP[vir_dom.info()[0]] != u'shutoff':
> -            raise InvalidOperation('KCHSNAP0001E', {'vm': vm_name})
> -
>           # if the VM has a non-CDROM disk with type 'raw', abort.
>           for storage_name in self.vmstorages.get_list(vm_name):
>               storage = self.vmstorage.lookup(vm_name, storage_name)
> diff --git a/tests/test_model.py b/tests/test_model.py
> index b53822a..3740143 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -144,8 +144,13 @@ class ModelTests(unittest.TestCase):
>               info = inst.vm_lookup('kimchi-vm')
>               self.assertEquals('running', info['state'])
>   
> -            self.assertRaises(InvalidOperation, inst.vmsnapshots_create,
> -                              u'kimchi-vm')
> +            # make sure creating a snapshot of a running VM works
> +            task = inst.vmsnapshots_create(u'kimchi-vm')
> +            inst.task_wait(task['id'])
> +            task = inst.task_lookup(task['id'])
> +            self.assertEquals('finished', task['status'])
> +            inst.vmsnapshot_delete(u'kimchi-vm',
> +                                   task['target_uri'].split('/')[-1])
>   
>               inst.vm_poweroff(u'kimchi-vm')
>               vm = inst.vm_lookup(u'kimchi-vm')