[Kimchi-devel] [PATCH V11 4/7] manually manage the metadata element

Tue Apr 29 00:13:34 UTC 2014

On 04/29/2014 06:04 AM, Aline Manera wrote:
> On 04/28/2014 12:44 PM, shaohef at linux.vnet.ibm.com wrote:
>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>
>> virDomain.metadata and virDomain.setMetadata does not work in all livirt
>> versions used in the supported distros.
>>
>> So if libvirt do not support these two API.
>>
>> let kimchi manually manage the metadata element
>>
>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>> src/kimchi/model/utils.py | 105 
>> +++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 85 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py
>> index 3136402..2614008 100644
>> --- a/src/kimchi/model/utils.py
>> +++ b/src/kimchi/model/utils.py
>> @@ -18,9 +18,10 @@
>> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>> 02110-1301 USA
>>
>> from kimchi.exception import OperationFailed
>> +from kimchi.model.config import CapabilitiesModel
>> import libvirt
>> -from lxml import etree
>> -from lxml.builder import E
>> +from lxml import etree, objectify
>> +from lxml.builder import E, ElementMaker
>>
>>
>> KIMCHI_META_URL = "https://github.com/kimchi-project/kimchi"
>> @@ -37,6 +38,15 @@ def get_vm_name(vm_name, t_name, name_list):
>> raise OperationFailed("KCHUTILS0003E")
>>
>>
>> +def create_metadata_node(tag):
>> + if CapabilitiesModel().metadata_support:
>> + return E(tag)
>> + else:
>> + EM = ElementMaker(namespace=KIMCHI_META_URL,
>> + nsmap={KIMCHI_NAMESPACE: KIMCHI_META_URL})
>> + return EM(tag)
>> +
>> +
>> def get_vm_config_flag(dom, mode="persistent"):
>> # libvirt.VIR_DOMAIN_AFFECT_CURRENT is 0
>> # VIR_DOMAIN_AFFECT_LIVE is 1, VIR_DOMAIN_AFFECT_CONFIG is 2
>> @@ -50,6 +60,30 @@ def get_vm_config_flag(dom, mode="persistent"):
>> return flag[mode]
>>
>
>> +def _kimchi_set_metadata_node(dom, node_xml):
>> + node = etree.fromstring(node_xml)
>> + # some other tools will not let libvirt create a persistent
>> + # configuration, raise exception.
>> + if not dom.isPersistent():
>> + msg = 'The VM has not a persistent configuration'
>> + raise OperationFailed("KCHVM0030E",
>> + {'name': dom.name(), "err": msg})
>> + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
>> + root = etree.fromstring(xml)
>> + metadata = root.find("metadata")
>> + if metadata is None:
>> + metadata = E.metadata()
>> + root.append(metadata)
>> + kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL)
>> + if kimchi is None:
>> + kimchi = create_metadata_node("kimchi")
>> + metadata.append(kimchi)
>> + old_node = kimchi.find(node.tag)
>> + (kimchi.replace(old_node, node) if old_node is not None
>> + else kimchi.append(node))
>> + dom.connect().defineXML(etree.tostring(root))
>> +
>> +
>
> There is a some duplicated code here and in set_metadata_node()
> We should have special function only for special matters.
> See below
>
>
>> def libvirt_get_kimchi_metadata_node(dom, mode="current"):
>> try:
>> xml = dom.metadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
>> @@ -61,29 +95,60 @@ def libvirt_get_kimchi_metadata_node(dom, 
>> mode="current"):
>>
>>
>> def set_metadata_node(dom, node_xml, mode="all"):
>
> Why node_xml? Use node directly as we use ET to create the element in 
> model/vms.py
> That way we avoid ET node -> string -> ET node
>
>> - element = etree.fromstring(node_xml)
>> - kimchi = libvirt_get_kimchi_metadata_node(dom, mode)
>> - old_node = None
>> - if kimchi is not None:
>> - old_node = kimchi.find(element.tag)
>> - else:
>> - kimchi = E.kimchi()
>> - (kimchi.replace(old_node, element) if old_node is not None
>> - else kimchi.append(element))
>> + if CapabilitiesModel().metadata_support:
>> + element = etree.fromstring(node_xml)
>> + kimchi = libvirt_get_kimchi_metadata_node(dom, mode)
>> + old_node = None
>> + if kimchi is not None:
>> + old_node = kimchi.find(element.tag)
>> + else:
>> + kimchi = E.kimchi()
>> + (kimchi.replace(old_node, element) if old_node is not None
>> + else kimchi.append(element))
>
> There is already an if/else statement above, use the same one to 
> append or replace the new node
>
>
> if kimchi is None:
> node = kimchi.find(element.tag)
> kimchi.replace(node, element)
For node maybe None.
In [145]: kimchi.replace(node, element)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-145-bb05d26e1b86> in <module>()
----> 1 metadata.replace(node, element)

TypeError: Argument 'old_element' has incorrect type (expected 
lxml.etree._Element, got NoneType)

