Reviewed by: Archana Singh <archus@linux.vnet.ibm.com> On 09/13/2016 02:00 PM, Suresh Babu Angadi wrote:
On 09/13/2016 01:46 PM, Archana Singh wrote:
Looks good, a minor comment.
On 09/13/2016 01:29 PM, sureshab@linux.vnet.ibm.com wrote:
From: Suresh Babu Angadi <sureshab@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@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: