[PATCHv2 0/3] Fix vm creation on readonly pool type
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Some pool types (scsi, iscsi, mpath) do not allow create/delete storage volume, so it is wrong to delete these volumes when vm delete. Fix it by just delete non read only pool types. Royce Lv (3): Fix vm creation storage rollback clean Prevent volume create and delete for certain pool types Clear pep8 failure in make check src/kimchi/config.py.in | 3 +++ src/kimchi/control/base.py | 2 ++ src/kimchi/i18n.py | 1 + src/kimchi/isoinfo.py | 4 ++-- src/kimchi/model/storageservers.py | 4 ++-- src/kimchi/model/storagevolumes.py | 18 +++++++++++++++--- src/kimchi/model/vms.py | 14 ++++++-------- tests/test_model.py | 8 ++++---- 8 files changed, 35 insertions(+), 19 deletions(-) -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> For pools which not allowed create and delete volume, if create vm fails, vm storage volume should not be destoryed. Put the constant in config.py to avoid circular import. For storage volume deletion, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/config.py.in | 3 +++ src/kimchi/model/vms.py | 14 ++++++-------- tests/test_model.py | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 32d61c6..00aab27 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -35,6 +35,9 @@ from kimchi.xmlutils import xpath_get_text DEFAULT_LOG_LEVEL = "debug" +# Storage pool constant for read-only pool types +READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath'] + def get_session_path(): return os.path.join(paths.state_dir, 'sessions') diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..4744bb6 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -30,6 +30,7 @@ from cherrypy.process.plugins import BackgroundTask from kimchi import vnc from kimchi import xmlutils +from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -186,7 +187,7 @@ class VMsModel(object): # If storagepool is SCSI, volumes will be LUNs and must be passed by # the user from UI or manually. vol_list = [] - if t._get_storage_type() == 'scsi': + if t._get_storage_type() in READONLY_POOL_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,10 @@ class VMsModel(object): try: conn.defineXML(xml.encode('utf-8')) except libvirt.libvirtError as e: - for v in vol_list: - vol = conn.storageVolLookupByPath(v['path']) - vol.delete(0) + if t._get_storage_type() not in READONLY_POOL_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()}) @@ -357,10 +359,6 @@ class VMModel(object): dom.undefine() - for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True) diff --git a/tests/test_model.py b/tests/test_model.py index 0dbc279..b0039a3 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -245,7 +245,7 @@ class ModelTests(unittest.TestCase): vm_info = inst.vm_lookup(params['name']) disk_path = '/var/lib/libvirt/images/%s-0.img' % vm_info['uuid'] self.assertTrue(os.access(disk_path, os.F_OK)) - self.assertFalse(os.access(disk_path, os.F_OK)) + self.assertTrue(os.access(disk_path, os.F_OK)) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_storagepool(self): -- 1.8.1.2
On 02/25/2014 04:07 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
For pools which not allowed create and delete volume, if create vm fails, vm storage volume should not be destoryed. Put the constant in config.py to avoid circular import. For storage volume deletion, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/config.py.in | 3 +++ src/kimchi/model/vms.py | 14 ++++++-------- tests/test_model.py | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 32d61c6..00aab27 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -35,6 +35,9 @@ from kimchi.xmlutils import xpath_get_text
DEFAULT_LOG_LEVEL = "debug"
+# Storage pool constant for read-only pool types +READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath'] +
def get_session_path(): return os.path.join(paths.state_dir, 'sessions') diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..4744bb6 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -30,6 +30,7 @@ from cherrypy.process.plugins import BackgroundTask
from kimchi import vnc from kimchi import xmlutils +from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -186,7 +187,7 @@ class VMsModel(object): # If storagepool is SCSI, volumes will be LUNs and must be passed by # the user from UI or manually. vol_list = [] - if t._get_storage_type() == 'scsi': + if t._get_storage_type() in READONLY_POOL_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,10 @@ class VMsModel(object): try: conn.defineXML(xml.encode('utf-8')) except libvirt.libvirtError as e: - for v in vol_list: - vol = conn.storageVolLookupByPath(v['path']) - vol.delete(0) + if t._get_storage_type() not in READONLY_POOL_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()})
@@ -357,10 +359,6 @@ class VMModel(object):
dom.undefine()
- for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True)
diff --git a/tests/test_model.py b/tests/test_model.py index 0dbc279..b0039a3 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -245,7 +245,7 @@ class ModelTests(unittest.TestCase): vm_info = inst.vm_lookup(params['name']) disk_path = '/var/lib/libvirt/images/%s-0.img' % vm_info['uuid'] self.assertTrue(os.access(disk_path, os.F_OK)) - self.assertFalse(os.access(disk_path, os.F_OK)) + self.assertTrue(os.access(disk_path, os.F_OK))
I don't understand this change. After the rollback block the vm will be deleted and the disk_path should not exist anymore.
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_storagepool(self):
On 2014年02月25日 22:38, Aline Manera wrote:
> On 02/25/2014 04:07 AM, lvroyce@linux.vnet.ibm.com wrote:
>> From: Royce Lv <lvroyce@linux.vnet.ibm.com>
>>
>> For pools which not allowed create and delete volume,
>> if create vm fails, vm storage volume should not be destoryed.
>> Put the constant in config.py to avoid circular import.
>> For storage volume deletion, work around it by delete vol clear part,
>> in the future, this will be fixed by volume reference information
>> tracking.
>>
>> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com>
>> ---
>> src/kimchi/config.py.in | 3 +++
>> src/kimchi/model/vms.py | 14 ++++++--------
>> tests/test_model.py | 2 +-
>> 3 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>> index 32d61c6..00aab27 100644
>> --- a/src/kimchi/config.py.in
>> +++ b/src/kimchi/config.py.in
>> @@ -35,6 +35,9 @@ from kimchi.xmlutils import xpath_get_text
>>
>> DEFAULT_LOG_LEVEL = "debug"
>>
>> +# Storage pool constant for read-only pool types
>> +READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath']
>> +
>>
>> def get_session_path():
>> return os.path.join(paths.state_dir, 'sessions')
>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
>> index b482e80..4744bb6 100644
>> --- a/src/kimchi/model/vms.py
>> +++ b/src/kimchi/model/vms.py
>> @@ -30,6 +30,7 @@ from cherrypy.process.plugins import BackgroundTask
>>
>> from kimchi import vnc
>> from kimchi import xmlutils
>> +from kimchi.config import READONLY_POOL_TYPE
>> from kimchi.exception import InvalidOperation, InvalidParameter
>> from kimchi.exception import MissingParameter, NotFoundError,
>> OperationFailed
>> from kimchi.model.config import CapabilitiesModel
>> @@ -186,7 +187,7 @@ class VMsModel(object):
>> # If storagepool is SCSI, volumes will be LUNs and must be passed by
>> # the user from UI or manually.
>> vol_list = []
>> - if t._get_storage_type() == 'scsi':
>> + if t._get_storage_type() in READONLY_POOL_TYPE:
>> if not params.get('volumes'):
>> raise MissingParameter('KCHVM0017E')
>> else:
>> @@ -218,9 +219,10 @@ class VMsModel(object):
>> try:
>> conn.defineXML(xml.encode('utf-8'))
>> except libvirt.libvirtError as e:
>> - for v in vol_list:
>> - vol = conn.storageVolLookupByPath(v['path'])
>> - vol.delete(0)
>> + if t._get_storage_type() not in READONLY_POOL_TYPE:
>> + for v in vol_list:
>> + vol = conn.storageVolLookupByPath(v['path'])
>> + vol.delete(0)
>> raise OperationFailed("KCHVM0007E", {'name': name,
>> 'err': e.get_error_message()})
>>
>> @@ -357,10 +359,6 @@ class VMModel(object):
>>
>> dom.undefine()
>>
>> - for path in paths:
>> - vol = conn.storageVolLookupByPath(path)
>> - vol.delete(0)
>> -
>> with self.objstore as session:
>> session.delete('vm', dom.UUIDString(), ignore_missing=True)
>>
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index 0dbc279..b0039a3 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -245,7 +245,7 @@ class ModelTests(unittest.TestCase):
>> vm_info = inst.vm_lookup(params['name'])
>> disk_path = '/var/lib/libvirt/images/%s-0.img' % vm_info['uuid']
>> self.assertTrue(os.access(disk_path, os.F_OK))
>> - self.assertFalse(os.access(disk_path, os.F_OK))
>> + self.assertTrue(os.access(disk_path, os.F_OK))
>
> I don't understand this change.
> After the rollback block the vm will be deleted and the disk_path
> should not exist anymore. See lines up:
- for path in paths:
- vol = conn.storageVolLookupByPath(path)
- vol.delete(0)
-
When delete vm, clear the volume delete part, because if the volumes of
vm are based on scsi pool and we are trying to delete them, this will
result in error.
This is a workaround, as I said in commit msg:
this will be fixed by volume reference information tracking.
>
>>
>> @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>> def test_storagepool(self):
>
On 02/26/2014 09:11 AM, Royce Lv wrote:
> On 2014年02月25日 22:38, Aline Manera wrote:
>> On 02/25/2014 04:07 AM, lvroyce@linux.vnet.ibm.com wrote:
>>> From: Royce Lv <lvroyce@linux.vnet.ibm.com>
>>>
>>> For pools which not allowed create and delete volume,
>>> if create vm fails, vm storage volume should not be destoryed.
>>> Put the constant in config.py to avoid circular import.
>>> For storage volume deletion, work around it by delete vol clear part,
>>> in the future, this will be fixed by volume reference information
>>> tracking.
>>>
>>> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com>
>>> ---
>>> src/kimchi/config.py.in | 3 +++
>>> src/kimchi/model/vms.py | 14 ++++++--------
>>> tests/test_model.py | 2 +-
>>> 3 files changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>>> index 32d61c6..00aab27 100644
>>> --- a/src/kimchi/config.py.in
>>> +++ b/src/kimchi/config.py.in
>>> @@ -35,6 +35,9 @@ from kimchi.xmlutils import xpath_get_text
>>>
>>> DEFAULT_LOG_LEVEL = "debug"
>>>
>>> +# Storage pool constant for read-only pool types
>>> +READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath']
>>> +
>>>
>>> def get_session_path():
>>> return os.path.join(paths.state_dir, 'sessions')
>>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
>>> index b482e80..4744bb6 100644
>>> --- a/src/kimchi/model/vms.py
>>> +++ b/src/kimchi/model/vms.py
>>> @@ -30,6 +30,7 @@ from cherrypy.process.plugins import BackgroundTask
>>>
>>> from kimchi import vnc
>>> from kimchi import xmlutils
>>> +from kimchi.config import READONLY_POOL_TYPE
>>> from kimchi.exception import InvalidOperation, InvalidParameter
>>> from kimchi.exception import MissingParameter, NotFoundError,
>>> OperationFailed
>>> from kimchi.model.config import CapabilitiesModel
>>> @@ -186,7 +187,7 @@ class VMsModel(object):
>>> # If storagepool is SCSI, volumes will be LUNs and must be passed by
>>> # the user from UI or manually.
>>> vol_list = []
>>> - if t._get_storage_type() == 'scsi':
>>> + if t._get_storage_type() in READONLY_POOL_TYPE:
>>> if not params.get('volumes'):
>>> raise MissingParameter('KCHVM0017E')
>>> else:
>>> @@ -218,9 +219,10 @@ class VMsModel(object):
>>> try:
>>> conn.defineXML(xml.encode('utf-8'))
>>> except libvirt.libvirtError as e:
>>> - for v in vol_list:
>>> - vol = conn.storageVolLookupByPath(v['path'])
>>> - vol.delete(0)
>>> + if t._get_storage_type() not in READONLY_POOL_TYPE:
>>> + for v in vol_list:
>>> + vol = conn.storageVolLookupByPath(v['path'])
>>> + vol.delete(0)
>>> raise OperationFailed("KCHVM0007E", {'name': name,
>>> 'err': e.get_error_message()})
>>>
>>> @@ -357,10 +359,6 @@ class VMModel(object):
>>>
>>> dom.undefine()
>>>
>>> - for path in paths:
>>> - vol = conn.storageVolLookupByPath(path)
>>> - vol.delete(0)
>>> -
>>> with self.objstore as session:
>>> session.delete('vm', dom.UUIDString(), ignore_missing=True)
>>>
>>> diff --git a/tests/test_model.py b/tests/test_model.py
>>> index 0dbc279..b0039a3 100644
>>> --- a/tests/test_model.py
>>> +++ b/tests/test_model.py
>>> @@ -245,7 +245,7 @@ class ModelTests(unittest.TestCase):
>>> vm_info = inst.vm_lookup(params['name'])
>>> disk_path = '/var/lib/libvirt/images/%s-0.img' % vm_info['uuid']
>>> self.assertTrue(os.access(disk_path, os.F_OK))
>>> - self.assertFalse(os.access(disk_path, os.F_OK))
>>> + self.assertTrue(os.access(disk_path, os.F_OK))
>>
>> I don't understand this change.
>> After the rollback block the vm will be deleted and the disk_path
>> should not exist anymore. See lines up:
> - for path in paths:
> - vol = conn.storageVolLookupByPath(path)
> - vol.delete(0)
> -
> When delete vm, clear the volume delete part, because if the volumes
> of vm are based on scsi pool and we are trying to delete them, this
> will result in error.
> This is a workaround, as I said in commit msg:
> this will be fixed by volume reference information tracking.
Don't you only remove this part only for READONLY_POOL_TYPE?
When storage pool is a dir, we should keep deleting the storage volume
while deleting the vm
>>
>>>
>>> @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>>> def test_storagepool(self):
>>
>
From: Royce Lv <lvroyce@linux.vnet.ibm.com> As iscsi, scsi, mpath pools do not support create/delete storage volumes, when creating and deleting volumes, operation on these pool types should be eleminated. REF: http://libvirt.org/storage.html Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 2 ++ src/kimchi/i18n.py | 1 + src/kimchi/model/storagevolumes.py | 18 +++++++++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index e8f03ae..300981c 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -130,6 +130,8 @@ class Resource(object): return self.delete() except NotFoundError, e: raise cherrypy.HTTPError(404, e.message) + except InvalidParameter, e: + raise cherrypy.HTTPError(400, e.message) elif method == 'PUT': try: return self.update() diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 7d08254..03ca3cb 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -145,6 +145,7 @@ messages = { "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), + "KCHVOL0012E": _("Storage type %(type)s does not support volume create and delete"), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 20c65b9..6beccfb 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -25,7 +25,8 @@ import os import libvirt from kimchi import xmlutils -from kimchi.exception import InvalidOperation, IsoFormatError +from kimchi.config import READONLY_POOL_TYPE +from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.isoinfo import IsoImage from kimchi.model.storagepools import StoragePoolModel @@ -40,8 +41,9 @@ VOLUME_TYPE_MAP = {0: 'file', class StorageVolumesModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.objstore = kargs['objstore'] - def create(self, pool, params): + def create(self, pool_name, params): vol_xml = """ <volume> <name>%(name)s</name> @@ -59,12 +61,16 @@ class StorageVolumesModel(object): name = params['name'] try: - pool = StoragePoolModel.get_storagepool(pool, self.conn) + pool = StoragePoolModel.get_storagepool(pool_name, self.conn) xml = vol_xml % params except KeyError, item: raise MissingParameter("KCHVOL0004E", {'item': item, 'volume': name}) + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool_name) + if pool_info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) try: pool.createXML(xml, 0) except libvirt.libvirtError as e: @@ -89,6 +95,7 @@ class StorageVolumesModel(object): class StorageVolumeModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.objstore = kargs['objstore'] def _get_storagevolume(self, pool, name): pool = StoragePoolModel.get_storagepool(pool, self.conn) @@ -139,6 +146,11 @@ class StorageVolumeModel(object): {'name': name, 'err': e.get_error_message()}) def delete(self, pool, name): + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool) + if pool_info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) + volume = self._get_storagevolume(pool, name) try: volume.delete(0) -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> isoinfo.py, storageservers.py and test_model.py fail in make check because of pep8, fix it. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/isoinfo.py | 4 ++-- src/kimchi/model/storageservers.py | 4 ++-- tests/test_model.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 5629391..4d4578f 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -186,7 +186,7 @@ class IsoImage(object): for i in xrange(1, 4): fmt = IsoImage.EL_TORITO_BOOT_RECORD ptr = i * IsoImage.SECTOR_SIZE - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] if len(tmp_data) < fmt.size: return @@ -215,7 +215,7 @@ class IsoImage(object): {'filename': self.path}) fmt = IsoImage.EL_TORITO_BOOT_ENTRY - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] (boot, media_type, load_seg, sys_type, pad0, sectors, load_rba) = self._unpack(fmt, tmp_data) if boot == 0x88: diff --git a/src/kimchi/model/storageservers.py b/src/kimchi/model/storageservers.py index 26e1f6f..26d53f3 100644 --- a/src/kimchi/model/storageservers.py +++ b/src/kimchi/model/storageservers.py @@ -67,8 +67,8 @@ class StorageServerModel(object): for pool in pools: try: pool_info = self.pool.lookup(pool) - if pool_info['source'] and \ - pool_info['source']['addr'] == server: + if (pool_info['source'] and + pool_info['source']['addr'] == server): return dict(host=server) except NotFoundError: # Avoid inconsistent pool result because of lease between list diff --git a/tests/test_model.py b/tests/test_model.py index b0039a3..10c6d57 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -921,7 +921,7 @@ class ModelTests(unittest.TestCase): for repo in test_repos: inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+len(test_repos), len(host_repos)) + self.assertEquals(system_host_repos + len(test_repos), len(host_repos)) for repo in test_repos: repo_info = inst.repository_lookup(repo.get('repo_id')) @@ -959,7 +959,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos)) new_repo = {'repo_id': 'fedora-fake', 'repo_name': 'Fedora 19 Update FAKE', @@ -989,7 +989,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos)) repo_info = inst.repository_lookup(repo.get('repo_id')) self.assertEquals(True, repo_info.get('enabled')) -- 1.8.1.2
pep8 1.3.3 can checkout these error, but pep8 1.4.6 cannot. Refer to pep8 doc, these are errors. so fix them. On 2014年02月25日 15:07, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
isoinfo.py, storageservers.py and test_model.py fail in make check because of pep8, fix it.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/isoinfo.py | 4 ++-- src/kimchi/model/storageservers.py | 4 ++-- tests/test_model.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 5629391..4d4578f 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -186,7 +186,7 @@ class IsoImage(object): for i in xrange(1, 4): fmt = IsoImage.EL_TORITO_BOOT_RECORD ptr = i * IsoImage.SECTOR_SIZE - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] if len(tmp_data) < fmt.size: return
@@ -215,7 +215,7 @@ class IsoImage(object): {'filename': self.path})
fmt = IsoImage.EL_TORITO_BOOT_ENTRY - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] (boot, media_type, load_seg, sys_type, pad0, sectors, load_rba) = self._unpack(fmt, tmp_data) if boot == 0x88: diff --git a/src/kimchi/model/storageservers.py b/src/kimchi/model/storageservers.py index 26e1f6f..26d53f3 100644 --- a/src/kimchi/model/storageservers.py +++ b/src/kimchi/model/storageservers.py @@ -67,8 +67,8 @@ class StorageServerModel(object): for pool in pools: try: pool_info = self.pool.lookup(pool) - if pool_info['source'] and \ - pool_info['source']['addr'] == server: + if (pool_info['source'] and + pool_info['source']['addr'] == server): return dict(host=server) except NotFoundError: # Avoid inconsistent pool result because of lease between list diff --git a/tests/test_model.py b/tests/test_model.py index b0039a3..10c6d57 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -921,7 +921,7 @@ class ModelTests(unittest.TestCase): for repo in test_repos: inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+len(test_repos), len(host_repos)) + self.assertEquals(system_host_repos + len(test_repos), len(host_repos))
for repo in test_repos: repo_info = inst.repository_lookup(repo.get('repo_id')) @@ -959,7 +959,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo)
host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos))
new_repo = {'repo_id': 'fedora-fake', 'repo_name': 'Fedora 19 Update FAKE', @@ -989,7 +989,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo)
host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos))
repo_info = inst.repository_lookup(repo.get('repo_id')) self.assertEquals(True, repo_info.get('enabled'))
participants (3)
-
Aline Manera -
lvroyce＠linux.vnet.ibm.com -
Royce Lv