2:06 p.m.
Crístian Viana (2):
Only allow VM snapshots to be taken on 'qcow2' disks
Revert "snapshot: Clone snapshots when cloning a VM"
src/kimchi/i18n.py | 1 +
src/kimchi/model/vms.py | 29 +----------------------------
src/kimchi/model/vmsnapshots.py | 17 +++++++++++++++--
tests/test_model.py | 11 -----------
tests/test_rest.py | 19 ++-----------------
5 files changed, 19 insertions(+), 58 deletions(-)
--
1.9.3
2:06 p.m.
New subject: [PATCH 1/2] Only allow VM snapshots to be taken on 'qcow2' disks
VM snapshot support in libvirt may not work correctly with some image
types, like 'raw'.
To make sure snapshots work correctly, only allow them to be taken on
VMs with 'qcow2' images, which is the most supported image type for that
operation.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/i18n.py | 1 +
src/kimchi/model/vmsnapshots.py | 17 +++++++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 58d1f36..e43fe9c 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -319,6 +319,7 @@ messages = {
"KCHSNAP0006E": _("Unable to delete snapshot '%(name)s' on
virtual machine '%(vm)s'. Details: %(err)s"),
"KCHSNAP0008E": _("Unable to retrieve current snapshot on virtual
machine '%(vm)s'. Details: %(err)s"),
"KCHSNAP0009E": _("Unable to revert virtual machine '%(vm)s'
to snapshot '%(name)s'. Details: %(err)s"),
+ "KCHSNAP0010E": _("Unable to create snapshot on virtual machine
'%(vm)s' because it contains a disk with format '%(format)s'; only
'qcow2' is supported."),
"KCHCPUINF0001E": _("The number of vCPUs is too large for this
system."),
"KCHCPUINF0002E": _("Invalid vCPU/topology combination."),
diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py
index 0da9dc3..bb9bfaa 100644
--- a/src/kimchi/model/vmsnapshots.py
+++ b/src/kimchi/model/vmsnapshots.py
@@ -27,6 +27,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.vmstorages import VMStorageModel, VMStoragesModel
from kimchi.utils import add_task
@@ -35,12 +36,14 @@ class VMSnapshotsModel(object):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
self.task = TaskModel(**kargs)
+ self.vmstorages = VMStoragesModel(**kargs)
+ self.vmstorage = VMStorageModel(**kargs)
def create(self, vm_name, params={}):
"""Create a snapshot with the current domain state.
- The VM must be stopped before creating a snapshot on it; otherwise, an
- exception will be raised.
+ The VM must be stopped and contain only disks with format 'qcow2';
+ otherwise an exception will be raised.
Parameters:
vm_name -- the name of the VM where the snapshot will be created.
@@ -55,6 +58,16 @@ class VMSnapshotsModel(object):
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)
+ type = storage['type']
+ format = storage['format']
+
+ if type != u'cdrom' and format != u'qcow2':
+ raise InvalidOperation('KCHSNAP0010E', {'vm': vm_name,
+ 'format': format})
+
name = params.get('name', unicode(int(time.time())))
task_params = {'vm_name': vm_name, 'name': name}
--
1.9.3
2:06 p.m.
New subject: [PATCH 2/2] Revert "snapshot: Clone snapshots when cloning a VM"
This reverts commit c087f65f12af7394230c300ca63e3fd79fe65498.
Conflicts:
src/kimchi/mockmodel.py
---
src/kimchi/model/vms.py | 29 +----------------------------
tests/test_model.py | 11 -----------
tests/test_rest.py | 19 ++-----------------
3 files changed, 3 insertions(+), 56 deletions(-)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 4b112fe..3aa1145 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -374,15 +374,10 @@ class VMModel(object):
# create new guest
cb('defining new VM')
try:
- vir_new_dom = vir_conn.defineXML(xml)
+ vir_conn.defineXML(xml)
except libvirt.libvirtError, e:
raise OperationFailed('KCHVM0035E', {'name': name,
'err': e.message})
- rollback.prependDefer(vir_new_dom.undefine)
-
- # copy snapshots
- cb('copying VM snapshots')
- self._clone_copy_snapshots(name, new_name)
rollback.commitAll()
@@ -546,28 +541,6 @@ class VMModel(object):
# remove the new object store entry should an error occur later
rollback.prependDefer(_rollback_objstore)
- def _clone_copy_snapshots(self, vm_name, new_vm_name):
- dom = self.get_vm(new_vm_name, self.conn)
- flags = libvirt.VIR_DOMAIN_XML_SECURE
-
- # libvirt's Test driver does not support the function
- # "virDomainListAllSnapshots", so "VMSnapshots.get_list" will
raise
- # "OperationFailed" in that case.
- try:
- snapshot_names = self.vmsnapshots.get_list(vm_name)
- except OperationFailed, e:
- kimchi_log.error('cannot list snapshots: %s; '
- 'skipping snapshot cloning...' % e.message)
- else:
- for s in snapshot_names:
- vir_snap = self.vmsnapshot.get_vmsnapshot(vm_name, s)
- try:
- snap_xml = vir_snap.getXMLDesc(flags).decode('utf-8')
- dom.snapshotCreateXML(snap_xml)
- except libvirt.libvirtError, e:
- raise OperationFailed('KCHVM0035E', {'name':
vm_name,
- 'err': e.message})
-
def _build_access_elem(self, users, groups):
auth = config.get("authentication", "method")
auth_elem = E.auth(type=auth)
diff --git a/tests/test_model.py b/tests/test_model.py
index 1355295..319408b 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -161,17 +161,6 @@ class ModelTests(unittest.TestCase):
self.assertEquals(sorted([params['name'], snap_name],
key=unicode.lower), snaps)
- # Clone the VM and check whether the snapshots have been cloned
- # as well
- task = inst.vm_clone(u'kimchi-vm')
- clone_name = task['target_uri'].split('/')[-1]
- rollback.prependDefer(inst.vm_delete, clone_name)
- inst.task_wait(task['id'])
- task = inst.task_lookup(task['id'])
- self.assertEquals('finished', task['status'])
- clone_snaps = inst.vmsnapshots_get_list(clone_name)
- self.assertEquals(snaps, clone_snaps)
-
snap = inst.vmsnapshot_lookup(u'kimchi-vm', snap_name)
current_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm')
self.assertEquals(snap, current_snap)
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 2e5cff1..fa15ef6 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -428,8 +428,8 @@ class RestTests(unittest.TestCase):
resp = self.request('/vms/test-vm/snapshots', '{}',
'GET')
self.assertEquals(200, resp.status)
- orig_snaps = json.loads(resp.read())
- self.assertEquals(2, len(orig_snaps))
+ snaps = json.loads(resp.read())
+ self.assertEquals(2, len(snaps))
# Look up current snapshot (the one created above)
resp = self.request('/vms/test-vm/snapshots/current', '{}',
'GET')
@@ -451,21 +451,6 @@ class RestTests(unittest.TestCase):
current_snap = json.loads(resp.read())
self.assertEquals(snap, current_snap)
- # Clone a VM with snapshot
- resp = self.request('/vms/test-vm/clone', '{}', 'POST')
- self.assertEquals(202, resp.status)
- task = json.loads(resp.read())
- wait_task(self._task_lookup, task['id'])
- resp = self.request('/tasks/%s' % task['id'], '{}',
'GET')
- self.assertEquals(200, resp.status)
- task = json.loads(resp.read())
- self.assertEquals('finished', task['status'])
- vm_name = task['target_uri'].split('/')[-1]
- resp = self.request('/vms/%s/snapshots' % vm_name, '{}',
'GET')
- self.assertEquals(200, resp.status)
- clone_snaps = json.loads(resp.read())
- self.assertEquals(orig_snaps, clone_snaps)
-
# Delete a snapshot
resp = self.request('/vms/test-vm/snapshots/%s' %
params['name'],
'{}', 'DELETE')
--
1.9.3
3674
days inactive
3683
days old
3 comments
2 participants
participants (2)
-
Aline Manera -
Crístian Viana