diff --git a/tests/xmlparse-xml/change-guest-in.xml b/tests/xmlparse-xml/change-guest-in.xml index ab56c03d2..298e5056d 100644 --- a/tests/xmlparse-xml/change-guest-in.xml +++ b/tests/xmlparse-xml/change-guest-in.xml @@ -19,6 +19,8 @@ + + destroy diff --git a/tests/xmlparse-xml/change-guest-out.xml b/tests/xmlparse-xml/change-guest-out.xml index 9cd0ddf76..dfceaf58e 100644 --- a/tests/xmlparse-xml/change-guest-out.xml +++ b/tests/xmlparse-xml/change-guest-out.xml @@ -21,6 +21,8 @@ qemuvendor + + diff --git a/tests/xmlparse.py b/tests/xmlparse.py index d9bf574d3..db679497b 100644 --- a/tests/xmlparse.py +++ b/tests/xmlparse.py @@ -168,7 +168,10 @@ class XMLParseTest(unittest.TestCase): check = self._make_checker(guest.cpu.features[0]) check("name", "x2apic") check("policy", "force", "disable") - guest.cpu.remove_feature(guest.cpu.features[1]) + rmfeat = guest.cpu.features[3] + guest.cpu.remove_feature(rmfeat) + self.assertEquals(rmfeat.get_xml_config(), + """""") guest.cpu.add_feature("addfeature") check = self._make_checker(guest.numatune) diff --git a/virtinst/cpu.py b/virtinst/cpu.py index af3744e49..b2fe1e40b 100644 --- a/virtinst/cpu.py +++ b/virtinst/cpu.py @@ -17,7 +17,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, # MA 02110-1301 USA. -from virtinst.xmlbuilder import XMLBuilder, XMLProperty +from virtinst.xmlbuilder import XMLBuilder, XMLProperty, XMLChildProperty class CPUFeature(XMLBuilder): @@ -28,28 +28,10 @@ class CPUFeature(XMLBuilder): POLICIES = ["force", "require", "optional", "disable", "forbid"] _XML_ROOT_XPATH = "/domain/cpu/feature" - _XML_PROP_ORDER = ["_xmlname", "policy"] + _XML_PROP_ORDER = ["name", "policy"] - def __init__(self, conn, name, parsexml=None, parsexmlnode=None): - XMLBuilder.__init__(self, conn, parsexml, parsexmlnode) - - self._name = name - self._xmlname = name - - def _get_name(self): - return self._xmlname - name = property(_get_name) - - def _name_xpath(self): - return "./cpu/feature[@name='%s']/@name" % self._name - _xmlname = XMLProperty(name="feature name", - make_getter_xpath_cb=_name_xpath, - make_setter_xpath_cb=_name_xpath) - def _policy_xpath(self): - return "./cpu/feature[@name='%s']/@policy" % self._name - policy = XMLProperty(name="feature policy", - make_getter_xpath_cb=_policy_xpath, - make_setter_xpath_cb=_policy_xpath) + name = XMLProperty("./@name") + policy = XMLProperty("./@policy") class CPU(XMLBuilder): @@ -61,36 +43,17 @@ class CPU(XMLBuilder): _XML_ROOT_XPATH = "/domain/cpu" _XML_PROP_ORDER = ["mode", "match", "model", "vendor", - "sockets", "cores", "threads", "_features"] - - def __init__(self, conn, parsexml=None, parsexmlnode=None): - self._features = [] - XMLBuilder.__init__(self, conn, parsexml, parsexmlnode) - - def _parsexml(self, xml, node): - XMLBuilder._parsexml(self, xml, node) - - for node in self._xml_node.children or []: - if node.name != "feature": - continue - if not node.prop("name"): - continue - feature = CPUFeature(self.conn, node.prop("name"), - parsexmlnode=self._xml_node) - self._features.append(feature) + "sockets", "cores", "threads", "features"] def add_feature(self, name, policy="require"): - feature = CPUFeature(self.conn, name, parsexmlnode=self._xml_node) + feature = CPUFeature(self.conn) + feature.name = name feature.policy = policy - self._features.append(feature) + self._add_child(feature) def remove_feature(self, feature): - self._features.remove(feature) - feature.clear() - - def _get_features(self): - return self._features[:] - features = property(_get_features) + self._remove_child(feature) + features = XMLChildProperty(CPUFeature) def copy_host_cpu(self): """ diff --git a/virtinst/device.py b/virtinst/device.py index 6bcebf41e..f7dce3804 100644 --- a/virtinst/device.py +++ b/virtinst/device.py @@ -71,6 +71,7 @@ class VirtualDevice(XMLBuilder): @classmethod def register_type(cls): + cls._XML_ROOT_XPATH = "/domain/devices/%s" % cls.virtual_device_type VirtualDevice.virtual_device_classes[cls.virtual_device_type] = cls # General device type (disk, interface, etc.) @@ -82,7 +83,6 @@ class VirtualDevice(XMLBuilder): @param conn: libvirt connection to validate device against """ - self._XML_ROOT_XPATH = "/domain/devices/%s" % self.virtual_device_type XMLBuilder.__init__(self, conn, parsexml, parsexmlnode) diff --git a/virtinst/guest.py b/virtinst/guest.py index 2fae924a2..3b7a3c664 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -35,7 +35,7 @@ from virtinst import Seclabel from virtinst import CPU from virtinst import DomainNumatune from virtinst import DomainFeatures -from virtinst.xmlbuilder import XMLBuilder, XMLProperty +from virtinst.xmlbuilder import XMLBuilder, XMLProperty, XMLChildProperty from virtinst import osdict @@ -92,8 +92,6 @@ class Guest(XMLBuilder): "seclabel"] def __init__(self, conn, parsexml=None, parsexmlnode=None): - self._devices = [] - self._install_devices = [] XMLBuilder.__init__(self, conn, parsexml, parsexmlnode) self.autostart = False @@ -102,6 +100,7 @@ class Guest(XMLBuilder): self._os_variant = None self._random_uuid = None + self._install_devices = [] # The libvirt virDomain object we 'Create' self.domain = None @@ -194,16 +193,6 @@ class Guest(XMLBuilder): # Device Add/Remove Public API methods # ######################################## - def _dev_build_list(self, devtype, devlist=None): - if devlist is None: - devlist = self._devices - - newlist = [] - for i in devlist: - if devtype == "all" or i.virtual_device_type == devtype: - newlist.append(i) - return newlist - def add_device(self, dev): """ Add the passed device to the guest's device list. @@ -211,12 +200,15 @@ class Guest(XMLBuilder): @param dev: VirtualDevice instance to attach to guest @param set_defaults: Whether to set defaults for the device """ - self._track_device(dev) - self._recalculate_device_xpaths() self._add_child(dev) - def _track_device(self, dev): - self._devices.append(dev) + def remove_device(self, dev): + """ + Remove the passed device from the guest's device list + + @param dev: VirtualDevice instance + """ + self._remove_child(dev) def get_devices(self, devtype): """ @@ -226,8 +218,14 @@ class Guest(XMLBuilder): @param devtype: Device type to search for (one of VirtualDevice.virtual_device_types) """ - devlist = self._dev_build_list(devtype) - return self._dev_build_list(devtype, devlist) + newlist = [] + for i in self._devices: + if devtype == "all" or i.virtual_device_type == devtype: + newlist.append(i) + return newlist + _devices = XMLChildProperty( + [VirtualDevice.virtual_device_classes[_n] + for _n in VirtualDevice.virtual_device_types]) def get_all_devices(self): """ @@ -239,50 +237,6 @@ class Guest(XMLBuilder): return retlist all_devices = property(lambda s: s.get_all_devices()) - def remove_device(self, dev): - """ - Remove the passed device from the guest's device list - - @param dev: VirtualDevice instance - """ - self._devices.remove(dev) - self._remove_child(dev) - self._recalculate_device_xpaths() - - - ################################ - # Private xml building methods # - ################################ - - def _parsexml(self, xml, node): - XMLBuilder._parsexml(self, xml, node) - - for node in self._xml_node.children or []: - if node.name != "devices": - continue - - devnodes = [ - x for x in node.children if - (x.name in VirtualDevice.virtual_device_classes and - x.parent == node) - ] - for devnode in devnodes: - objclass = VirtualDevice.virtual_device_classes[devnode.name] - dev = objclass(self.conn, parsexmlnode=self._xml_node) - self._track_device(dev) - - self._recalculate_device_xpaths() - - def _recalculate_device_xpaths(self): - count = {} - for dev in self.get_all_devices(): - devtype = dev.virtual_device_type - if devtype not in count: - count[devtype] = 1 - newpath = "./devices/%s[%d]" % (devtype, count[devtype]) - dev.set_root_xpath(newpath) - count[devtype] += 1 - ############################ # Install Helper functions # @@ -312,9 +266,9 @@ class Guest(XMLBuilder): # We do a shallow copy of the device list here, and set the defaults. # This way, default changes aren't persistent, and we don't need # to worry about when to call set_defaults - data = (self._devices, self.features, self.os) + data = (self._devices[:], self.features, self.os) try: - self._devices = [dev.copy() for dev in self._devices] + self._propstore["_devices"] = [dev.copy() for dev in self._devices] self.features = self.features.copy() self.os = self.os.copy() support.set_rhel6(self._is_rhel6()) @@ -324,7 +278,7 @@ class Guest(XMLBuilder): return data def _finish_get_xml(self, data): - self._devices, self.features, self.os = data + self._propstore["_devices"], self.features, self.os = data support.set_rhel6(False) def get_install_xml(self, *args, **kwargs): @@ -370,7 +324,6 @@ class Guest(XMLBuilder): self.bootloader = "/usr/bin/pygrub" self.os.clear() - self._recalculate_device_xpaths() return self.get_xml_config() def get_continue_inst(self): @@ -688,7 +641,7 @@ class Guest(XMLBuilder): def _check_address_multi(self): addresses = {} - for d in self._devices: + for d in self.all_devices: if d.address.type != d.address.ADDRESS_TYPE_PCI: continue diff --git a/virtinst/osxml.py b/virtinst/osxml.py index c5fd0f1c5..9a3246113 100644 --- a/virtinst/osxml.py +++ b/virtinst/osxml.py @@ -17,26 +17,12 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, # MA 02110-1301 USA. -from virtinst.xmlbuilder import XMLBuilder, XMLProperty +from virtinst.xmlbuilder import XMLBuilder, XMLProperty, XMLChildProperty -class BootDevice(XMLBuilder): +class _BootDevice(XMLBuilder): _XML_ROOT_XPATH = "/domain/os/boot" - - def __init__(self, conn, dev, parsexml=None, parsexmlnode=None): - XMLBuilder.__init__(self, conn, parsexml, parsexmlnode) - self._dev = dev - self._xmldev = dev - - def _get_dev(self): - return self._xmldev - dev = property(_get_dev) - - def _dev_xpath(self): - return "./os/boot[@dev='%s']/@dev" % self._dev - _xmldev = XMLProperty(name="boot dev type", - make_getter_xpath_cb=_dev_xpath, - make_setter_xpath_cb=_dev_xpath) + dev = XMLProperty("./@dev") class OSXML(XMLBuilder): @@ -50,24 +36,6 @@ class OSXML(XMLBuilder): boot_devices = [BOOT_DEVICE_HARDDISK, BOOT_DEVICE_CDROM, BOOT_DEVICE_FLOPPY, BOOT_DEVICE_NETWORK] - def __init__(self, *args, **kwargs): - self._bootdevs = [] - XMLBuilder.__init__(self, *args, **kwargs) - - def _parsexml(self, xml, node): - XMLBuilder._parsexml(self, xml, node) - - for node in self._xml_node.children or []: - if node.name != "boot" or not node.prop("dev"): - continue - bootdev = BootDevice(self.conn, node.prop("dev"), - parsexmlnode=self._xml_node) - self._bootdevs.append(bootdev) - - def clear(self): - XMLBuilder.clear(self) - self.bootorder = [] - def is_hvm(self): return self.os_type == "hvm" def is_xenpv(self): @@ -95,10 +63,13 @@ class OSXML(XMLBuilder): return [dev.dev for dev in self._bootdevs] def _set_bootorder(self, newdevs): for dev in self._bootdevs: - dev.clear() - self._bootdevs = [BootDevice(self.conn, d, - parsexmlnode=self._xml_node) - for d in newdevs] + self._remove_child(dev) + + for d in newdevs: + dev = _BootDevice(self.conn) + dev.dev = d + self._add_child(dev) + _bootdevs = XMLChildProperty(_BootDevice) bootorder = property(_get_bootorder, _set_bootorder) enable_bootmenu = XMLProperty(xpath="./os/bootmenu/@enable", is_yesno=True) diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py index ba712ded6..4fa6e097d 100644 --- a/virtinst/xmlbuilder.py +++ b/virtinst/xmlbuilder.py @@ -246,8 +246,42 @@ def _remove_xpath_node(ctx, xpath, dofree=True, unlinkroot=True): node.freeNode() +class XMLChildProperty(property): + """ + Property that points to a class used for parsing a subsection of + of the parent XML. For example when we deligate parsing + /domain/cpu/feature of the /domain/cpu class. + """ + def __init__(self, child_classes): + self.child_classes = util.listify(child_classes) + property.__init__(self, self._fget) + + def __repr__(self): + return "" % (str(self.child_classes), id(self)) + + def _findpropname(self, xmlbuilder): + for key, val in xmlbuilder._all_child_props().items(): + if val is self: + return key + raise RuntimeError("Didn't find expected property=%s" % self) + + def _get_list(self, xmlbuilder): + propname = self._findpropname(xmlbuilder) + if propname not in xmlbuilder._propstore: + xmlbuilder._propstore[propname] = [] + return xmlbuilder._propstore[propname] + + def _fget(self, xmlbuilder): + return self._get_list(xmlbuilder)[:] + + def append(self, xmlbuilder, obj): + self._get_list(xmlbuilder).append(obj) + def remove(self, xmlbuilder, obj): + self._get_list(xmlbuilder).remove(obj) + + class XMLProperty(property): - def __init__(self, doc=None, xpath=None, name=None, + def __init__(self, xpath=None, name=None, doc=None, set_converter=None, validate_cb=None, make_getter_xpath_cb=None, make_setter_xpath_cb=None, is_bool=False, is_int=False, is_yesno=False, is_onoff=False, @@ -341,10 +375,10 @@ class XMLProperty(property): Map the raw property() instance to the param name it's exposed as in the XMLBuilder class. This is just for debug purposes. """ - for key, val in xmlbuilder.all_xml_props().items(): + for key, val in xmlbuilder._all_xml_props().items(): if val is self: return key - raise RuntimeError("Didn't find expected property") + raise RuntimeError("Didn't find expected property=%s" % self) def _xpath_for_getter(self, xmlbuilder): ret = self._xpath @@ -586,31 +620,34 @@ class _XMLState(object): self.xml_root_doc = None self.dump_xpath = None self.root_xpath = None + self.node_top_xpath = None self.is_build = False - if not (parsexml or parsexmlnode): - parsexml = self.make_xml_stub() + if not parsexml and not parsexmlnode: self.is_build = True - self._parse(parsexml, parsexmlnode) def _parse(self, xml, node): - if xml: + if node: + self.xml_root_doc = None + self.xml_node = node + self.is_build = (getattr(node, "virtinst_is_build", False) or + self.is_build) + else: + if not xml: + xml = self.make_xml_stub() doc = libxml2.parseDoc(xml) self.xml_root_doc = _DocCleanupWrapper(doc) self.xml_node = doc.children self.xml_node.virtinst_is_build = self.is_build + self.xml_node.virtinst_node_top_xpath = self.orig_root_xpath # This just stores a reference to our root doc wrapper in # the root node, so when the node goes away it triggers # auto free'ing of the doc self.xml_node.virtinst_root_doc = self.xml_root_doc - else: - self.xml_root_doc = None - self.xml_node = node - self.is_build = (getattr(node, "virtinst_is_build", False) or - self.is_build) + self.node_top_xpath = self.xml_node.virtinst_node_top_xpath self.xml_ctx = _make_xml_context(self.xml_node) self.set_root_xpath(self.root_xpath) @@ -677,6 +714,14 @@ class XMLBuilder(object): ret = copy.copy(self) ret._propstore = ret._propstore.copy() ret._proporder = ret._proporder[:] + + # XMLChildProperty stores a list in propstore, which dict shallow + # copy won't fix for us. + for name, value in ret._propstore.items(): + if type(value) is not list: + continue + ret._propstore[name] = [obj.copy() for obj in ret._propstore[name]] + return ret def set_root_xpath(self, xpath): @@ -696,17 +741,6 @@ class XMLBuilder(object): def fix_relative_xpath(self, xpath): return self._xmlstate.fix_relative_xpath(xpath) - def all_xml_props(self): - """ - Return a list of all XMLProperty instances that this class has. - """ - ret = {} - for c in reversed(type.mro(self.__class__)[:-1]): - for key, val in c.__dict__.items(): - if val.__class__ is XMLProperty: - ret[key] = val - return ret - ############################ # Public XML managing APIs # @@ -726,8 +760,11 @@ class XMLBuilder(object): """ Wipe out all properties of the object """ - for prop in self.all_xml_props().values(): + for prop in self._all_xml_props().values(): prop._clear(self) + for prop in self._all_child_props().values(): + for obj in prop._get_list(self)[:]: + self._remove_child(obj) def validate(self): """ @@ -761,13 +798,6 @@ class XMLBuilder(object): """ ignore = data - def _parsexml(self, xml, node): - """ - Subclasses hook here to do things like parse into - internal state - """ - self._xmlstate = _XMLState(self._XML_ROOT_XPATH, xml, node) - ################ # Internal API # @@ -775,6 +805,76 @@ class XMLBuilder(object): _xml_node = property(lambda s: s._xmlstate.xml_node) + def _all_xml_props(self): + """ + Return a list of all XMLProperty instances that this class has. + """ + ret = {} + for c in reversed(type.mro(self.__class__)[:-1]): + for key, val in c.__dict__.items(): + if val.__class__ is XMLProperty: + ret[key] = val + return ret + + def _all_child_props(self): + """ + Return a list of all XMLChildProperty instances that this class has. + """ + ret = {} + for c in reversed(type.mro(self.__class__)[:-1]): + for key, val in c.__dict__.items(): + if val.__class__ is XMLChildProperty: + ret[key] = val + return ret + + def _find_child_prop(self, child_class): + xmlprops = self._all_child_props() + for xmlprop in xmlprops.values(): + if child_class in xmlprop.child_classes: + return xmlprop + raise RuntimeError("programming error: " + "Didn't find child property for child_class=%s" % + child_class) + + def _add_child(self, obj): + """ + Insert the passed XMLBuilder object into our XML document. The + object needs to have an associated mapping via XMLChildProperty + """ + xmlprop = self._find_child_prop(obj.__class__) + xmlprop.append(self, obj) + self._set_child_xpaths() + + if not obj._xmlstate.is_build: + if not obj.get_root_xpath(): + raise RuntimeError("programming error: must set device " + "root xpath before add_child") + orig_xpath = obj.get_root_xpath() + obj.set_root_xpath(None) + xml = obj.get_xml_config() + obj.set_root_xpath(orig_xpath) + + use_xpath = orig_xpath.rsplit("/", 1)[0] + newnode = libxml2.parseDoc(xml).children + _build_xpath_node(self._xmlstate.xml_ctx, use_xpath, newnode) + obj.set_root_xpath(orig_xpath) + obj._xmlstate._parse(None, self._xmlstate.xml_node) + + def _remove_child(self, obj): + """ + Remove the passed XMLBuilder object from our XML document, but + ensure it's data isn't altered. + """ + xmlprop = self._find_child_prop(obj.__class__) + xmlprop.remove(self, obj) + + xpath = obj.get_root_xpath() + xml = obj.get_xml_config() + obj.set_root_xpath(None) + obj._xmlstate._parse(xml, None) + _remove_xpath_node(self._xmlstate.xml_ctx, xpath, dofree=False) + self._set_child_xpaths() + def _all_subelement_props(self): """ Return a list of all sub element properties this class tracks. @@ -782,51 +882,63 @@ class XMLBuilder(object): a parent class, which alters the parent XML. VirtualDevice.address is an example. """ - xmlprops = self.all_xml_props() + xmlprops = self._all_xml_props() + childprops = self._all_child_props() ret = [] + propmap = {} for propname in self._XML_PROP_ORDER: if propname not in xmlprops: - ret.extend(util.listify(getattr(self, propname))) + propmap[propname] = getattr(self, propname) + for propname in childprops: + propmap[propname] = getattr(self, propname) + + for objs in propmap.values(): + ret.extend(util.listify(objs)) return ret - def _add_child(self, dev): - """ - Insert the passed XMLBuilder object into our XML document at the - specified path - """ - if not dev._xmlstate.is_build: - if not dev.get_root_xpath(): - raise RuntimeError("programming error: must set device " - "root xpath before add_child") - orig_xpath = dev.get_root_xpath() - dev.set_root_xpath(None) - xml = dev.get_xml_config() - dev.set_root_xpath(orig_xpath) - - use_xpath = orig_xpath.rsplit("/", 1)[0] - newnode = libxml2.parseDoc(xml).children - _build_xpath_node(self._xmlstate.xml_ctx, use_xpath, newnode) - dev.set_root_xpath(orig_xpath) - - dev._xmlstate._parse(None, self._xmlstate.xml_node) - - def _remove_child(self, dev): - """ - Remove the passed XMLBuilder object from our XML document, but - ensure it's data isn't altered. - """ - xpath = dev.get_root_xpath() - xml = dev.get_xml_config() - dev.set_root_xpath(None) - dev._xmlstate._parse(xml, None) - if xpath: - _remove_xpath_node(self._xmlstate.xml_ctx, xpath, dofree=False) - ################################# # Private XML building routines # ################################# + def _parsexml(self, xml, node): + if self._xmlstate: + raise RuntimeError("_parsexml can't be called twice") + self._xmlstate = _XMLState(self._XML_ROOT_XPATH, xml, node) + + # Walk the XML tree and hand of parsing to any registered + # child classes + for xmlprop in self._all_child_props().values(): + for child_class in xmlprop.child_classes: + prop_path = child_class._XML_ROOT_XPATH.replace( + self._xmlstate.node_top_xpath, ".") + + nodecount = int(self._xml_node.xpathEval( + "count(%s)" % prop_path)) + for ignore in range(nodecount): + xmlprop.append(self, + child_class(self.conn, parsexmlnode=self._xml_node)) + self._set_child_xpaths() + + def _set_child_xpaths(self): + """ + Walk the list of child properties and make sure their + xpaths point at their particular element + """ + typecount = {} + for propname in self._all_child_props(): + for obj in getattr(self, propname): + class_type = obj.__class__ + if class_type not in typecount: + typecount[class_type] = 0 + idx = typecount[class_type] + + prop_path = obj._XML_ROOT_XPATH.replace( + self._xmlstate.node_top_xpath, ".") + obj.set_root_xpath(prop_path + ("[%d]" % (idx + 1))) + typecount[class_type] += 1 + + def _do_get_xml_config(self): xmlstub = self._xmlstate.make_xml_stub() @@ -866,7 +978,7 @@ class XMLBuilder(object): def _do_add_parse_bits(self, node, ctx): # Set all defaults if the properties have one registered - xmlprops = self.all_xml_props() + xmlprops = self._all_xml_props() for prop in xmlprops.values(): prop._set_default(self)