[libvirt] [PATCH 1/5] Implement public API for virDomainGetFSInfo

[John Ferlan]

On 09/30/2014 08:20 PM, Tomoki Sekiyama wrote:
> virDomainGetFSInfo returns a list of filesystems information mounted in the
> guest, which contains mountpoints, device names, filesystem types, and
> device aliases named by libvirt. This will be useful, for example, to
> specify mountpoints to fsfreeze when taking snapshot of a part of disks.
> 
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama at hds.com>
> ---
>  include/libvirt/libvirt.h.in |   21 +++++++++++++
>  src/driver.h                 |    6 ++++
>  src/libvirt.c                |   68 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    6 ++++
>  4 files changed, 101 insertions(+)
> 

Since this is a new API you might need to make some changes in
libvirt-python (as well as others) in order to ensure the API is in each
as well.


> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 5217ab3..c60c3d7 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -5651,6 +5651,27 @@ int virDomainFSThaw(virDomainPtr dom,
>                      unsigned int nmountpoints,
>                      unsigned int flags);
>  
> +/**
> + * virDomainFSInfo:
> + *
> + * The data structure containing mounted file systems within a guset
> + *
> + */
> +typedef struct _virDomainFSInfo virDomainFSInfo;
> +typedef virDomainFSInfo *virDomainFSInfoPtr;
> +struct _virDomainFSInfo {
> +    char *mountpoint; /* path to mount point */
> +    char *name;       /* device name in the guest (e.g. "sda1") */
> +    char *type;       /* filesystem type */
> +    char **devAlias;  /* NULL-terminated array of disk device aliases */
> +};
> +
> +void virDomainFSInfoFree(virDomainFSInfoPtr info);
> +
> +int virDomainGetFSInfo(virDomainPtr dom,
> +                       virDomainFSInfoPtr **info,
> +                       unsigned int flags);
> +
>  int virDomainGetTime(virDomainPtr dom,
>                       long long *seconds,
>                       unsigned int *nseconds,
> diff --git a/src/driver.h b/src/driver.h


FYI: Recent upstream changes will cause you to find a new home for
things in driver.h.  See commit-id 'd21d35e3' - this will now be in
driver-hypervisor.h.

You may want to watch Daniel's series of patches because his patches
will probably modify your libvirt.c changes as well


> index dc62a8e..33e3bd9 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -1195,6 +1195,11 @@ typedef int
>                        unsigned int flags);
>  
>  typedef int
> +(*virDrvDomainGetFSInfo)(virDomainPtr dom,
> +                         virDomainFSInfoPtr **info,
> +                         unsigned int flags);
> +
> +typedef int
>  (*virDrvNodeGetFreePages)(virConnectPtr conn,
>                            unsigned int npages,
>                            unsigned int *pages,
> @@ -1445,6 +1450,7 @@ struct _virDriver {
>      virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
>      virDrvConnectGetAllDomainStats connectGetAllDomainStats;
>      virDrvNodeAllocPages nodeAllocPages;
> +    virDrvDomainGetFSInfo domainGetFSInfo;
>  };
>  
>  
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 245c373..77ec851 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21200,6 +21200,74 @@ virDomainFSThaw(virDomainPtr dom,
>      return -1;
>  }
>  
> +
> +/**
> + * virDomainGetFSInfo:
> + * @dom: a domain object
> + * @info: a pointer to a variable to store an array of mount points information
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Get a list of mapping informaiton for each mounted file systems within the

s/informaiton/information

> + * specified guest and the disks.
> + *
> + * Returns the number of returned mount points, or -1 and sets @info to
> + * NULL in case of error. On success, the array stored into @info is
> + * guaranteed to have an extra allocated element set to NULL but not included
> + * in the return count, to make iteration easier. The caller is responsible
> + * for calling virDomainFSInfoFree() on each array element, then calling
> + * free() on @info.

NOTE: See comment in patch 3 - I see no reason to allocate an extra
element for @info.  Since you are returning the count of elements,
that's all the caller should access.

> + */
> +int
> +virDomainGetFSInfo(virDomainPtr dom,
> +                   virDomainFSInfoPtr **info,
> +                   unsigned int flags)
> +{
> +    VIR_DOMAIN_DEBUG(dom, "info=%p, flags=%x", info, flags);
> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(dom, -1);
> +    virCheckReadOnlyGoto(dom->conn->flags, error);

Since it's touched and assumed later on:

    virCheckNonNullArgGoto(info, error);

and since on error you are indicating you guarantee it being NULL on
error, you may as well initialize it

        *info = NULL;

John
> +
> +    if (dom->conn->driver->domainGetFSInfo) {
> +        int ret = dom->conn->driver->domainGetFSInfo(dom, info, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virReportUnsupportedError();
> +
> + error:
> +    virDispatchError(dom->conn);
> +    return -1;
> +}
> +
> +
> +/**
> + * virDomainFSInfoFree:
> + * @info: pointer to a FSInfo object
> + *
> + * Frees all the memory occupied by @info.
> + */
> +void
> +virDomainFSInfoFree(virDomainFSInfoPtr info)
> +{
> +    char **alias;
> +
> +    if (!info)
> +        return;
> +
> +    VIR_FREE(info->mountpoint);
> +    VIR_FREE(info->name);
> +    VIR_FREE(info->type);
> +
> +    for (alias = info->devAlias; alias && *alias; alias++)
> +        VIR_FREE(*alias);
> +    VIR_FREE(info->devAlias);
> +}
> +
> +
>  /**
>   * virDomainGetTime:
>   * @dom: a domain object
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 5f95802..f688e76 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -684,4 +684,10 @@ LIBVIRT_1.2.9 {
>          virNodeAllocPages;
>  } LIBVIRT_1.2.8;
>  
> +LIBVIRT_1.2.10 {
> +    global:
> +        virDomainFSInfoFree;
> +        virDomainGetFSInfo;
> +} LIBVIRT_1.2.9;
> +
>  # .... define new API here using predicted next version number ....
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>