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

[Boris Fiuczynski]

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 at linux.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk at linux.ibm.com>
>> ---
>> 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 at redhat.com>
> 
> 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