[Kimchi-devel] [PATCH V2 3/7] (WIP) Storagepool SCSI/FC: Assign SCSI fibre channel LUN as disk to a new guest

Rodrigo Trujillo rodrigo.trujillo at linux.vnet.ibm.com
Tue Jan 28 21:19:02 UTC 2014


On 01/25/2014 09:08 PM, Aline Manera wrote:
> On 01/23/2014 10:29 PM, Rodrigo Trujillo wrote:
>> This patch implements basic routines to add a disk (scsi) to a new
>> vm template. At this moment, the LUN that will be assigned to guest
>> template will be the first and with more space found.
>
> Which LUN used to assigned to a VM should be provided by user.
> As you suggested in RFC patches, we will show the user the LUNs and he 
> will select which one he want to assign to vm
The LUN can and cannot be passed through the JSON  API when creating the 
VM, if not passed, a LUN will be selected automatically
>
>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>> ---
>>   src/kimchi/model.py      | 24 ++++++++++++++++++++++++
>>   src/kimchi/vmtemplate.py | 40 +++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
>> index 0eb8cc4..552f69d 100644
>> --- a/src/kimchi/model.py
>> +++ b/src/kimchi/model.py
>> @@ -1534,9 +1534,33 @@ class LibvirtVMTemplate(VMTemplate):
>>           xml = pool.XMLDesc(0)
>>           return xmlutils.xpath_get_text(xml, "/pool/target/path")[0]
>>
>> +    def _get_storage_type(self):
>> +        pool = self._storage_validate()
>> +        xml = pool.XMLDesc(0)
>> +        return xmlutils.xpath_get_text(xml, "/pool/@type")[0]
>> +
>
>> +    def _get_available_volumes(self):
>> +        pool = self._storage_validate()
>> +        vol_names = pool.listVolumes()
>> +        res = []
>> +        for vol in vol_names:
>> +            # Considering a volume as available if there is not any 
>> format type
>> +            xml = pool.storageVolLookupByName(vol).XMLDesc(0)
>> +            if (xmlutils.xpath_get_text(xml, 
>> "/volume/target/format/@type")[0]
>> +            == 'unknown'):
>> +                res.append((
>> +                xmlutils.xpath_get_text(xml, "/volume/target/path")[0],
>> +                int (xmlutils.xpath_get_text(xml,
>> +                    "/volume/capacity")[0]) / 1024**3 )
>> +                )
>> +        res.sort(key=lambda x: x[1])
>> +        return res
>> +
>
> Not sure we need this. The user may want to assign an existing volume 
> to a VM.
> We should show all of them and he choice what he want to use.
This only happens when user has UI.  I left the option to not pass any 
lun, when consuming the API
strictly, so higher LUN will be selected automatically


>
>>       def fork_vm_storage(self, vm_uuid):
>>           # Provision storage:
>>           # TODO: Rebase on the storage API once upstream
>
>> +        if self._get_storage_type() == 'scsi':
>> +            return []
>
> You should not call fork_vm_storage() in that case.
> Move this condition to before it.

ACK

>
>>           pool = self._storage_validate()
>>           vol_list = self.to_volume_list(vm_uuid)
>>           for v in vol_list:
>> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
>> index 58147e3..d6f807a 100644
>> --- a/src/kimchi/vmtemplate.py
>> +++ b/src/kimchi/vmtemplate.py
>> @@ -31,6 +31,7 @@ from kimchi import isoinfo
>>   from kimchi import osinfo
>>   from kimchi.exception import InvalidParameter, IsoFormatError
>>   from kimchi.isoinfo import IsoImage
>> +from kimchi.utils import is_libvirt_version_lesser
>>
>>
>>   QEMU_NAMESPACE = 
>> "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'"
>> @@ -180,6 +181,32 @@ class VMTemplate(object):
>>               graphics_xml = graphics_xml + spicevmc_xml
>>           return graphics_xml
>>
>> +    def _get_scsi_disks_xml(self):
>> +        ret = ""
>> +        # Passthrough configuration
>> +        disk_xml = """
>> +            <disk type='volume' device='lun'>
>> +              <driver name='qemu' type='raw'/>
>> +              <source dev='%(src)s'/>
>> +              <target dev='%(dev)s' bus='scsi'/>
>> +            </disk>"""
>> +        if is_libvirt_version_lesser('1.0.5'):
>> +            disk_xml = disk_xml.replace('volume','block')
>> +
>
>> +        luns = self._get_available_volumes()
>> +        for i, disk in enumerate(self.info['disks']):
>> +            index = disk.get('index', i)
>> +            try:
>> +                # Get luns with more space firstly
>> +                src = luns.pop()[0]
>> +            except IndexError:
>> +                # Skip if there is not available luns
>> +                continue
>
> That info is provided by user.
>
> It should be:
>
> def _get_scsi_vol_xml(self, vol)

The volume information provided by user is being modified in patch [6/7]
The xml provides a DISK, just followed previous functions pattern.

>
>> +            dev = "sd%s" % string.lowercase[index]
>> +            params = {'src': src, 'dev': dev}
>> +            ret = ret + disk_xml % params
>> +        return ret
>> +
>>       def to_volume_list(self, vm_uuid):
>>           storage_path = self._get_storage_path()
>>           ret = []
>> @@ -225,7 +252,6 @@ class VMTemplate(object):
>>           params = dict(self.info)
>>           params['name'] = vm_name
>>           params['uuid'] = vm_uuid
>> -        params['disks'] = self._get_disks_xml(vm_uuid)
>>           params['networks'] = self._get_networks_xml()
>>           params['qemu-namespace'] = ''
>>           params['cdroms'] = ''
>> @@ -233,6 +259,12 @@ class VMTemplate(object):
>>           graphics = kwargs.get('graphics')
>>           params['graphics'] = self._get_graphics_xml(graphics)
>
>> +        # SCSI FC disk are different
>> +        if self._get_storage_type() == 'scsi':
>> +            params['disks'] = self._get_scsi_disks_xml()
>> +        else:
>> +            params['disks'] = self._get_disks_xml(vm_uuid)
>> +
>
> Then my vm only can have scsi volumes or file ones? never both?
At this moment, yes, as Kimchi only supports 1 disk.

>
> You can add a 'type' key to 'disks'
>
> So we can have:
>
> disks = [{index: 0, size: 10, volume: "/home/alinefm/my-disk.img", 
> type: file}, {index:1, size: 0, volume: 'unit-0-0-1', type: lun}]
>
> Then _get_disks_xml() get the correct xml according to file

I think this is another scope and another feature: "Support multiple disks".
I would love code this, but not in the scope of this patch

>
>>           qemu_stream_dns = kwargs.get('qemu_stream_dns', False)
>>           libvirt_stream = kwargs.get('libvirt_stream', False)
>>           cdrom_xml = self._get_cdrom_xml(libvirt_stream, 
>> qemu_stream_dns)
>> @@ -292,3 +324,9 @@ class VMTemplate(object):
>>
>>       def _get_storage_path(self):
>>           return ''
>> +
>> +    def _get_storage_type(self):
>> +        return ''
>> +
>> +    def _get_available_volumes(self):
>> +        return []
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>




More information about the Kimchi-devel mailing list