[virt-tools-list] [PATCH] virt-manager:Valid address type change in network device

[Cole Robinson]

On 05/29/2012 10:56 PM, Lin Qing wrote:
> On 05/30/2012 06:58 AM, Cole Robinson wrote:
>> On 05/23/2012 04:05 AM, Qing Lin wrote:
>>> Function set_address(addr) will only change the value of
>>> VirtualDeviceAddress._type,doesn't map change in xml.So,
>>> it will cause change lost problem.Using assignment statements
>>> will not only change the value of VirtualDeviceAddress._type
>>> but also change the xml.
>>>
>>> Signed-off-by: Qing Lin<qinglbj at linux.vnet.ibm.com>
>>> Signed-off-by: Li Zhang<zhlcindy at linux.vnet.ibm.com>
>>> ---
>>>   src/virtManager/domain.py |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/virtManager/domain.py b/src/virtManager/domain.py
>>> index b547f91..893c5df 100644
>>> --- a/src/virtManager/domain.py
>>> +++ b/src/virtManager/domain.py
>>> @@ -626,7 +626,7 @@ class vmmDomain(vmmLibvirtObject):
>>>           def change(editdev):
>>>               if editdev.model != newmodel:
>>>                   editdev.address.clear()
>>> -                editdev.set_address(addr)
>>> +                editdev.address.type = addr
>>>               editdev.model = newmodel
>>>           return self._redefine_device(change, devobj)
>>>
>> Sorry for being slow to respond, I'll be quicker going forward.
>>
>> Can you give an example of where the current code is failing, and what case
>> this fixes for you? Before and after XML would help.
> Thanks Cole.I will explain in detail.
> spapr-vlan is a network  interface type that pseries  machine supported.
> when we change a network  interface  device model in virt-manager form one to
> another.
> the xml will be changed like:
> 
> (libvirtobject:135): Redefining 'vm1' with XML diff:
> --- Original XML
> +++ New XML
> @@ -47,8 +47,7 @@
> <interface type="network">
> <mac address="9a:ea:d6:ab:b7:36"/>
> <source network="default"/>
> - <model type="rtl8139"/>
> - <address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/>
> + <model type="virtio"/>
> </interface>
> 
> Is it fine ,except when we change a network  interface  device model in
> virt-manager form any one   to spapr-vlan.
> 
> (libvirtobject:135): Redefining 'vm1' with XML diff:
> --- Original XML
> +++ New XML
> @@ -47,8 +47,7 @@
> <interface type="network">
> <mac address="9a:ea:d6:ab:b7:36"/>
> <source network="default"/>
> - <model type="rtl8139"/>
> - <address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/>
> + <model type="spapr-vlan"/>
> </interface>
> 
> The xml change is not enough .We have to set address type for spapr-vlan in
> xml too.What we expect is like below:
> 
> (libvirtobject:135): Redefining 'vm1' with XML diff:
> --- Original XML
> +++ New XML
> @@ -47,8 +47,8 @@
> <interface type="network">
> <mac address="9a:ea:d6:ab:b7:36"/>
> <source network="default"/>
> - <model type="rtl8139"/>
> - <address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/>
> + <model type="spapr-vlan"/>
> + <address type="spapr-vio"/>
> </interface>
> 

The fact that we require this is a libvirt bug IMO. Libvirt should be smart
enough to realize that <model type='spapr-vlan'/> implies <address
type='spapr-vio'/>. Libvirt already does similar stuff for <disk> devices.

So I'd prefer that this is fixed in libvirt.

Thanks,
Cole