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

[Archana Singh]

On 08/30/2016 10:14 PM, Aline Manera wrote:
>
>
> 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.
So even without this patch if system is x86 and has 
"/domain/devices/serial/@type" it will not enter the first 'if' 
statement. If I understood correctly, purpose of this check is to ensure 
that guest has "/domain/devices/serial/@type" otherwise return false.

And with this patch first check(platform.machine() != 's390x' ) in first 
'if' statement will always be true and hence 'if' statement is similar 
to what it was before this patch.

This patch ensure that on s390x system even if guest does not have 
"/domain/devices/serial/@type" it should not enter in first if 
statement. And for all other system, it should enter in first if 
statement in case guest does not have 
"/domain/devices/serial/@type"(similar to what it was before)

Am I miss understood some thing?

> 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"
>