[Kimchi-devel] [PATCH V2] Create logical pool from existing VG
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Tue Nov 17 05:13:23 UTC 2015
Patch seems ok, just some comments below
On 11/11/2015 04:41 PM, Aline Manera wrote:
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
> ---
> src/wok/plugins/kimchi/API.json | 5 +
> src/wok/plugins/kimchi/control/host.py | 22 ++++
> src/wok/plugins/kimchi/disks.py | 123 +++++++++++++++++++++
> src/wok/plugins/kimchi/docs/API.md | 25 ++++-
> src/wok/plugins/kimchi/i18n.py | 4 +
> src/wok/plugins/kimchi/mockmodel.py | 27 +++++
> src/wok/plugins/kimchi/model/host.py | 29 +++++
> src/wok/plugins/kimchi/model/libvirtstoragepool.py | 9 +-
> src/wok/plugins/kimchi/model/storagepools.py | 30 +++--
> .../plugins/kimchi/tests/test_mock_storagepool.py | 8 +-
> 10 files changed, 265 insertions(+), 17 deletions(-)
>
> diff --git a/src/wok/plugins/kimchi/API.json b/src/wok/plugins/kimchi/API.json
> index 961f35f..7daa2a9 100644
> --- a/src/wok/plugins/kimchi/API.json
> +++ b/src/wok/plugins/kimchi/API.json
> @@ -141,6 +141,11 @@
> "error": "KCHPOOL0025E"
> }
> }
> + },
> + "from_vg": {
> + "description": "Use an existing VG or create a new one",
This description is strange, please, check what is written in API.MD
> + "type": "boolean",
> + "error": "KCHPOOL0026E"
> }
> }
> }
> diff --git a/src/wok/plugins/kimchi/control/host.py b/src/wok/plugins/kimchi/control/host.py
> index 9bc703a..39df0d0 100644
> --- a/src/wok/plugins/kimchi/control/host.py
> +++ b/src/wok/plugins/kimchi/control/host.py
> @@ -35,12 +35,34 @@ class Host(Resource):
> self.devices = Devices(self.model)
> self.cpuinfo = CPUInfo(self.model)
> self.partitions = Partitions(self.model)
> + self.vgs = VolumeGroups(self.model)
>
> @property
> def data(self):
> return {}
>
>
> +class VolumeGroups(Collection):
> + def __init__(self, model):
> + super(VolumeGroups, self).__init__(model)
> + self.role_key = 'host'
> + self.uri_fmt = "/host/vgs"
> + self.admin_methods = ['GET']
> + self.resource = VolumeGroup
> +
> +
> +class VolumeGroup(Resource):
> + def __init__(self, model, id=None):
> + super(VolumeGroup, self).__init__(model, id)
> + self.role_key = 'host'
> + self.uri_fmt = "/host/vgs/%s"
> + self.admin_methods = ['GET']
> +
> + @property
> + def data(self):
> + return self.info
> +
> +
> class VMHolders(SimpleCollection):
> def __init__(self, model, device_id):
> super(VMHolders, self).__init__(model)
> diff --git a/src/wok/plugins/kimchi/disks.py b/src/wok/plugins/kimchi/disks.py
> index eb40e3a..0ac792e 100644
> --- a/src/wok/plugins/kimchi/disks.py
> +++ b/src/wok/plugins/kimchi/disks.py
> @@ -194,3 +194,126 @@ def get_partition_details(name):
> dev['path'] = dev_path
> dev['name'] = name
> return dev
> +
> +
> +def vgs():
> + """
> + lists all volume groups in the system. All size units are in bytes.
> +
> + [{'vgname': 'vgtest', 'size': 999653638144L, 'free': 0}]
> + """
> + cmd = ['vgs',
> + '--units',
> + 'b',
> + '--nosuffix',
> + '--noheading',
> + '--unbuffered',
> + '--options',
> + 'vg_name,vg_size,vg_free']
> +
> + proc = subprocess.Popen(cmd,
> + stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE)
> +
> + out, err = proc.communicate()
> +
> + if proc.returncode != 0:
> + raise OperationFailed("KCHDISKS0003E", {'err': err})
> +
> + if not out:
> + return []
> +
> + # remove blank spaces and create a list of VGs
> + vgs = map(lambda v: v.strip(), out.strip('\n').split('\n'))
> +
> + # create a dict based on data retrieved from vgs
> + return map(lambda l: {'vgname': l[0],
> + 'size': long(l[1]),
> + 'free': long(l[2])},
> + [fields.split() for fields in vgs])
> +
> +
> +def lvs(vgname=None):
> + """
> + lists all logical volumes found in the system. It can be filtered by
> + the volume group. All size units are in bytes.
> +
> + [{'lvname': 'lva', 'path': '/dev/vgtest/lva', 'size': 12345L},
> + {'lvname': 'lvb', 'path': '/dev/vgtest/lvb', 'size': 12345L}]
> + """
> + cmd = ['lvs',
> + '--units',
> + 'b',
> + '--nosuffix',
> + '--noheading',
> + '--unbuffered',
> + '--options',
> + 'lv_name,lv_path,lv_size,vg_name']
> +
> + proc = subprocess.Popen(cmd,
> + stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE)
> +
> + out, err = proc.communicate()
> +
> + if proc.returncode != 0:
> + raise OperationFailed("KCHDISKS0003E", {'err': err})
> +
> + if not out:
> + return []
> +
> + # remove blank spaces and create a list of LVs filtered by vgname, if
> + # provided
> + lvs = filter(lambda f: vgname is None or vgname in f,
> + map(lambda v: v.strip(), out.strip('\n').split('\n')))
> +
> + # create a dict based on data retrieved from lvs
> + return map(lambda l: {'lvname': l[0],
> + 'path': l[1],
> + 'size': long(l[2])},
> + [fields.split() for fields in lvs])
> +
> +
> +def pvs(vgname=None):
> + """
> + lists all physical volumes in the system. It can be filtered by the
> + volume group. All size units are in bytes.
> +
> + [{'pvname': '/dev/sda3',
> + 'size': 469502001152L,
> + 'uuid': 'kkon5B-vnFI-eKHn-I5cG-Hj0C-uGx0-xqZrXI'},
> + {'pvname': '/dev/sda2',
> + 'size': 21470642176L,
> + 'uuid': 'CyBzhK-cQFl-gWqr-fyWC-A50Y-LMxu-iHiJq4'}]
> + """
> + cmd = ['pvs',
> + '--units',
> + 'b',
> + '--nosuffix',
> + '--noheading',
> + '--unbuffered',
> + '--options',
> + 'pv_name,pv_size,pv_uuid,vg_name']
> +
> + proc = subprocess.Popen(cmd,
> + stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE)
> +
> + out, err = proc.communicate()
> +
> + if proc.returncode != 0:
> + raise OperationFailed("KCHDISKS0003E", {'err': err})
> +
> + if not out:
> + return []
> +
> + # remove blank spaces and create a list of PVs filtered by vgname, if
> + # provided
> + pvs = filter(lambda f: vgname is None or vgname in f,
> + map(lambda v: v.strip(), out.strip('\n').split('\n')))
> +
> + # create a dict based on data retrieved from pvs
> + return map(lambda l: {'pvname': l[0],
> + 'size': long(l[1]),
> + 'uuid': l[2]},
> + [fields.split() for fields in pvs])
> diff --git a/src/wok/plugins/kimchi/docs/API.md b/src/wok/plugins/kimchi/docs/API.md
> index 52368b7..14f014d 100644
> --- a/src/wok/plugins/kimchi/docs/API.md
> +++ b/src/wok/plugins/kimchi/docs/API.md
> @@ -444,7 +444,8 @@ A interface represents available network interface on VM.
>
> * **GET**: Retrieve a summarized list of all defined Storage Pools
> * **POST**: Create a new Storage Pool
> - * name: The name of the Storage Pool.
> + * name: The name of the Storage Pool. It corresponds to the VG name while
> + creating a logical storage pool from an existing VG.
> * type: The type of the defined Storage Pool.
> Supported types: 'dir', 'kimchi-iso', 'netfs', 'logical', 'iscsi', 'scsi'
> * path: The path of the defined Storage Pool.
> @@ -466,6 +467,8 @@ A interface represents available network interface on VM.
> * username: Login username of the iSCSI target.
> * password: Login password of the iSCSI target.
> * adapter_name: SCSI host name.
> + * from_vg: Indicate if a logical pool will be created from an
> + existing VG or not.
>
> ### Resource: Storage Pool
>
> @@ -897,6 +900,26 @@ List of available groups.
> Otherwise blank.
> * available: false, if the partition is in use by system; true, otherwise.
>
> +### Collection: Volume Groups
> +
> +**URI:** /plugins/kimchi/host/vgs
> +
> +**Methods:**
> +
> +* **GET**: Retrieves a detailed list of all volume groups in the host.
> +
> +### Resource: Volume Group
> +
> +**URI:** /plugins/kimchi/host/vgs/*:name*
> +
> +**Methods:**
> +
> +* **GET**: Retrieve the description of a single Volume Group:
> + * name: The volume group name. Used to identify it in this API.
> + * lvs: The logical volumes associated to this volume group.
> + * pvs: The phisical volumes associated to this volume group.
> + * free: Amount of free space in the volume group.
> + * size: Total size of the volume group.
>
> ### Collection: Peers
>
> diff --git a/src/wok/plugins/kimchi/i18n.py b/src/wok/plugins/kimchi/i18n.py
> index 59b61de..a4092e7 100644
> --- a/src/wok/plugins/kimchi/i18n.py
> +++ b/src/wok/plugins/kimchi/i18n.py
> @@ -29,6 +29,7 @@ messages = {
>
> "KCHDISKS0001E": _("Error while getting block devices. Details: %(err)s"),
> "KCHDISKS0002E": _("Error while getting block device information for %(device)s."),
> + "KCHDISKS0003E": _("Unable to retrieve LVM information. Details: %(err)s"),
>
Not a big issue, but why not KCHLVMXXX ?
> "KCHPART0001E": _("Partition %(name)s does not exist in the host"),
>
> @@ -199,6 +200,7 @@ messages = {
> "KCHPOOL0035E": _("Unable to delete pool %(name)s as it is associated with some templates"),
> "KCHPOOL0036E": _("A volume group named '%(name)s' already exists. Please, choose another name to create the logical pool."),
> "KCHPOOL0037E": _("Unable to update database with deep scan information due error: %(err)s"),
> + "KCHPOOL0038E": _("No volume group '%(name)s' found. Please, specify an existing volume group to create the logical pool from."),
>
> "KCHVOL0001E": _("Storage volume %(name)s already exists"),
> "KCHVOL0002E": _("Storage volume %(name)s does not exist in storage pool %(pool)s"),
> @@ -294,4 +296,6 @@ messages = {
> "KCHCPUINF0002E": _("Invalid vCPU/topology combination."),
> "KCHCPUINF0003E": _("This host (or current configuration) does not allow CPU topology."),
>
> + "KCHLVMS0001E": _("Invalid volume group name parameter: %(name)s."),
> +
> }
> diff --git a/src/wok/plugins/kimchi/mockmodel.py b/src/wok/plugins/kimchi/mockmodel.py
> index 895401e..9186f78 100644
> --- a/src/wok/plugins/kimchi/mockmodel.py
> +++ b/src/wok/plugins/kimchi/mockmodel.py
> @@ -39,6 +39,7 @@ from wok.plugins.kimchi.model.libvirtstoragepool import NetfsPoolDef
> from wok.plugins.kimchi.model.libvirtstoragepool import StoragePoolDef
> from wok.plugins.kimchi.model.model import Model
> from wok.plugins.kimchi.model.storagepools import StoragePoolModel
> +from wok.plugins.kimchi.model.storagepools import StoragePoolsModel
> from wok.plugins.kimchi.model.storagevolumes import StorageVolumeModel
> from wok.plugins.kimchi.model.storagevolumes import StorageVolumesModel
> from wok.plugins.kimchi.model import storagevolumes
> @@ -73,6 +74,7 @@ class MockModel(Model):
> defaults.update(mockmodel_defaults)
> osinfo.defaults = dict(defaults)
>
> + self._mock_vgs = MockVolumeGroups()
> self._mock_partitions = MockPartitions()
> self._mock_devices = MockDevices()
> self._mock_storagevolumes = MockStorageVolumes()
> @@ -113,6 +115,7 @@ class MockModel(Model):
> setattr(self, m, mock_method)
>
> DeviceModel.lookup = self._mock_device_lookup
> + StoragePoolsModel._check_lvm = self._check_lvm
> StoragePoolModel._update_lvm_disks = self._update_lvm_disks
> StorageVolumesModel.get_list = self._mock_storagevolumes_get_list
> StorageVolumeModel.doUpload = self._mock_storagevolume_doUpload
> @@ -252,6 +255,10 @@ class MockModel(Model):
>
> return MockModel._libvirt_get_vol_path(pool, vol)
>
> + def _check_lvm(self, name, from_vg):
> + # do not do any verification while using MockModel
> + pass
> +
> def _update_lvm_disks(self, pool_name, disks):
> conn = self.conn.get()
> pool = conn.storagePoolLookupByName(pool_name.encode('utf-8'))
> @@ -334,6 +341,12 @@ class MockModel(Model):
> def _mock_partition_lookup(self, name):
> return self._mock_partitions.partitions[name]
>
> + def _mock_volumegroups_get_list(self):
> + return self._mock_vgs.data.keys()
> +
> + def _mock_volumegroup_lookup(self, name):
> + return self._mock_vgs.data[name]
> +
> def _mock_vm_clone(self, name):
> new_name = get_next_clone_name(self.vms_get_list(), name)
> snapshots = MockModel._mock_snapshots.get(name, [])
> @@ -418,6 +431,20 @@ class MockStorageVolumes(object):
> 'isvalid': True}}
>
>
> +class MockVolumeGroups(object):
> + def __init__(self):
> + self.data = {"hostVG": {"lvs": [],
> + "name": "hostVG",
> + "pvs": ["/dev/vdx"],
> + "free": 5347737600,
> + "size": 5347737600},
> + "kimchiVG": {"lvs": [],
> + "name": "kimchiVG",
> + "pvs": ["/dev/vdz", "/dev/vdw"],
> + "free": 10695475200,
> + "size": 10695475200}}
> +
> +
> class MockPartitions(object):
> def __init__(self):
> self.partitions = {"vdx": {"available": True, "name": "vdx",
> diff --git a/src/wok/plugins/kimchi/model/host.py b/src/wok/plugins/kimchi/model/host.py
> index 96f4bea..4a37dc8 100644
> --- a/src/wok/plugins/kimchi/model/host.py
> +++ b/src/wok/plugins/kimchi/model/host.py
> @@ -241,3 +241,32 @@ class PartitionModel(object):
>
> def lookup(self, name):
> return disks.get_partition_details(name)
> +
> +
> +class VolumeGroupsModel(object):
> + def __init__(self, **kargs):
> + pass
> +
> + def get_list(self):
> + if not disks.vgs():
> + return []
The if above is not necessary
> + return [vg['vgname'] for vg in disks.vgs()]
> +
> +
> +class VolumeGroupModel(object):
> + def __init__(self, **kargs):
> + pass
> +
> + def lookup(self, name):
> + def _format(vg):
> + return {'name': vg['vgname'],
> + 'size': vg['size'],
> + 'free': vg['free'],
> + 'pvs': [pv['pvname'] for pv in disks.pvs(vg['vgname'])],
> + 'lvs': [lv['lvname'] for lv in disks.lvs(vg['vgname'])]}
> +
> + vgs = [_format(vg) for vg in disks.vgs() if vg['vgname'] == name]
> + if not vgs:
> + raise InvalidParameter("KCHLVMS0001E", {'name': name})
> +
> + return vgs[0]
> diff --git a/src/wok/plugins/kimchi/model/libvirtstoragepool.py b/src/wok/plugins/kimchi/model/libvirtstoragepool.py
> index 5da0b3b..0fa8ce0 100644
> --- a/src/wok/plugins/kimchi/model/libvirtstoragepool.py
> +++ b/src/wok/plugins/kimchi/model/libvirtstoragepool.py
> @@ -144,11 +144,12 @@ class LogicalPoolDef(StoragePoolDef):
> pool = E.pool(type='logical')
> pool.append(E.name(self.poolArgs['name']))
>
> - source = E.source()
> - for device_path in self.poolArgs['source']['devices']:
> - source.append(E.device(path=device_path))
> + if not self.poolArgs['source'].get('from_vg', False):
> + source = E.source()
> + for device_path in self.poolArgs['source']['devices']:
> + source.append(E.device(path=device_path))
> + pool.append(source)
>
> - pool.append(source)
> pool.append(E.target(E.path(self.path)))
> return ET.tostring(pool, encoding='unicode', pretty_print=True)
>
> diff --git a/src/wok/plugins/kimchi/model/storagepools.py b/src/wok/plugins/kimchi/model/storagepools.py
> index cc0bc7c..ec866c5 100644
> --- a/src/wok/plugins/kimchi/model/storagepools.py
> +++ b/src/wok/plugins/kimchi/model/storagepools.py
> @@ -142,9 +142,27 @@ class StoragePoolsModel(object):
> raise OperationFailed("KCHPOOL0006E",
> {'err': e.get_error_message()})
>
> + def _check_lvm(self, name, from_vg):
> + vgdisplay_cmd = ['vgdisplay', name.encode('utf-8')]
> + output, error, returncode = run_command(vgdisplay_cmd)
> + # From vgdisplay error codes:
> + # 1 error reading VGDA
> + # 2 volume group doesn't exist
> + # 3 not all physical volumes of volume group online
> + # 4 volume group not found
> + # 5 no volume groups found at all
> + # 6 error reading VGDA from lvmtab
> + if from_vg and returncode in [2, 4, 5]:
> + raise InvalidOperation("KCHPOOL0038E", {'name': name})
> +
> + if not from_vg and returncode not in [2, 4, 5]:
> + raise InvalidOperation("KCHPOOL0036E", {'name': name})
> +
> def create(self, params):
> task_id = None
> conn = self.conn.get()
> + from_vg = params.get('source', {}).get('from_vg', False)
> +
> try:
> name = params['name']
> if name == ISO_POOL_NAME:
> @@ -154,17 +172,7 @@ class StoragePoolsModel(object):
> # used before but a volume group will already exist with this name
> # So check the volume group does not exist to create the pool
> if params['type'] == 'logical':
> - vgdisplay_cmd = ['vgdisplay', name.encode('utf-8')]
> - output, error, returncode = run_command(vgdisplay_cmd)
> - # From vgdisplay error codes:
> - # 1 error reading VGDA
> - # 2 volume group doesn't exist
> - # 3 not all physical volumes of volume group online
> - # 4 volume group not found
> - # 5 no volume groups found at all
> - # 6 error reading VGDA from lvmtab
> - if returncode not in [2, 4, 5]:
> - raise InvalidOperation("KCHPOOL0036E", {'name': name})
> + self._check_lvm(name, from_vg)
>
> if params['type'] == 'kimchi-iso':
> task_id = self._do_deep_scan(params)
> diff --git a/src/wok/plugins/kimchi/tests/test_mock_storagepool.py b/src/wok/plugins/kimchi/tests/test_mock_storagepool.py
> index ea9843b..a2609eb 100644
> --- a/src/wok/plugins/kimchi/tests/test_mock_storagepool.py
> +++ b/src/wok/plugins/kimchi/tests/test_mock_storagepool.py
> @@ -65,6 +65,10 @@ class MockStoragepoolTests(unittest.TestCase):
> )
>
> def test_storagepool(self):
> + # MockModel always returns 2 VGs (hostVG, kimchiVG)
> + vgs = json.loads(self.request('/plugins/kimchi/host/vgs').read())
> + vg_names = [vg['name'] for vg in vgs]
> +
> # MockModel always returns 2 partitions (vdx, vdz)
> partitions = json.loads(
> self.request('/plugins/kimchi/host/partitions').read()
> @@ -89,7 +93,9 @@ class MockStoragepoolTests(unittest.TestCase):
> 'source': {'host': '127.0.0.1',
> 'target': 'iqn.2015-01.localhost.kimchiUnitTest'}},
> {'type': 'logical', 'name': u'kīмсhīUnitTestLogicalPool',
> - 'source': {'devices': [devs[0]]}}]
> + 'source': {'devices': [devs[0]]}},
> + {'type': 'logical', 'name': vg_names[0],
> + 'source': {'from_vg': True}}]
>
> def _do_test(params):
> name = params['name']
More information about the Kimchi-devel
mailing list