On 02/10/2014 12:17 AM, Sheldon wrote:
a minor comment below
On 02/07/2014 08:53 PM, Daniel Barboza wrote:
> From: Daniel Henrique Barboza<danielhb(a)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.
need to update API.json?
Good point.
I've just checked the API.json and there is no entry for
storagepool_update so it
will need to add a new entry for that
> Signed-off-by: Daniel Henrique
Barboza<danielhb(a)linux.vnet.ibm.com>
> ---
> src/kimchi/control/storagepools.py | 2 +-
> src/kimchi/model/storagepools.py | 35
> +++++++++++++++++++++++++++--------
> 2 files changed, 28 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..89cb61f 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,34 @@ class StoragePoolModel(object):
> pass
> return res
>
> + def _update_lvm_disks(self, pool_name, disks):
> + 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)
> + pool_refresh_cmd = ["virsh", "pool-refresh", pool_name]
src/kimchi/model/storagepools.py:
pool = self.get_storagepool(pool_name , self.conn)
pool.refresh(0)
> + output, error, returncode = run_command(pool_refresh_cmd)
> + if returncode != 0:
> + kimchi_log.error('Could not refresh pool %s using
> libvirt, '
> + 'error: %s', pool_name, error)
> + raise OperationFailed('Error while refreshing pool %s '
> + 'using libvirt.', pool_name)
> +
> 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'] not in [True, False]:
After updating the API.json you can remove the above verification as
jsonschema will care about it
> + raise InvalidOperation("Autostart flag must
be true
> or false")
> + if params['autostart']:
> + pool.setAutostart(1)
> + else:
> + pool.setAutostart(0)
do you need to check this pool is a logical pool here as you docs says.
Good point. =)
* disks: Adds one or more disks to the pool (for 'logical'
pool only)
> + if 'disks' in params:
> + self._update_lvm_disks(name, params['disks'])
> ident = pool.name()
> return ident
>