On 08/30/2016 10:14 PM, Aline Manera wrote:
On 08/30/2016 09:43 AM, archus(a)linux.vnet.ibm.com wrote:
> From: Archana Singh <archus(a)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(a)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"