Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-20 Thread mpolednik 
Martin Polednik has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/54802/1/lib/vdsm/hostdev.py
File lib/vdsm/hostdev.py:

Line 118:   'usb_device': _parse_usb_address}
Line 119: 
Line 120: params = {}
Line 121: 
Line 122: devXML = etree.fromstring(device_xml.decode('ascii', 
errors='ignore'))
> I did not understand your comment, but hey - 'replace' give the annoying un
Yes, that worries me. And what worries me more is that BIOS should *never* use 
anything more exotic than ascii. If there is an issue, it will most likely be 
unprintable character.
Line 123: name = devXML.find('name').text
Line 124: if name != 'computer':
Line 125: params['parent'] = devXML.find('parent').text
Line 126: 


-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-19 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1: Code-Review+1

seems OK, but not completely sure swallowing errors is a good idea.
While I clear my thoughts, partial ACK.

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-19 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1: Code-Review+2

OK, please backport!

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-19 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 2:

* #1315435::Update tracker: OK
* Set MODIFIED::bug 1315435#1315435OK

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-19 Thread danken 
Dan Kenigsberg has submitted this change and it was merged.

Change subject: hostdev: decode XML string when parsing devices
..


hostdev: decode XML string when parsing devices

As there is no encoding of information and libvirt does not handle
encoding, our best bet is ascii. This patch therefore converts device
XMLs to ascii, ignoring the errors found.

Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Bug-Url: https://bugzilla.redhat.com/1315435
Signed-off-by: Martin Polednik 
Reviewed-on: https://gerrit.ovirt.org/54802
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/hostdev.py
M tests/devices/data/Makefile.am
A tests/devices/data/computer.xml
M tests/hostdevTests.py
M tests/vmfakelib.py
5 files changed, 32 insertions(+), 2 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Looks good to me, but someone else must approve
  Martin Polednik: Verified



-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-19 Thread mpolednik 
Martin Polednik has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/54802/1//COMMIT_MSG
Commit Message:

Line 10: ascii
> why is that? I would assume that UTF8 is likely to be used if a crazy BIOS 
Simple answer would be "we can't know". Speaking of which, BIOS should not ever 
use non-ascii characters, and issue in this case is more likely invalid XML 
character rather than character in different encoding.


https://gerrit.ovirt.org/#/c/54802/1/lib/vdsm/hostdev.py
File lib/vdsm/hostdev.py:

Line 118:   'usb_device': _parse_usb_address}
Line 119: 
Line 120: params = {}
Line 121: 
Line 122: devXML = etree.fromstring(device_xml.decode('ascii', 
errors='ignore'))
> I think that 'replace' would give the user a hint about information
See the other comment - it's most likely case of un-printable characters. Those 
would be displayed as escaped hex codes, which may rather devalue the 
information user sees.
Line 123: name = devXML.find('name').text
Line 124: if name != 'computer':
Line 125: params['parent'] = devXML.find('parent').text
Line 126: 


-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-18 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/54802/1/lib/vdsm/hostdev.py
File lib/vdsm/hostdev.py:

Line 118:   'usb_device': _parse_usb_address}
Line 119: 
Line 120: params = {}
Line 121: 
Line 122: devXML = etree.fromstring(device_xml.decode('ascii', 
errors='ignore'))
> See the other comment - it's most likely case of un-printable characters. T
I did not understand your comment, but hey - 'replace' give the annoying 
unicode replace char. Is that what worries you?

>>> 'ÿ'.decode('ascii', 
>>> 'ignore')
u''
>>> 'ÿ'.decode('ascii', 
>>> 'replace')
u'\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd\ufffd'
Line 123: name = devXML.find('name').text
Line 124: if name != 'computer':
Line 125: params['parent'] = devXML.find('parent').text
Line 126: 


-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-18 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

If the system's already complaining, there is even little reason to add our 
whine on top of that. :)

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-18 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/54802/1//COMMIT_MSG
Commit Message:

Line 10: ascii
why is that? I would assume that UTF8 is likely to be used if a crazy BIOS 
likes to add Hebrew or Czech strings.


https://gerrit.ovirt.org/#/c/54802/1/lib/vdsm/hostdev.py
File lib/vdsm/hostdev.py:

Line 118:   'usb_device': _parse_usb_address}
Line 119: 
Line 120: params = {}
Line 121: 
Line 122: devXML = etree.fromstring(device_xml.decode('ascii', 
errors='ignore'))
I think that 'replace' would give the user a hint about information
Line 123: name = devXML.find('name').text
Line 124: if name != 'computer':
Line 125: params['parent'] = devXML.find('parent').text
Line 126: 


-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-18 Thread mpolednik 
Martin Polednik has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

