3:19 p.m.
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>
> ---
> 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