1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] nodedev: update mdev_types caps before dumpxml

Wu Zongyong posted 1 patch 6 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1515411322-22846-1-git-send-email-cordius.wu@huawei.com
src/node_device/node_device_linux_sysfs.c | 21 +++++++++++++++++++++
src/node_device/node_device_udev.c        | 29 +++++++++++++++++++++++++++++
src/node_device/node_device_udev.h        | 18 +++++++++++++-----
3 files changed, 63 insertions(+), 5 deletions(-)
[libvirt] [PATCH v2] nodedev: update mdev_types caps before dumpxml
Posted by Wu Zongyong 6 years, 3 months ago 
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@huawei.com>
---

v2:
- fix mistakes pointed out by Erik

 src/node_device/node_device_linux_sysfs.c | 21 +++++++++++++++++++++
 src/node_device/node_device_udev.c        | 29 +++++++++++++++++++++++++++++
 src/node_device/node_device_udev.h        | 18 +++++++++++++-----
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
index 6f438e5..8b00aff 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,24 @@ 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;
+
+    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 +209,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..781facf 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -506,6 +506,35 @@ udevPCIGetMdevTypesCap(struct udev_device *device,
 }
 
 
+int
+udevPCISysfsGetMdevTypesCap(const char *sysfsPath,
+                            virNodeDevCapPCIDevPtr pci_dev)
+{
+    int ret = -1;
+    udevEventDataPtr priv = NULL;
+    struct udev *udev = NULL;
+    struct udev_device *device = NULL;
+
+    priv = driver->privateData;
+    udev = udev_monitor_get_udev(priv->udev_monitor);
+    device = udev_device_new_from_syspath(udev, sysfsPath);
+    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)
diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h
index f15e520..bbdc70f 100644
--- a/src/node_device/node_device_udev.h
+++ b/src/node_device/node_device_udev.h
@@ -19,10 +19,18 @@
  *
  * Author: Dave Allan <dallan@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"
+
+int
+udevPCISysfsGetMdevTypesCap(const char *sysfsPath, virNodeDevCapPCIDevPtr pci_dev);
+
+
+#endif /* __VIR_NODE_DEVICE_UDEV_H__ */
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nodedev: update mdev_types caps before dumpxml
Posted by Erik Skultety 6 years, 3 months ago 
On Mon, Jan 08, 2018 at 07:35:22PM +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@huawei.com>
> ---
>
> v2:
> - fix mistakes pointed out by Erik
>
>  src/node_device/node_device_linux_sysfs.c | 21 +++++++++++++++++++++
>  src/node_device/node_device_udev.c        | 29 +++++++++++++++++++++++++++++
>  src/node_device/node_device_udev.h        | 18 +++++++++++++-----
>  3 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
> index 6f438e5..8b00aff 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,24 @@ 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;

You also need to clear the VIR_NODE_DEV_CAP_FLAG_PCI_MDEV flag, otherwise the
device:
1) is still listed as mdev_types capable using virsh nodedev-list --cap
mdev_types
2) formats an empty mdev_types element

> +
> +    if (udevPCISysfsGetMdevTypesCap(sysfsPath, pci_dev) < 0)
> +        return -1;
> +
> +    return 0;
> +}

Still leaking memory, because you didn't drop the old way of querying the
capabilities - there's udevPCIGetMdevTypesCap() at the end of udevProcessPCI
that has to be dropped.

> +
>
>  /* nodeDeviceSysfsGetPCIRelatedCaps() get info that is stored in sysfs
>   * about devices related to this device, i.e. things that can change
> @@ -190,6 +209,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..781facf 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -506,6 +506,35 @@ udevPCIGetMdevTypesCap(struct udev_device *device,
>  }
>
>
> +int
> +udevPCISysfsGetMdevTypesCap(const char *sysfsPath,
> +                            virNodeDevCapPCIDevPtr pci_dev)
> +{
> +    int ret = -1;
> +    udevEventDataPtr priv = NULL;

initialize @priv ^here...

> +    struct udev *udev = NULL;
> +    struct udev_device *device = NULL;
> +
> +    priv = driver->privateData;

rather than ^here...

you're missing virObjectLock(priv) here...

> +    udev = udev_monitor_get_udev(priv->udev_monitor);
> +    device = udev_device_new_from_syspath(udev, sysfsPath);

...and virObjectUnlock(priv) here...

> +    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)
> diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h
> index f15e520..bbdc70f 100644
> --- a/src/node_device/node_device_udev.h
> +++ b/src/node_device/node_device_udev.h
> @@ -19,10 +19,18 @@
>   *
>   * Author: Dave Allan <dallan@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"
> +
> +int
> +udevPCISysfsGetMdevTypesCap(const char *sysfsPath, virNodeDevCapPCIDevPtr pci_dev);
> +
> +
> +#endif /* __VIR_NODE_DEVICE_UDEV_H__ */
> --

Now that I'm looking at this hunk, would you mind splitting the patch in 2 and
move this include guard-related change into a separate patch?

Thanks,
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.