6:14 a.m.
Some capabilities of node devices rely on what driver they bound to,
and therefore, these capabilities may change when the driver change.
In current implemention, it is not consistent between real status and
the status we get by invoking nodedev interfaces.
So, this series of patches try to manually update devices' capabilities
each time before nodedev driver interfaces invoked.
Wu Zongyong (3):
v2->v3:
1. split a single patch to three part
2. fix memory leak and lock problems
3. update caps before invoking nodedev driver interfaces (refer to patch 3/3)
nodedev: add macro guard to node_device_udev header file
nodedev: update mdev_types caps before dumpxml
nodedev: update caps before invoking nodedev driver interfaces
src/node_device/node_device_driver.c | 55 +++++++++++++++++++++++++++++++
src/node_device/node_device_linux_sysfs.c | 22 +++++++++++++
src/node_device/node_device_udev.c | 37 +++++++++++++++++----
src/node_device/node_device_udev.h | 18 +++++++---
4 files changed, 121 insertions(+), 11 deletions(-)
--
1.9.1
6:14 a.m.
New subject: [libvirt] [PATCH 1/3] nodedev: add macro guard to node_device_udev header file
Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
---
src/node_device/node_device_udev.h | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h
index f15e520..300f57e 100644
--- a/src/node_device/node_device_udev.h
+++ b/src/node_device/node_device_udev.h
@@ -19,10 +19,15 @@
*
* Author: Dave Allan <dallan(a)redhat.com>
*/
+#ifndef __VIR_NODE_DEVICE_UDEV_H__
+# define __VIR_NODE_DEVICE_UDEV_H__
-#include <libudev.h>
-#include <stdint.h>
+# include <libudev.h>
+# include <stdint.h>
-#define SYSFS_DATA_SIZE 4096
-#define DMI_DEVPATH "/sys/devices/virtual/dmi/id"
-#define DMI_DEVPATH_FALLBACK "/sys/class/dmi/id"
+# define SYSFS_DATA_SIZE 4096
+# define DMI_DEVPATH "/sys/devices/virtual/dmi/id"
+# define DMI_DEVPATH_FALLBACK "/sys/class/dmi/id"
+
+
+#endif /* __VIR_NODE_DEVICE_UDEV_H__ */
--
1.9.1
6:14 a.m.
New subject: [libvirt] [PATCH 2/3] nodedev: update mdev_types caps before dumpxml
In current implemention, mdev_types caps keep constant all
the time. But, it is possible that a device capable of
mdev_types sometime(for example:bind to proper driver) and
incapable of mdev_types at other times(for example: unbind
from its driver).
We should keep the info of xml dumped out consistent with
real status of the device.
Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
---
src/node_device/node_device_linux_sysfs.c | 22 ++++++++++++++++++
src/node_device/node_device_udev.c | 37 ++++++++++++++++++++++++++-----
src/node_device/node_device_udev.h | 3 +++
3 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 6f438e5..388ffb4 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -29,6 +29,7 @@
#include "dirname.h"
#include "node_device_driver.h"
#include "node_device_hal.h"
+#include "node_device_udev.h"
#include "node_device_linux_sysfs.h"
#include "virerror.h"
#include "viralloc.h"
@@ -175,6 +176,25 @@ nodeDeviceSysfsGetPCIIOMMUGroupCaps(virNodeDevCapPCIDevPtr pci_dev)
return ret;
}
+static int
+nodeDeviceSysfsGetPCIMdevTypesCaps(const char *sysfsPath,
+ virNodeDevCapPCIDevPtr pci_dev)
+{
+ size_t i;
+
+ /* this could be a refresh, so clear out the old data */
+ for (i = 0; i < pci_dev->nmdev_types; i++)
+ virNodeDevCapMdevTypeFree(pci_dev->mdev_types[i]);
+ VIR_FREE(pci_dev->mdev_types);
+ pci_dev->nmdev_types = 0;
+ pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+
+ if (udevPCISysfsGetMdevTypesCap(sysfsPath, pci_dev) < 0)
+ return -1;
+
+ return 0;
+}
+
/* nodeDeviceSysfsGetPCIRelatedCaps() get info that is stored in sysfs
* about devices related to this device, i.e. things that can change
@@ -190,6 +210,8 @@ nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath,
return -1;
if (nodeDeviceSysfsGetPCIIOMMUGroupCaps(pci_dev) < 0)
return -1;
+ if (nodeDeviceSysfsGetPCIMdevTypesCaps(sysfsPath, pci_dev) < 0)
+ return -1;
return 0;
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index e0fca61..7b0014e 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -506,6 +506,37 @@ udevPCIGetMdevTypesCap(struct udev_device *device,
}
+int
+udevPCISysfsGetMdevTypesCap(const char *sysfsPath,
+ virNodeDevCapPCIDevPtr pci_dev)
+{
+ int ret = -1;
+ struct udev *udev = NULL;
+ struct udev_device *device = NULL;
+ udevEventDataPtr priv = driver->privateData;
+
+ virObjectLock(priv);
+ udev = udev_monitor_get_udev(priv->udev_monitor);
+ device = udev_device_new_from_syspath(udev, sysfsPath);
+ virObjectUnlock(priv);
+
+ if (!device) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create udev device from path %s"),
+ sysfsPath);
+ goto cleanup;
+ }
+
+ if (udevPCIGetMdevTypesCap(device, pci_dev) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ udev_device_unref(device);
+ return ret;
+}
+
+
static int
udevProcessPCI(struct udev_device *device,
virNodeDeviceDefPtr def)
@@ -597,12 +628,6 @@ udevProcessPCI(struct udev_device *device,
}
}
- /* check whether the device is mediated devices framework capable, if so,
- * process it
- */
- if (udevPCIGetMdevTypesCap(device, pci_dev) < 0)
- goto cleanup;
-
ret = 0;
cleanup:
diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h
index 300f57e..bbdc70f 100644
--- a/src/node_device/node_device_udev.h
+++ b/src/node_device/node_device_udev.h
@@ -29,5 +29,8 @@
# define DMI_DEVPATH "/sys/devices/virtual/dmi/id"
# define DMI_DEVPATH_FALLBACK "/sys/class/dmi/id"
+int
+udevPCISysfsGetMdevTypesCap(const char *sysfsPath, virNodeDevCapPCIDevPtr pci_dev);
+
#endif /* __VIR_NODE_DEVICE_UDEV_H__ */
--
1.9.1
4:07 a.m.
New subject: [libvirt] [PATCH 2/3] nodedev: update mdev_types caps before dumpxml
On Wed, Jan 10, 2018 at 08:14:50PM +0800, Wu Zongyong wrote:
In current implemention, mdev_types caps keep constant all
the time. But, it is possible that a device capable of
mdev_types sometime(for example:bind to proper driver) and
incapable of mdev_types at other times(for example: unbind
from its driver).
We should keep the info of xml dumped out consistent with
real status of the device.
Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
For patches 1 and 2 (I'll tune the commit message for this one a bit before
pushing):
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
8:09 p.m.
New subject: [libvirt] [PATCH 2/3] nodedev: update mdev_types caps before dumpxml
Would you push this two patches before release of 4.0.0?
Thanks,
Zongyong Wu
-----Original Message-----
From: Erik Skultety [mailto:eskultet@redhat.com]
Sent: Thursday, January 11, 2018 6:07 PM
To: Wuzongyong (Euler Dept) <cordius.wu(a)huawei.com>
Cc: libvir-list(a)redhat.com; weijinfen <weijinfen(a)huawei.com>; Wanzongshun
(Vincent) <wanzongshun(a)huawei.com>
Subject: Re: [PATCH 2/3] nodedev: update mdev_types caps before dumpxml
On Wed, Jan 10, 2018 at 08:14:50PM +0800, Wu Zongyong wrote:
> In current implemention, mdev_types caps keep constant all the time.
> But, it is possible that a device capable of mdev_types sometime(for
> example:bind to proper driver) and incapable of mdev_types at other
> times(for example: unbind from its driver).
> We should keep the info of xml dumped out consistent with real status
> of the device.
>
> Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
For patches 1 and 2 (I'll tune the commit message for this one a bit
before
pushing):
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
2:24 a.m.
New subject: [libvirt] [PATCH 2/3] nodedev: update mdev_types caps before dumpxml
On Thu, Jan 18, 2018 at 02:09:14AM +0000, Wuzongyong (Euler Dept) wrote:
Would you push this two patches before release of 4.0.0?
Well, I don't think it's that critical, in fact I think we can easily wait
until 4.1, especially since we're in RC2, I'd rather not push anything
unnecessary just before the release, hope you can understand that.
Thanks,
Erik
6:14 a.m.
New subject: [libvirt] [PATCH 3/3] nodedev: update caps before invoking nodedev driver interfaces
Some capabilities of node devices rely on what driver they bound to,
and therefore, these capabilities may change when the driver change.
So, it is necessary to manually update devices' capabilities each time
before nodedev driver interfaces invoked.
Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
---
src/node_device/node_device_driver.c | 55 ++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index facfeb6..d854516 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -155,6 +155,42 @@ nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def ATTRIBUTE_UNUSED)
#endif
+static int
+nodeConnectUpdateAllNodeDevicesCaps(virConnectPtr conn,
+ virNodeDeviceObjListFilter filter)
+{
+ int ret = -1;
+ size_t i;
+ virNodeDevicePtr *devices;
+
+ if (virNodeDeviceObjListExport(conn, driver->devs, &devices, filter, 0) <
0)
+ return -1;
+
+ for (i = 0; devices[i]; i++) {
+ virNodeDeviceObjPtr obj;
+ virNodeDeviceDefPtr def;
+
+ if (!(obj = virNodeDeviceObjListFindByName(driver->devs,
devices[i]->name)))
+ goto cleanup;
+ def = virNodeDeviceObjGetDef(obj);
+
+ if (nodeDeviceUpdateCaps(def) < 0) {
+ virNodeDeviceObjEndAPI(&obj);
+ goto cleanup;
+ }
+
+ virNodeDeviceObjEndAPI(&obj);
+ }
+
+ ret = 0;
+ cleanup:
+ for (i = 0; devices[i]; i++)
+ virObjectUnref(devices[i]);
+ VIR_FREE(devices);
+ return ret;
+}
+
+
void
nodeDeviceLock(void)
{
@@ -179,6 +215,9 @@ nodeNumOfDevices(virConnectPtr conn,
virCheckFlags(0, -1);
+ if (nodeConnectUpdateAllNodeDevicesCaps(conn, virNodeNumOfDevicesCheckACL) < 0)
+ return -1;
+
return virNodeDeviceObjListNumOfDevices(driver->devs, conn, cap,
virNodeNumOfDevicesCheckACL);
}
@@ -196,6 +235,9 @@ nodeListDevices(virConnectPtr conn,
virCheckFlags(0, -1);
+ if (nodeConnectUpdateAllNodeDevicesCaps(conn, virNodeListDevicesCheckACL) < 0)
+ return -1;
+
return virNodeDeviceObjListGetNames(driver->devs, conn,
virNodeListDevicesCheckACL,
cap, names, maxnames);
@@ -212,6 +254,10 @@ nodeConnectListAllNodeDevices(virConnectPtr conn,
if (virConnectListAllNodeDevicesEnsureACL(conn) < 0)
return -1;
+ if (nodeConnectUpdateAllNodeDevicesCaps(conn,
+ virConnectListAllNodeDevicesCheckACL) <
0)
+ return -1;
+
return virNodeDeviceObjListExport(conn, driver->devs, devices,
virConnectListAllNodeDevicesCheckACL,
flags);
@@ -248,6 +294,9 @@ nodeDeviceLookupByName(virConnectPtr conn,
if (virNodeDeviceLookupByNameEnsureACL(conn, def) < 0)
goto cleanup;
+ if (nodeDeviceUpdateCaps(def) < 0)
+ goto cleanup;
+
if ((device = virGetNodeDevice(conn, name))) {
if (VIR_STRDUP(device->parent, def->parent) < 0) {
virObjectUnref(device);
@@ -370,6 +419,9 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device)
if (virNodeDeviceNumOfCapsEnsureACL(device->conn, def) < 0)
goto cleanup;
+ if (nodeDeviceUpdateCaps(def) < 0)
+ goto cleanup;
+
for (caps = def->caps; caps; caps = caps->next) {
++ncaps;
@@ -411,6 +463,9 @@ nodeDeviceListCaps(virNodeDevicePtr device,
if (virNodeDeviceListCapsEnsureACL(device->conn, def) < 0)
goto cleanup;
+ if (nodeDeviceUpdateCaps(def) < 0)
+ goto cleanup;
+
for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next)
{
if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type))
< 0)
goto cleanup;
--
1.9.1
4:15 a.m.
New subject: [libvirt] [PATCH 3/3] nodedev: update caps before invoking nodedev driver interfaces
On Wed, Jan 10, 2018 at 08:14:51PM +0800, Wu Zongyong wrote:
Some capabilities of node devices rely on what driver they bound to,
and therefore, these capabilities may change when the driver change.
So, it is necessary to manually update devices' capabilities each time
before nodedev driver interfaces invoked.
Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
---
Thank you for posting the patch, since I hadn't noticed the problem with other
APIs until I read it.
It was a sad realization that a driver change is not reflected by a udev/kernel
CHANGE event, that would make things much much simpler. I have an idea to either
make this patch shorter or not needed at all though, we'll see when I finish my
patch (it's a long-needed refactor). Despite I haven't found any major flaws in
this patch, let's just put it on hold for a while until I finish my
investigation/work on my patch and see we can really do better here.
Thanks,
Erik
7:39 p.m.
New subject: [libvirt] [PATCH 3/3] nodedev: update caps before invoking nodedev driver interfaces
It is a bit inefficient about my patch, and it is imperfect.
I wish to see a better idea to fix this problem.
And again, thank you for your correction to my patches.
Thanks,
Zongyong Wu
-----Original Message-----
From: Erik Skultety [mailto:eskultet@redhat.com]
Sent: Thursday, January 11, 2018 6:16 PM
To: Wuzongyong (Euler Dept) <cordius.wu(a)huawei.com>
Cc: libvir-list(a)redhat.com; weijinfen <weijinfen(a)huawei.com>; Wanzongshun
(Vincent) <wanzongshun(a)huawei.com>
Subject: Re: [PATCH 3/3] nodedev: update caps before invoking nodedev
driver interfaces
On Wed, Jan 10, 2018 at 08:14:51PM +0800, Wu Zongyong wrote:
> Some capabilities of node devices rely on what driver they bound to,
> and therefore, these capabilities may change when the driver change.
> So, it is necessary to manually update devices' capabilities each time
> before nodedev driver interfaces invoked.
>
> Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com>
> ---
Thank you for posting the patch, since I hadn't noticed the problem with
other APIs until I read it.
It was a sad realization that a driver change is not reflected by a
udev/kernel CHANGE event, that would make things much much simpler. I have
an idea to either make this patch shorter or not needed at all though,
we'll see when I finish my patch (it's a long-needed refactor). Despite I
haven't found any major flaws in this patch, let's just put it on hold for
a while until I finish my investigation/work on my patch and see we can
really do better here.
Thanks,
Erik
3:33 a.m.
On Wed, Jan 10, 2018 at 08:14:48PM +0800, Wu Zongyong wrote:
Some capabilities of node devices rely on what driver they bound to,
and therefore, these capabilities may change when the driver change.
In current implemention, it is not consistent between real status and
the status we get by invoking nodedev interfaces.
So, this series of patches try to manually update devices' capabilities
each time before nodedev driver interfaces invoked.
Hi, so I finally posted the series I mentioned in my previous reviews,
unfortunately, after applying your patches there were a lot of merge conflicts
so I had to write them from scratch, so I at least added you as an honorable
mention in patch equivalents of your patches 2 and 3. Unless someone objects to
the series as a whole, your patch 1 will be unnecessary.
Feel free to have a look at the series [1] and test whether it works as
expected (it did in my testing, both virsh and python).
Erik
[1] https://www.redhat.com/archives/libvir-list/2018-January/msg00851.html
Wu Zongyong (3):
v2->v3:
1. split a single patch to three part
2. fix memory leak and lock problems
3. update caps before invoking nodedev driver interfaces (refer to patch 3/3)
nodedev: add macro guard to node_device_udev header file
nodedev: update mdev_types caps before dumpxml
nodedev: update caps before invoking nodedev driver interfaces
src/node_device/node_device_driver.c | 55 +++++++++++++++++++++++++++++++
src/node_device/node_device_linux_sysfs.c | 22 +++++++++++++
src/node_device/node_device_udev.c | 37 +++++++++++++++++----
src/node_device/node_device_udev.h | 18 +++++++---
4 files changed, 121 insertions(+), 11 deletions(-)
--
1.9.1
2496
days inactive
2511
days old
9 comments
3 participants
participants (3)
-
Erik Skultety -
Wu Zongyong -
Wuzongyong (Euler Dept)