[PATCH 0/5] CDROM eject support
From: Royce Lv <lvroyce@linux.vnet.ibm.com> This patchset added a new api of vm storage eject, also fix previous vmstorage not work for persistent configuration error. Royce Lv (5): Fix wrong create/update/delete flags for vmstorages Adding test case for updating flags Change doc and controllor to support cdrom eject Update model to support cdrom eject Add testcase for cdrom eject docs/API.md | 3 +++ src/kimchi/control/vm/storages.py | 1 + src/kimchi/model/vmstorages.py | 40 ++++++++++++++++++++++++++------------- tests/test_model.py | 10 ++++++++++ 4 files changed, 41 insertions(+), 13 deletions(-) -- 1.8.3.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> For vmstorages now, we always use 'CURRENT' flag, this will result in change not take effective when vm's reboot. Now use get_vm_config_flag probe the current status of vm, and change according to vm status. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index ecc20cf..6511515 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -32,6 +32,7 @@ from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError from kimchi.exception import OperationFailed from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.storagevolumes import StorageVolumeModel +from kimchi.model.utils import get_vm_config_flag from kimchi.utils import check_url_path from kimchi.osinfo import lookup from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list @@ -176,7 +177,7 @@ class VMStoragesModel(object): try: conn = self.conn.get() dom = conn.lookupByName(vm_name) - dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + dom.attachDeviceFlags(dev_xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) return params['dev'] @@ -224,7 +225,7 @@ class VMStorageModel(object): dom = conn.lookupByName(vm_name) disk = get_device_xml(dom, dev_name) dom.detachDeviceFlags(etree.tostring(disk), - libvirt.VIR_DOMAIN_AFFECT_CURRENT) + get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0010E", {'error': e.message}) @@ -239,7 +240,7 @@ class VMStorageModel(object): xml = _get_storage_xml(dev_info) try: - dom.updateDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) return dev_name -- 1.8.3.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/03/2014 04:20 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
For vmstorages now, we always use 'CURRENT' flag, this will result in change not take effective when vm's reboot. Now use get_vm_config_flag probe the current status of vm, and change according to vm status.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index ecc20cf..6511515 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -32,6 +32,7 @@ from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError from kimchi.exception import OperationFailed from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.storagevolumes import StorageVolumeModel +from kimchi.model.utils import get_vm_config_flag from kimchi.utils import check_url_path from kimchi.osinfo import lookup from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list @@ -176,7 +177,7 @@ class VMStoragesModel(object): try: conn = self.conn.get() dom = conn.lookupByName(vm_name) - dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + dom.attachDeviceFlags(dev_xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) return params['dev'] @@ -224,7 +225,7 @@ class VMStorageModel(object): dom = conn.lookupByName(vm_name) disk = get_device_xml(dom, dev_name) dom.detachDeviceFlags(etree.tostring(disk), - libvirt.VIR_DOMAIN_AFFECT_CURRENT) + get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0010E", {'error': e.message})
@@ -239,7 +240,7 @@ class VMStorageModel(object): xml = _get_storage_xml(dev_info)
try: - dom.updateDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) return dev_name
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add test case for flags change to make sure it works after vm powered off. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index e3dff95..b78b52e 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -247,9 +247,14 @@ class ModelTests(unittest.TestCase): # Hot plug a disk inst.vm_start(vm_name) disk = _attach_disk() + # VM disk still there after powered off + inst.vm_poweroff(vm_name) + disk_info = inst.vmstorage_lookup(vm_name, disk) + self.assertEquals(u'disk', disk_info['type']) inst.vmstorage_delete(vm_name, disk) # Hot plug 'ide' bus disk does not work + inst.vm_start(vm_name) self.assertRaises(InvalidOperation, _attach_disk, 'ide') inst.vm_poweroff(vm_name) -- 1.8.3.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/03/2014 04:20 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add test case for flags change to make sure it works after vm powered off.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tests/test_model.py b/tests/test_model.py index e3dff95..b78b52e 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -247,9 +247,14 @@ class ModelTests(unittest.TestCase): # Hot plug a disk inst.vm_start(vm_name) disk = _attach_disk() + # VM disk still there after powered off + inst.vm_poweroff(vm_name) + disk_info = inst.vmstorage_lookup(vm_name, disk) + self.assertEquals(u'disk', disk_info['type']) inst.vmstorage_delete(vm_name, disk)
# Hot plug 'ide' bus disk does not work + inst.vm_start(vm_name) self.assertRaises(InvalidOperation, _attach_disk, 'ide') inst.vm_poweroff(vm_name)
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add eject action to vmstorage actions, and add this action to doc. Instead of reuse 'update', make eject a single action to avoid empty path introduced confusion. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 3 +++ src/kimchi/control/vm/storages.py | 1 + 2 files changed, 4 insertions(+) diff --git a/docs/API.md b/docs/API.md index 9217a37..6c14806 100644 --- a/docs/API.md +++ b/docs/API.md @@ -155,6 +155,9 @@ Represents a snapshot of the Virtual Machine's primary monitor. * path: Path of cdrom iso. Can not be blank. Now just support cdrom type. * **DELETE**: Remove the storage. +**Actions (POST):** + +* eject: Eject cdrom from device. ### Collection: Templates diff --git a/src/kimchi/control/vm/storages.py b/src/kimchi/control/vm/storages.py index 984c4d2..a8a037a 100644 --- a/src/kimchi/control/vm/storages.py +++ b/src/kimchi/control/vm/storages.py @@ -39,6 +39,7 @@ class VMStorage(Resource): self.info = {} self.model_args = [self.vm, self.ident] self.uri_fmt = '/vms/%s/storages/%s' + self.eject = self.generate_action_handler('eject') self.update_params = ['path'] @property -- 1.8.3.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/03/2014 04:20 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add eject action to vmstorage actions, and add this action to doc. Instead of reuse 'update', make eject a single action to avoid empty path introduced confusion.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 3 +++ src/kimchi/control/vm/storages.py | 1 + 2 files changed, 4 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 9217a37..6c14806 100644 --- a/docs/API.md +++ b/docs/API.md @@ -155,6 +155,9 @@ Represents a snapshot of the Virtual Machine's primary monitor. * path: Path of cdrom iso. Can not be blank. Now just support cdrom type. * **DELETE**: Remove the storage.
+**Actions (POST):** + +* eject: Eject cdrom from device.
### Collection: Templates diff --git a/src/kimchi/control/vm/storages.py b/src/kimchi/control/vm/storages.py index 984c4d2..a8a037a 100644 --- a/src/kimchi/control/vm/storages.py +++ b/src/kimchi/control/vm/storages.py @@ -39,6 +39,7 @@ class VMStorage(Resource): self.info = {} self.model_args = [self.vm, self.ident] self.uri_fmt = '/vms/%s/storages/%s' + self.eject = self.generate_action_handler('eject') self.update_params = ['path']
@property
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Modify xml generation to adjust to cdrom eject change. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 6511515..354fc36 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -49,10 +49,22 @@ def _get_device_bus(dev_type, dom): return lookup(distro, version)[dev_type+'_bus'] -def _get_storage_xml(params): +def _get_storage_xml(params, ignore_source=False): src_type = params.get('src_type') disk = E.disk(type=src_type, device=params.get('type')) disk.append(E.driver(name='qemu', type=params['format'])) + + disk.append(E.target(dev=params.get('dev'), bus=params['bus'])) + if params.get('address'): + # ide disk target id is always '0' + disk.append(E.address( + type='drive', controller=params['address']['controller'], + bus=params['address']['bus'], target='0', + unit=params['address']['unit'])) + + if ignore_source: + return ET.tostring(disk) + # Working with url paths if src_type == 'network': output = urlparse.urlparse(params.get('path')) @@ -67,13 +79,6 @@ def _get_storage_xml(params): source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params.get('path')) disk.append(source) - disk.append(E.target(dev=params.get('dev'), bus=params['bus'])) - if params.get('address'): - # ide disk target id is always '0' - disk.append(E.address( - type='drive', controller=params['address']['controller'], - bus=params['address']['bus'], target='0', - unit=params['address']['unit'])) return ET.tostring(disk) @@ -230,17 +235,25 @@ class VMStorageModel(object): raise OperationFailed("KCHVMSTOR0010E", {'error': e.message}) def update(self, vm_name, dev_name, params): - params['src_type'] = _check_path(params['path']) + if params.get('path'): + params['src_type'] = _check_path(params['path']) + ignore_source = False + else: + params['src_type'] = 'file' + ignore_source = True dom = VMModel.get_vm(vm_name, self.conn) dev_info = self.lookup(vm_name, dev_name) if dev_info['type'] != 'cdrom': raise InvalidOperation("KCHVMSTOR0006E") dev_info.update(params) - xml = _get_storage_xml(dev_info) + xml = _get_storage_xml(dev_info, ignore_source) try: dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) return dev_name + + def eject(self, vm_name, dev_name): + return self.update(vm_name, dev_name, dict()) -- 1.8.3.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/03/2014 04:20 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Modify xml generation to adjust to cdrom eject change.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 6511515..354fc36 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -49,10 +49,22 @@ def _get_device_bus(dev_type, dom): return lookup(distro, version)[dev_type+'_bus']
-def _get_storage_xml(params): +def _get_storage_xml(params, ignore_source=False): src_type = params.get('src_type') disk = E.disk(type=src_type, device=params.get('type')) disk.append(E.driver(name='qemu', type=params['format'])) + + disk.append(E.target(dev=params.get('dev'), bus=params['bus'])) + if params.get('address'): + # ide disk target id is always '0' + disk.append(E.address( + type='drive', controller=params['address']['controller'], + bus=params['address']['bus'], target='0', + unit=params['address']['unit'])) + + if ignore_source: + return ET.tostring(disk) + # Working with url paths if src_type == 'network': output = urlparse.urlparse(params.get('path')) @@ -67,13 +79,6 @@ def _get_storage_xml(params): source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params.get('path')) disk.append(source)
- disk.append(E.target(dev=params.get('dev'), bus=params['bus'])) - if params.get('address'): - # ide disk target id is always '0' - disk.append(E.address( - type='drive', controller=params['address']['controller'], - bus=params['address']['bus'], target='0', - unit=params['address']['unit'])) return ET.tostring(disk)
@@ -230,17 +235,25 @@ class VMStorageModel(object): raise OperationFailed("KCHVMSTOR0010E", {'error': e.message})
def update(self, vm_name, dev_name, params): - params['src_type'] = _check_path(params['path']) + if params.get('path'): + params['src_type'] = _check_path(params['path']) + ignore_source = False + else: + params['src_type'] = 'file' + ignore_source = True dom = VMModel.get_vm(vm_name, self.conn)
dev_info = self.lookup(vm_name, dev_name) if dev_info['type'] != 'cdrom': raise InvalidOperation("KCHVMSTOR0006E") dev_info.update(params) - xml = _get_storage_xml(dev_info) + xml = _get_storage_xml(dev_info, ignore_source)
try: dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) return dev_name + + def eject(self, vm_name, dev_name): + return self.update(vm_name, dev_name, dict())
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add cdrom eject testcase to make sure it works properly. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index b78b52e..142e4fc 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -318,6 +318,11 @@ class ModelTests(unittest.TestCase): inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) self.assertEquals(iso_path, cdrom_info['path']) + + # eject cdrom + cdrom_dev = inst.vmstorage_eject(vm_name, cdrom_dev) + cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) + self.assertEquals('', cdrom_info['path']) inst.vm_poweroff(vm_name) # removing non existent cdrom -- 1.8.3.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/03/2014 04:20 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add cdrom eject testcase to make sure it works properly.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tests/test_model.py b/tests/test_model.py index b78b52e..142e4fc 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -318,6 +318,11 @@ class ModelTests(unittest.TestCase): inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) self.assertEquals(iso_path, cdrom_info['path']) + + # eject cdrom + cdrom_dev = inst.vmstorage_eject(vm_name, cdrom_dev) + cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) + self.assertEquals('', cdrom_info['path']) inst.vm_poweroff(vm_name)
# removing non existent cdrom
participants (2)
-
Aline Manera -
lvroyce＠linux.vnet.ibm.com