[PATCH v2 0/4] Add disks to LVM pool backend

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Changes in v2: - added API.json parameter verification - added disk verification - added mockmodel improvements - other improvements based on feedback This patch series implements the backend changes to allow disks/partitions to be added to an existing LVM (logical) pool. The process, as discussed previously in kimchi mailing list, is to use 'vgextend' and 'virsh pool-refresh' commands to add the disks and then refresh the pool. It is also worth noticing that the existing data from the disks/partitions being added to the pool will be erased/lost. To test the patch: $ curl -u root -H 'Content-type: application/json' -H 'Accept: application/json' -X PUT localhost:8000/storagepools/<pool_name> -d '{"disks": ["/path/to/disk1", "/path/to/disk2"] }' Tip: to test the patch over and over with the same pool and disks without the need to destroy/create the pool: $ vgreduce <pool_name> disk1 disk2 $ virsh pool-refresh <pool_name> The above commands will restore the logical pool to its previous state, before adding disks 'disk1' and 'disk2' Daniel Henrique Barboza (4): Add disks to LVM pool: control and model changes Add disks to LVM pool: API.md changes Add disks to LVM pool: mockmodel changes Add disks to LVM pool: API.json changes docs/API.md | 5 ++++- src/kimchi/API.json | 16 +++++++++++++ src/kimchi/control/storagepools.py | 2 +- src/kimchi/mockmodel.py | 36 +++++++++++++++++++++++------ src/kimchi/model/storagepools.py | 46 +++++++++++++++++++++++++++++++------- 5 files changed, 88 insertions(+), 17 deletions(-) -- 1.8.3.1

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Added a new update parameter to storagepool resource. Changed the logic in the model to accept any update parameter ('autostart' or 'disks'). 'disks' parameter will add disks/partitions to the target LVM pool. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/control/storagepools.py | 2 +- src/kimchi/model/storagepools.py | 46 +++++++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index ae5185c..af10acd 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -80,7 +80,7 @@ class StoragePools(Collection): class StoragePool(Resource): def __init__(self, model, ident): super(StoragePool, self).__init__(model, ident) - self.update_params = ["autostart"] + self.update_params = ["autostart", "disks"] self.uri_fmt = "/storagepools/%s" self.activate = self.generate_action_handler('activate') self.deactivate = self.generate_action_handler('deactivate') diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 233a8a7..067311f 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -28,7 +28,7 @@ from kimchi.exception import InvalidOperation, MissingParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.libvirtstoragepool import StoragePoolDef from kimchi.utils import add_task, kimchi_log - +from kimchi.utils import run_command ISO_POOL_NAME = u'kimchi_isos' POOL_STATE_MAP = {0: 'inactive', @@ -200,15 +200,45 @@ class StoragePoolModel(object): pass return res + def _update_lvm_disks(self, pool_name, disks): + # check if all the disks/partitions exists in the host + for disk in disks: + blkid_cmd = ['blkid', disk] + output, error, returncode = run_command(blkid_cmd) + if returncode != 0: + kimchi_log.error('%s is not a valid disk/partition. Could not ' + 'add it to the pool %s.', disk, pool_name) + raise OperationFailed('%s is not a valid disk/partition. ' + 'Could not add it to the pool %s.', disk, + pool_name) + # add disks to the lvm pool using vgextend + virsh refresh + vgextend_cmd = ["vgextend", pool_name] + vgextend_cmd += disks + output, error, returncode = run_command(vgextend_cmd) + if returncode != 0: + kimchi_log.error('Could not add disks to pool %s, ' + 'error: %s', pool_name, error) + raise OperationFailed('Error while adding disks to pool %s.', + pool_name) + # refreshing pool state + pool = self.get_storagepool(pool_name, self.conn) + pool.refresh(0) + def update(self, name, params): - autostart = params['autostart'] - if autostart not in [True, False]: - raise InvalidOperation("Autostart flag must be true or false") pool = self.get_storagepool(name, self.conn) - if autostart: - pool.setAutostart(1) - else: - pool.setAutostart(0) + if 'autostart' in params: + if params['autostart']: + pool.setAutostart(1) + else: + pool.setAutostart(0) + if 'disks' in params: + # check if pool is type 'logical' + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] + if pool_type != 'logical': + raise InvalidOperation("Operation available only for " + "'logical' type storage pool.") + self._update_lvm_disks(name, params['disks']) ident = pool.name() return ident -- 1.8.3.1

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 07:52 PM, Daniel Barboza wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Added a new update parameter to storagepool resource. Changed the logic in the model to accept any update parameter ('autostart' or 'disks'). 'disks' parameter will add disks/partitions to the target LVM pool.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/control/storagepools.py | 2 +- src/kimchi/model/storagepools.py | 46 +++++++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index ae5185c..af10acd 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -80,7 +80,7 @@ class StoragePools(Collection): class StoragePool(Resource): def __init__(self, model, ident): super(StoragePool, self).__init__(model, ident) - self.update_params = ["autostart"] + self.update_params = ["autostart", "disks"] self.uri_fmt = "/storagepools/%s" self.activate = self.generate_action_handler('activate') self.deactivate = self.generate_action_handler('deactivate') diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 233a8a7..067311f 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -28,7 +28,7 @@ from kimchi.exception import InvalidOperation, MissingParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.libvirtstoragepool import StoragePoolDef from kimchi.utils import add_task, kimchi_log - +from kimchi.utils import run_command
ISO_POOL_NAME = u'kimchi_isos' POOL_STATE_MAP = {0: 'inactive', @@ -200,15 +200,45 @@ class StoragePoolModel(object): pass return res
+ def _update_lvm_disks(self, pool_name, disks): + # check if all the disks/partitions exists in the host + for disk in disks: + blkid_cmd = ['blkid', disk] + output, error, returncode = run_command(blkid_cmd) + if returncode != 0: + kimchi_log.error('%s is not a valid disk/partition. Could not ' + 'add it to the pool %s.', disk, pool_name) + raise OperationFailed('%s is not a valid disk/partition. ' + 'Could not add it to the pool %s.', disk, + pool_name) + # add disks to the lvm pool using vgextend + virsh refresh + vgextend_cmd = ["vgextend", pool_name] + vgextend_cmd += disks + output, error, returncode = run_command(vgextend_cmd) + if returncode != 0: + kimchi_log.error('Could not add disks to pool %s, ' + 'error: %s', pool_name, error) + raise OperationFailed('Error while adding disks to pool %s.', + pool_name) + # refreshing pool state + pool = self.get_storagepool(pool_name, self.conn) + pool.refresh(0) + def update(self, name, params): - autostart = params['autostart'] - if autostart not in [True, False]: - raise InvalidOperation("Autostart flag must be true or false") pool = self.get_storagepool(name, self.conn) - if autostart: - pool.setAutostart(1) - else: - pool.setAutostart(0) + if 'autostart' in params: + if params['autostart']: + pool.setAutostart(1) + else: + pool.setAutostart(0) + if 'disks' in params: + # check if pool is type 'logical' + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] + if pool_type != 'logical': + raise InvalidOperation("Operation available only for " + "'logical' type storage pool.") + self._update_lvm_disks(name, params['disks']) ident = pool.name() return ident

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Adding documentation about the new 'disks' update parameter, pool type 'logical' only. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index 580728c..48a293f 100644 --- a/docs/API.md +++ b/docs/API.md @@ -319,7 +319,10 @@ A interface represents available network interface on VM. * path: export path of this storage pool(for 'netfs' pool) * **PUT**: Set whether the Storage Pool should be enabled automatically when the system boots - * autostart: Toggle the autostart flag of the VM + * autostart: Toggle the autostart flag of the VM. This flag sets whether + the Storage Pool should be enabled automatically when the + system boots + * disks: Adds one or more disks to the pool (for 'logical' pool only) * **DELETE**: Remove the Storage Pool * **POST**: *See Storage Pool Actions* -- 1.8.3.1

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 07:52 PM, Daniel Barboza wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Adding documentation about the new 'disks' update parameter, pool type 'logical' only.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/docs/API.md b/docs/API.md index 580728c..48a293f 100644 --- a/docs/API.md +++ b/docs/API.md @@ -319,7 +319,10 @@ A interface represents available network interface on VM. * path: export path of this storage pool(for 'netfs' pool) * **PUT**: Set whether the Storage Pool should be enabled automatically when the system boots - * autostart: Toggle the autostart flag of the VM + * autostart: Toggle the autostart flag of the VM. This flag sets whether + the Storage Pool should be enabled automatically when the + system boots + * disks: Adds one or more disks to the pool (for 'logical' pool only) * **DELETE**: Remove the Storage Pool * **POST**: *See Storage Pool Actions*

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Eliminated the parameter validation that is now being done in API.json. To simulate the operation of add disks in the mockmodel, for each disk (which is validated with real data from the host), for each new disk added a fixed amount is added in the 'capacity' and 'available' fields on the pool. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 4e276eb..441c0e4 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -52,7 +52,7 @@ from kimchi.model.utils import get_vm_name from kimchi.model.vms import VM_STATIC_UPDATE_PARAMS from kimchi.objectstore import ObjectStore from kimchi.screenshot import VMScreenshot -from kimchi.utils import template_name_from_uri, pool_name_from_uri +from kimchi.utils import pool_name_from_uri, run_command, template_name_from_uri from kimchi.vmtemplate import VMTemplate @@ -315,13 +315,35 @@ class MockModel(object): storagepool.refresh() return storagepool.info + def _update_lvm_disks(self, pool_name, disks): + pool = self._get_storagepool(pool_name) + # check if all the disks/partitions exists in the host + for disk in disks: + blkid_cmd = ['blkid', disk] + output, error, returncode = run_command(blkid_cmd) + if returncode != 0: + raise OperationFailed('%s is not a valid disk/partition. ' + 'Could not add it to the pool %s.', disk, + pool_name) + # Adding disks to the lvm pool by adding extra capacity. + # Fixed amount for each disk present based in the + # values used in MockStoragePool. + capacity_per_disk = 1024 << 20 + capacity_added = capacity_per_disk * len(disks) + pool.info['capacity'] += capacity_added + pool.info['available'] += capacity_added + def storagepool_update(self, name, params): - autostart = params['autostart'] - if autostart not in [True, False]: - raise InvalidOperation("Autostart flag must be true or false") - storagepool = self._get_storagepool(name) - storagepool.info['autostart'] = autostart - ident = storagepool.name + pool = self._get_storagepool(name) + if 'autostart' in params: + pool.info['autostart'] = params['autostart'] + if 'disks' in params: + # check if pool is type 'logical' + if pool.info['type'] != 'logical': + raise InvalidOperation("Operation available only for " + "'logical' type storage pool.") + self._update_lvm_disks(name, params['disks']) + ident = pool.name return ident def storagepool_activate(self, name): -- 1.8.3.1

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 07:52 PM, Daniel Barboza wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Eliminated the parameter validation that is now being done in API.json.
To simulate the operation of add disks in the mockmodel, for each disk (which is validated with real data from the host), for each new disk added a fixed amount is added in the 'capacity' and 'available' fields on the pool.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 4e276eb..441c0e4 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -52,7 +52,7 @@ from kimchi.model.utils import get_vm_name from kimchi.model.vms import VM_STATIC_UPDATE_PARAMS from kimchi.objectstore import ObjectStore from kimchi.screenshot import VMScreenshot -from kimchi.utils import template_name_from_uri, pool_name_from_uri +from kimchi.utils import pool_name_from_uri, run_command, template_name_from_uri from kimchi.vmtemplate import VMTemplate
@@ -315,13 +315,35 @@ class MockModel(object): storagepool.refresh() return storagepool.info
+ def _update_lvm_disks(self, pool_name, disks): + pool = self._get_storagepool(pool_name) + # check if all the disks/partitions exists in the host + for disk in disks: + blkid_cmd = ['blkid', disk] + output, error, returncode = run_command(blkid_cmd) + if returncode != 0: + raise OperationFailed('%s is not a valid disk/partition. ' + 'Could not add it to the pool %s.', disk, + pool_name) + # Adding disks to the lvm pool by adding extra capacity. + # Fixed amount for each disk present based in the + # values used in MockStoragePool. + capacity_per_disk = 1024 << 20 + capacity_added = capacity_per_disk * len(disks) + pool.info['capacity'] += capacity_added + pool.info['available'] += capacity_added + def storagepool_update(self, name, params): - autostart = params['autostart'] - if autostart not in [True, False]: - raise InvalidOperation("Autostart flag must be true or false") - storagepool = self._get_storagepool(name) - storagepool.info['autostart'] = autostart - ident = storagepool.name + pool = self._get_storagepool(name) + if 'autostart' in params: + pool.info['autostart'] = params['autostart'] + if 'disks' in params: + # check if pool is type 'logical' + if pool.info['type'] != 'logical': + raise InvalidOperation("Operation available only for " + "'logical' type storage pool.") + self._update_lvm_disks(name, params['disks']) + ident = pool.name return ident
def storagepool_activate(self, name):

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds a new section 'storagepool_update' in API.json in order to remove parameter validation inside the model. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/API.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 08c77c5..f025661 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -94,6 +94,22 @@ } } }, + "storagepool_update": { + "type": "object", + "properties": { + "autostart": { + "description": "Set autostart value of the pool", + "type": "boolean" + }, + "disks": { + "description": "List of disks/partitions to be added", + "type": "array", + "items": { "type": "string" }, + "minItems": 1, + "uniqueItems": true + } + } + }, "vms_create": { "type": "object", "properties": { -- 1.8.3.1

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 07:52 PM, Daniel Barboza wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds a new section 'storagepool_update' in API.json in order to remove parameter validation inside the model.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/kimchi/API.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 08c77c5..f025661 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -94,6 +94,22 @@ } } }, + "storagepool_update": { + "type": "object", + "properties": { + "autostart": { + "description": "Set autostart value of the pool", + "type": "boolean" + }, + "disks": { + "description": "List of disks/partitions to be added", + "type": "array", + "items": { "type": "string" }, + "minItems": 1, + "uniqueItems": true + } + } + }, "vms_create": { "type": "object", "properties": {
participants (2)
-
Aline Manera
-
Daniel Barboza