diff --git a/tests/cli-test-xml/compare/virt-xml-edit-clear-clock.xml b/tests/cli-test-xml/compare/virt-xml-edit-clear-clock.xml index 6807d15d..9a096d39 100644 --- a/tests/cli-test-xml/compare/virt-xml-edit-clear-clock.xml +++ b/tests/cli-test-xml/compare/virt-xml-edit-clear-clock.xml @@ -1,20 +1,12 @@ - -- + - - - -- + destroy restart - restart -@@ - - - -+ - Domain 'test-for-virtxml' defined successfully. Changes will take effect after the next domain shutdown. \ No newline at end of file diff --git a/tests/cli-test-xml/compare/virt-xml-edit-clear-cpu.xml b/tests/cli-test-xml/compare/virt-xml-edit-clear-cpu.xml index bda57cd3..6e593e95 100644 --- a/tests/cli-test-xml/compare/virt-xml-edit-clear-cpu.xml +++ b/tests/cli-test-xml/compare/virt-xml-edit-clear-cpu.xml @@ -17,16 +17,10 @@ - - - -- ++ + - -@@ - - - -+ - Domain 'test-for-virtxml' defined successfully. Changes will take effect after the next domain shutdown. \ No newline at end of file diff --git a/virtinst/cli.py b/virtinst/cli.py index 5d330d8f..48f53a61 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -1028,17 +1028,27 @@ class VirtCLIParser(object): def __init_global_params(self): def set_clearxml_cb(opts, inst, cliname, val): - ignore = opts = cliname + ignore = opts + ignore = cliname if not self.objclass and not self.clear_attr: raise RuntimeError("Don't know how to clearxml --%s" % self.cli_arg_name) if val is not True: return + clear_inst = inst if self.clear_attr: - getattr(inst, self.clear_attr).clear() - else: - inst.clear() + clear_inst = getattr(inst, self.clear_attr) + + # If there's any opts remaining, leave the root stub element + # in place, so virt-xml updates are done in place. + # + # So --edit --cpu clearxml=yes should remove the entire + # block. But --edit --cpu clearxml=yes,model=foo should leave + # a stub in place, so that it gets model=foo in place, + # otherwise the newly created cpu block gets appened to the + # end of the domain XML, which gives an ugly diff + clear_inst.clear(leave_stub=bool(opts.opts)) self.set_param(None, "clearxml", setter_cb=set_clearxml_cb, is_onoff=True) diff --git a/virtinst/support.py b/virtinst/support.py index 18e74ed0..8e12e888 100644 --- a/virtinst/support.py +++ b/virtinst/support.py @@ -312,9 +312,9 @@ SUPPORT_CONN_MEM_STATS_PERIOD = _make( function="virDomain.setMemoryStatsPeriod", version="1.1.1", hv_version={"qemu": 0}) SUPPORT_CONN_SPICE_GL = _make(version="1.3.3", - hv_version={"qemu": "2.5.92", "test": 0}) + hv_version={"qemu": "2.7.92", "test": 0}) SUPPORT_CONN_VIDEO_VIRTIO_ACCEL3D = _make(version="1.3.0", - hv_version={"qemu": "2.5.0", "test": 0}) + hv_version={"qemu": "2.7.0", "test": 0}) # This is for disk . xen supports this, but it's diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py index ba2c19a5..d5475e7d 100644 --- a/virtinst/xmlbuilder.py +++ b/virtinst/xmlbuilder.py @@ -38,6 +38,10 @@ _trackprops = bool("VIRTINST_TEST_SUITE" in os.environ) _allprops = [] _seenprops = [] +# Convenience global to prevent _remove_xpath_node from unlinking the +# top relavtive node in certain cases +_top_node = None + class _DocCleanupWrapper(object): def __init__(self, doc): @@ -224,17 +228,17 @@ def _remove_xpath_node(ctx, xpath, dofree=True): if not is_orig: continue + if node == root_node or node == _top_node: + # Don't unlink the root node, since it's spread out to all + # child objects and it ends up wreaking havoc. + break + # Look for preceding whitespace and remove it white = node.get_prev() if white and white.type == "text" and not white.content.count("<"): white.unlinkNode() white.freeNode() - if node == root_node: - # Don't unlink the root node, since it's spread out to all - # child objects and it ends up wreaking havoc. - break - node.unlinkNode() if dofree: node.freeNode() @@ -836,17 +840,28 @@ class XMLBuilder(object): finally: self._finish_get_xml(data) - def clear(self): + def clear(self, leave_stub=False): """ Wipe out all properties of the object + + :param leave_stub: if True, don't unlink the top stub node, + see virtinst/cli usage for an explanation """ - props = self._all_xml_props().values() - props += self._all_child_props().values() - for prop in props: - prop.clear(self) + global _top_node + old_top_node = _top_node + try: + if leave_stub: + _top_node = _get_xpath_node(self._xmlstate.xml_ctx, + self.get_root_xpath()) + props = self._all_xml_props().values() + props += self._all_child_props().values() + for prop in props: + prop.clear(self) + finally: + _top_node = old_top_node is_child = bool(re.match("^.*\[\d+\]$", self.get_root_xpath())) - if is_child: + if is_child or leave_stub: # User requested to clear an object that is the child of # another object (xpath ends in [1] etc). We can't fully remove # the node in that case, since then the xmlbuilder object is @@ -855,7 +870,8 @@ class XMLBuilder(object): node = _get_xpath_node(self._xmlstate.xml_ctx, self.get_root_xpath()) indent = 2 * self.get_root_xpath().count("/") - node.setContent("\n" + (indent * " ")) + if node: + node.setContent("\n" + (indent * " ")) else: _remove_xpath_node(self._xmlstate.xml_ctx, self.get_root_xpath())