I know it's tempting to at least log something, but reading dmesg, you would 
probably have about 17 messages mentioning broken bios. Do we also need to 
report the same thing?

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-16 Thread mpolednik 
Martin Polednik has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-16 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: hostdev: decode XML string when parsing devices
..


Patch Set 1:

* #1315435::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1315435::OK, public bug
* Check Product::#1315435::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: decode XML string when parsing devices

2016-03-16 Thread mpolednik 
Martin Polednik has uploaded a new change for review.

Change subject: hostdev: decode XML string when parsing devices
..

hostdev: decode XML string when parsing devices

As there is no encoding of information and libvirt does not handle
encoding, our best bet is ascii. This patch therefore converts device
XMLs to ascii, ignoring the errors found.

Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Bug-Url: https://bugzilla.redhat.com/1315435
Signed-off-by: Martin Polednik 
---
M lib/vdsm/hostdev.py
M tests/devices/data/Makefile.am
A tests/devices/data/computer.xml
M tests/hostdevTests.py
M tests/vmfakelib.py
5 files changed, 32 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/54802/1

diff --git a/lib/vdsm/hostdev.py b/lib/vdsm/hostdev.py
index bae9baf..48933b6 100644
--- a/lib/vdsm/hostdev.py
+++ b/lib/vdsm/hostdev.py
@@ -119,7 +119,7 @@
 
 params = {}
 
-devXML = etree.fromstring(device_xml)
+devXML = etree.fromstring(device_xml.decode('ascii', errors='ignore'))
 name = devXML.find('name').text
 if name != 'computer':
 params['parent'] = devXML.find('parent').text
diff --git a/tests/devices/data/Makefile.am b/tests/devices/data/Makefile.am
index 040ab8e..f0136a8 100644
--- a/tests/devices/data/Makefile.am
+++ b/tests/devices/data/Makefile.am
@@ -21,6 +21,7 @@
 vdsmdevdatatestsdir = ${vdsmtestsdir}/devices/data
 
 dist_vdsmdevdatatests_DATA = \
+   computer.xml \
net_em1_28_d2_44_55_66_88.xml \
pci__00_02_0.xml \
pci__00_09_0.xml \
diff --git a/tests/devices/data/computer.xml b/tests/devices/data/computer.xml
new file mode 100644
index 000..ac9941f
--- /dev/null
+++ b/tests/devices/data/computer.xml
@@ -0,0 +1,17 @@
+
+  computer
+  
+ÿ
+
+  ÿ
+  ÿ
+  ÿ
+  77ad6766-3273-498f-bb18-03db6f46c62c
+
+
+  Intel Corp.
+  FYBYT10H.86A.0050.2015.0326.1731
+  03/26/2015
+
+  
+
diff --git a/tests/hostdevTests.py b/tests/hostdevTests.py
index f28a446..5af9cd5 100644
--- a/tests/hostdevTests.py
+++ b/tests/hostdevTests.py
@@ -38,6 +38,7 @@
 _SRIOV_PF = 'pci__05_00_1'
 _SRIOV_VF = 'pci__05_10_7'
 _ADDITIONAL_DEVICE = 'pci__00_09_0'
+_COMPUTER_DEVICE = 'computer'
 _NET_DEVICE = 'net_em1_28_d2_44_55_66_88'
 
 _DEVICE_XML = {
@@ -228,6 +229,8 @@
 'domain': '0',
 'function': '0'}}
 
+_COMPUTER_DEVICE_PARSED = {'capability': 'system'}
+
 _NET_DEVICE_PARSED = {
 'parent': 'pci__00_19_0',
 'capability': 'net',
@@ -330,6 +333,14 @@
 
 self.assertEquals(ADDITIONAL_DEVICE_PARSED, deviceXML)
 
+def testParseDeviceParamsInvalidEncoding(self):
+deviceXML = hostdev._parse_device_params(
+libvirtconnection.get().nodeDeviceLookupByName(
+_COMPUTER_DEVICE).XMLDesc()
+)
+
+self.assertEquals(_COMPUTER_DEVICE_PARSED, deviceXML)
+
 def testParseSRIOV_PFDeviceParams(self):
 deviceXML = hostdev._parse_device_params(
 libvirtconnection.get().nodeDeviceLookupByName(
diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py
index 21e6cb3..614e66d 100644
--- a/tests/vmfakelib.py
+++ b/tests/vmfakelib.py
@@ -298,7 +298,8 @@
 
 def __init__(self, xml):
 self.xml = xml
-self._name = etree.fromstring(self.XMLDesc(0)).find('name').text
+self._name = etree.fromstring(self.XMLDesc(0).decode(
+'ascii', errors='ignore')).find('name').text
 
 def XMLDesc(self, flags=0):
 return self.xml


-- 
To view, visit https://gerrit.ovirt.org/54802
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf9b9699790c83bb82a88a700c63d05604588d02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches