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