[Kimchi-devel] [PATCH][Kimchi] Recognize different interfaces type when cloning vm

[Aline Manera]

On 06/16/2016 07:26 PM, Ramon Medeiros wrote:
> When cloning a VM, it's needed to add a new mac address. model/vms.py
> was selection <interface type=network /> only. Using correct xpath
> framework, now all <interface> tag with type as attribute and <mac
> address> as a subelement, is selected. After all, they will received new
> mac address. This way, it's not needed to check if the interfaces has it
> own mac address.
>
> Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
> ---
>   model/vms.py | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/model/vms.py b/model/vms.py
> index f069c4e..9753a10 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -83,12 +83,10 @@ VM_ONLINE_UPDATE_PARAMS = ['graphics', 'groups', 'memory', 'users']
>   VM_OFFLINE_UPDATE_PARAMS = ['cpu_info', 'graphics', 'groups', 'memory',


>                               'name', 'users']
>
> +XPATH_DOMAIN_CHANGE_MAC = "./devices/interface[@type='%s']/mac[@address='%s']"

You don't need to filter the interface type as you are going to change 
all MAC address.

So ./devices/interfaces/mac[@address=%s] should work for your matters.

>   XPATH_DOMAIN_DISK = "/domain/devices/disk[@device='disk']/source/@file"
>   XPATH_DOMAIN_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']"
>   XPATH_DOMAIN_NAME = '/domain/name'
> -XPATH_DOMAIN_MAC = "/domain/devices/interface[@type='network']/mac/@address"
> -XPATH_DOMAIN_MAC_BY_ADDRESS = "./devices/interface[@type='network']/"\
> -                              "mac[@address='%s']"
>   XPATH_DOMAIN_MEMORY = '/domain/memory'
>   XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit'
>   XPATH_DOMAIN_UUID = '/domain/uuid'
> @@ -97,6 +95,7 @@ XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id'
>   XPATH_CPU = './cpu'
>   XPATH_NAME = './name'
>   XPATH_NUMA_CELL = './cpu/numa/cell'

> +XPATH_MAC = "./devices/interface[@type]/mac/[@address]"

The same I commented above.

>   XPATH_TOPOLOGY = './cpu/topology'
>   XPATH_VCPU = './vcpu'
>   XPATH_MAX_MEMORY = './maxMemory'
> @@ -417,19 +416,25 @@ class VMModel(object):
>           The XML descriptor <xml> with the new MAC addresses instead of the
>           old ones.
>           """
> -        old_macs = xpath_get_text(xml, XPATH_DOMAIN_MAC)
> -        new_macs = []
> +        # get dict with macs and interfaces
> +        old_macs_int = {}

> +        for node in ET.fromstring(xml).findall(XPATH_MAC):
> +            old_macs_int[node.get("address")] = node.getparent().get("type")

Why do you need to record the network type and it will not be changed?
Don't you need to get only the MAC address value and identify a new 
value to be placed on?

And seems you can continue to use xpath_get_text() to do that.

>
> -        for mac in old_macs:
> +        # update mac address
> +        new_macs = []
> +        for mac_addr in old_macs_int:
>               while True:
>                   new_mac = model.vmifaces.VMIfacesModel.random_mac()
>                   # make sure the new MAC doesn't conflict with the original VM
>                   # and with the new values on the new VM.
> -                if new_mac not in (old_macs + new_macs):
> +                if new_mac not in (old_macs_int.keys() + new_macs):
>                       new_macs.append(new_mac)
>                       break
>
> -            xml = xml_item_update(xml, XPATH_DOMAIN_MAC_BY_ADDRESS % mac,
> +            dev_type = old_macs_int[mac_addr]
> +            xml = xml_item_update(xml, XPATH_DOMAIN_CHANGE_MAC % (dev_type,
> +                                                                  mac_addr),
>                                     new_mac, 'address')
>
>           return xml