Comments below
On 11/11/2015 11:43 AM, pvital(a)linux.vnet.ibm.com wrote:
From: Paulo Vital <pvital(a)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(a)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 +++++++++++++++++++++++++++-
src/wok/plugins/kimchi/tests/test_model.py | 25 +++++++++++++++++++++++++
5 files changed, 70 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).
According to your code, below, it is possible to create a new disk image
only passing CAPACITY
Must change here, or the code.
+ * 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'.
Add MiB in the capacity
description
### 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'."),
Typo: must
"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..29cdc73 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.get('format', 'raw')
Is
there any reason to default format be different of QCOW2 ?
You could check the default value being used in Templates (template.conf)
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')
Here I see a issue. The condition to create a new volume is passing a
CAPACITY only.
What happens if 'vol' is not passed ? I think you are going to have
exception in [1] and [2]
I think you should check for vol= new_vol, then check the capacity and
issue a error, if capacity not there
+ if params.get('capacity'):
+ # If 'capacity' is in the parameters a new storage volume will
+ # be created and allocated to the VM.
[1]
+ 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'])
[2]
+ 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']
diff --git a/src/wok/plugins/kimchi/tests/test_model.py
b/src/wok/plugins/kimchi/tests/test_model.py
index fdbe3b4..8cc3f14 100644
--- a/src/wok/plugins/kimchi/tests/test_model.py
+++ b/src/wok/plugins/kimchi/tests/test_model.py
@@ -406,6 +406,19 @@ class ModelTests(unittest.TestCase):
self.assertEquals(expect_bus, disk_info['bus'])
return disk
+ def _attach_new_disk(expect_bus=modern_disk_bus):
+ disk_args = {"type": "disk",
+ "pool": pool,
+ "vol": 'new_vol',
+ "format": 'qcow2',
+ "capacity": 1024}
+ new_disk = inst.vmstorages_create(vm_name, disk_args)
+
+ # Check the bus type to be 'virtio'
+ disk_info = inst.vmstorage_lookup(vm_name, new_disk)
+ self.assertEquals(expect_bus, disk_info['bus'])
+ return new_disk
+
inst = model.Model(objstore_loc=self.tmp_store)
with RollbackContext() as rollback:
path = os.path.join(TMP_DIR, 'kimchi-images')
@@ -466,6 +479,18 @@ class ModelTests(unittest.TestCase):
self.assertEquals(u'disk', disk_info['type'])
inst.vmstorage_delete(vm_name, disk)
+ # Create a new volume when attaching a disks
+ inst.vm_start(vm_name)
+ prev_disk_count = len(inst.vmstorages_get_list(vm_name))
+ new_disk = _attach_new_disk()
+ curr_disk_count = len(inst.vmstorages_get_list(vm_name))
+ self.assertEquals(prev_disk_count+1, curr_disk_count)
+ # The number of disks must be the same after VM off.
Is it necessary to stop the VM to remove the disk ? Can I remove the
disk with the vm running ?
You could add one more disk and remove it with vm running.
+ inst.vm_poweroff(vm_name)
+ inst.vmstorage_delete(vm_name, new_disk)
+ after_disk_count = len(inst.vmstorages_get_list(vm_name))
+ self.assertEquals(prev_disk_count, after_disk_count)
+
# Specifying pool and path at same time will fail
disk_args = {"type": "disk",
"pool": pool,