[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune

[Adam Litke]

On Thu, Nov 10, 2011 at 04:32:51AM +0800, Lei HH Li wrote:
> This patch add new pulic API virDomainSetBlockIoTune and 
> virDomainGetBlockIoTune.
> 
> 
> Signed-off-by: Zhi Yong Wu <wuzhy at linux.vnet.ibm.com>
> Signed-off-by: Lei Li <lilei at linux.vnet.ibm.com>
> ---
>  include/libvirt/libvirt.h.in |   26 +++++++++
>  src/driver.h                 |   19 +++++++
>  src/libvirt.c                |  115 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    2 +
>  4 files changed, 162 insertions(+), 0 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index aa320b6..a79c35e 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1640,6 +1640,32 @@ int    virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
>  int           virDomainBlockPull(virDomainPtr dom, const char *path,
>                                   unsigned long bandwidth, unsigned int flags);
> 
> +/*
> + * Block I/O throttling support
> + */
> +
> +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
> +struct _virDomainBlockIoTuneInfo {
> +    unsigned long long total_bytes_sec;
> +    unsigned long long read_bytes_sec;
> +    unsigned long long write_bytes_sec;
> +    unsigned long long total_iops_sec;
> +    unsigned long long read_iops_sec;
> +    unsigned long long write_iops_sec;
> +};
> +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;
> +
> +int
> +virDomainSetBlockIoTune(virDomainPtr dom,
> +                        const char *disk,
> +                        virDomainBlockIoTuneInfoPtr info,
> +                        unsigned int flags);
> +int
> +virDomainGetBlockIoTune(virDomainPtr dom,
> +                        const char *disk,
> +                        virDomainBlockIoTuneInfoPtr reply,
> +                        unsigned int flags);
> +
> 
>  /*
>   * NUMA support
> diff --git a/src/driver.h b/src/driver.h
> index 4c14aaa..9628ad7 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -741,6 +741,23 @@ typedef int
>                               unsigned long bandwidth, unsigned int flags);
> 
> 
> +/*
> + * Block I/O throttling support
> + */
> +
> +typedef int
> +    (*virDrvDomainSetBlockIoTune)(virDomainPtr dom,
> +                                  const char *disk,
> +                                  virDomainBlockIoTuneInfoPtr info,
> +                                  unsigned int flags);
> +
> +typedef int
> +    (*virDrvDomainGetBlockIoTune)(virDomainPtr dom,
> +                                      const char *disk,
> +                                      virDomainBlockIoTuneInfoPtr reply,
> +                                      unsigned int flags);
> +
> +
>  /**
>   * _virDriver:
>   *
> @@ -899,6 +916,8 @@ struct _virDriver {
>      virDrvDomainGetBlockJobInfo domainGetBlockJobInfo;
>      virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed;
>      virDrvDomainBlockPull domainBlockPull;
> +    virDrvDomainSetBlockIoTune domainSetBlockIoTune;
> +    virDrvDomainGetBlockIoTune domainGetBlockIoTune;
>  };
> 
>  typedef int
> diff --git a/src/libvirt.c b/src/libvirt.c
> index b0d1e01..79ac84d 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -17083,3 +17083,118 @@ error:
>      virDispatchError(dom->conn);
>      return -1;
>  }
> +
> +/**
> + * virDomainSetBlockIoTune:
> + * @dom: pointer to domain object
> + * @disk: Fully-qualified disk name
> + * @info: Specify block I/O limits in bytes
> + * @flags: An OR'ed set of virDomainModificationImpact
> + *
> + * This function is mainly to enable Block I/O throttling function in libvirt.
> + * It is used to change the block I/O throttling setting for specified domain.
> + *
> + * Returns 0 if the operation has started, -1 on failure.
> + */
> +int virDomainSetBlockIoTune(virDomainPtr dom,
> +                            const char *disk,
> +                            virDomainBlockIoTuneInfoPtr info,
> +                            unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(dom, "disk=%p, info=%p, flags=%x",
> +                     disk, info, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
> +        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +    conn = dom->conn;
> +
> +    if (dom->conn->flags & VIR_CONNECT_RO) {
> +        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +        goto error;
> +    }
> +
> +    if (!disk) {
> +        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> +        goto error;
> +    }
> +
> +    if (!info) {
> +        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> +        goto error;
> +    }
> +
> +    if (conn->driver->domainSetBlockIoTune) {
> +        int ret;
> +        ret = conn->driver->domainSetBlockIoTune(dom, disk, info, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(dom->conn);
> +    return -1;
> +}
> +
> +/**
> + * virDomainGetBlockIoTune:
> + * @dom: pointer to domain object
> + * @disk: Fully-qualified disk name
> + * @reply: Specify block I/O info in bytes
> + * @flags: An OR'ed set of virDomainModificationImpact
> + *
> + * This function is mainly to enable Block I/O throttling function in libvirt.
> + * It is used to get the block I/O throttling setting for specified domain.
> + *
> + * Returns 0 if the operation has started, -1 on failure.

I think this might be a mistake from copying the comment from somewhere else.
Should it read: "Returns -1 if an error occurred, 0 otherwise."?

> + */
> +
> +int virDomainGetBlockIoTune(virDomainPtr dom,
> +                            const char *disk,
> +                            virDomainBlockIoTuneInfoPtr reply,
> +                            unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(dom, "disk=%p, reply=%p, flags=%x",
> +                     disk, reply, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
> +        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +    conn = dom->conn;
> +
> +    if (!disk) {
> +        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> +        goto error;
> +    }

Should you also check that reply is not NULL?

> +
> +    if (conn->driver->domainGetBlockIoTune) {
> +        int ret;
> +        ret = conn->driver->domainGetBlockIoTune(dom, disk, reply, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(dom->conn);
> +    return -1;
> +
> +}
> +
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index bcefb10..4808891 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -496,6 +496,8 @@ LIBVIRT_0.9.7 {
>          virDomainSnapshotGetParent;
>          virDomainSnapshotListChildrenNames;
>          virDomainSnapshotNumChildren;
> +        virDomainSetBlockIoTune;
> +        virDomainGetBlockIoTune;
>  } LIBVIRT_0.9.5;
> 
>  # .... define new API here using predicted next version number ....
> -- 
> 1.7.1
> 

-- 
Adam Litke <agl at us.ibm.com>
IBM Linux Technology Center