[Kimchi-devel] [RFC: PATCH v1] Prevent disks from being added more than once
Christy Perez
christy at linux.vnet.ibm.com
Wed Sep 24 22:29:40 UTC 2014
The concept of a ref count for disks had been added, but this count
was dependant upon the pool name for a disk. However, the pool name
wasn't being stored in the VM XML so it wasn't possible to retreive
the pool for the disk in order to update the ref count.
To fix this, this patch changes the way that disks are added when
created in libvirt storage pools. Instead of files, they are treated
as volumes in pools.
Note: Because of this modification, a small inconsistency in the API,
which states that the pool for a vm's "storages" is returned, is
corrected. The pool is returned as an empty string if the disk or
volume was not create from a pool (e.g. a remote ISO).
Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
---
src/kimchi/model/storagevolumes.py | 13 ++++++--
src/kimchi/model/vmstorages.py | 68 +++++++++++++++++++++++++++++++++++---
src/kimchi/vmdisks.py | 33 ++++++++++++++----
src/kimchi/vmtemplate.py | 41 ++++++++++++++++-------
4 files changed, 129 insertions(+), 26 deletions(-)
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index f9f226f..af97748 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -249,19 +249,26 @@ def _get_ref_cnt(self, pool, name, path):
try:
ref_cnt = session.get('storagevolume', vol_id)['ref_cnt']
except NotFoundError:
+ kimchi_log.info('Volume %s not found in obj store.' % vol_id)
# Fix storage volume created outside kimchi scope
ref_cnt = 0
- # try to find this volume in exsisted vm
+ # try to find this volume in exsisting vm
vms = VMsModel.get_vms(self.conn)
for vm in vms:
dom = VMModel.get_vm(vm, self.conn)
storages = get_vm_disk_list(dom)
for disk in storages:
- d_info = get_vm_disk(dom, disk)
+ d_info = get_vm_disk(dom, disk, self.conn)
if path == d_info['path']:
ref_cnt = ref_cnt + 1
- session.store('storagevolume', vol_id,
+ try:
+ session.store('storagevolume', vol_id,
{'ref_cnt': ref_cnt})
+ except Exception as e:
+ # Just log an error if the obj store fails:
+ kimchi_log.warning('Unable to store storage volume id in '
+ 'objectstore due error: %s', e.message)
+
except Exception as e:
# This exception is going to catch errors returned by 'with',
# specially ones generated by 'session.store'. It is outside
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
index 4d6c0b2..1b2c0fe 100644
--- a/src/kimchi/model/vmstorages.py
+++ b/src/kimchi/model/vmstorages.py
@@ -32,7 +32,7 @@
from kimchi.model.vms import DOM_STATE_MAP, VMModel
from kimchi.model.storagevolumes import StorageVolumeModel
from kimchi.model.utils import get_vm_config_flag
-from kimchi.utils import check_url_path
+from kimchi.utils import check_url_path, kimchi_log
from kimchi.osinfo import lookup
from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list
from kimchi.vmdisks import DEV_TYPE_SRC_ATTR_MAP
@@ -73,6 +73,11 @@ def _get_storage_xml(params, ignore_source=False):
source = E.source(protocol=output.scheme, name=output.path)
source.append(host)
disk.append(source)
+ elif src_type == 'volume':
+ source = E.source()
+ source.set('pool', params.get('pool'))
+ source.set('volume', params.get('vol'))
+ disk.append(source)
else:
# Fixing source attribute
source = E.source()
@@ -138,6 +143,8 @@ def create(self, vm_name, params):
# Path will never be blank due to API.json verification.
# There is no need to cover this case here.
params['format'] = 'raw'
+ vol_info = None
+ vol_store_name = None
if not ('vol' in params) ^ ('path' in params):
raise InvalidParameter("KCHVMSTOR0017E")
if params.get('vol'):
@@ -146,6 +153,7 @@ def create(self, vm_name, params):
vol_info = StorageVolumeModel(
conn=self.conn,
objstore=self.objstore).lookup(pool, params['vol'])
+ vol_store_name = '%s:%s' % (pool, params['vol'])
except KeyError:
raise InvalidParameter("KCHVMSTOR0012E")
except Exception as e:
@@ -158,6 +166,10 @@ def create(self, vm_name, params):
params['format'] = vol_info['format']
params['path'] = vol_info['path']
params['src_type'] = _check_path(params['path'])
+ if params['src_type'] is 'file' and params.get('pool') and \
+ params.get('vol'):
+ params['src_type'] = 'volume'
+ # change from 'file' to volume if in a pool ^^
if (params['bus'] not in HOTPLUG_TYPE
and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
raise InvalidOperation('KCHVMSTOR0011E')
@@ -171,6 +183,16 @@ def create(self, vm_name, params):
dom.attachDeviceFlags(dev_xml, get_vm_config_flag(dom, 'all'))
except Exception as e:
raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
+
+ try:
+ new_ref_cnt = vol_info['ref_cnt'] + 1
+ with self.objstore as session:
+ session.store('storagevolume', vol_store_name, {'ref_cnt': new_ref_cnt})
+ except Exception as e:
+ # Just log an error if the obj store fails:
+ kimchi_log.warning('Unable to store storage volume id in '
+ 'objectstore due error: %s', e.message)
+
return params['dev']
def _get_storage_device_name(self, vm_name, params):
@@ -192,25 +214,41 @@ def get_list(self, vm_name):
dom = VMModel.get_vm(vm_name, self.conn)
return get_vm_disk_list(dom)
-
class VMStorageModel(object):
def __init__(self, **kargs):
self.conn = kargs['conn']
+ self.objstore = kargs['objstore']
def lookup(self, vm_name, dev_name):
# Retrieve disk xml and format return dict
dom = VMModel.get_vm(vm_name, self.conn)
- return get_vm_disk(dom, dev_name)
+ disk = get_vm_disk(dom, dev_name, self.conn)
+ # Also include the pool and volume
+ pool_name = ''
+ vol_name = ''
+ try:
+ pool_name = disk.get('pool')
+ vol_name = disk.get('volume')
+ except Exception as e:
+ kimchi_log.warning('VMDisk lookup error: %s' % e.message)
+
+ disk['pool'] = pool_name
+ disk['volume'] = vol_name
+ return disk
def delete(self, vm_name, dev_name):
# Get storage device xml
dom = VMModel.get_vm(vm_name, self.conn)
+ disk_info = None
+ disk_xml = None
+ pool_name = ''
+ vol_name = ''
try:
- bus_type = self.lookup(vm_name, dev_name)['bus']
+ disk_info = self.lookup(vm_name, dev_name)
+ bus_type = disk_info['bus']
except NotFoundError:
raise
- dom = VMModel.get_vm(vm_name, self.conn)
if (bus_type not in HOTPLUG_TYPE and
DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
raise InvalidOperation('KCHVMSTOR0011E')
@@ -224,10 +262,30 @@ def delete(self, vm_name, dev_name):
except Exception as e:
raise OperationFailed("KCHVMSTOR0010E", {'error': e.message})
+ # Get the pool and vol of a disk so we can
+ # decrement its ref_cnt in the obj store
+ pool_name = disk_info.get('pool')
+ vol_name = disk_info.get('volume')
+
+ if(pool_name and vol_name):
+ try:
+ vol_id = '%s:%s' % (pool_name, vol_name)
+ with self.objstore as session:
+ vol = session.get('storagevolume', vol_id)
+ if vol:
+ ref_cnt = vol['ref_cnt']
+ if ref_cnt > 0:
+ ref_cnt = ref_cnt - 1
+ session.store('storagevolume', vol_id, {'ref_cnt': ref_cnt})
+ except Exception as e:
+ kimchi_log.warning('Unable to decrement volume ref count due to %s.'
+ % e.message)
+
def update(self, vm_name, dev_name, params):
path = params.get('path')
if path and len(path) != 0:
params['src_type'] = _check_path(path)
+ # change from 'file' to 'volume' if pool ^^
ignore_source = False
else:
params['src_type'] = 'file'
diff --git a/src/kimchi/vmdisks.py b/src/kimchi/vmdisks.py
index 6012ada..185a499 100644
--- a/src/kimchi/vmdisks.py
+++ b/src/kimchi/vmdisks.py
@@ -17,6 +17,8 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import libvirt
+import os
from lxml import objectify
from kimchi.exception import NotFoundError
@@ -34,37 +36,56 @@ def get_device_xml(dom, dev_name):
return None
return disk[0]
+def get_pool_xml(conn, pool_name):
+ conn = conn.get()
+ return objectify.fromstring(conn.\
+ storagePoolLookupByName(pool_name).XMLDesc(0))
-def get_vm_disk(dom, dev_name):
+def get_vm_disk(dom, dev_name, conn):
# Retrieve disk xml and format return dict
disk = get_device_xml(dom, dev_name)
if disk is None:
raise NotFoundError(
"KCHVMSTOR0007E",
{'dev_name': dev_name, 'vm_name': dom.name()})
- path = ""
+ path = ''
dev_bus = 'ide'
+ pool_name = ''
+ vol_name = ''
try:
source = disk.source
if source is not None:
- src_type = disk.attrib['type']
+ src_type = disk.get('type')
if src_type == 'network':
host = source.host
path = (source.attrib['protocol'] + '://' +
host.attrib['name'] + ':' +
host.attrib['port'] + source.attrib['name'])
+ elif src_type == 'volume':
+ pool_name = disk.source.get('pool')
+ pool = get_pool_xml(conn, pool_name)
+ pool_path = str(pool.target.path)
+ vol_name = disk.source.attrib['volume']
+ path = os.path.join(pool_path, vol_name)
else:
path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]]
# Retrieve storage bus type
dev_bus = disk.target.attrib['bus']
- except:
- pass
+ except Exception as e:
+ kimchi_log.warning("get_vm_disk exception: %s " % e.message)
+
dev_type = disk.attrib['device']
+
+ # Pool and volume may be empty strings, if the user passed a disk name.
+ # If the disk was from a storage pool, let's use that so we can keep track
+ # of its ref_cnt in the objects store.
return {'dev': dev_name,
'type': dev_type,
'path': path,
'format': disk.driver.attrib['type'],
- 'bus': dev_bus}
+ 'bus': dev_bus,
+ 'pool': pool_name,
+ 'volume': vol_name}
def get_vm_disk_list(dom):
diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index 5f22db9..1940911 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -187,24 +187,41 @@ def _get_cdrom_xml(self, libvirt_stream_protocols, qemu_stream_dns):
return network_file % (params)
def _get_disks_xml(self, vm_uuid):
- storage_path = self._get_storage_path()
+ def TYPE(v):
+ return {'type': v}
+
+ #storage_path = self._get_storage_path()
+ _pool = pool_name_from_uri(self.info['storagepool'])
ret = ""
for i, disk in enumerate(self.info['disks']):
index = disk.get('index', i)
- volume = "%s-%s.img" % (vm_uuid, index)
- src = os.path.join(storage_path, volume)
+ _volume = "%s-%s.img" % (vm_uuid, index)
+ #src = os.path.join(storage_path, volume)
dev = "%s%s" % (self._bus_to_dev[self.info['disk_bus']],
string.lowercase[index])
fmt = 'raw' if self._get_storage_type() in ['logical'] else 'qcow2'
- params = {'src': src, 'dev': dev, 'bus': self.info['disk_bus'],
- 'type': fmt}
- ret += """
- <disk type='file' device='disk'>
- <driver name='qemu' type='%(type)s' cache='none'/>
- <source file='%(src)s' />
- <target dev='%(dev)s' bus='%(bus)s' />
- </disk>
- """ % params
+ #params = {'src': src, 'dev': dev, 'bus': self.info['disk_bus'],
+ # 'type': fmt}
+ #ret += """
+ #<disk type='file' device='disk'>
+ # <driver name='qemu' type='%(type)s' cache='none'/>
+ # <source file='%(src)s' />
+ # <target dev='%(dev)s' bus='%(bus)s' />
+ #</disk>
+ #""" % params
+
+ # Instead do:
+ #<disk type='volume'>
+ # <driver name='qemu' type='%(type)s' cache='none'/>
+ # <source pool='%(pool)s' volume='%(volume)s' />
+ # <target dev='%(dev)s' bus='%(bus)s />
+ #</disk>
+ disk = E.disk(
+ E.driver(TYPE(fmt), name='qemu', cache='none'),
+ E.source(pool=_pool, volume=_volume),
+ E.target(dev=dev, bus=self.info['disk_bus']),
+ TYPE('volume'))
+ ret += etree.tostring(disk)
return ret
def _get_graphics_xml(self, params):
--
1.9.3
More information about the Kimchi-devel
mailing list