xmlbuilder: Few misc cleanups and streamlinings

tests/xmlparse.py

@@ -189,7 +189,9 @@ class XMLParseTest(unittest.TestCase):
         self.assertTrue(guest.clock.get_xml_config().startswith("<clock"))
 
         check = self._make_checker(guest.seclabel)
-        check("model", None, "default")
+        check("model", None)
+        guest.seclabel.model = "default"
+        self.assertEquals(guest.seclabel.model, "testSecurity")
         check("type", None, "static")
         check("label", None, "frob")
         self.assertTrue(

virtinst/CPU.py

@@ -23,7 +23,9 @@ import libxml2
 
 
 def _int_or_none(val):
-    return val and int(val) or val
+    if val is None:
+        return None
+    return int(val)
 
 
 class CPUFeature(XMLBuilder):
@@ -105,7 +107,7 @@ class CPU(XMLBuilder):
 
     def _get_features(self):
         return self._features[:]
-    features = XMLProperty(_get_features)
+    features = property(_get_features)
 
     def add_feature(self, name, policy="require"):
         feature = CPUFeature(self.conn)
@@ -166,24 +168,21 @@ class CPU(XMLBuilder):
         return self._sockets
     def _set_sockets(self, val):
         self._sockets = _int_or_none(val)
-    sockets = XMLProperty(_get_sockets, _set_sockets,
+    sockets = XMLProperty(_get_sockets, _set_sockets, is_int=True,
-                            get_converter=lambda s, x: _int_or_none(x),
                           xpath="./cpu/topology/@sockets")
 
     def _get_cores(self):
         return self._cores
     def _set_cores(self, val):
         self._cores = _int_or_none(val)
-    cores = XMLProperty(_get_cores, _set_cores,
+    cores = XMLProperty(_get_cores, _set_cores, is_int=True,
-                          get_converter=lambda s, x: _int_or_none(x),
                         xpath="./cpu/topology/@cores")
 
     def _get_threads(self):
         return self._threads
     def _set_threads(self, val):
         self._threads = _int_or_none(val)
-    threads = XMLProperty(_get_threads, _set_threads,
+    threads = XMLProperty(_get_threads, _set_threads, is_int=True,
-                            get_converter=lambda s, x: _int_or_none(x),
                           xpath="./cpu/topology/@threads")
 
     def clear_attrs(self):

virtinst/Seclabel.py

@@ -106,9 +106,13 @@ class Seclabel(XMLBuilder):
         return self._model
     def set_model(self, val):
         self._model = val
+    def _set_model_converter(self, val):
+        if val == "default":
+            return self._get_default_model()
+        return val
     model = XMLProperty(get_model, set_model,
-                          xpath="./seclabel/@model",
+                        set_converter=_set_model_converter,
-                          default_converter=_get_default_model)
+                          xpath="./seclabel/@model")
 
     def get_label(self):
         return self._label

virtinst/VirtualCharDevice.py

@@ -275,6 +275,7 @@ class VirtualCharDevice(VirtualDevice):
             return "./source[@mode='connect']/@mode"
         return "./source/@mode"
     source_mode = XMLProperty(get_source_mode, set_source_mode,
+                              name="char sourcemode",
                                 xml_get_xpath=_sourcemode_xpath,
                                 xml_set_xpath=_sourcemode_xpath)
 
@@ -285,6 +286,7 @@ class VirtualCharDevice(VirtualDevice):
     def _sourcehost_xpath(self):
         return "./source[@mode='%s']/@host" % self.source_mode
     source_host = XMLProperty(get_source_host, set_source_host,
+                              name="char sourcehost",
                                 xml_get_xpath=_sourcehost_xpath,
                                 xml_set_xpath=_sourcehost_xpath)
 
@@ -295,6 +297,7 @@ class VirtualCharDevice(VirtualDevice):
     def _sourceport_xpath(self):
         return "./source[@mode='%s']/@service" % self.source_mode
     source_port = XMLProperty(get_source_port, set_source_port,
+                              name="char sourceport",
                                 xml_get_xpath=_sourceport_xpath,
                                 xml_set_xpath=_sourceport_xpath)
 

virtinst/VirtualDisk.py

@@ -431,7 +431,7 @@ class VirtualDisk(VirtualDevice):
         return ret
     def _xml_set_xpath(self):
         return "./source/@" + self.disk_type_to_target_prop(self.type)
-    path = XMLProperty(_get_path, _set_path,
+    path = XMLProperty(_get_path, _set_path, name="disk path",
                          xml_get_xpath=_xml_get_xpath,
                          xml_set_xpath=_xml_set_xpath,
                          clear_first=["./source/@" + target for target in

virtinst/VirtualFilesystem.py

@@ -146,7 +146,7 @@ class VirtualFilesystem(VirtualDevice):
     def _xml_set_source_xpath(self):
         ret = "./source/@" + self.type_to_source_prop(self.type)
         return ret
-    source = XMLProperty(_get_source, _set_source,
+    source = XMLProperty(_get_source, _set_source, name="filesystem source",
                            xml_get_xpath=_xml_get_source_xpath,
                            xml_set_xpath=_xml_set_source_xpath)
 

virtinst/VirtualGraphics.py

@@ -252,8 +252,7 @@ class VirtualGraphics(VirtualDevice):
             raise ValueError(_("TLS port must be a number between "
                                "5900 and 65535, or -1 for auto allocation"))
         self._tlsPort = val
-    tlsPort = XMLProperty(get_tlsPort, set_tlsPort,
+    tlsPort = XMLProperty(get_tlsPort, set_tlsPort, is_int=True,
-                            get_converter=lambda s, x: int(x or -1),
                           xpath="./@tlsPort")
 
     channel_main_mode = _get_mode_prop(CHANNEL_TYPE_MAIN)
@@ -313,9 +312,12 @@ class VirtualGraphics(VirtualDevice):
         port = self.port
         if port is None:
             port = -1
+        tlsport = self.tlsPort
+        if tlsport is None:
+            tlsport = -1
         return self._build_xml(port=port, keymap=self.keymap,
                                passwd=self.passwd, listen=self.listen,
-                               tlsPort=self.tlsPort, canautoport=True,
+                               tlsPort=tlsport, canautoport=True,
                                passwdValidTo=self.passwdValidTo)
 
     def _vnc_config(self):

virtinst/xmlbuilder.py

@@ -222,11 +222,11 @@ def _remove_xpath_node(ctx, xpath, dofree=True):
 
 
 class XMLProperty(property):
-    def __init__(self, fget=None, fset=None, doc=None,
+    def __init__(self, fget=None, fset=None, doc=None, xpath=None, name=None,
-                 xpath=None, get_converter=None, set_converter=None,
+                 get_converter=None, set_converter=None,
                  xml_get_xpath=None, xml_set_xpath=None,
                  is_bool=False, is_tri=False, is_int=False, is_multi=False,
-                 default_converter=None, clear_first=None):
+                 clear_first=None):
         """
         Set a XMLBuilder class property that represents a value in the
         <domain> XML. For example
@@ -243,6 +243,8 @@ class XMLProperty(property):
         @param doc: option doc string for the property
         @param xpath: xpath string which maps to the associated property
                       in a typical XML document
+        @param name: Just a string to print for debugging, only needed
+            if xpath isn't specified.
         @param get_converter:
         @param set_converter: optional function for converting the property
             value from the virtinst API to the guest XML. For example,
@@ -258,14 +260,15 @@ class XMLProperty(property):
             no value set.
         @param is_multi: Whether data is coming multiple or a single node
         @param is_int: Whethere this is an integer property in the XML
-        @param default_converter: If the virtinst value is "default", use
-                                  this function to get the actual XML value
         @param clear_first: List of xpaths to unset before any 'set' operation.
             For those weird interdependent XML props like disk source type and
             path attribute.
         """
 
         self._xpath = xpath
+        self._name = name or xpath
+        if not self._name:
+            raise RuntimeError("XMLProperty: name or xpath must be passed.")
 
         self._is_tri = is_tri
         self._is_bool = is_bool or is_tri
@@ -277,7 +280,6 @@ class XMLProperty(property):
 
         self._convert_value_for_getter_cb = get_converter
         self._convert_value_for_setter_cb = set_converter
-        self._default_converter = default_converter
         self._setter_clear_these_first = clear_first or []
 
         if not fget:
@@ -297,10 +299,7 @@ class XMLProperty(property):
     ##################
 
     def __repr__(self):
-        ret = property.__repr__(self)
+        return "<XMLProperty %s %s>" % (str(self._name), id(self))
-        if self._xpath:
-            ret = "<XMLProperty %s>" % str(self._xpath)
-        return ret
 
 
     ####################
@@ -348,13 +347,19 @@ class XMLProperty(property):
         return propstore.get(self._findpropname(xmlbuilder), None)
 
     def _xpath_for_getter(self, xmlbuilder):
+        ret = self._xpath
         if self._xpath_for_getter_cb:
-            return self._xpath_for_getter_cb(xmlbuilder)
+            ret = self._xpath_for_getter_cb(xmlbuilder)
-        return self._xpath
+        if ret is None:
+            raise RuntimeError("%s: didn't generate any setter xpath." % self)
+        return ret
     def _xpath_for_setter(self, xmlbuilder):
+        ret = self._xpath
         if self._xpath_for_setter_cb:
-            return self._xpath_for_setter_cb(xmlbuilder)
+            ret = self._xpath_for_setter_cb(xmlbuilder)
-        return self._xpath
+        if ret is None:
+            raise RuntimeError("%s: didn't generate any setter xpath." % self)
+        return ret
 
     def _xpath_list_for_setter(self, xpath, setval, nodelist):
         if not self._is_multi:
@@ -378,8 +383,6 @@ class XMLProperty(property):
         val = self._orig_fget(xmlbuilder)
         if self._convert_value_for_setter_cb:
             val = self._convert_value_for_setter_cb(xmlbuilder, val)
-        elif self._default_converter and val == "default":
-            val = self._default_converter(xmlbuilder)
         return val
 
     def _build_node_list(self, xmlbuilder, xpath):
@@ -425,12 +428,6 @@ class XMLProperty(property):
             return self._convert_raw_getter(fgetval)
 
         xpath = self._xpath_for_getter(xmlbuilder)
-        if xpath is None:
-            return self._convert_raw_getter(fgetval)
-
-        if self._default_converter and fgetval == "default":
-            return fgetval
-
         nodelist = self._build_node_list(xmlbuilder, xpath)
 
         if nodelist:
@@ -466,9 +463,6 @@ class XMLProperty(property):
             return
 
         xpath = self._xpath_for_setter(xmlbuilder)
-        if xpath is None:
-            return
-
         setval = self._convert_value_for_setter(xmlbuilder)
         nodelist = self._build_node_list(xmlbuilder, xpath)
         clearlist = self._build_clear_list(xmlbuilder, nodelist)