Edward Haas has posted comments on this change.

Change subject: virt net: Support VM migration on OVS based networks
......................................................................


Patch Set 10:

(4 comments)

https://gerrit.ovirt.org/#/c/59645/10/tests/network/vm_migrate_hook_test.py
File tests/network/vm_migrate_hook_test.py:

PS10, Line 95: is_vlan_net
> ovs_is_vlan_net
It's not only for OVS


PS10, Line 96: stdin = BytesIO(from_xml)
> this is not stdin but your custom *in. the same for stdout
please propose a better name, I was fine with this one.


https://gerrit.ovirt.org/#/c/59645/10/vdsm/virt/Makefile.am
File vdsm/virt/Makefile.am:

Line 28:        domain_descriptor.py \
Line 29:        migration.py \
Line 30:        recovery.py \
Line 31:        vm.py \
Line 32:        vm_migrate_hook.py \
> why do we need twice? just for tests?
I guess... and to have it under the sources.
Line 33:        vmtune.py \
Line 34:        vmxml.py \
Line 35:        $(NULL)
Line 36: 


https://gerrit.ovirt.org/#/c/59645/10/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

PS10, Line 122: elem_virtualport.set('type', 'openvswitch')
> if there is a virtualport we assume that it has type=openvswitch?
Yes, we do not support anything else.
Not even sure there is anything else.


-- 
To view, visit https://gerrit.ovirt.org/59645
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d32f9605f9ca99d1e07062108f2567806ac59c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Edward Haas <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to