On 09/02/2016 12:51 PM, Archana Singh wrote:
Missed to replay for second validation, added comment inline.
Thanks,
Archana Singh
On 09/02/2016 09:48 AM, Archana Singh wrote:
HI Aline,
Please find my comment below. Please let me know if it make sense to you.
Thanks,
Archana Singh
On 08/30/2016 11:56 PM, Archana Singh wrote:
On 08/30/2016 10:14 PM, Aline Manera wrote:
On 08/30/2016 09:43 AM, archus@linux.vnet.ibm.com wrote:
From: Archana Singh <archus@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@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.
Yes, it has to be validated for s390x as well.
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
So summary is first one has to be ignored on s390x but second one has to be validated on s390x also.
OK. Got it. I thought the both cases should be ignored in case of s390x.
return False
expr = "/domain/devices/console/@type"
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel