[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Eric Blake
eblake at redhat.com
Wed Nov 23 03:38:02 UTC 2011
On 11/15/2011 02:02 AM, Lei Li wrote:
> Support Block I/O Throttle setting and query to remote driver.
>
> Signed-off-by: Lei Li <lilei at linux.vnet.ibm.com>
> Signed-off-by: Zhi Yong Wu <wuzhy at linux.vnet.ibm.com>
> ---
> daemon/remote.c | 109 ++++++++++++++++++++++++++++++++++++++++++
> src/remote/remote_driver.c | 96 +++++++++++++++++++++++++++++++++++++
> src/remote/remote_protocol.x | 27 ++++++++++-
> src/remote_protocol-structs | 24 +++++++++
> 4 files changed, 255 insertions(+), 1 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index aa3f768..227d36e 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1886,6 +1886,115 @@ cleanup:
> return rv;
> }
>
> +static int
> +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED,
This should be named remoteDispatchDomainSetBlockIoTune, to match the
public API naming.
In fact, by naming it correctly, we can rely on autogen to write this
function for us.
> +
> +static int
> +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client ATTRIBUTE_UNUSED,
> + virNetMessagePtr hdr ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr,
> + remote_domain_get_block_io_throttle_args *args,
> + remote_domain_get_block_io_throttle_ret *ret)
Again, naming should be consistent, but this time we have to provide a
manual version.
> +{
> + virDomainPtr dom = NULL;
> + int rv = -1;
> + int i;
> + virTypedParameterPtr params;
> + int nparams = args->nparams;
> + struct daemonClientPrivate *priv =
> + virNetServerClientGetPrivateData(client);
> +
> + if (!priv->conn) {
> + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
> + goto cleanup;
> + }
> +
> + if (nparams > REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) {
More naming; the constant should be
REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX.
> + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
> + goto cleanup;
> + }
> +
> + if (VIR_ALLOC_N(params, nparams) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
> + goto cleanup;
> +
> + if (virDomainGetBlockIoTune(dom, args->disk, params, &nparams, args->flags) < 0)
Given my tweaks in 1/8, we want to be able to handle NULL over the wire;
this means that this type is changed to remote_string, and that
args->disk is now char** instead of char* and requires slightly
different handling.
> +
> +cleanup:
> + if (rv < 0) {
> + virNetMessageSaveError(rerr);
> + if (ret->params.params_val) {
> + for (i = 0; i < nparams; i++)
> + VIR_FREE(ret->params.params_val[i].field);
> + VIR_FREE(ret->params.params_val);
> + }
> + }
Memory leak on any string parameters, as well as on params itself.
> +++ b/src/remote/remote_driver.c
> @@ -2178,6 +2178,100 @@ done:
> return rv;
> }
>
> +static int remoteDomainSetBlockIoTune(virDomainPtr domain,
This one can be auto-generated.
> +
> +static int remoteDomainGetBlockIoTune(virDomainPtr domain,
> + const char *disk,
> + virTypedParameterPtr params,
> + int *nparams,
> + unsigned int flags)
> +{
> + int rv = -1;
> + remote_domain_get_block_io_throttle_args args;
> + remote_domain_get_block_io_throttle_ret ret;
Naming of these types.
> + struct private_data *priv = domain->conn->privateData;
> +
> + remoteDriverLock(priv);
> +
> + make_nonnull_domain(&args.dom, domain);
> + args.disk = (char *)disk;
Type of this argument.
> + if (remoteDeserializeTypedParameters(ret.params.params_val,
> + ret.params.params_len,
> + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX,
> + params,
> + nparams) < 0)
> + goto cleanup;
> +
> + rv = 0;
> +
> +cleanup:
> + xdr_free ((xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret,
> + (char *) &ret);
> + rv = 0;
Oops - that negated any previously reported error.
> +++ b/src/remote/remote_protocol.x
> @@ -125,6 +125,9 @@ const REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX = 16;
> /* Upper limit on list of memory parameters. */
> const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16;
>
> +/* Upper limit on list of blockio throttle parameters. */
> +const REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX = 16;
As mentioned above, I tweaked this naming.
> +
> +struct remote_domain_get_block_io_throttle_args {
> + remote_nonnull_domain dom;
> + remote_nonnull_string disk;
as well as this.
> @@ -2564,7 +2586,10 @@ enum remote_procedure {
> REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen priority:high */
> REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */
> REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, /* skipgen skipgen */
> - REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249 /* skipgen skipgen */
> + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249, /* skipgen skipgen */
> + REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250, /* skipgen skipgen */
Missing blank line after a multiple of 10.
> + REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251 /* skipgen skipgen */
> +
>
> /*
> * Notice how the entries are grouped in sets of 10 ?
Here's what I'll be squashing in, when I'm ready to push this:
diff --git i/daemon/remote.c w/daemon/remote.c
index e74c671..2a2c551 100644
--- i/daemon/remote.c
+++ w/daemon/remote.c
@@ -1886,53 +1886,12 @@ cleanup:
}
static int
-remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server
ATTRIBUTE_UNUSED,
- virNetServerClientPtr client
ATTRIBUTE_UNUSED,
- virNetMessagePtr hdr
ATTRIBUTE_UNUSED,
- virNetMessageErrorPtr rerr,
-
remote_domain_set_block_io_throttle_args *args)
-{
- virDomainPtr dom = NULL;
- int rv = -1;
- virTypedParameterPtr params = NULL;
- int nparams;
- struct daemonClientPrivate *priv =
- virNetServerClientGetPrivateData(client);
-
- if (!priv->conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
- goto cleanup;
-
- if ((params = remoteDeserializeTypedParameters(args->params.params_val,
- args->params.params_len,
-
REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX,
- &nparams)) == NULL)
- goto cleanup;
-
- rv = virDomainSetBlockIoTune(dom, args->disk, params, nparams,
args->flags);
-
- if (rv < 0)
- goto cleanup;
-
-cleanup:
- if (rv < 0)
- virNetMessageSaveError(rerr);
- if (dom)
- virDomainFree(dom);
- return rv;
-}
-
-static int
-remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server
ATTRIBUTE_UNUSED,
- virNetServerClientPtr client
ATTRIBUTE_UNUSED,
- virNetMessagePtr hdr
ATTRIBUTE_UNUSED,
- virNetMessageErrorPtr rerr,
-
remote_domain_get_block_io_throttle_args *args,
-
remote_domain_get_block_io_throttle_ret *ret)
+remoteDispatchDomainGetBlockIoTune(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+ virNetMessagePtr hdr ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_block_io_tune_args
*args,
+ remote_domain_get_block_io_tune_ret
*ret)
{
virDomainPtr dom = NULL;
int rv = -1;
@@ -1947,7 +1906,7 @@
remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server
ATTRIBUTE_UNUSED,
goto cleanup;
}
- if (nparams > REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) {
+ if (nparams > REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX) {
virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
}
@@ -1960,7 +1919,8 @@
remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server
ATTRIBUTE_UNUSED,
if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
goto cleanup;
- if (virDomainGetBlockIoTune(dom, args->disk, params, &nparams,
args->flags) < 0)
+ if (virDomainGetBlockIoTune(dom, args->disk ? *args->disk : NULL,
+ params, &nparams, args->flags) < 0)
goto cleanup;
/* In this case, we need to send back the number of parameters
@@ -1971,7 +1931,7 @@
remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server
ATTRIBUTE_UNUSED,
goto success;
}
- /* Serialise the block I/O throttle. */
+ /* Serialise the block I/O tuning parameters. */
if (remoteSerializeTypedParameters(params, nparams,
&ret->params.params_val,
&ret->params.params_len,
@@ -1982,14 +1942,10 @@ success:
rv = 0;
cleanup:
- if (rv < 0) {
+ if (rv < 0)
virNetMessageSaveError(rerr);
- if (ret->params.params_val) {
- for (i = 0; i < nparams; i++)
- VIR_FREE(ret->params.params_val[i].field);
- VIR_FREE(ret->params.params_val);
- }
- }
+ virTypedParameterArrayClear(params, nparams);
+ VIR_FREE(params);
if (dom)
virDomainFree(dom);
return rv;
diff --git i/src/libvirt.c w/src/libvirt.c
index a64bc07..074bc22 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -17336,4 +17336,3 @@ error:
virDispatchError(dom->conn);
return -1;
}
-
diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c
index fa2d2c7..c37954e 100644
--- i/src/remote/remote_driver.c
+++ w/src/remote/remote_driver.c
@@ -2178,43 +2178,6 @@ done:
return rv;
}
-static int remoteDomainSetBlockIoTune(virDomainPtr domain,
- const char *disk,
- virTypedParameterPtr params,
- int nparams,
- unsigned int flags)
-{
- int rv = -1;
- remote_domain_set_block_io_throttle_args args;
- struct private_data *priv = domain->conn->privateData;
-
- remoteDriverLock(priv);
-
- memset(&args, 0, sizeof(args));
-
- make_nonnull_domain(&args.dom, domain);
- args.disk = (char *)disk;
- args.flags = flags;
-
- if (remoteSerializeTypedParameters(params, nparams,
&args.params.params_val, &args.params.params_len) < 0) {
-
xdr_free((xdrproc_t)xdr_remote_domain_set_block_io_throttle_args, (char
*)&args);
- goto done;
- }
-
- if (call(domain->conn, priv, 0,
REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE,
- (xdrproc_t) xdr_remote_domain_set_block_io_throttle_args,
- (char *) &args,
- (xdrproc_t) xdr_void,
- (char *) NULL) == -1) {
- goto done;
- }
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
- return rv;
-}
-
static int remoteDomainGetBlockIoTune(virDomainPtr domain,
const char *disk,
virTypedParameterPtr params,
@@ -2222,24 +2185,24 @@ static int
remoteDomainGetBlockIoTune(virDomainPtr domain,
unsigned int flags)
{
int rv = -1;
- remote_domain_get_block_io_throttle_args args;
- remote_domain_get_block_io_throttle_ret ret;
+ remote_domain_get_block_io_tune_args args;
+ remote_domain_get_block_io_tune_ret ret;
struct private_data *priv = domain->conn->privateData;
remoteDriverLock(priv);
make_nonnull_domain(&args.dom, domain);
- args.disk = (char *)disk;
+ args.disk = disk ? (char **)&disk : NULL;
args.nparams = *nparams;
args.flags = flags;
memset(&ret, 0, sizeof(ret));
- if (call(domain->conn, priv, 0,
REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE,
- (xdrproc_t) xdr_remote_domain_get_block_io_throttle_args,
+ if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE,
+ (xdrproc_t) xdr_remote_domain_get_block_io_tune_args,
(char *) &args,
- (xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret,
+ (xdrproc_t) xdr_remote_domain_get_block_io_tune_ret,
(char *) &ret) == -1) {
goto done;
}
@@ -2263,10 +2226,8 @@ static int
remoteDomainGetBlockIoTune(virDomainPtr domain,
rv = 0;
cleanup:
- xdr_free ((xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret,
+ xdr_free ((xdrproc_t) xdr_remote_domain_get_block_io_tune_ret,
(char *) &ret);
- rv = 0;
-
done:
remoteDriverUnlock(priv);
return rv;
diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x
index 9ab3314..106a982 100644
--- i/src/remote/remote_protocol.x
+++ w/src/remote/remote_protocol.x
@@ -125,8 +125,8 @@ const REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX = 16;
/* Upper limit on list of memory parameters. */
const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16;
-/* Upper limit on list of blockio throttle parameters. */
-const REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX = 16;
+/* Upper limit on list of blockio tuning parameters. */
+const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16;
/* Upper limit on list of node cpu stats. */
const REMOTE_NODE_CPU_STATS_MAX = 16;
@@ -1078,22 +1078,22 @@ struct remote_domain_block_pull_args {
unsigned int flags;
};
-struct remote_domain_set_block_io_throttle_args {
+struct remote_domain_set_block_io_tune_args {
remote_nonnull_domain dom;
remote_nonnull_string disk;
- remote_typed_param params<REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX>;
+ remote_typed_param params<REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX>;
unsigned int flags;
};
-struct remote_domain_get_block_io_throttle_args {
+struct remote_domain_get_block_io_tune_args {
remote_nonnull_domain dom;
- remote_nonnull_string disk;
+ remote_string disk;
int nparams;
unsigned int flags;
};
-struct remote_domain_get_block_io_throttle_ret {
- remote_typed_param params<REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX>;
+struct remote_domain_get_block_io_tune_ret {
+ remote_typed_param params<REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX>;
int nparams;
};
@@ -2587,9 +2587,9 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen
autogen priority:high */
REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249, /* skipgen skipgen */
- REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250, /* skipgen skipgen */
- REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251 /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 250, /* autogen autogen */
+ REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 251 /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs
index eabf5e5..853eac4 100644
--- i/src/remote_protocol-structs
+++ w/src/remote_protocol-structs
@@ -759,22 +759,22 @@ struct remote_domain_block_pull_args {
uint64_t bandwidth;
u_int flags;
};
-struct remote_domain_set_block_io_throttle_args {
+struct remote_domain_set_block_io_tune_args {
remote_nonnull_domain dom;
- remote_nonnull_string disk;
+ remote_nonnull_string disk;
struct {
u_int params_len;
remote_typed_param * params_val;
} params;
u_int flags;
};
-struct remote_domain_get_block_io_throttle_args {
+struct remote_domain_get_block_io_tune_args {
remote_nonnull_domain dom;
- remote_nonnull_string disk;
+ remote_string disk;
int nparams;
u_int flags;
};
-struct remote_domain_get_block_io_throttle_ret {
+struct remote_domain_get_block_io_tune_ret {
struct {
u_int params_len;
remote_typed_param * params_val;
@@ -2029,6 +2029,6 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248,
REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249,
- REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250,
- REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251,
+ REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 250,
+ REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 251,
};
--
Eric Blake eblake at redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111122/9ed9d8b2/attachment-0001.sig>
More information about the libvir-list
mailing list