Edward Haas has posted comments on this change. Change subject: virt net: Support VM migration on OVS based networks ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/59645/4/vdsm/virt/vm_migrate_hook.py File vdsm/virt/vm_migrate_hook.py: Line 29: Line 30: _DEBUG_MODE = False Line 31: Line 32: if _DEBUG_MODE: Line 33: LOG_FILE = '/tmp/libvirthook_ovs_migrate.log' > this could result in a big mess if you enable DEBUG and run two VMs, as bot Done Line 34: log = open(LOG_FILE, 'w') Line 35: Line 36: Line 37: def main(domain, event, phase, *args, **kwargs): PS4, Line 62: info > I'd use vm_conf, since this the name all over virt. Done Line 66: target_vm_nets_by_mac = {dev['macAddr']: dev['network'] Line 67: for dev in target_vm_info['devices'] Line 68: if dev['type'] == 'interface'} Line 69: Line 70: for interface in devices.findall('interface'): > I think that findall('./interface') is safer (in case there's a nested 'int From the documentation it seems to be identical, it is usually used if from root you want to look for interfaces, like this: root.findall('./devices/interface'). This syntax won't lookup interface entities recursively and I consider that a good thing. Line 71: if interface.get('type') == 'bridge': Line 72: _bind_iface_to_bridge(interface, target_vm_nets_by_mac) Line 73: Line 74: -- 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: 4 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]
