[Kimchi-devel] [PATCH V1] [Kimchi] For s390x architecture, serial type is not validated in vm xml as as not supported on s390x.

[Aline Manera]

On 08/30/2016 09:43 AM, archus at linux.vnet.ibm.com wrote:
> From: Archana Singh <archus at linux.vnet.ibm.com>
>
> This changes ensure that in vm xml serial type is not validated for s390x
> as not supported in s390x architecture and only console type is validated
> in _vm_check_serial.
>
> Signed-off-by: Archana Singh <archus at linux.vnet.ibm.com>
> ---
>   model/vms.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/model/vms.py b/model/vms.py
> index 7f607f5..d3d2cb1 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -1486,7 +1486,8 @@ class VMModel(object):
>           xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
>
>           expr = "/domain/devices/serial/@type"

> -        if not xpath_get_text(xml, expr):
> +        # on s390x serial is not supported
> +        if platform.machine() != 's390x' and not xpath_get_text(xml, expr):

Seems this validation is not correct.

The full code is:

         expr = "/domain/devices/serial/@type"
         if not xpath_get_text(xml, expr):
             return False

         expr = "/domain/devices/console/@type"
         if not xpath_get_text(xml, expr):
             return False

With this patch, it turned to:

         expr = "/domain/devices/serial/@type"
         if platform.machine() != 's390x' and not xpath_get_text(xml, 
expr):
             return False

         expr = "/domain/devices/console/@type"
         if not xpath_get_text(xml, expr):
             return False

If the system is x86 and the guest does have 
"/domain/devices/serial/@type" it will not enter the first 'if' statement.
And the second if will be validated for all the platforms, including 
s390x. Which means, the function _vm_check_serial can still return True 
for a s390x system.

Does that make sense for you?

Said that, I suggest to add a exclusive if statement to cover the s390x 
system:

if platform.machine() == 's390x':
     return False


>               return False
>
>           expr = "/domain/devices/console/@type"