3:25 p.m.
Hi John, thanks for your review.
On 10/23/14, 9:16 , "John Ferlan" <jferlan(a)redhat.com> wrote:
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(a)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.
OK, I¹ll add a patch for libvirt-python in v2.
> 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
Thanks for the info ‹ I¹ll rebase it after the Daniel¹s series is merged.
>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.
This design was copied from virNetworkGetDHCPLeases.
If it isn¹t a standard way to return an array, I¹ll remove the extra
element.
> + */
> +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
OK, I¹ll add the check and initialization.
Thanks,
Tomoki Sekiyama
> +
> + 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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>