Looks good, a minor comment. On 09/13/2016 01:29 PM, sureshab(a)linux.vnet.ibm.com wrote: > From: Suresh Babu Angadi <sureshab(a)in.ibm.com&gt; > > 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(a)in.ibm.com&gt; > --- > 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.

> + > + 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:

On 09/13/2016 01:46 PM, Archana Singh wrote: > Looks good, a minor comment. > > > On 09/13/2016 01:29 PM, sureshab(a)linux.vnet.ibm.com wrote: >> From: Suresh Babu Angadi <sureshab(a)in.ibm.com&gt; >> >> 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(a)in.ibm.com&gt; >> --- >> 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: >