[PATCH] [Kimchi 0/2] Issue 793: Disable Libvirt hot-plugging support for multi-function adapters
Libvirt does not support multi-function devices hot-plug (but it will). So this patchset will disable hot-plugging for such devices. Jose Ricardo Ziviani (2): Add multi-function field in PCI information Disable hotplugging buttons of multi-function devices model/host.py | 27 ++++++++++++++++++++++++++- model/vmhostdevs.py | 3 ++- ui/js/src/kimchi.guest_edit_main.js | 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) -- 1.9.1
- This commit adds a new field in the PCI device data structure to notify the client that such PCI is a multi-function device. The goal is to make it easy to identify whether a given device is multi-function or not in the client side. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/host.py | 27 ++++++++++++++++++++++++++- model/vmhostdevs.py | 3 ++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/model/host.py b/model/host.py index 628ae71..72d7d0e 100644 --- a/model/host.py +++ b/model/host.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import libvirt +from collections import defaultdict from lxml import objectify from wok.exception import InvalidParameter @@ -144,6 +145,21 @@ class DevicesModel(object): class DeviceModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.iommu_groups = self._get_iommu_groups() + + def _get_iommu_groups(self): + iommu_groups = defaultdict(list) + conn = self.conn.get() + + devices = DevicesModel(conn=self.conn).get_list() + for device in devices: + info = hostdev.get_dev_info(conn.nodeDeviceLookupByName(device)) + if 'iommuGroup' not in info: + continue + iommu_group_nr = int(info['iommuGroup']) + iommu_groups[iommu_group_nr].append(device) + + return iommu_groups def lookup(self, nodedev_name): conn = self.conn.get() @@ -151,7 +167,16 @@ class DeviceModel(object): dev = conn.nodeDeviceLookupByName(nodedev_name) except: raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) - return hostdev.get_dev_info(dev) + + info = hostdev.get_dev_info(dev) + info['multifunction'] = self.is_multifunction_pci(info) + return info + + def is_multifunction_pci(self, info): + if 'iommuGroup' not in info: + return False + iommu_group_nr = int(info['iommuGroup']) + return len(self.iommu_groups[iommu_group_nr]) > 1 @staticmethod def _toint(num_str): diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index c4fb815..de52cd8 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -279,7 +279,8 @@ class VMHostDevModel(object): return {'name': dev_name, 'type': e.attrib['type'], 'product': dev_info.get('product', None), - 'vendor': dev_info.get('vendor', None)} + 'vendor': dev_info.get('vendor', None), + 'multifunction': dev_info.get('multifunction', None)} raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name}) -- 1.9.1
On 12/08/2015 11:52 AM, Jose Ricardo Ziviani wrote:
- This commit adds a new field in the PCI device data structure to notify the client that such PCI is a multi-function device. The goal is to make it easy to identify whether a given device is multi-function or not in the client side.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/host.py | 27 ++++++++++++++++++++++++++- model/vmhostdevs.py | 3 ++- 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/model/host.py b/model/host.py index 628ae71..72d7d0e 100644 --- a/model/host.py +++ b/model/host.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
import libvirt +from collections import defaultdict from lxml import objectify
from wok.exception import InvalidParameter @@ -144,6 +145,21 @@ class DevicesModel(object): class DeviceModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.iommu_groups = self._get_iommu_groups() + + def _get_iommu_groups(self): + iommu_groups = defaultdict(list) + conn = self.conn.get() + + devices = DevicesModel(conn=self.conn).get_list()
Why not use 'conn=self.conn' in the first line and use only conn object, instead of 'conn=self.conn'? Then you can use conn.get().nodeDevice... in the below line also.
+ for device in devices: + info = hostdev.get_dev_info(conn.nodeDeviceLookupByName(device)) + if 'iommuGroup' not in info: + continue + iommu_group_nr = int(info['iommuGroup']) + iommu_groups[iommu_group_nr].append(device) + + return iommu_groups
def lookup(self, nodedev_name): conn = self.conn.get() @@ -151,7 +167,16 @@ class DeviceModel(object): dev = conn.nodeDeviceLookupByName(nodedev_name) except: raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) - return hostdev.get_dev_info(dev) + + info = hostdev.get_dev_info(dev) + info['multifunction'] = self.is_multifunction_pci(info) + return info + + def is_multifunction_pci(self, info): + if 'iommuGroup' not in info: + return False + iommu_group_nr = int(info['iommuGroup']) + return len(self.iommu_groups[iommu_group_nr]) > 1
@staticmethod def _toint(num_str): diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index c4fb815..de52cd8 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -279,7 +279,8 @@ class VMHostDevModel(object): return {'name': dev_name, 'type': e.attrib['type'], 'product': dev_info.get('product', None), - 'vendor': dev_info.get('vendor', None)} + 'vendor': dev_info.get('vendor', None), + 'multifunction': dev_info.get('multifunction', None)}
raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name})
Hey Paulo, I believe this is a minor issue and does not impact the code. Reviewed-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> On 12/08/2015 04:21 PM, Paulo Ricardo Paz Vital wrote:
On 12/08/2015 11:52 AM, Jose Ricardo Ziviani wrote:
- This commit adds a new field in the PCI device data structure to notify the client that such PCI is a multi-function device. The goal is to make it easy to identify whether a given device is multi-function or not in the client side.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/host.py | 27 ++++++++++++++++++++++++++- model/vmhostdevs.py | 3 ++- 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/model/host.py b/model/host.py index 628ae71..72d7d0e 100644 --- a/model/host.py +++ b/model/host.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
import libvirt +from collections import defaultdict from lxml import objectify
from wok.exception import InvalidParameter @@ -144,6 +145,21 @@ class DevicesModel(object): class DeviceModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.iommu_groups = self._get_iommu_groups() + + def _get_iommu_groups(self): + iommu_groups = defaultdict(list) + conn = self.conn.get() + + devices = DevicesModel(conn=self.conn).get_list() Why not use 'conn=self.conn' in the first line and use only conn object, instead of 'conn=self.conn'? Then you can use conn.get().nodeDevice... in the below line also.
+ for device in devices: + info = hostdev.get_dev_info(conn.nodeDeviceLookupByName(device)) + if 'iommuGroup' not in info: + continue + iommu_group_nr = int(info['iommuGroup']) + iommu_groups[iommu_group_nr].append(device) + + return iommu_groups
def lookup(self, nodedev_name): conn = self.conn.get() @@ -151,7 +167,16 @@ class DeviceModel(object): dev = conn.nodeDeviceLookupByName(nodedev_name) except: raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) - return hostdev.get_dev_info(dev) + + info = hostdev.get_dev_info(dev) + info['multifunction'] = self.is_multifunction_pci(info) + return info + + def is_multifunction_pci(self, info): + if 'iommuGroup' not in info: + return False + iommu_group_nr = int(info['iommuGroup']) + return len(self.iommu_groups[iommu_group_nr]) > 1
@staticmethod def _toint(num_str): diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index c4fb815..de52cd8 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -279,7 +279,8 @@ class VMHostDevModel(object): return {'name': dev_name, 'type': e.attrib['type'], 'product': dev_info.get('product', None), - 'vendor': dev_info.get('vendor', None)} + 'vendor': dev_info.get('vendor', None), + 'multifunction': dev_info.get('multifunction', None)}
raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name})
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
I'm going to send a new patch On 08-12-2015 16:21, Paulo Ricardo Paz Vital wrote:
On 12/08/2015 11:52 AM, Jose Ricardo Ziviani wrote:
- This commit adds a new field in the PCI device data structure to notify the client that such PCI is a multi-function device. The goal is to make it easy to identify whether a given device is multi-function or not in the client side.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/host.py | 27 ++++++++++++++++++++++++++- model/vmhostdevs.py | 3 ++- 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/model/host.py b/model/host.py index 628ae71..72d7d0e 100644 --- a/model/host.py +++ b/model/host.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
import libvirt +from collections import defaultdict from lxml import objectify
from wok.exception import InvalidParameter @@ -144,6 +145,21 @@ class DevicesModel(object): class DeviceModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.iommu_groups = self._get_iommu_groups() + + def _get_iommu_groups(self): + iommu_groups = defaultdict(list) + conn = self.conn.get() + + devices = DevicesModel(conn=self.conn).get_list()
Why not use 'conn=self.conn' in the first line and use only conn object, instead of 'conn=self.conn'? Then you can use conn.get().nodeDevice... in the below line also.
+ for device in devices: + info = hostdev.get_dev_info(conn.nodeDeviceLookupByName(device)) + if 'iommuGroup' not in info: + continue + iommu_group_nr = int(info['iommuGroup']) + iommu_groups[iommu_group_nr].append(device) + + return iommu_groups
def lookup(self, nodedev_name): conn = self.conn.get() @@ -151,7 +167,16 @@ class DeviceModel(object): dev = conn.nodeDeviceLookupByName(nodedev_name) except: raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) - return hostdev.get_dev_info(dev) + + info = hostdev.get_dev_info(dev) + info['multifunction'] = self.is_multifunction_pci(info) + return info + + def is_multifunction_pci(self, info): + if 'iommuGroup' not in info: + return False + iommu_group_nr = int(info['iommuGroup']) + return len(self.iommu_groups[iommu_group_nr]) > 1
@staticmethod def _toint(num_str): diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index c4fb815..de52cd8 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -279,7 +279,8 @@ class VMHostDevModel(object): return {'name': dev_name, 'type': e.attrib['type'], 'product': dev_info.get('product', None), - 'vendor': dev_info.get('vendor', None)} + 'vendor': dev_info.get('vendor', None), + 'multifunction': dev_info.get('multifunction', None)}
raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name})
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Jose Ricardo Ziviani ----------------------------- Software Engineer Linux Technology Center - IBM
- Libvirt does not support multi-function devices currently. This commit disables + and - button while the VM is running if the PCI device is multi-function. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_edit_main.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js index c9a4c94..b358f59 100644 --- a/ui/js/src/kimchi.guest_edit_main.js +++ b/ui/js/src/kimchi.guest_edit_main.js @@ -486,6 +486,9 @@ kimchi.guest_edit_main = function() { $('.body', '#form-guest-edit-pci').append(itemNode); pciEnabled || $('button', itemNode).remove(); $('button i', itemNode).addClass(iconClass); + if (kimchi.thisVMState === "running" && arrPCIDevices[i].multifunction) { + $('button', itemNode).prop("disabled", true); + } $('button', itemNode).on('click', function(event) { event.preventDefault(); var obj = $(this); -- 1.9.1
Reviewed-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> On 12/08/2015 11:52 AM, Jose Ricardo Ziviani wrote:
- Libvirt does not support multi-function devices currently. This commit disables + and - button while the VM is running if the PCI device is multi-function.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_edit_main.js | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js index c9a4c94..b358f59 100644 --- a/ui/js/src/kimchi.guest_edit_main.js +++ b/ui/js/src/kimchi.guest_edit_main.js @@ -486,6 +486,9 @@ kimchi.guest_edit_main = function() { $('.body', '#form-guest-edit-pci').append(itemNode); pciEnabled || $('button', itemNode).remove(); $('button i', itemNode).addClass(iconClass); + if (kimchi.thisVMState === "running" && arrPCIDevices[i].multifunction) { + $('button', itemNode).prop("disabled", true); + } $('button', itemNode).on('click', function(event) { event.preventDefault(); var obj = $(this);
participants (3)
-
Jose Ricardo Ziviani -
Paulo Ricardo Paz Vital -
Rodrigo Trujillo