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

Re: [libvirt] [PATCH 1/4] Introduce virNodeAllocPages



On 09/18/2014 10:24 AM, Michal Privoznik wrote:
> A long time ago in a galaxy far, far away it has been decided
> that libvirt will manage not only domains but host as well. And
> with my latest work on qemu driver supporting huge pages, we miss
> the cherry on top: an API to allocate huge pages on the run.
> Currently users are forced to log into the host and adjust the
> huge pages pool themselves.  However, with this API the problem
> is gone - they can both size up and size down the pool.
> 
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  daemon/remote.c              | 37 ++++++++++++++++++++++++
>  include/libvirt/libvirt.h.in | 16 +++++++++++
>  src/driver.h                 | 10 +++++++
>  src/libvirt.c                | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |  1 +
>  src/remote/remote_driver.c   | 47 +++++++++++++++++++++++++++++++
>  src/remote/remote_protocol.x | 20 ++++++++++++-
>  src/remote_protocol-structs  | 17 +++++++++++
>  8 files changed, 214 insertions(+), 1 deletion(-)
> 

> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 724314e..1a47bae 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -5505,6 +5505,22 @@ int virNodeGetFreePages(virConnectPtr conn,
>                          unsigned int cellcount,
>                          unsigned long long *counts,
>                          unsigned int flags);
> +
> +typedef enum {
> +    VIR_NODE_ALLOC_PAGES_ADD = 0, /* Add @pageCounts to the pages pool. This
> +                                     can be used only to size up the pool. */
> +    VIR_NODE_ALLOC_PAGES_SET = (1 << 0), /* Don't add @pageCounts, instead set
> +                                            passed number of pages. This can be
> +                                            used to free allocated pages. */

> +} virNodeAllocPagesFlags;
> +
> +int virNodeAllocPages(virConnectPtr conn,
> +                      unsigned int npages,
> +                      unsigned int *pageSizes,
> +                      unsigned long long *pageCounts,
> +                      int startCell,
> +                      unsigned int cellCount,
> +                      unsigned int flags);
>  /**
>   * virSchedParameterType:
>   *
> diff --git a/src/driver.h b/src/driver.h
> index bb748c4..dc62a8e 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -1212,6 +1212,15 @@ typedef int
>                                    virDomainStatsRecordPtr **retStats,
>                                    unsigned int flags);
>  
> +typedef int
> +(*virDrvNodeAllocPages)(virConnectPtr conn,
> +                        unsigned int npages,
> +                        unsigned int *pageSizes,
> +                        unsigned long long *pageCounts,
> +                        int startCell,
> +                        unsigned int cellCount,
> +                        unsigned int flags);
> +
>  typedef struct _virDriver virDriver;
>  typedef virDriver *virDriverPtr;
>  
> @@ -1435,6 +1444,7 @@ struct _virDriver {
>      virDrvNodeGetFreePages nodeGetFreePages;
>      virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
>      virDrvConnectGetAllDomainStats connectGetAllDomainStats;
> +    virDrvNodeAllocPages nodeAllocPages;
>  };
>  
>  
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 7c63825..27445e5 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21841,3 +21841,70 @@ virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats)
>  
>      VIR_FREE(stats);
>  }
> +
> +
> +/**
> + * virNodeAllocPages:
> + * @conn: pointer to the hypervisor connection
> + * @npages: number of items in the @pageSizes array
> + * @pageSizes: which huge page sizes to allocate
> + * @pageCounts: how many pages should be allocated
> + * @startCell: index of first cell to allocate pages on
> + * @cellCount: number of consecutive cells to allocate pages on
> + * @flags: extra flags; binary-OR of virConnectGetAllDomainStatsFlags

virNodeAllocPagesFlags

> + *
> + * Sometimes, when trying to start a new domain, it may be
> + * necessary to reserve some huge pages in the system pool which
> + * can be then allocated by the domain. This API serves that
> + * purpose. On its input, @pageSizes and @pageCounts are arrays
> + * of the same cardinality of @npages. The @pageSizes contains
> + * page sizes which are to be allocated in the system (the size
> + * unit is kibibytes), and @pageCounts then contains the number
> + * of pages to reserve. Depending on @flags specified, the number
> + * of pages is either added into the pool and the pool is sized
> + * up (@flags have VIR_NODE_ALLOC_PAGES_ADD set) or the number of
> + * pages is looked at the new size of pool and the pool can be
> + * both sized up or down (@flags have VIR_NODE_ALLOC_PAGES_SET
> + * set).  The pages pool can be allocated over several NUMA nodes
> + * at once, just point at @startCell and tell how many subsequent
> + * NUMA nodes should be taken in.
> + *
> + * Returns: the number of nodes successfully adjusted or -1 in
> + * case of an error.
> + */
> +int
> +virNodeAllocPages(virConnectPtr conn,
> +                  unsigned int npages,
> +                  unsigned int *pageSizes,
> +                  unsigned long long *pageCounts,
> +                  int startCell,

If -1 has special meaning here, it should be documented above.
If not, I'm not sure whether this should be unsinged, or left as signed to
match the GetFreePages API.

> +                  unsigned int cellCount,
> +                  unsigned int flags)
> +{
> +    VIR_DEBUG("conn=%p npages=%u pageSizes=%p pageCounts=%p "
> +              "startCell=%d cellCount=%u flagx=%x",
> +              conn, npages, pageSizes, pageCounts, startCell,
> +              cellCount, flags);
> +
> +    virResetLastError();
> +
> +    virCheckConnectReturn(conn, -1);
> +    virCheckNonZeroArgGoto(npages, error);
> +    virCheckNonNullArgGoto(pageSizes, error);
> +    virCheckNonNullArgGoto(pageCounts, error);
> +
> +    if (conn->driver->nodeAllocPages) {
> +        int ret;
> +        ret = conn->driver->nodeAllocPages(conn, npages, pageSizes,
> +                                           pageCounts, startCell,
> +                                           cellCount, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virReportUnsupportedError();
> + error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index e1f013f..89fbc71 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -677,6 +677,7 @@ LIBVIRT_1.2.8 {
>          virDomainListGetStats;
>          virDomainOpenGraphicsFD;
>          virDomainStatsRecordListFree;
> +        virNodeAllocPages;
>  } LIBVIRT_1.2.7;
>  

This should be in 1.2.9 (same goes for the comments in the drivers).

ACK series with the nits fixed.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature


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