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: