[Kimchi-devel] [PATCH 2/2] add a smart way to get interface model

[Aline Manera]

On 04/22/2014 11:03 PM, Sheldon wrote:
> On 04/23/2014 02:01 AM, Aline Manera wrote:
>> On 04/22/2014 09:48 AM, shaohef at linux.vnet.ibm.com wrote:
>>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>>
>>> criterias are:
>>> 1. if there are interfaces already attached to a vm, use the same model
>>> 2. if "vmifaces" element in metatdata, get the model from metatdata
>>> 3. if "OS" element in metatdata, get the model from "OS" metatdata
>>
>> What is the difference between getting the model from "vmifaces" and 
>> "OS" metadata?
>> From my view, the "vmifaces" element was set based on OS during VM 
>> creation so it will be the same
>> if we get "OS" element and find the iface model on osinfo.py dict
> There are difference. we will not update "OS", after we install the VM.
> User may install ubuntu 12.10, the the guest OS can upgrade itself to 
> ubuntu 13.04.
> The kimchi will do not update "OS" metadata. For it does not know the 
> guest os change.
> It is still remain ubuntu 12.10.
>
> And as we discuss last week, we don not support user change their OS.
> For example, user insall window, and now him change it to ubuntu.
> We have get agreement that we suggested them to delete the old VM if 
> he do not want to use it any more,
> and create a new template to create a new one.
>
> So we assume the guest "OS" are the same distribution of the "OS" 
> metadata.
> But the guest "OS" version maybe higher than the the "OS" metadat.
> This is good, for the higher version can be compatible with the lower 
> one.
>
> The "ifaces" are different.
> But we will update "ifaces" according to user's choice, when he 
> updates the the model.
> Patch will later.
> Only user knows which is the best model that fit the guest OS.
> For example, user maybe install windows, there will be virtio nic 
> diver for windows,
> but it may not works well, maybe a bug in OS. So it will change it to 
> "e1000" or other model.
>

Ok. I haven't thought we would update the "iface" metadata element 
according to user choice.

> We has get some feedback for these case.
>
> Not sure we still need to be compatible with other VM management.
> So there will be no model info in metadata.
> But we can add this metadata when user update their model.
>>
>> I'd say the criterias are:
>> 1) "vmifaces" metadata
>> 2) if "vmifaces" metadata does not exist AND there are interfaces, 
>> use the same model
>> 3) if "vmifaces" metadata does not exist AND there is NOT interface, 
>> leave model empty to libvirt fill it properly
>>
>>
> What about the criterias:
> 1) "vmifaces" metadata
> 2) if "vmifaces" metadata does not exist AND there are interfaces, use 
> the same model
> 3) choose the model by "OS"
> 4) if "vmifaces" metadata does not exist AND there is NOT 
> interface,and no "OS" metadat,
> leave model empty to libvirt fill it properly

ACK

>
>>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>> ---
>>> src/kimchi/model/vmifaces.py | 31 +++++++++++++++++++++++++++++--
>>> 1 file changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/kimchi/model/vmifaces.py 
>>> b/src/kimchi/model/vmifaces.py
>>> index 30e5373..da9721b 100644
>>> --- a/src/kimchi/model/vmifaces.py
>>> +++ b/src/kimchi/model/vmifaces.py
>>> @@ -23,6 +23,7 @@ import libvirt
>>> from lxml import etree, objectify
>>> from lxml.builder import E
>>>
>>> +from kimchi import osinfo
>>> from kimchi.exception import InvalidOperation, InvalidParameter, 
>>> NotFoundError
>>> from kimchi.model.utils import get_vm_metadata_element
>>> from kimchi.model.utils import set_vm_metadata_element
>>> @@ -45,6 +46,28 @@ class VMIfacesModel(object):
>>> vmiface = objectify.fromstring(iface_xml)
>>> return vmiface.attrib.get("model")
>>>
>>> + def _choose_model(self, vm):
>>> + # first find a model from the exist iface of vm
>>> + for iface in VMIfacesModel.get_vmifaces(vm, self.conn):
>>> + if iface.find("model") is not None:
>>> + return iface.model.get('type')
>>> +
>>> + dom = VMModel.get_vm(vm, self.conn)
>>> + # then get model from vmiface metadata
>>> + model = self._vm_get_iface_metadata(dom)
>>> + if model is not None:
>>> + return model
>>> +
>>> + # at last from OS metadata
>>> + distro, version = VMModel.vm_get_os_metadata(dom)
>>> + if distro is not None:
>>> + entry = osinfo.lookup(distro, version)
>>> + VMIfaceModel.vm_updata_iface_metadata(
>>> + dom, {"model": entry['nic_model']})
>>> + return entry['nic_model']
>>> +
>>> + return None
>>> +
>>> def create(self, vm, params):
>>> def randomMAC():
>>> mac = [0x52, 0x54, 0x00,
>>> @@ -76,8 +99,12 @@ class VMIfacesModel(object):
>>> children = [E.mac(address=mac)]
>>> ("network" in params.keys() and
>>> children.append(E.source(network=params['network'])))
>>> - ("model" in params.keys() and
>>> - children.append(E.model(type=params['model'])))
>>> +
>>> + model = params.get("model")
>>> + if "network" in params and model is None:
>>> + model = self._choose_model(vm)
>>> + model is not None and children.append(E.model({"type": model}))
>>> +
>>> attrib = {"type": params["type"]}
>>>
>>> xml = etree.tostring(E.interface(*children, **attrib))
>>
>>
>>
>
>