[Kimchi-devel] [PATCH] Create new storage volume when attaching disk to a guest.
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Tue Nov 10 20:33:18 UTC 2015
I believe this new behavior deserves at least one new unit test
(or changing an existing one) to ensure that it works as expected
in the future.
All existing unit tests passed (= no new failures) in both x86 and ppc
running pkvm.
One more comment below:
On 11/10/2015 04:12 PM, pvital at linux.vnet.ibm.com wrote:
> From: Paulo Vital <pvital at linux.vnet.ibm.com>
>
> Add back-end support to create new storage volume (new virtual disk) when
> attaching disk to a guest created before.
>
> There are three essential parameters to create the new volume:
> * vol: Storage volume name of disk image, that should be 'new_vol'.
> * capacity: The total space which can be used to store new volumes.
> The unit is bytes.
> * format: The format of the defined Storage Volume. Only used when creating
> a storage volume with 'capacity'.
>
> Signed-off-by: Paulo Vital <pvital at linux.vnet.ibm.com>
> ---
> src/wok/plugins/kimchi/API.json | 12 ++++++++++++
> src/wok/plugins/kimchi/docs/API.md | 6 +++++-
> src/wok/plugins/kimchi/i18n.py | 1 +
> src/wok/plugins/kimchi/model/vmstorages.py | 28 +++++++++++++++++++++++++++-
> 4 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/src/wok/plugins/kimchi/API.json b/src/wok/plugins/kimchi/API.json
> index 961f35f..8822520 100644
> --- a/src/wok/plugins/kimchi/API.json
> +++ b/src/wok/plugins/kimchi/API.json
> @@ -564,6 +564,18 @@
> "type": "string",
> "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
> "error": "KCHVMSTOR0003E"
> + },
> + "capacity": {
> + "description": "The total size (MiB) of the storage volume",
> + "type": "number",
> + "minimum": 1,
> + "error": "KCHVOL0020E"
> + },
> + "format": {
> + "description": "The format of the volume",
> + "type": "string",
> + "pattern": "^(|bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$",
> + "error": "KCHVOL0015E"
> }
> }
> },
> diff --git a/src/wok/plugins/kimchi/docs/API.md b/src/wok/plugins/kimchi/docs/API.md
> index 52368b7..d00c2fe 100644
> --- a/src/wok/plugins/kimchi/docs/API.md
> +++ b/src/wok/plugins/kimchi/docs/API.md
> @@ -205,7 +205,11 @@ Represents a snapshot of the Virtual Machine's primary monitor.
> * type: The type of the storage (currently support 'cdrom' and 'disk').
> * path: Path of cdrom iso.
> * pool: Storage pool which disk image file locate in.
> - * vol: Storage volume name of disk image.
> + * vol: Storage volume name of disk image ('new_vol' for create a new volume).
> + * capacity: The total space which can be used to store new volumes.
> + The unit is bytes.
> + * format: The format of the defined Storage Volume. Only used when creating
> + a storage volume with 'capacity'.
>
> ### Sub-resource: storage
> **URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*
> diff --git a/src/wok/plugins/kimchi/i18n.py b/src/wok/plugins/kimchi/i18n.py
> index 42a5e16..53a0b6b 100644
> --- a/src/wok/plugins/kimchi/i18n.py
> +++ b/src/wok/plugins/kimchi/i18n.py
> @@ -278,6 +278,7 @@ messages = {
> "KCHVMSTOR0016E": _("Volume already in use by other virtual machine."),
> "KCHVMSTOR0017E": _("Only one of path or pool/volume can be specified to add a new virtual machine disk"),
> "KCHVMSTOR0018E": _("Volume chosen with format %(format)s does not fit in the storage type %(type)s"),
> + "KCHVMSTOR0019E": _("The format msust be used only when creating a new storage volume with 'capacity'."),
>
> "KCHSNAP0001E": _("Virtual machine '%(vm)s' must be stopped before creating a snapshot of it."),
> "KCHSNAP0002E": _("Unable to create snapshot '%(name)s' on virtual machine '%(vm)s'. Details: %(err)s"),
> diff --git a/src/wok/plugins/kimchi/model/vmstorages.py b/src/wok/plugins/kimchi/model/vmstorages.py
> index 23db0a6..f6b3918 100644
> --- a/src/wok/plugins/kimchi/model/vmstorages.py
> +++ b/src/wok/plugins/kimchi/model/vmstorages.py
> @@ -22,12 +22,14 @@ from lxml import etree
>
> from wok.exception import InvalidOperation, InvalidParameter, NotFoundError
> from wok.exception import OperationFailed
> +from wok.model.tasks import TaskModel
> from wok.utils import wok_log
>
> from wok.plugins.kimchi.model.config import CapabilitiesModel
> from wok.plugins.kimchi.model.diskutils import get_disk_used_by
> from wok.plugins.kimchi.model.diskutils import set_disk_used_by
> from wok.plugins.kimchi.model.storagevolumes import StorageVolumeModel
> +from wok.plugins.kimchi.model.storagevolumes import StorageVolumesModel
> from wok.plugins.kimchi.model.utils import get_vm_config_flag
> from wok.plugins.kimchi.model.vms import DOM_STATE_MAP, VMModel
> from wok.plugins.kimchi.osinfo import lookup
> @@ -51,6 +53,7 @@ class VMStoragesModel(object):
> self.conn = kargs['conn']
> self.objstore = kargs['objstore']
> self.caps = CapabilitiesModel(**kargs)
> + self.task = TaskModel(**kargs)
>
> def _get_available_bus_address(self, bus_type, vm_name):
> if bus_type not in ['ide']:
> @@ -85,9 +88,12 @@ class VMStoragesModel(object):
> if not ('vol' in params) ^ ('path' in params):
> raise InvalidParameter("KCHVMSTOR0017E")
>
> + if ('format' in params) and ('capacity' not in params):
> + raise InvalidParameter("KCHVMSTOR0019E")
> +
> dom = VMModel.get_vm(vm_name, self.conn)
> params['bus'] = _get_device_bus(params['type'], dom)
> - params['format'] = 'raw'
> + params['format'] = params['format'] if 'format' in params else 'raw'
Why not use dict.get() instead? It's simpler and produces the same effect:
params['format'] = params.get('format', 'raw')
>
> dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems()
> if bus == params['bus']]
> @@ -102,6 +108,26 @@ class VMStoragesModel(object):
> DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
> raise InvalidOperation('KCHVMSTOR0011E')
>
> + if params.get('capacity'):
> + # If 'capacity' is in the parameters a new storage volume will
> + # be created and allocated to the VM.
> + if params['vol'] == 'new_vol':
> + # Set the name of the volume as the same pattern used when
> + # creating a VM based on Template (VM UUID + disk index).
> + # Otherwise, use the name provided by the user.
> + params['vol'] = "%s-%s.img" % (dom.UUIDString(),
> + params['index'])
> + vol_info = dict(name=params['vol'],
> + type=params['type'],
> + capacity=params['capacity'],
> + allocation=params['capacity'],
> + format=params['format'],
> + )
> + storage_volumes = StorageVolumesModel(conn=self.conn,
> + objstore=self.objstore)
> + mytask = storage_volumes.create(params['pool'], vol_info)
> + self.task.wait(mytask['id'])
> +
> if params.get('vol'):
> try:
> pool = params['pool']
More information about the Kimchi-devel
mailing list