diff --git a/tests/cli-test-xml/compare/clone-auto1.xml b/tests/cli-test-xml/compare/clone-auto1.xml index 1af5374cb..1c90162e6 100644 --- a/tests/cli-test-xml/compare/clone-auto1.xml +++ b/tests/cli-test-xml/compare/clone-auto1.xml @@ -20,10 +20,10 @@ /usr/lib/xen/bin/qemu-dm +
- diff --git a/tests/clone-xml/managed-storage-out.xml b/tests/clone-xml/managed-storage-out.xml index eeb0bd45d..3f172cb89 100644 --- a/tests/clone-xml/managed-storage-out.xml +++ b/tests/clone-xml/managed-storage-out.xml @@ -18,9 +18,9 @@ /usr/bin/qemu-kvm + - diff --git a/tests/xmlparse.py b/tests/xmlparse.py index ef7e7e88f..97dc0694a 100644 --- a/tests/xmlparse.py +++ b/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) diff --git a/virtManager/domain.py b/virtManager/domain.py index 414ce7ccd..a054d4d45 100644 --- a/virtManager/domain.py +++ b/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): diff --git a/virtinst/VirtualDisk.py b/virtinst/VirtualDisk.py index 2c0cbe318..983448c19 100644 --- a/virtinst/VirtualDisk.py +++ b/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 refresh_prop_xml(propname): - # When parsing, we can pull info from _propstore or the - # backing XML. The problem is that disk XML has several - # interdependent properties that we need to update if - # one or the other changes. - # - # This will update the XML value with the newly determined - # default value, but it won't edit propstore. This means - 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 sync_path_props(self): + """ + Fills in the values of type, driver_type, and driver_name for + the associated backing storage. This needs to be manually called + if changing an existing disk's media. + """ + self.type = self._get_default_type() + self.driver_name = self._get_default_driver_name() + self.driver_type = self._get_default_driver_type() def __managed_storage(self): """ diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py index b2c542ace..f13b26e97 100644 --- a/virtinst/xmlbuilder.py +++ b/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,9 +846,8 @@ 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) + for prop in xmlprops.values(): + prop._set_default(self) # Set up preferred XML ordering do_order = self._proporder[:]