> else:
> kimchi = E.kimchi()
> kimchi.append(element)
>
>
> Joining common code in one place:
>
> def set_metadata_node(dom, node, mode="all")
> kimchi = get_kimchi_metadata_node(dom, mode)
> if kimchi is None:
> kimchi = create_metadata_node("kimchi")
> else:
> old_node = kimchi.find(node.tag)
> kimchi.replace(old_node, node)
>
> if CapabilitiesModel().metadata_support:
> return dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, node_xml,
> KIMCHI_NAMESPACE, KIMCHI_META_URL,
> flags=get_vm_config_flag(dom, mode))
>
> # Without metadata support
> # some other tools will not let libvirt create a persistent
> # configuration, raise exception.
> if not dom.isPersistent():
> msg = 'The VM has not a persistent configuration'
> raise OperationFailed("KCHVM0030E", + {'name': dom.name(), "err": msg})
>
> xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
> root = etree.fromstring(xml)
> metadata = root.find("metadata")
> if metadtaa is None:
> metadata = E.metadata()
> root.append(metadata)
>
> old_node = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL)
> metadata.replace(old_node, kimchi) if old_node else 
> metadata.append(kimchi)
> dom.connect().defineXML(etree.tostring(root))
>
> def get_kimchi_metadata_node(dom, mode):
> if CapabilitiesModel().metadata_support:
> xml = dom.metadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
> KIMCHI_META_URL,
> flags=get_vm_config_flag(dom, mode))
> return etree.fromstring(xml)
>
> # Without metadata support
> xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
> root = etree.fromstring(xml)
> metadata = root.find("metadata")
> return root.find("metadata/{%s}kimchi" % KIMCHI_META_URL)
>
>
>>
>> - node_xml = etree.tostring(kimchi)
>> - # From libvirt doc, Passing None for @metadata says to remove that
>> - # element from the domain XML (passing the empty string leaves the
>> - # element present). Do not support remove the old metadata.
>> - dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, node_xml,
>> - KIMCHI_NAMESPACE, KIMCHI_META_URL,
>> - flags=get_vm_config_flag(dom, mode))
>> + node_xml = etree.tostring(kimchi)
>> + # From libvirt doc, Passing None for @metadata says to remove that
>> + # element from the domain XML (passing the empty string leaves the
>> + # element present). Do not support remove the old metadata.
>> + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, node_xml,
>> + KIMCHI_NAMESPACE, KIMCHI_META_URL,
>> + flags=get_vm_config_flag(dom, mode))
>> + else:
>> + # FIXME remove this code when all distro libvirt supports metadata 
>> element
>> + _kimchi_set_metadata_node(dom, node_xml)
>>
>>
>> -def get_metadata_node(dom, tag, mode="current"):
>> - kimchi = libvirt_get_kimchi_metadata_node(dom, mode)
>> +def _kimchi_get_metadata_node(dom, tag):
>> + # some other tools will not let libvirt create a persistent
>> + # configuration, just return empty
>> + if not dom.isPersistent():
>> + return ""
>> + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
>> + root = etree.fromstring(xml)
>> + kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL)
>
>
>> + #remove the "kimchi" prifix of xml
>> if kimchi is not None:
>> + for child in kimchi.getiterator():
>> + i = child.tag.find('}')
>> + if i >= 0:
>> + child.tag = child.tag[i+1:]
>> + objectify.deannotate(kimchi, cleanup_namespaces=True)
>> + kimchi_xml = etree.tostring(kimchi)
>> + kimchi = etree.fromstring(kimchi_xml)
>> node = kimchi.find(tag)
>> if node is not None:
>> return etree.tostring(node)
>> return ""
>
> What about?
>
> if kimchi is None:
> return ""
>
> kimchi_xml = etree.tostring(kimchi)
> Kimchi = kimchi_xml.replace("{%s}" % KIMCHI_META_URL, "")

it should be kimchi_xml.replace(xmlns:* 
="https://github.com/kimchi-project/kimchi, "")
also it should remove the ”kimchi:“
<kimchi:access ">
<kimchi:user>user1</kimchi:user>
<kimchi:user>user2</kimchi:user>
<kimchi:group>group1</kimchi:group>
<kimchi:group>group2</kimchi:group>
</kimchi:access>

or it will report:

In [149]: etree.fromstring(kimchi_xml)
File "<string>", line unknown
XMLSyntaxError: Namespace prefix kimchi on access is not defined, line 
2, column 19

I can use RE to handle the xml directly if you do not like handle the ET 
node directly.

> node = kimchi.find(tag)
> if node is not None:
> return etree.tostring(node)
>
> return ""
>
>
>> +
>> +
>> +def get_metadata_node(dom, tag, mode="current"):
>> + if CapabilitiesModel().metadata_support:
>> + kimchi = libvirt_get_kimchi_metadata_node(dom, mode)
>> + if kimchi is not None:
>> + node = kimchi.find(tag)
>> + if node is not None:
>> + return etree.tostring(node)
>> + return ""
>> + else:
>> + # FIXME remove this code when all distro libvirt supports metadata 
>> element
>> + return _kimchi_get_metadata_node(dom, tag)
>
>
>

-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center