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

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(a)linux.vnet.ibm.com&gt; >> --- >> 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(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel