VirtualDisk: Make the driver/type auto changes opt-in

It was hard to get this right, so just require that any API users
which are changing the path of an existing disk call sync_path_props()

tests/cli-test-xml/compare/clone-auto1.xml

@@ -20,10 +20,10 @@
   <devices>
     <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
     <disk type="block" device="floppy">
+      <driver type="vmdk"/>
       <source dev="/disk-pool/diskvol1-clone"/>
       <target dev="fda" bus="fdc"/>
       <address type="drive" controller="0" bus="0" target="0" unit="0"/>
-      <driver type="vmdk"/>
     </disk>
     <disk type="block" device="disk">
       <source dev="/disk-pool/diskvol2"/>

tests/clone-xml/managed-storage-out.xml

@@ -18,9 +18,9 @@
   <devices>
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type="file" device="disk">
+      <driver name="qemu" type="vmdk"/>
       <source file="/default-pool/new1.img"/>
       <target dev="hda" bus="ide"/>
-      <driver name="qemu" type="vmdk"/>
     </disk>
     <disk type="block" device="disk">
       <source dev="/disk-pool/new2.img"/>

tests/xmlparse.py

@@ -264,6 +264,7 @@ class XMLParseTest(unittest.TestCase):
 
         check = self._make_checker(disk1)
         check("path", "/tmp/test.img", "/dev/loop0")
+        disk1.sync_path_props()
         check("driver_name", None, "test")
         check("driver_type", None, "raw")
         check("serial", "WD-WMAP9A966149", "frob")
@@ -271,6 +272,7 @@ class XMLParseTest(unittest.TestCase):
         check = self._make_checker(disk3)
         check("type", "block", "dir", "file", "block")
         check("path", "/dev/loop0", None)
+        disk3.sync_path_props()
         check("device", "cdrom", "floppy")
         check("read_only", True, False)
         check("target", "hdc", "fde")
@@ -279,6 +281,7 @@ class XMLParseTest(unittest.TestCase):
 
         check = self._make_checker(disk6)
         check("path", None, "/default-pool/default-vol")
+        disk6.sync_path_props()
         check("shareable", False, True)
         check("driver_cache", None, "writeback")
         check("driver_io", None, "threads")
@@ -757,23 +760,28 @@ class XMLParseTest(unittest.TestCase):
         disk = guest.get_devices("disk")[0]
         check = self._make_checker(disk)
         check("path", None, "/default-pool/default-vol")
+        disk.sync_path_props()
 
         disk = guest.get_devices("disk")[1]
         check = self._make_checker(disk)
         check("path", None, "/default-pool/default-vol")
         check("path", "/default-pool/default-vol", "/disk-pool/diskvol1")
+        disk.sync_path_props()
 
         disk = guest.get_devices("disk")[2]
         check = self._make_checker(disk)
         check("path", None, "/disk-pool/diskvol1")
+        disk.sync_path_props()
 
         disk = guest.get_devices("disk")[3]
         check = self._make_checker(disk)
         check("path", None, "/default-pool/default-vol")
+        disk.sync_path_props()
 
         disk = guest.get_devices("disk")[4]
         check = self._make_checker(disk)
         check("path", None, "/disk-pool/diskvol1")
+        disk.sync_path_props()
 
         self._alter_compare(guest.get_xml_config(), outfile)
 

virtManager/domain.py

@@ -607,6 +607,7 @@ class vmmDomain(vmmLibvirtObject):
     def define_storage_media(self, devobj, newpath):
         def change(editdev):
             editdev.path = newpath
+            editdev.sync_path_props()
         return self._redefine_device(change, devobj)
     def define_disk_readonly(self, devobj, do_readonly):
         def change(editdev):

virtinst/VirtualDisk.py

@@ -418,7 +418,7 @@ class VirtualDisk(VirtualDevice):
             raise ValueError("Can't change disk path if storage creation info "
                              "has been set.")
         self._change_backend(val, None)
-        self._refresh_backend_settings()
+        self._xmlpath = self.path
     path = property(_get_path, _set_path)
 
 
@@ -580,7 +580,7 @@ class VirtualDisk(VirtualDevice):
         self._storage_creator = creator
         if self._storage_creator:
             self._storage_creator.fake = bool(fake)
-            self._refresh_backend_settings()
+            self._xmlpath = self.path
         else:
             if (vol_install or clone_path):
                 raise RuntimeError("Need storage creation but it "
@@ -599,26 +599,15 @@ class VirtualDisk(VirtualDevice):
                                 path, vol_object, None, None)
         self._storage_backend = backend
 
-    def _refresh_backend_settings(self):
+    def sync_path_props(self):
-        def refresh_prop_xml(propname):
+        """
-            # When parsing, we can pull info from _propstore or the
+        Fills in the values of type, driver_type, and driver_name for
-            # backing XML. The problem is that disk XML has several
+        the associated backing storage. This needs to be manually called
-            # interdependent properties that we need to update if
+        if changing an existing disk's media.
-            # one or the other changes.
+        """
-            #
+        self.type = self._get_default_type()
-            # This will update the XML value with the newly determined
+        self.driver_name = self._get_default_driver_name()
-            # default value, but it won't edit propstore. This means
+        self.driver_type = self._get_default_driver_type()
-            if self.is_build():
-                return
-            prop = self.all_xml_props()[propname]
-            candefault, val = getattr(prop, "_default_get_value")(self)
-            if candefault:
-                getattr(prop, "_set_xml")(self, val)
-
-        refresh_prop_xml("type")
-        refresh_prop_xml("driver_name")
-        refresh_prop_xml("driver_type")
-        self._xmlpath = self.path
 
     def __managed_storage(self):
         """

virtinst/xmlbuilder.py

@@ -420,6 +420,8 @@ class XMLProperty(property):
         Return (can use default, default value)
         """
         ret = (False, -1)
+        if not xmlbuilder._xmlstate.is_build:
+            return ret
         if not self._prop_is_unset(xmlbuilder):
             return ret
         if not self._default_cb:
@@ -488,8 +490,13 @@ class XMLProperty(property):
         Fetch value at user request. If we are parsing existing XML and
         the user hasn't done a 'set' yet, return the value from the XML,
         otherwise return the value from propstore
+
+        If this is a built from scratch object, we never pull from XML
+        since it's known to the empty, and we may want to return
+        a 'default' value
         """
-        if self._prop_is_unset(xmlbuilder) and not xmlbuilder.is_build():
+        if (self._prop_is_unset(xmlbuilder) and
+            not xmlbuilder._xmlstate.is_build):
             val = self._get_xml(xmlbuilder)
         else:
             val = self._nonxml_fget(xmlbuilder)
@@ -521,10 +528,6 @@ class XMLProperty(property):
         self._nonxml_fset(xmlbuilder,
                           self._convert_set_value(xmlbuilder, val))
 
-        if xmlbuilder.is_build():
-            return
-        self._convert_set_value(xmlbuilder, val)
-
     def _set_xml(self, xmlbuilder, setval, root_node=None):
         """
         Actually set the passed value in the XML document
@@ -692,13 +695,6 @@ class XMLBuilder(object):
                     ret[key] = val
         return ret
 
-    def is_build(self):
-        """
-        True if guest is building XML from scratch and not parsing
-        pre-existing XML
-        """
-        return bool(self._xmlstate.is_build)
-
 
     ############################
     # Public XML managing APIs #
@@ -786,7 +782,7 @@ class XMLBuilder(object):
         Insert the passed XMLBuilder object into our XML document at the
         specified path
         """
-        if not dev.is_build():
+        if not dev._xmlstate.is_build:
             newnode = libxml2.parseDoc(dev.get_xml_config()).children
             _build_xpath_node(self._xmlstate.xml_ctx,
                               dev.get_root_xpath(), newnode)
@@ -815,7 +811,7 @@ class XMLBuilder(object):
         try:
             node = None
             ctx = None
-            if self.is_build():
+            if self._xmlstate.is_build:
                 node = self._xmlstate.xml_node.docCopyNodeList(
                     self._xmlstate.xml_node.doc)
                 ctx = _make_xml_context(node)
@@ -850,7 +846,6 @@ class XMLBuilder(object):
         # Set all defaults if the properties have one registered
         xmlprops = self.all_xml_props()
 
-        if self.is_build():
         for prop in xmlprops.values():
             prop._set_default(self)