Re: [PATCH 02/10] util: refactor mdev_types method from PCI to mdev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/4/20 6:38 PM, Ján Tomko wrote:
On a Friday in 2020, Boris Fiuczynski wrote:
Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes
into mdev for later reuse.

Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxx>
---
src/conf/node_device_conf.c |  2 +-
src/libvirt_private.syms    |  2 +-
src/util/virmdev.c          | 65 +++++++++++++++++++++++++++++++++++++
src/util/virmdev.h          |  4 +++
src/util/virpci.c           | 60 ----------------------------------
src/util/virpci.h           |  3 --
6 files changed, 71 insertions(+), 65 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 1f679a7b45..2086a4270b 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2528,57 +2528,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
    return 0;
}

-
-ssize_t
-virPCIGetMdevTypes(const char *sysfspath,
-                   virMediatedDeviceTypePtr **types)
-{
-    ssize_t ret = -1;
-    int dirret = -1;
-    DIR *dir = NULL;

commit c0ae4919e386cda6e21d3ba022ee187e8b09793b
     change DIR* int g_autoptr(DIR) where appropriate

touched this code so it no longer applies cleanly.

yes, correct. Laine's change was commited after I sent this series.


-    struct dirent *entry;
-    g_autofree char *types_path = NULL;
-    g_autoptr(virMediatedDeviceType) mdev_type = NULL;
-    virMediatedDeviceTypePtr *mdev_types = NULL;
-    size_t ntypes = 0;
-    size_t i;
-
-    types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath);
-
-    if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0)
-        goto cleanup;
-
-    if (dirret == 0) {
-        ret = 0;
-        goto cleanup;
-    }
-
-    while ((dirret = virDirRead(dir, &entry, types_path)) > 0) {
-        g_autofree char *tmppath = NULL;
-        /* append the type id to the path and read the attributes from there */
-        tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name);
-
-        if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0)
-            goto cleanup;
-
-        if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0)
-            goto cleanup;
-    }
-
-    if (dirret < 0)
-        goto cleanup;
-
-    *types = g_steal_pointer(&mdev_types);
-    ret = ntypes;
-    ntypes = 0;
- cleanup:
-    for (i = 0; i < ntypes; i++)
-        virMediatedDeviceTypeFree(mdev_types[i]);
-    VIR_FREE(mdev_types);
-    VIR_DIR_CLOSE(dir);
-    return ret;
-}
-
#else
static const char *unsupported = N_("not supported on non-linux platforms");

This also needs to be copied to the new file...

Sorry for missing this.




@@ -2653,15 +2602,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path G_GNUC_UNUSED,
    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
    return -1;
}
-
-
-ssize_t
-virPCIGetMdevTypes(const char *sysfspath G_GNUC_UNUSED,
-                   virMediatedDeviceTypePtr **types G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));

otherwise this won't compile.

Yes, that is correct.
I did my checks on a supported platform only and therefore did not catch this.


-    return -1;
-}
#endif /* __linux__ */


With that fixed (no need to resend - this is a refactor so I'll push it
regardless of the rest of the series):

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Thanks for fixing this and also for your review of this and the other refactoring patches ... and for pushing them. :-)


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux