[Kimchi-devel] [PATCH] Abstract vm element updater for common usage

[Aline Manera]

On 01/17/2014 12:06 PM, lvroyce0210 at gmail.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Different kinds of vm elements will be added through different
> libvirt apis, this patch abstract a framework for this update.
> Wrap correspondent libvirt api in vm element class,
> and format their own libvirt call parameters in each class.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/model.py | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index 2c6d3a1..0f82b61 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -78,7 +78,6 @@ ISO_POOL_NAME = u'kimchi_isos'
>   GUESTS_STATS_INTERVAL = 5
>   HOST_STATS_INTERVAL = 1
>   VM_STATIC_UPDATE_PARAMS = {'name': './name'}
> -VM_LIVE_UPDATE_PARAMS = {}
>   STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
>                                'path': '/pool/source/dir/@path'}}
>
> @@ -490,7 +489,13 @@ class Model(object):
>           return dom
>
>       def _live_vm_update(self, dom, params):
> -        pass
> +        try:
> +            for key, val in params.items():
> +                element = VMElementDef.create(key, val, dom)
> +                element.add_to_vm()
> +        except libvirt.libvirtError as e:
> +            raise OperationFailed("Cannot apply change on vm: %s",
> +                                  e.get_error_message())
>
>       def vm_update(self, name, params):
>           dom = self._get_vm(name)
> @@ -1510,6 +1515,62 @@ class LibvirtVMScreenshot(VMScreenshot):
>               os.close(fd)
>
>
> +class VMElementDef(object):

What about VMDeviceDef()

> +    @classmethod
> +    def create(cls, elem_type, elemArgs, dom):
> +        for klass in cls.__subclasses__():
> +            if elem_type == klass.elemType:
> +                return klass(elemArgs, dom)
> +        raise OperationFailed('Unsupported vm element type: %s' % elemArgs['type'])
> +
> +    def __init__(self, elemArgs, dom):
> +        self.dom = dom
> +        self.elemArgs = elemArgs
> +
> +    def add_to_vm(self):
> +        """Live update domain with correspondant libvirt API.
> +        This should be paired with cancel() operation to cancel failed update."""
> +        pass
> +
> +    def delete_from_vm(self):
> +        """Revert operation of live update vm."""
> +        pass
> +
> +    @property
> +    def _update_param(self):
> +        ''' Subclasses have to override this method to actually generate the
> +        update param.'''
> +        raise OperationFailed('Update param generater is not implemented: %s' % self)
> +
> +
> +class VMDiskDef(VMElementDef):
> +    elemType = 'disk'
> +
> +    def __init__(self, elemArgs, dom):
> +        super(VMDiskDef, self).__init__(elemArgs, dom)
> +


> +    def add_to_vm(self):
> +        self.dom.attachDeviceFlags(xml=self._update_param['xml'],
> +            flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT|libvirt.VIR_DOMAIN_AFFECT_CONFIG)
> +
> +    def delete_from_vm(self):
> +        self.dom.detachDeviceFlags(xml=self._update_param['xml'],
> +            flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT|libvirt.VIR_DOMAIN_AFFECT_CONFIG)
> +

Those functions will be the same for all kind of devices.
The difference is on the xml passed to attachDeviceFlags() and 
detachDeviceFlags()


So I suggest to move those function to VMDeviceDef() and create a 
function get_device_xml() that will return the xml needed to 
attachDeviceFlags() and detachDeviceFlags()


> +    @property
> +    def _update_param(self):
> +        self.elemArgs['readonly'] = "<readonly/>" \
> +            if self.elemArgs['target']['type'] == 'cdrom' else None
> +        xml = """
> +            <disk type='{source[type]}' device='{target[type]}'>
> +            <source file='{source[path]}'/>
> +            <target dev='{target[name]}' bus='{target[bus]}'/>
> +            {readonly}
> +            </disk>
> +        """.format(**self.elemArgs)
> +        return dict(xml=xml)
> +
> +
>   class StoragePoolDef(object):
>       @classmethod
>       def create(cls, poolArgs):