[libvirt] [PATCH 01/16] util: Introduce new module virmdev

[Michal Privoznik]

On 06.02.2017 13:19, Erik Skultety wrote:
> Beside creation, disposal, getter, and setter methods the module exports
> methods to work with lists of mediated devices.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  po/POTFILES.in           |   1 +
>  src/Makefile.am          |   1 +
>  src/libvirt_private.syms |  17 +++
>  src/util/virmdev.c       | 375 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virmdev.h       |  85 +++++++++++
>  5 files changed, 479 insertions(+)
>  create mode 100644 src/util/virmdev.c
>  create mode 100644 src/util/virmdev.h
> 


> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> new file mode 100644
> index 0000000..0b70798
> --- /dev/null
> +++ b/src/util/virmdev.c
> @@ -0,0 +1,375 @@
> +/*
> + * virmdev.c: helper APIs for managing host MDEV devices
> + *
> + * Copyright (C) 2017-2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Authors:
> + *     Erik Skultety <eskultet at redhat.com>
> + */
> +
> +#include <config.h>
> +
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <limits.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +
> +#include "virmdev.h"
> +#include "dirname.h"
> +#include "virlog.h"
> +#include "viralloc.h"
> +#include "vircommand.h"
> +#include "virerror.h"
> +#include "virfile.h"
> +#include "virkmod.h"
> +#include "virstring.h"
> +#include "virutil.h"
> +#include "viruuid.h"
> +#include "virhostdev.h"
> +
> +VIR_LOG_INIT("util.mdev");
> +
> +struct _virMediatedDevice {
> +    char *path;             /* sysfs path */
> +    bool managed;
> +
> +    char *used_by_drvname;
> +    char *used_by_domname;
> +};
> +
> +struct _virMediatedDeviceList {
> +    virObjectLockable parent;
> +
> +    size_t count;
> +    virMediatedDevicePtr *devs;
> +};
> +
> +
> +/* For virReportOOMError()  and virReportSystemError() */
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +static virClassPtr virMediatedDeviceListClass;
> +
> +static void virMediatedDeviceListDispose(void *obj);
> +
> +static int virMediatedOnceInit(void)
> +{
> +    if (!(virMediatedDeviceListClass = virClassNew(virClassForObjectLockable(),
> +                                                   "virMediatedDeviceList",
> +                                                   sizeof(virMediatedDeviceList),
> +                                                   virMediatedDeviceListDispose)))
> +        return -1;
> +
> +    return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virMediated)
> +
> +#ifdef __linux__
> +# define MDEV_SYSFS "/sys/class/mdev_bus/"

Some functions in this block look not-linux specific at all. For
instance there's nothing linux specific about virMediatedDeviceFree(),
GetPath() and so on. Usually, if running on unsupported environment,
erroring out in New() or Create() methods is good. The rest of the code
is not platform/environment/stellar constellation specific.

> +
> +virMediatedDevicePtr
> +virMediatedDeviceNew(virPCIDeviceAddressPtr pciaddr, const char *uuidstr)
> +{
> +    virMediatedDevicePtr dev = NULL, ret = NULL;
> +    char *pcistr = NULL;
> +
> +    if (virAsprintf(&pcistr, "%.4x:%.2x:%.2x.%.1x", pciaddr->domain,
> +                    pciaddr->bus, pciaddr->slot, pciaddr->function) < 0)
> +        return NULL;
> +
> +    if (VIR_ALLOC(dev) < 0)
> +        goto cleanup;
> +
> +    if (virAsprintf(&dev->path, MDEV_SYSFS "%s/%s", pcistr, uuidstr) < 0)
> +       goto cleanup;
> +
> +    ret = dev;
> +    dev = NULL;
> +
> + cleanup:
> +    VIR_FREE(pcistr);
> +    virMediatedDeviceFree(dev);
> +    return ret;
> +}
> +
> +
> +void
> +virMediatedDeviceFree(virMediatedDevicePtr dev)
> +{
> +    if (!dev)
> +        return;
> +    VIR_FREE(dev->path);
> +    VIR_FREE(dev->used_by_drvname);
> +    VIR_FREE(dev->used_by_domname);
> +    VIR_FREE(dev);
> +}
> +
> +
> +const char *
> +virMediatedDeviceGetPath(virMediatedDevicePtr dev)
> +{
> +    return dev->path;
> +}
> +
> +
> +/* Returns an absolute canonicalized path to the device used to control the
> + * mediated device's IOMMU group (e.g. "/dev/vfio/15")
> + */
> +char *
> +virMediatedDeviceGetIOMMUGroupDev(virMediatedDevicePtr dev)
> +{
> +    char *resultpath = NULL;
> +    char *iommu_linkpath = NULL;
> +    char *vfio_path = NULL;
> +
> +    if (virAsprintf(&iommu_linkpath, "%s/iommu_group", dev->path) < 0)
> +        return NULL;
> +
> +    if (virFileIsLink(iommu_linkpath) != 1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("IOMMU group file %s is not a symlink"),
> +                       iommu_linkpath);
> +        goto cleanup;
> +    }
> +
> +    if (virFileResolveLink(iommu_linkpath, &resultpath) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to resolve IOMMU group symlink %s"),
> +                       iommu_linkpath);
> +        goto cleanup;
> +    }
> +
> +    if (virAsprintf(&vfio_path, "/dev/vfio/%s", last_component(resultpath)) < 0)
> +        goto cleanup;
> +
> + cleanup:
> +    VIR_FREE(resultpath);
> +    VIR_FREE(iommu_linkpath);
> +    return vfio_path;
> +}
> +
> +
> +void
> +virMediatedDeviceGetUsedBy(virMediatedDevicePtr dev,
> +                           char **drvname, char **domname)

s/char **/const char **/ as we want to let caller know to not free the
values.

> +{
> +    *drvname = dev->used_by_drvname;
> +    *domname = dev->used_by_domname;
> +}
> +
> +



> +
> +
> +virMediatedDevicePtr
> +virMediatedDeviceListGet(virMediatedDeviceListPtr list,
> +                         int idx)

s/int/ssize_t/

Not that it would matter too much, but I had to point something else too :-)

> +{
> +    if (idx >= list->count)
> +        return NULL;
> +    if (idx < 0)
> +        return NULL;
> +
> +    return list->devs[idx];
> +}
> +
> +

Michal