Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
Michal Skrivanek has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 4: >During vm migration we invoke a libvirt hook: vm_migrate_hook.py. >This was added recently, and adds some special ovs handling to our >vNics. >But, this change broke all hooks which relied on modifying the nic >domxml, >like: vmfex, openstack, and now the ovn hook. >These hooks applied custom changes to the NIC xml. yes. And that should have been avoided in the first place. >These custom changes were now lost, as a result of vm_migrate_hook >applying >the special ovs handling. >So we needed a way to disable this special handling for some of the >NICs. IMO you should stop modifying libvirt xml unless it is something we can't really avoid, not without a discussion with libvirt team, without a plan how to get rid of such modification. Alternatively - if there is a fundamental disagreement between the responsibilities/ownership of networks then we need to work with libvirt to resolve that. We can't rely on libvirt for migration and "step on their things" at the same time. >Please note that this is not something that introduces something >new, but >something which tries to fix the functionality broken by >introducing the >libvirt hook. Again, I would much rather try to discuss all the other options to be able to avoid such a hook. >So far everyone agrees that this is not a nice solution, but so far >no one >was able to suggest any better. >If you don't agree with the way this is solved, please suggest a >nicer >alternative. The fact that the problematic libvirt hook was merged is unfortunate. I suggest to discuss and revisit the whole problem and design a proper sustainable solution -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
Marcin Mirecki has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 4: During vm migration we invoke a libvirt hook: vm_migrate_hook.py. This was added recently, and adds some special ovs handling to our vNics. But, this change broke all hooks which relied on modifying the nic domxml, like: vmfex, openstack, and now the ovn hook. These hooks applied custom changes to the NIC xml. These custom changes were now lost, as a result of vm_migrate_hook applying the special ovs handling. So we needed a way to disable this special handling for some of the NICs. To be able to do it for any hooks, we needed something easily extendable (as for example this could break future external network providers), so we went for plugins. We tried some other options (like using the existing vdsm hooks) but these looked even worse. Please note that this is not something that introduces something new, but something which tries to fix the functionality broken by introducing the libvirt hook. So far everyone agrees that this is not a nice solution, but so far no one was able to suggest any better. If you don't agree with the way this is solved, please suggest a nicer alternative. -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
Michal Skrivanek has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 4: Code-Review-1 I don't think we want to have plugins of plugins. What for? Why not build it into vdsm itself (whatever is needed to be done). Waht's the exact problem we're trying to solve - is it really the only way like that? -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
gerrit-hooks has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
gerrit-hooks has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
gerrit-hooks has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
Marcin Mirecki has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 1: Code-Review-1 Verified-1 -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
gerrit-hooks has posted comments on this change. Change subject: draft: vm_migration_libvirt_hook_plugins .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64001 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: draft: vm_migration_libvirt_hook_plugins
Marcin Mirecki has uploaded a new change for review.
Change subject: draft: vm_migration_libvirt_hook_plugins
..
draft: vm_migration_libvirt_hook_plugins
Quick draft for vm migration libvirt hook plugins
Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc
Signed-off-by: mirecki
---
M vdsm/virt/vm_migrate_hook.py
A vdsm/virt/vm_migrate_plugins/__init__.py
2 files changed, 72 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/64001/1
diff --git a/vdsm/virt/vm_migrate_hook.py b/vdsm/virt/vm_migrate_hook.py
index 633e4f0..063fdd0 100755
--- a/vdsm/virt/vm_migrate_hook.py
+++ b/vdsm/virt/vm_migrate_hook.py
@@ -28,6 +28,8 @@
from vdsm.config import config
from vdsm.network import api as net_api
+import vm_migrate_plugins
+
_DEBUG_MODE = False
LOG_FILE = '/tmp/libvirthook_ovs_migrate.log'
@@ -82,24 +84,29 @@
def _set_bridge_interfaces(devices, target_vm_conf):
-target_vm_nets_by_vnic_mac = {dev['macAddr']: dev['network']
- for dev in target_vm_conf['devices']
- if dev.get('type') == 'interface'}
+target_vm_conf_by_mac = {dev['macAddr']: dev
+ for dev in target_vm_conf['devices']
+ if dev.get('type') == 'interface'}
+
for interface in devices.findall('interface'):
if interface.get('type') == 'bridge':
-_bind_iface_to_bridge(interface, target_vm_nets_by_vnic_mac)
+_bind_iface_to_bridge(interface, target_vm_conf_by_mac)
-def _bind_iface_to_bridge(interface, target_vm_nets_by_vnic_mac):
+def _bind_iface_to_bridge(interface, target_vm_conf_by_mac):
elem_macaddr = interface.find('mac')
mac_addr = elem_macaddr.get('address')
-target_vm_net = target_vm_nets_by_vnic_mac[mac_addr]
-target_ovs_bridge = net_api.ovs_bridge(target_vm_net)
-if target_ovs_bridge:
-_bind_iface_to_ovs_bridge(interface, target_ovs_bridge, target_vm_net)
-else:
-_bind_iface_to_linux_bridge(interface, target_vm_net)
+interface_conf = target_vm_conf_by_mac[mac_addr]
+target_vm_net = interface_conf['network']
+if vm_migrate_plugins.do_default_processing(interface, interface_conf):
+target_ovs_bridge = net_api.ovs_bridge(target_vm_net)
+if target_ovs_bridge:
+_bind_iface_to_ovs_bridge(interface, target_ovs_bridge,
+ target_vm_net)
+else:
+_bind_iface_to_linux_bridge(interface, target_vm_net)
+vm_migrate_plugins.process(interface, interface_conf)
def _bind_iface_to_ovs_bridge(interface, target_ovs_bridge, target_vm_net):
diff --git a/vdsm/virt/vm_migrate_plugins/__init__.py
b/vdsm/virt/vm_migrate_plugins/__init__.py
new file mode 100644
index 000..3e4b090
--- /dev/null
+++ b/vdsm/virt/vm_migrate_plugins/__init__.py
@@ -0,0 +1,54 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import abc
+from importlib import import_module
+from pkgutil import iter_modules
+
+import six
+
+
+_DRIVERS = []
+
+
+def do_default_processing(domxml, device_conf):
+result = True
+for driver in _DRIVERS:
+result = result and driver.create().do_default_processing(domxml,
+ device_conf)
+return result
+
+
+def process(domxml, device_conf):
+for driver in _DRIVERS:
+driver.create().process(domxml, device_conf)
+
+
+class VmMigrateDriver():
+
+@abc.abstractmethod
+def do_default_processing(self, domxml, device_conf):
+pass
+
+@abc.abstractmethod
+def process(self, domxml, device_conf):
+pass
+
+for _, module, _ in iter_modules([__path__[0]]):
+_DRIVERS.append(import_module('{}.{}'.format(__name__, module)))
--
To view, visit https://gerrit.ovirt.org/64001
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e13a94fa28968e37cd2d4d99fe540c8b762f7cc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki
__
