[Kimchi-devel] [PATCH v2] [Kimchi 2/2] added 'console' parameter to vms api for s390x

Suresh Babu Angadi sureshab at linux.vnet.ibm.com
Tue Sep 13 08:30:16 UTC 2016



On 09/13/2016 01:46 PM, Archana Singh wrote:
> Looks good, a minor comment.
>
>
> On 09/13/2016 01:29 PM, sureshab at linux.vnet.ibm.com wrote:
>> From: Suresh Babu Angadi <sureshab at in.ibm.com>
>>
>> In case of s390x architecture, console type can be
>> either virtio/sclp. Extended current code to support
>> console configuration in case of s390x
>>
>> * added 'console' parameter as optional attribute for s390x
>>    which returns current console type attached to guest
>> * made changes in model code to support update of 'console'
>>    attribute for guest when running on s390x
>> * added appropriate error codes in i18n.py
>> * added API.json changes to validate 'console' attribute
>> * made changes in xml code generation, default value as
>>    'virtio'
>>
>> Signed-off-by: Suresh Babu Angadi <sureshab at in.ibm.com>
>> ---
>>   API.json           |  8 +++++-
>>   i18n.py            |  2 ++
>>   model/vms.py       | 75 
>> +++++++++++++++++++++++++++++++++++++-----------------
>>   xmlutils/serial.py |  5 +++-
>>   4 files changed, 64 insertions(+), 26 deletions(-)
>>
>> diff --git a/API.json b/API.json
>> index fe5579e..f0861a2 100644
>> --- a/API.json
>> +++ b/API.json
>> @@ -414,7 +414,13 @@
>>                       }
>>                   },
>>                   "cpu_info": { "$ref": "#/kimchitype/cpu_info" },
>> -                "memory": { "$ref": "#/kimchitype/memory" }
>> +                "memory": { "$ref": "#/kimchitype/memory" },
>> +                "console": {
>> +                    "description": "type of the console attached to 
>> the guest in s390x architecture",
>> +                    "type": "string",
>> +                    "pattern": "^sclp|virtio$",
>> +                    "error": "KCHVM0088E"
>> +                }
>>               },
>>               "additionalProperties": false
>>           },
>> diff --git a/i18n.py b/i18n.py
>> index 1684dc4..c46caf4 100644
>> --- a/i18n.py
>> +++ b/i18n.py
>> @@ -137,6 +137,8 @@ messages = {
>>       "KCHVM0084E": _("Error occured while retrieving the Virt Viewer 
>> file for virtual machine %(name)s : %(err)s"),
>>       "KCHVM0085E": _("Virtual machine title must be a string"),
>>       "KCHVM0086E": _("Virtual machine description must be a string"),
>> +    "KCHVM0087E": _("console parameter is only supported for 
>> s390x/s390 architecture."),
>> +    "KCHVM0088E": _("invalid console type, supported types are 
>> sclp/virtio."),
>>
>>       "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly 
>> assigned host device %(dev_name)s."),
>>       "KCHVMHDEV0002E": _("The host device %(dev_name)s is not 
>> allowed to directly assign to VM."),
>> diff --git a/model/vms.py b/model/vms.py
>> index b889166..5855a11 100644
>> --- a/model/vms.py
>> +++ b/model/vms.py
>> @@ -85,7 +85,7 @@ VM_ONLINE_UPDATE_PARAMS = ['graphics', 'groups', 
>> 'memory', 'users']
>>   # update parameters which are updatable when the VM is offline
>>   VM_OFFLINE_UPDATE_PARAMS = ['cpu_info', 'graphics', 'groups', 
>> 'memory',
>>                               'name', 'users', 'bootorder', 'bootmenu',
>> -                            'description', 'title']
>> +                            'description', 'title', 'console']
>>
>>   XPATH_DOMAIN_DISK = 
>> "/domain/devices/disk[@device='disk']/source/@file"
>>   XPATH_DOMAIN_DISK_BY_FILE = 
>> "./devices/disk[@device='disk']/source[@file='%s']"
>> @@ -96,6 +96,7 @@ XPATH_DOMAIN_MEMORY = '/domain/memory'
>>   XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit'
>>   XPATH_DOMAIN_UUID = '/domain/uuid'
>>   XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id'
>> +XPATH_DOMAIN_CONSOLE_TARGET = "/domain/devices/console/target/@type"
>>
>>   XPATH_BOOT = 'os/boot/@dev'
>>   XPATH_BOOTMENU = 'os/bootmenu/@enable'
>> @@ -109,6 +110,7 @@ XPATH_TITLE = './title'
>>   XPATH_TOPOLOGY = './cpu/topology'
>>   XPATH_VCPU = './vcpu'
>>   XPATH_MAX_MEMORY = './maxMemory'
>> +XPATH_CONSOLE_TARGET = "./devices/console/target"
>>
>>   # key: VM name; value: lock object
>>   vm_locks = {}
>> @@ -263,6 +265,9 @@ class VMModel(object):
>>           return sockets and cores and threads
>>
>>       def update(self, name, params):
>> +        if platform.machine() not in ['s390x', 's390'] and\
>> +                        'console' in params:
>> +            raise InvalidParameter('KCHVM0087E')
>>           lock = vm_locks.get(name)
>>           if lock is None:
>>               lock = threading.Lock()
>> @@ -793,6 +798,19 @@ class VMModel(object):
>>           # update <os>
>>           return ET.tostring(et)
>>
>> +    def _update_s390x_console(self, xml, params):
>> +        if xpath_get_text(xml, XPATH_DOMAIN_CONSOLE_TARGET):
>> +            # if console is defined, update console
>> +            return xml_item_update(xml, XPATH_CONSOLE_TARGET,
>> +                                   params.get('console'), 'type')
>> +        # if console is not defined earlier, add console
>> +        console = E.console(type="pty")
>> +        console.append(E.target(type=params.get('console'), port='0'))
>> +        et = ET.fromstring(xml)
>> +        devices = et.find('devices')
>> +        devices.append(console)
>> +        return ET.tostring(et)
>> +
>>       def _static_vm_update(self, vm_name, dom, params):
>>           old_xml = new_xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
>>           params = copy.deepcopy(params)
>> @@ -861,6 +879,9 @@ class VMModel(object):
>>           if "bootorder" or "bootmenu" in params:
>>               new_xml = self._update_bootorder(new_xml, params)
>>
>> +        if platform.machine() in ['s390', 's390x'] and 
>> params.get('console'):
>> +            new_xml = self._update_s390x_console(new_xml, params)
>> +
>>           snapshots_info = []
>>           conn = self.conn.get()
>>           try:
>> @@ -1303,29 +1324,35 @@ class VMModel(object):
>>           bootmenu = "yes" if "yes" in xpath_get_text(xml, 
>> XPATH_BOOTMENU) \
>>               else "no"
>>
>> -        return {'name': name,
>> -                'title': "".join(xpath_get_text(xml, XPATH_TITLE)),
>> -                'description': "".join(xpath_get_text(xml, 
>> XPATH_DESCRIPTION)),
>> -                'state': state,
>> -                'stats': res,
>> -                'uuid': dom.UUIDString(),
>> -                'memory': {'current': memory, 'maxmemory': maxmemory},
>> -                'cpu_info': cpu_info,
>> -                'screenshot': screenshot,
>> -                'icon': icon,
>> -                # (type, listen, port, passwd, passwdValidTo)
>> -                'graphics': {"type": graphics[0],
>> -                             "listen": graphics[1],
>> -                             "port": graphics_port,
>> -                             "passwd": graphics[3],
>> -                             "passwdValidTo": graphics[4]},
>> -                'users': users,
>> -                'groups': groups,
>> -                'access': 'full',
>> -                'persistent': True if dom.isPersistent() else False,
>> -                'bootorder': boot,
>> -                'bootmenu': bootmenu
>> -                }
>> +        vm_info = {'name': name,
>> +                   'title': "".join(xpath_get_text(xml, XPATH_TITLE)),
>> +                   'description':
>> +                       "".join(xpath_get_text(xml, XPATH_DESCRIPTION)),
>> +                   'state': state,
>> +                   'stats': res,
>> +                   'uuid': dom.UUIDString(),
>> +                   'memory': {'current': memory, 'maxmemory': 
>> maxmemory},
>> +                   'cpu_info': cpu_info,
>> +                   'screenshot': screenshot,
>> +                   'icon': icon,
>> +                   # (type, listen, port, passwd, passwdValidTo)
>> +                   'graphics': {"type": graphics[0],
>> +                                "listen": graphics[1],
>> +                                "port": graphics_port,
>> +                                "passwd": graphics[3],
>> +                                "passwdValidTo": graphics[4]},
>> +                   'users': users,
>> +                   'groups': groups,
>> +                   'access': 'full',
>> +                   'persistent': True if dom.isPersistent() else False,
>> +                   'bootorder': boot,
>> +                   'bootmenu': bootmenu
>> +                   }
>> +        if platform.machine() in ['s390', 's390x']:
>> +            vm_console = xpath_get_text(xml, 
>> XPATH_DOMAIN_CONSOLE_TARGET)
>> +            vm_info['console'] = vm_console[0] if vm_console else ''
> in case console is not set, do we need to have that key in vm_info as 
> blank or we should not have the key itself.
if running on s390x, we should have it as blank if no console attached 
to guest.
>> +
>> +        return vm_info
>>
>>       def _vm_get_disk_paths(self, dom):
>>           xml = dom.XMLDesc(0)
>> diff --git a/xmlutils/serial.py b/xmlutils/serial.py
>> index c823ee6..f61eefa 100644
>> --- a/xmlutils/serial.py
>> +++ b/xmlutils/serial.py
>> @@ -38,6 +38,7 @@ def get_serial_xml(params):
>>        </console>
>>
>>       For s390x
>> +    target type can be sclp/virtio
>>       <console type='pty'>
>>         <target type='sclp' port='0'/>
>>       </console>
>> @@ -50,8 +51,10 @@ def get_serial_xml(params):
>>           return ET.tostring(console, encoding='utf-8', 
>> pretty_print=True)
>>       # for s390x
>>       elif params["arch"] in ["s390x"]:
>> +        # if params doesn't have console parameter, use virtio as 
>> default
>> +        console_type = params.get('console', 'virtio')
>>           console = E.console(type="pty")
>> -        console.append(E.target(type="sclp", port='0'))
>> +        console.append(E.target(type=console_type, port='0'))
>>           return ET.tostring(console, encoding='utf-8', 
>> pretty_print=True)
>>       # for x
>>       else:
>

-- 
Regards,
Suresh Babu Angadi




More information about the Kimchi-devel mailing list