From 76d5113941934ce270e787717757adbf6a8c9c6c Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso@redhat.com>
Date: Fri, 10 May 2019 17:47:54 -0400
Subject: [PATCH] xmlbuilder: Make is_* handling more future proof

If libvirt changes XML handling in the future, we shouldn't restrict
what we return to the user with the is_yesno/is_onoff convertors
---
 tests/xmlparse.py      | 23 +++++++++++++++++++++
 virtinst/xmlbuilder.py | 46 ++++++++++++++++++++++++++++++------------
 2 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/tests/xmlparse.py b/tests/xmlparse.py
index 66ff9859..14630fb4 100644
--- a/tests/xmlparse.py
+++ b/tests/xmlparse.py
@@ -1504,3 +1504,26 @@ class XMLParseTest(unittest.TestCase):
         guest = virtinst.Guest(self.conn, parsexml=open(infile).read())
 
         utils.diff_compare(guest.get_xml(), outfile)
+
+    def testYesNoUnexpectedParse(self):
+        # Make sure that if we see an unexpected yes/no or on/off value,
+        # we just return it to the user and don't error. Libvirt could
+        # change our assumptions and we shouldn't be too restrictive
+        xml = ("<hostdev managed='foo'>\n  <rom bar='wibble'/>\n"
+            "  <source><address bus='hello'/></source>\n</hostdev>")
+        dev = virtinst.DeviceHostdev(self.conn, parsexml=xml)
+
+        self.assertEqual(dev.managed, "foo")
+        self.assertEqual(dev.rom_bar, "wibble")
+        self.assertEqual(dev.scsi_bus, "hello")
+
+        dev.managed = "test1"
+        dev.rom_bar = "test2"
+        self.assertEqual(dev.managed, "test1")
+        self.assertEqual(dev.rom_bar, "test2")
+
+        try:
+            dev.scsi_bus = "goodbye"
+            raise AssertionError("Expected ValueError")
+        except ValueError:
+            pass
diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py
index d02ea7a7..ea5869d9 100644
--- a/virtinst/xmlbuilder.py
+++ b/virtinst/xmlbuilder.py
@@ -223,16 +223,30 @@ class XMLProperty(_XMLPropertyBase):
     def _convert_get_value(self, val):
         # pylint: disable=redefined-variable-type
         if self._is_bool:
-            ret = bool(val)
+            return bool(val)
         elif self._is_int and val is not None:
-            intkwargs = {}
-            if "0x" in str(val):
-                intkwargs["base"] = 16
-            ret = int(val, **intkwargs)
-        elif self._is_yesno and val is not None:
-            ret = bool(val == "yes")
-        elif self._is_onoff and val is not None:
-            ret = bool(val == "on")
+            try:
+                intkwargs = {}
+                if "0x" in str(val):
+                    intkwargs["base"] = 16
+                ret = int(val, **intkwargs)
+            except ValueError as e:
+                logging.debug("Error converting XML value to int: %s", e)
+                ret = val
+        elif self._is_yesno:
+            if val == "yes":
+                ret = True
+            elif val == "no":
+                ret = False
+            else:
+                ret = val
+        elif self._is_onoff:
+            if val == "on":
+                ret = True
+            elif val == "off":
+                ret = False
+            else:
+                ret = val
         else:
             ret = val
         return ret
@@ -240,10 +254,16 @@ class XMLProperty(_XMLPropertyBase):
     def _convert_set_value(self, val):
         if self._do_abspath and val is not None:
             val = os.path.abspath(val)
-        elif self._is_onoff and val is not None:
-            val = bool(val) and "on" or "off"
-        elif self._is_yesno and val is not None:
-            val = bool(val) and "yes" or "no"
+        elif self._is_onoff:
+            if val is True:
+                val = "on"
+            elif val is False:
+                val = "off"
+        elif self._is_yesno:
+            if val is True:
+                val = "yes"
+            elif val is False:
+                val = "no"
         elif self._is_int and val is not None:
             intkwargs = {}
             if "0x" in str(val):