[libvirt] [PATCH v2 0/3] nodedev: add class info for pci capability
Diff from v1 [1] =============== - add patch to use/restrict name "klass" in headers instead of class, _class - add news patch [1] https://www.redhat.com/archives/libvir-list/2019-February/msg01036.html Nikolay Shirokovskiy (3): conf: don't use "class" as name in _virNodeDevCapPCIDev xml: nodedev: add class info for pci capability news: update for new class element of PCI nodedev capability cfg.mk | 8 ++++++++ docs/formatnode.html.in | 5 +++++ docs/news.xml | 5 +++++ docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 17 +++++++++++++++-- src/conf/node_device_conf.h | 4 ++-- src/node_device/node_device_udev.c | 4 ++-- .../nodedevschemadata/pci_0000_00_02_0_header_type.xml | 1 + .../nodedevschemadata/pci_0000_00_1c_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml | 1 + .../pci_0000_02_10_7_sriov_zero_vfs_max_count.xml | 1 + tests/nodedevschemadata/pci_1002_71c4.xml | 1 + tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml | 1 + tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml | 1 + tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml | 1 + tests/nodedevschemadata/pci_82579LM_network_adapter.xml | 1 + 20 files changed, 55 insertions(+), 6 deletions(-) -- 2.16.5
Vim treats *.h files as cpp ones with respect to syntax highlighting. Thus "class" in _virNodeDevCapPCIDev highlighted mistakenly. This can be fixed by filetype detection code tunables but it is more convinient to skip this tuning by every project member. Let's just use "klass" as field name instead of _class or class and add syntax rule. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- cfg.mk | 8 ++++++++ src/conf/node_device_conf.c | 4 ++-- src/conf/node_device_conf.h | 4 ++-- src/node_device/node_device_udev.c | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cfg.mk b/cfg.mk index f99b8ae631..88198037cc 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1075,6 +1075,14 @@ sc_require_attribute_cleanup_initialization: halt='variable declared with a cleanup macro must be initialized' \ $(_sc_search_regexp) +# "class" in headers is not good because by default Vim treats it as a keyword +sc_prohibit_class_in_headers: + @prohibit=' +_?class *;' \ + in_vc_files='\.h$$' \ + halt='use klass instead of class or _class as name in header files' \ + $(_sc_search_regexp) + + # We don't use this feature of maint.mk. prev_version_file = /dev/null diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 1b1f57d065..5de51d1f6b 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -346,7 +346,7 @@ virNodeDeviceCapUSBInterfaceDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<number>%d</number>\n", data->usb_if.number); virBufferAsprintf(buf, "<class>%d</class>\n", - data->usb_if._class); + data->usb_if.klass); virBufferAsprintf(buf, "<subclass>%d</subclass>\n", data->usb_if.subclass); virBufferAsprintf(buf, "<protocol>%d</protocol>\n", @@ -1216,7 +1216,7 @@ virNodeDevCapUSBInterfaceParseXML(xmlXPathContextPtr ctxt, goto out; if (virNodeDevCapsDefParseULong("number(./class[1])", ctxt, - &usb_if->_class, def, + &usb_if->klass, def, _("no USB interface class supplied for '%s'"), _("invalid USB interface class supplied for '%s'")) < 0) goto out; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index ce789e9ee9..b13bc13b87 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -153,7 +153,7 @@ struct _virNodeDevCapPCIDev { unsigned int function; unsigned int product; unsigned int vendor; - unsigned int class; + unsigned int klass; char *product_name; char *vendor_name; virPCIDeviceAddressPtr physical_function; @@ -186,7 +186,7 @@ typedef struct _virNodeDevCapUSBIf virNodeDevCapUSBIf; typedef virNodeDevCapUSBIf *virNodeDevCapUSBIfPtr; struct _virNodeDevCapUSBIf { unsigned int number; - unsigned int _class; /* "class" is reserved in C */ + unsigned int klass; unsigned int subclass; unsigned int protocol; char *description; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 32e762009f..f0e61e4236 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -402,7 +402,7 @@ udevProcessPCI(struct udev_device *device, privileged = driver->privileged; nodeDeviceUnlock(); - if (udevGetUintProperty(device, "PCI_CLASS", &pci_dev->class, 16) < 0) + if (udevGetUintProperty(device, "PCI_CLASS", &pci_dev->klass, 16) < 0) goto cleanup; if ((p = strrchr(def->sysfs_path, '/')) == NULL || @@ -582,7 +582,7 @@ udevProcessUSBInterface(struct udev_device *device, return -1; if (udevGetUintSysfsAttr(device, "bInterfaceClass", - &usb_if->_class, 16) < 0) + &usb_if->klass, 16) < 0) return -1; if (udevGetUintSysfsAttr(device, "bInterfaceSubClass", -- 2.16.5
On Tue, Mar 12, 2019 at 02:14:39PM +0300, Nikolay Shirokovskiy wrote:
Vim treats *.h files as cpp ones with respect to syntax highlighting.
Surprised it doesn't use .hpp for C++ headers, but no matter.
Thus "class" in _virNodeDevCapPCIDev highlighted mistakenly. This can be fixed by filetype detection code tunables but it is more convinient to skip this tuning by every project member.
Let's just use "klass" as field name instead of _class or class and add syntax rule.
I think that's good practice regardless of the vimm issue !
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- cfg.mk | 8 ++++++++ src/conf/node_device_conf.c | 4 ++-- src/conf/node_device_conf.h | 4 ++-- src/node_device/node_device_udev.c | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/cfg.mk b/cfg.mk index f99b8ae631..88198037cc 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1075,6 +1075,14 @@ sc_require_attribute_cleanup_initialization: halt='variable declared with a cleanup macro must be initialized' \ $(_sc_search_regexp)
+# "class" in headers is not good because by default Vim treats it as a keyword +sc_prohibit_class_in_headers: + @prohibit=' +_?class *;' \ + in_vc_files='\.h$$' \
How about extending that rule to '.c' too. The .h check will cause most of the usage in .c files to be removed anyway, so might as well blacklist it explicitly in .c
+ halt='use klass instead of class or _class as name in header files' \ + $(_sc_search_regexp) + + # We don't use this feature of maint.mk. prev_version_file = /dev/null
With or without the .c blacklist too Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
This info can be useful to filter devices visible to mgmt clients so that they won't see devices that unsafe/not meaningful to pass thru. Provide class info the way it is provided by udev or kernel that is as single 6-digit hexadecimal. Class element is not optional. I guess this should not break users that use virNodeDeviceCreateXML because they probably specify only scsi_host capability on input and then node device driver gets other capabilities from udev after device appeared. HAL driver does not get support for the new element in this patch. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatnode.html.in | 5 +++++ docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 13 +++++++++++++ tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml | 1 + .../nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml | 1 + .../pci_0000_02_10_7_sriov_zero_vfs_max_count.xml | 1 + tests/nodedevschemadata/pci_1002_71c4.xml | 1 + tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml | 1 + tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml | 1 + tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml | 1 + tests/nodedevschemadata/pci_82579LM_network_adapter.xml | 1 + 16 files changed, 36 insertions(+) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 29244a8984..5095b97d8a 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -70,6 +70,10 @@ <dd>Describes a device on the host's PCI bus. Sub-elements include: <dl> + <dt><code>class</code></dt> + <dd>Combined class, subclass and + programming interface codes as 6-digit hexadecimal number. + <span class="since">Since 5.2.0</span></dd> <dt><code>domain</code></dt> <dd>Which domain the device belongs to.</dd> <dt><code>bus</code></dt> @@ -381,6 +385,7 @@ <name>igb</name> </driver> <capability type='pci'> + <class>0x020000</class> <domain>0</domain> <bus>2</bus> <slot>0</slot> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 0498489cfd..0f45d7993e 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -133,6 +133,11 @@ <value>pci</value> </attribute> + <element name='class'> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{6}</param> + </data> + </element> <element name='domain'> <ref name='unsignedLong'/> </element> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 5de51d1f6b..19c601ae11 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -208,6 +208,7 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf, { size_t i; + virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass); virBufferAsprintf(buf, "<domain>%d</domain>\n", data->pci_dev.domain); virBufferAsprintf(buf, "<bus>%d</bus>\n", data->pci_dev.bus); @@ -1644,6 +1645,18 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, orignode = ctxt->node; ctxt->node = node; + if (virNodeDevCapsDefParseHexId("string(./class[1])", ctxt, + &pci_dev->klass, def, + _("no PCI class supplied for '%s'"), + _("invalid PCI class supplied for '%s'")) < 0) + goto out; + + if (pci_dev->klass > 0xffffff) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid PCI class supplied for '%s'"), def->name); + goto out; + } + if (virNodeDevCapsDefParseULong("number(./domain[1])", ctxt, &pci_dev->domain, def, _("no PCI domain ID supplied for '%s'"), diff --git a/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml b/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml index 5150fd1e8b..387fce7051 100644 --- a/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml +++ b/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml @@ -2,6 +2,7 @@ <name>pci_0000_00_02_0</name> <parent>computer</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>0</bus> <slot>2</slot> diff --git a/tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml b/tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml index c1be9f7d9c..b07d14faa5 100644 --- a/tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml +++ b/tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml @@ -2,6 +2,7 @@ <name>pci_0000_00_1c_0</name> <parent>computer</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>0</bus> <slot>28</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml b/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml index a2d57569af..8e71e3f897 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml index 8f243b4d61..6fa2b406a5 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml index 9e8dace020..74036a65fa 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml index 4e6323a3ec..c30c0d0d2b 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml index 355eaaad97..096055e2ae 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_zero_vfs_max_count.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_zero_vfs_max_count.xml index e9eb122bfe..8259cd059b 100644 --- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_zero_vfs_max_count.xml +++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_zero_vfs_max_count.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_10_7</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>16</slot> diff --git a/tests/nodedevschemadata/pci_1002_71c4.xml b/tests/nodedevschemadata/pci_1002_71c4.xml index 6d5d85bc3c..2039e2201f 100644 --- a/tests/nodedevschemadata/pci_1002_71c4.xml +++ b/tests/nodedevschemadata/pci_1002_71c4.xml @@ -2,6 +2,7 @@ <name>pci_1002_71c4</name> <parent>pci_8086_27a1</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>1</bus> <slot>0</slot> diff --git a/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml b/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml index 8e8990073d..3ffe53b8c9 100644 --- a/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml +++ b/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml @@ -2,6 +2,7 @@ <name>pci_0000_00_03_0</name> <parent>computer</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>0</bus> <slot>3</slot> diff --git a/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml b/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml index 6e1dc868a6..6bd1292095 100644 --- a/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml +++ b/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml @@ -2,6 +2,7 @@ <name>pci_0000_02_00_0</name> <parent>pci_0000_00_04_0</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>2</bus> <slot>0</slot> diff --git a/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml b/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml index 18172e900b..59f5ec8622 100644 --- a/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml +++ b/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml @@ -2,6 +2,7 @@ <name>pci_0000_03_00_0</name> <parent>pci_0000_00_1c_1</parent> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>3</bus> <slot>0</slot> diff --git a/tests/nodedevschemadata/pci_82579LM_network_adapter.xml b/tests/nodedevschemadata/pci_82579LM_network_adapter.xml index 6e154d6de3..96a4c51762 100644 --- a/tests/nodedevschemadata/pci_82579LM_network_adapter.xml +++ b/tests/nodedevschemadata/pci_82579LM_network_adapter.xml @@ -5,6 +5,7 @@ <name>e1000e</name> </driver> <capability type='pci'> + <class>0xffffff</class> <domain>0</domain> <bus>0</bus> <slot>25</slot> -- 2.16.5
On Tue, Mar 12, 2019 at 02:14:40PM +0300, Nikolay Shirokovskiy wrote:
This info can be useful to filter devices visible to mgmt clients so that they won't see devices that unsafe/not meaningful to pass thru.
Provide class info the way it is provided by udev or kernel that is as single 6-digit hexadecimal.
Class element is not optional. I guess this should not break users that use virNodeDeviceCreateXML because they probably specify only scsi_host capability on input and then node device driver gets other capabilities from udev after device appeared.
HAL driver does not get support for the new element in this patch.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatnode.html.in | 5 +++++ docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 13 +++++++++++++ tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml | 1 + .../nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml | 1 + .../pci_0000_02_10_7_sriov_zero_vfs_max_count.xml | 1 + tests/nodedevschemadata/pci_1002_71c4.xml | 1 + tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml | 1 + tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml | 1 + tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml | 1 + tests/nodedevschemadata/pci_82579LM_network_adapter.xml | 1 + 16 files changed, 36 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Mar 12, 2019 at 02:14:40PM +0300, Nikolay Shirokovskiy wrote:
This info can be useful to filter devices visible to mgmt clients so that they won't see devices that unsafe/not meaningful to pass thru.
Provide class info the way it is provided by udev or kernel that is as single 6-digit hexadecimal.
Class element is not optional. I guess this should not break users that use virNodeDeviceCreateXML because they probably specify only scsi_host capability on input and then node device driver gets other capabilities from udev after device appeared.
This patch broke virt-manager testing suit [1]. It uses test driver a lot with custom XMLs providing required information to initialize test driver and one of the information is list of nodedev devices. One can use it like this: virsh --connect test:///path/to/host/definition And it will parse XML providing host definition which this patch breaks. Yes, we can possibly fix all the test cases in virt-manager but I would rather make this element optional as it is not required. Pavel [1] <https://ci.centos.org/view/libvirt/job/virt-manager-check/systems=libvirt-fedora-29/532/console>
On Wed, Mar 20, 2019 at 12:35:14PM +0100, Pavel Hrdina wrote:
On Tue, Mar 12, 2019 at 02:14:40PM +0300, Nikolay Shirokovskiy wrote:
This info can be useful to filter devices visible to mgmt clients so that they won't see devices that unsafe/not meaningful to pass thru.
Provide class info the way it is provided by udev or kernel that is as single 6-digit hexadecimal.
Class element is not optional. I guess this should not break users that use virNodeDeviceCreateXML because they probably specify only scsi_host capability on input and then node device driver gets other capabilities from udev after device appeared.
This patch broke virt-manager testing suit [1]. It uses test driver a lot with custom XMLs providing required information to initialize test driver and one of the information is list of nodedev devices.
One can use it like this:
virsh --connect test:///path/to/host/definition
And it will parse XML providing host definition which this patch breaks.
Yes, we can possibly fix all the test cases in virt-manager but I would rather make this element optional as it is not required.
Hmm, yes, I forgot that the test driver accepts /all/ the XML schemas as inputs, even if we don't otherwise support it via the normal APIs. I think the XML stability guarantee is not clearly defined wrt to the test driver, and indeed I think we're reasonable in breaking compat in this area...if we have to... but in this case I think we can be more graceful & avoid the break without causing ourselves trouble. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/news.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 81466c3d55..786d18d485 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -68,6 +68,11 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + Report class information for PCI node device capability. + </summary> + </change> </section> <section title="Bug fixes"> </section> -- 2.16.5
On Tue, Mar 12, 2019 at 02:14:41PM +0300, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/news.xml | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Pushed. Thanx! On 12.03.2019 14:14, Nikolay Shirokovskiy wrote:
Diff from v1 [1] =============== - add patch to use/restrict name "klass" in headers instead of class, _class - add news patch
[1] https://www.redhat.com/archives/libvir-list/2019-February/msg01036.html
Nikolay Shirokovskiy (3): conf: don't use "class" as name in _virNodeDevCapPCIDev xml: nodedev: add class info for pci capability news: update for new class element of PCI nodedev capability
cfg.mk | 8 ++++++++ docs/formatnode.html.in | 5 +++++ docs/news.xml | 5 +++++ docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 17 +++++++++++++++-- src/conf/node_device_conf.h | 4 ++-- src/node_device/node_device_udev.c | 4 ++-- .../nodedevschemadata/pci_0000_00_02_0_header_type.xml | 1 + .../nodedevschemadata/pci_0000_00_1c_0_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all.xml | 1 + .../pci_0000_02_10_7_sriov_pf_vfs_all_header_type.xml | 1 + tests/nodedevschemadata/pci_0000_02_10_7_sriov_vfs.xml | 1 + .../pci_0000_02_10_7_sriov_zero_vfs_max_count.xml | 1 + tests/nodedevschemadata/pci_1002_71c4.xml | 1 + tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml | 1 + tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml | 1 + tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml | 1 + tests/nodedevschemadata/pci_82579LM_network_adapter.xml | 1 + 20 files changed, 55 insertions(+), 6 deletions(-)
participants (3)
-
Daniel P. Berrangé -
Nikolay Shirokovskiy -
Pavel Hrdina