[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> Signed-off-by: Lei Li <lilei@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. + */ + +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; + } + + 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
On 11/09/2011 01:32 PM, Lei Li wrote:
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune.
Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> Signed-off-by: Lei Li<lilei@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;
This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/10/2011 04:43 AM, Eric Blake wrote:
On 11/09/2011 01:32 PM, Lei Li wrote:
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune.
Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> Signed-off-by: Lei Li<lilei@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;
This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure.
Hi Eric, We consider using virTypeParameter as Adam suggested at first, but when I dig into the code, I found struct virTypedParameter is mainly used by memtune, blkiotune...etc, supported by cgroup mechanism, different from Block I/O throttling which is supported through qemu monitor command 'block_set_io_throttle'. Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command. -- Lei
于 2011年11月10日 13:41, Lei Li 写道:
On 11/10/2011 04:43 AM, Eric Blake wrote:
On 11/09/2011 01:32 PM, Lei Li wrote:
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune.
Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> Signed-off-by: Lei Li<lilei@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;
This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure.
Hi Eric,
We consider using virTypeParameter as Adam suggested at first, but when I dig into the code, I found struct virTypedParameter is mainly used by memtune, blkiotune...etc, supported by cgroup mechanism, different from Block I/O throttling which is supported through qemu monitor command 'block_set_io_throttle'.
virTypedParameter is used between libvirt client and libvirtd. It isn't used between libvirtd and qemu.
Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command.
On 11/09/2011 11:11 PM, Xu He Jie wrote:
+++ 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 {
This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure.
virTypedParameter is used between libvirt client and libvirtd. It isn't used between libvirtd and qemu.
Correct - the public interface must use virTypedParameter. Once we get to qemu_driver.c, we can then convert from virTypedParameter to a struct specific to qemu, but that struct should probably be declared in qemu_monitor.h, and certainly NOT declared in libvirt.h.in.
Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command.
I have no problems with qemu monitor using a struct, my heartburn was with the introduction of a new public struct. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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@linux.vnet.ibm.com> Signed-off-by: Lei Li <lilei@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@us.ibm.com> IBM Linux Technology Center
participants (4)
-
Adam Litke -
Eric Blake -
Lei Li -
Xu He Jie