[libvirt] [PATCHv2 0/5] Add new command domiftune

This series adds a new command domiftune to get/set parameters of domain's network interfaces. Supported parameters are bandwidth settings. Currently the network interface bandwidth can only be set: - in domain's xml before the domain is up - when attaching an interface by attach-interface With this series, users can change network interface bandwidth settings using virsh, whether the domain is running or not. changes: - rebase - use vshGetTypedParamValue in patch 5 Hu Tao (5): Add API virDomain{S,G}etInterfaceParameters virDomain{S,G}etInterfaceParameters: the main entry points Add virDomain{S,G}etInterfaceparameters support to the remote driver Add virDomain{S,G}etInterfaceParameters support to qemu driver Enable the virDomain{S,G}etInterfaceParameters in virsh daemon/remote.c | 64 ++++++ include/libvirt/libvirt.h.in | 50 +++++ python/generator.py | 2 + src/driver.h | 12 ++ src/libvirt.c | 118 ++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 434 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 52 +++++ src/remote/remote_protocol.x | 28 +++- src/remote_protocol-structs | 24 +++ tools/virsh.c | 198 +++++++++++++++++++ tools/virsh.pod | 21 ++ 12 files changed, 1004 insertions(+), 1 deletions(-) -- 1.7.4.4

The APIs are used to set/get domain's network interface's parameters. Currently supported parameters are bandwidth settings. * include/libvirt/libvirt.h.in: new API and parameters definition * python/generator.py: fix compiler errors * src/driver.h: add new entry to the driver structure * src/libvirt_public.syms: export symbols --- include/libvirt/libvirt.h.in | 50 ++++++++++++++++++++++++++++++++++++++++++ python/generator.py | 2 + src/driver.h | 12 ++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 66 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7f26521..f59c3b1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -645,6 +645,48 @@ typedef virTypedParameter *virTypedParameterPtr; */ #define VIR_DOMAIN_SCHEDULER_SHARES "shares" +/** + * VIR_DOMAIN_BANDWIDTH_IN_AVERAGE: + * + * Macro represents the inbound average of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_AVERAGE "inbound.average" + +/** + * VIR_DOMAIN_BANDWIDTH_IN_PEAK: + * + * Macro represents the inbound peak of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_PEAK "inbound.peak" + +/** + * VIR_DOMAIN_BANDWIDTH_IN_BURST: + * + * Macro represents the inbound burst of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_BURST "inbound.burst" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE: + * + * Macro represents the outbound average of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE "outbound.average" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_PEAK: + * + * Macro represents the outbound peak of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_PEAK "outbound.peak" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_BURST: + * + * Macro represents the outbound burst of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst" + /* * Fetch scheduler parameters, caller allocates 'params' field of size 'nparams' */ @@ -1446,6 +1488,14 @@ int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, size_t size); +int virDomainSetInterfaceParameters (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags); +int virDomainGetInterfaceParameters (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags); int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats, diff --git a/python/generator.py b/python/generator.py index 181f70e..6fee3a4 100755 --- a/python/generator.py +++ b/python/generator.py @@ -419,6 +419,8 @@ skip_impl = ( 'virDomainBlockStatsFlags', 'virDomainSetBlockIoTune', 'virDomainGetBlockIoTune', + 'virDomainSetInterfaceParameters', + 'virDomainGetInterfaceParameters', ) qemu_skip_impl = ( diff --git a/src/driver.h b/src/driver.h index bbd6417..ec4abf3 100644 --- a/src/driver.h +++ b/src/driver.h @@ -375,6 +375,16 @@ typedef int (virDomainPtr domain, const char *path, struct _virDomainInterfaceStats *stats); +typedef int + (*virDrvDomainSetInterfaceParameters) (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags); +typedef int + (*virDrvDomainGetInterfaceParameters) (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags); typedef int (*virDrvDomainMemoryStats) @@ -883,6 +893,8 @@ struct _virDriver { virDrvDomainBlockStats domainBlockStats; virDrvDomainBlockStatsFlags domainBlockStatsFlags; virDrvDomainInterfaceStats domainInterfaceStats; + virDrvDomainSetInterfaceParameters domainSetInterfaceParameters; + virDrvDomainGetInterfaceParameters domainGetInterfaceParameters; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 3709f08..ea6d562 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -512,6 +512,8 @@ LIBVIRT_0.9.9 { global: virDomainGetNumaParameters; virDomainSetNumaParameters; + virDomainGetInterfaceParameters; + virDomainSetInterfaceParameters; } LIBVIRT_0.9.8; # .... define new API here using predicted next version number .... -- 1.7.4.4

On 2011年12月23日 15:09, Hu Tao wrote:
The APIs are used to set/get domain's network interface's parameters. Currently supported parameters are bandwidth settings.
* include/libvirt/libvirt.h.in: new API and parameters definition * python/generator.py: fix compiler errors
It's not to fix something, but to skip the Python API generation for newly introduced APIs.
* src/driver.h: add new entry to the driver structure * src/libvirt_public.syms: export symbols --- include/libvirt/libvirt.h.in | 50 ++++++++++++++++++++++++++++++++++++++++++ python/generator.py | 2 + src/driver.h | 12 ++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 66 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7f26521..f59c3b1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -645,6 +645,48 @@ typedef virTypedParameter *virTypedParameterPtr; */ #define VIR_DOMAIN_SCHEDULER_SHARES "shares"
+/** + * VIR_DOMAIN_BANDWIDTH_IN_AVERAGE: + * + * Macro represents the inbound average of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_AVERAGE "inbound.average" + +/** + * VIR_DOMAIN_BANDWIDTH_IN_PEAK: + * + * Macro represents the inbound peak of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_PEAK "inbound.peak" + +/** + * VIR_DOMAIN_BANDWIDTH_IN_BURST: + * + * Macro represents the inbound burst of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_IN_BURST "inbound.burst" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE: + * + * Macro represents the outbound average of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE "outbound.average" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_PEAK: + * + * Macro represents the outbound peak of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_PEAK "outbound.peak" + +/** + * VIR_DOMAIN_BANDWIDTH_OUT_BURST: + * + * Macro represents the outbound burst of NIC bandwidth. + */ +#define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst" + /* * Fetch scheduler parameters, caller allocates 'params' field of size 'nparams' */ @@ -1446,6 +1488,14 @@ int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, size_t size); +int virDomainSetInterfaceParameters (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags); +int virDomainGetInterfaceParameters (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags); int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats, diff --git a/python/generator.py b/python/generator.py index 181f70e..6fee3a4 100755 --- a/python/generator.py +++ b/python/generator.py @@ -419,6 +419,8 @@ skip_impl = ( 'virDomainBlockStatsFlags', 'virDomainSetBlockIoTune', 'virDomainGetBlockIoTune', + 'virDomainSetInterfaceParameters', + 'virDomainGetInterfaceParameters', )
qemu_skip_impl = ( diff --git a/src/driver.h b/src/driver.h index bbd6417..ec4abf3 100644 --- a/src/driver.h +++ b/src/driver.h @@ -375,6 +375,16 @@ typedef int (virDomainPtr domain, const char *path, struct _virDomainInterfaceStats *stats); +typedef int + (*virDrvDomainSetInterfaceParameters) (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags); +typedef int + (*virDrvDomainGetInterfaceParameters) (virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags);
typedef int (*virDrvDomainMemoryStats) @@ -883,6 +893,8 @@ struct _virDriver { virDrvDomainBlockStats domainBlockStats; virDrvDomainBlockStatsFlags domainBlockStatsFlags; virDrvDomainInterfaceStats domainInterfaceStats; + virDrvDomainSetInterfaceParameters domainSetInterfaceParameters; + virDrvDomainGetInterfaceParameters domainGetInterfaceParameters; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 3709f08..ea6d562 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -512,6 +512,8 @@ LIBVIRT_0.9.9 { global: virDomainGetNumaParameters; virDomainSetNumaParameters; + virDomainGetInterfaceParameters; + virDomainSetInterfaceParameters; } LIBVIRT_0.9.8;
# .... define new API here using predicted next version number ....
ACK with the commit message fixed. Regards, Osier

* src/libvirt.c: implement the main entry points --- src/libvirt.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 118 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 574be16..3d71d6f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7075,6 +7075,124 @@ error: return -1; } + /** + * virDomainSetInterfaceParameters: + * @domain: pointer to domain object + * @device: the interface name or mac address + * @params: pointer to interface parameter objects + * @nparams: number of interface parameter (this value can be the same or + * less than the number of parameters supported) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Currently this function sets bandwidth parameters of interface. + * This function may require privileged access to the hypervisor. + * + * Returns -1 in case of error, 0 in case of success. + */ +int +virDomainSetInterfaceParameters(virDomainPtr domain, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "device=%s, params=%p, nparams=%d, flags=%x", + device, params, nparams, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + if ((nparams <= 0) || (params == NULL)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (virTypedParameterValidateSet(domain, params, nparams) < 0) + return -1; + + conn = domain->conn; + + if (conn->driver->domainSetInterfaceParameters) { + int ret; + ret = conn->driver->domainSetInterfaceParameters(domain, device, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + /** + * virDomainGetInterfaceParameters: + * @domain: pointer to domain object + * @device: the interface name or mac address + * @params: pointer to interface parameter objects + * (return value, allocated by the caller) + * @nparams: pointer to number of interface parameter + * @flags: one of virDomainModificationImpact + * + * This function may require privileged access to the hypervisor. This function + * expects the caller to allocate the @params. + * + * Returns -1 in case of error, 0 in case of success. + */ + +int +virDomainGetInterfaceParameters(virDomainPtr domain, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "device=%s, params=%p, nparams=%d, flags=%x", + device, params, (nparams) ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if ((nparams == NULL) || (*nparams < 0) || + (params == NULL && *nparams != 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + conn = domain->conn; + + if (conn->driver->domainGetInterfaceParameters) { + int ret; + ret = conn->driver->domainGetInterfaceParameters (domain, device, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + /** * virDomainMemoryStats: * @dom: pointer to the domain object -- 1.7.4.4

On 2011年12月23日 15:09, Hu Tao wrote:
* src/libvirt.c: implement the main entry points --- src/libvirt.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 118 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 574be16..3d71d6f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7075,6 +7075,124 @@ error: return -1; }
+ /** + * virDomainSetInterfaceParameters: + * @domain: pointer to domain object + * @device: the interface name or mac address + * @params: pointer to interface parameter objects + * @nparams: number of interface parameter (this value can be the same or + * less than the number of parameters supported) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Currently this function sets bandwidth parameters of interface. + * This function may require privileged access to the hypervisor. + * + * Returns -1 in case of error, 0 in case of success. + */ +int +virDomainSetInterfaceParameters(virDomainPtr domain, + const char *device, + virTypedParameterPtr params, + int nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "device=%s, params=%p, nparams=%d, flags=%x", + device, params, nparams, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags& VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + if ((nparams<= 0) || (params == NULL)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (virTypedParameterValidateSet(domain, params, nparams)< 0) + return -1; + + conn = domain->conn; + + if (conn->driver->domainSetInterfaceParameters) { + int ret; + ret = conn->driver->domainSetInterfaceParameters(domain, device, params, nparams, flags); + if (ret< 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + /** + * virDomainGetInterfaceParameters: + * @domain: pointer to domain object + * @device: the interface name or mac address + * @params: pointer to interface parameter objects + * (return value, allocated by the caller) + * @nparams: pointer to number of interface parameter + * @flags: one of virDomainModificationImpact
Lacks documents like for other GetFooParameters APIs. E.g. <snip> * Get all numa parameters. On input, @nparams gives the size of the * @params array; on output, @nparams gives how many slots were filled * with parameter information, which might be less but will not exceed * the input value. * * As a special case, calling with @params as NULL and @nparams as 0 on * input will cause @nparams on output to contain the number of parameters * supported by the hypervisor. The caller should then allocate @params * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API * again. * * See virDomainGetMemoryParameters() for an equivalent usage example. </snip> Actually virDomainGetNumaParameters is also by you. :-)
+ * + * This function may require privileged access to the hypervisor. This function + * expects the caller to allocate the @params. + * + * Returns -1 in case of error, 0 in case of success. + */ + +int +virDomainGetInterfaceParameters(virDomainPtr domain, + const char *device, + virTypedParameterPtr params, + int *nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "device=%s, params=%p, nparams=%d, flags=%x", + device, params, (nparams) ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if ((nparams == NULL) || (*nparams< 0) || + (params == NULL&& *nparams != 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + conn = domain->conn; + + if (conn->driver->domainGetInterfaceParameters) { + int ret; + ret = conn->driver->domainGetInterfaceParameters (domain, device, params, nparams, flags); + if (ret< 0) + goto error; + return ret; + } + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + /** * virDomainMemoryStats: * @dom: pointer to the domain object
ACK with the documents added. Regards, Osier

* daemon/remote.c: implement the server side support * src/remote/remote_driver.c: implement the client side support * src/remote/remote_protocol.x: definitions for the mew entry points * src/remote_protocol-structs: structure definitions --- daemon/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 52 ++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 28 +++++++++++++++++- src/remote_protocol-structs | 24 +++++++++++++++ 4 files changed, 167 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 316d530..b7ba321 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3436,6 +3436,70 @@ cleanup: return rv; } +static int +remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_interface_parameters_args *args, + remote_domain_get_interface_parameters_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + const char *device = args->device; + int nparams = args->nparams; + unsigned int flags; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + flags = args->flags; + + if (nparams > REMOTE_DOMAIN_INTERFACE_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 (virDomainGetInterfaceParameters(dom, device, params, &nparams, flags) < 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len, + flags) < 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(params); + return rv; +} /*----- Helpers. -----*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index cd3da53..7580477 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4396,6 +4396,56 @@ call (virConnectPtr conn, } +static int +remoteDomainGetInterfaceParameters (virDomainPtr domain, + const char *device, + virTypedParameterPtr params, int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_interface_parameters_args args; + remote_domain_get_interface_parameters_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.device = (char *)device; + args.nparams = *nparams; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS, + (xdrproc_t) xdr_remote_domain_get_interface_parameters_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_interface_parameters_ret, (char *) &ret) == -1) + goto done; + + /* Handle the case when the caller does not know the number of parameters + * and is asking for the number of parameters supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_interface_parameters_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { @@ -4619,6 +4669,8 @@ static virDriver remote_driver = { .domainBlockStats = remoteDomainBlockStats, /* 0.3.2 */ .domainBlockStatsFlags = remoteDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ + .domainSetInterfaceParameters = remoteDomainSetInterfaceParameters, /* 0.9.9 */ + .domainGetInterfaceParameters = remoteDomainGetInterfaceParameters, /* 0.9.9 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ .domainMemoryPeek = remoteDomainMemoryPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 18dba8c..ca739ff 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -202,6 +202,11 @@ const REMOTE_CPU_BASELINE_MAX = 256; */ const REMOTE_DOMAIN_SEND_KEY_MAX = 16; +/* + * Upper limit on list of interface parameters + */ +const REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX = 16; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -608,6 +613,25 @@ struct remote_domain_interface_stats_ret { /* insert@2 */ hyper tx_drop; }; +struct remote_domain_set_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + remote_typed_param params<REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + int nparams; + unsigned int flags; +}; + +struct remote_domain_get_interface_parameters_ret { + remote_typed_param params<REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX>; + int nparams; +}; + struct remote_domain_memory_stats_args { remote_nonnull_domain dom; unsigned int maxStats; @@ -2627,7 +2651,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 252, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index f6d8ff5..2758315 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -328,6 +328,28 @@ struct remote_domain_interface_stats_ret { int64_t tx_errs; int64_t tx_drop; }; +struct remote_domain_set_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_get_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + int nparams; + u_int flags; +}; +struct remote_domain_get_interface_parameters_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + int nparams; +}; struct remote_domain_memory_stats_args { remote_nonnull_domain dom; u_int maxStats; @@ -2066,4 +2088,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, + REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, }; -- 1.7.4.4

On 2011年12月23日 15:09, Hu Tao wrote:
* daemon/remote.c: implement the server side support * src/remote/remote_driver.c: implement the client side support * src/remote/remote_protocol.x: definitions for the mew entry points
s/mew/new/
* src/remote_protocol-structs: structure definitions --- daemon/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 52 ++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 28 +++++++++++++++++- src/remote_protocol-structs | 24 +++++++++++++++ 4 files changed, 167 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 316d530..b7ba321 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3436,6 +3436,70 @@ cleanup: return rv; }
+static int +remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_interface_parameters_args *args, + remote_domain_get_interface_parameters_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + const char *device = args->device; + int nparams = args->nparams; + unsigned int flags; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + flags = args->flags; + + if (nparams> REMOTE_DOMAIN_INTERFACE_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 (virDomainGetInterfaceParameters(dom, device, params,&nparams, flags)< 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + if (remoteSerializeTypedParameters(params, nparams, +&ret->params.params_val, +&ret->params.params_len, + flags)< 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv< 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(params); + return rv; +}
/*----- Helpers. -----*/
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index cd3da53..7580477 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4396,6 +4396,56 @@ call (virConnectPtr conn, }
+static int +remoteDomainGetInterfaceParameters (virDomainPtr domain, + const char *device, + virTypedParameterPtr params, int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_interface_parameters_args args; + remote_domain_get_interface_parameters_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.device = (char *)device; + args.nparams = *nparams; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS, + (xdrproc_t) xdr_remote_domain_get_interface_parameters_args, (char *)&args, + (xdrproc_t) xdr_remote_domain_get_interface_parameters_ret, (char *)&ret) == -1) + goto done; + + /* Handle the case when the caller does not know the number of parameters + * and is asking for the number of parameters supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX, + params, + nparams)< 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_interface_parameters_ret, + (char *)&ret); +done: + remoteDriverUnlock(priv); + return rv; +} + static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { @@ -4619,6 +4669,8 @@ static virDriver remote_driver = { .domainBlockStats = remoteDomainBlockStats, /* 0.3.2 */ .domainBlockStatsFlags = remoteDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ + .domainSetInterfaceParameters = remoteDomainSetInterfaceParameters, /* 0.9.9 */ + .domainGetInterfaceParameters = remoteDomainGetInterfaceParameters, /* 0.9.9 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ .domainMemoryPeek = remoteDomainMemoryPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 18dba8c..ca739ff 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -202,6 +202,11 @@ const REMOTE_CPU_BASELINE_MAX = 256; */ const REMOTE_DOMAIN_SEND_KEY_MAX = 16;
+/* + * Upper limit on list of interface parameters + */ +const REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX = 16; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -608,6 +613,25 @@ struct remote_domain_interface_stats_ret { /* insert@2 */ hyper tx_drop; };
+struct remote_domain_set_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + remote_typed_param params<REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + int nparams; + unsigned int flags; +}; + +struct remote_domain_get_interface_parameters_ret { + remote_typed_param params<REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX>; + int nparams; +}; + struct remote_domain_memory_stats_args { remote_nonnull_domain dom; unsigned int maxStats; @@ -2627,7 +2651,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 252, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index f6d8ff5..2758315 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -328,6 +328,28 @@ struct remote_domain_interface_stats_ret { int64_t tx_errs; int64_t tx_drop; }; +struct remote_domain_set_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_get_interface_parameters_args { + remote_nonnull_domain dom; + remote_nonnull_string device; + int nparams; + u_int flags; +}; +struct remote_domain_get_interface_parameters_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + int nparams; +}; struct remote_domain_memory_stats_args { remote_nonnull_domain dom; u_int maxStats; @@ -2066,4 +2088,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, + REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, };
ACK

* src/qemu/qemu_driver.c: implement the qemu driver support --- src/qemu/qemu_driver.c | 434 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 434 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c908135..2fab489 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -119,6 +119,8 @@ #define QEMU_NB_BLKIO_PARAM 2 +#define QEMU_NB_BANDWIDTH_PARAM 6 + static void processWatchdogEvent(void *data, void *opaque); static int qemudShutdown(void); @@ -7846,6 +7848,436 @@ qemudDomainInterfaceStats (virDomainPtr dom, #endif static int +qemuDomainSetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; + int ret = -1; + virDomainNetDefPtr net = NULL; + bool isMac = false; + virNetDevBandwidthPtr bandwidth; + unsigned char mac[VIR_MAC_BUFLEN]; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + } + + if (VIR_ALLOC(bandwidth) < 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC(bandwidth->in) < 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->in, 0, sizeof(*bandwidth->in)); + if (VIR_ALLOC(bandwidth->out) < 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->out, 0, sizeof(*bandwidth->out)); + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->burst = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->burst = params[i].value.ui; + } else { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), + param->field); + goto cleanup; + } + } + + /* average is mandatory, peak and burst is optional. So if no + * average is given, we free inbound/outbound here which causes + * inbound/outbound won't be set. */ + if (!bandwidth->in->average) + VIR_FREE(bandwidth->in); + if (!bandwidth->out->average) + VIR_FREE(bandwidth->out); + + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + ret = 0; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (isMac) { + for (i = 0; i < vm->def->nnets; i++) { + if (memcmp(mac, vm->def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = vm->def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i < vm->def->nnets; i++) { + if (STREQ(device, vm->def->nets[i]->ifname)) { + net = vm->def->nets[i]; + break; + } + } + } + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("cannt find device %s"), + device); + goto cleanup; + } + if (virNetDevBandwidthSet(net->ifname, bandwidth) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + device); + ret = -1; + } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + } + if (ret < 0) + goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (isMac) { + for (i = 0; i < persistentDef->nnets; i++) { + if (memcmp(mac, persistentDef->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = persistentDef->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i < persistentDef->nnets; i++) { + if (STREQ(device, persistentDef->nets[i]->ifname)) { + net = persistentDef->nets[i]; + break; + } + } + } + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + ret = -1; + } + +cleanup: + if (bandwidth) { + VIR_FREE(bandwidth->in); + VIR_FREE(bandwidth->out); + VIR_FREE(bandwidth); + } + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; + virDomainNetDefPtr net = NULL; + bool isMac = false; + unsigned char mac[VIR_MAC_BUFLEN]; + int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + def = vm->def; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + } + + if ((*nparams) == 0) { + *nparams = QEMU_NB_BANDWIDTH_PARAM; + ret = 0; + goto cleanup; + } + + if ((*nparams) < QEMU_NB_BANDWIDTH_PARAM) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + } + + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + if (isMac) { + for (i = 0; i < def->nnets; i++) { + if (memcmp(mac, def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i < def->nnets; i++) { + if (STREQ(device, def->nets[i]->ifname)) { + net = def->nets[i]; + break; + } + } + } + + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + for (i = 0; i < *nparams && i < QEMU_NB_BANDWIDTH_PARAM; i++) { + params[i].value.ui = 0; + params[i].type = VIR_TYPED_PARAM_UINT; + + switch(i) { + case 0: /* inbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->average; + break; + case 1: /* inbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_PEAK); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->peak; + break; + case 2: /* inbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_BURST); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->burst; + break; + case 3: /* outbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->average; + break; + case 4: /* outbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_PEAK); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->peak; + break; + case 5: /* outbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_BURST); + goto cleanup; + } + if (net->bandwidth && net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->burst; + break; + default: + break; + /* should not hit here */ + } + } + + *nparams = QEMU_NB_BANDWIDTH_PARAM; + ret = 0; + +cleanup: + if (group) + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemudDomainMemoryStats (virDomainPtr dom, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -11636,6 +12068,8 @@ static virDriver qemuDriver = { .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ + .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ + .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ }; -- 1.7.4.4

On 2011年12月23日 15:09, Hu Tao wrote:
* src/qemu/qemu_driver.c: implement the qemu driver support --- src/qemu/qemu_driver.c | 434 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 434 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c908135..2fab489 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -119,6 +119,8 @@
#define QEMU_NB_BLKIO_PARAM 2
+#define QEMU_NB_BANDWIDTH_PARAM 6 + static void processWatchdogEvent(void *data, void *opaque);
static int qemudShutdown(void); @@ -7846,6 +7848,436 @@ qemudDomainInterfaceStats (virDomainPtr dom, #endif
static int +qemuDomainSetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; + int ret = -1; + virDomainNetDefPtr net = NULL; + bool isMac = false; + virNetDevBandwidthPtr bandwidth; + unsigned char mac[VIR_MAC_BUFLEN]; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + }
== From
+ + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + } + + if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + }
== To The above block can be shortened with using helper function virDomainLiveConfigHelperMethod. See commit ae52342.
+ + if (VIR_ALLOC(bandwidth)< 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC(bandwidth->in)< 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->in, 0, sizeof(*bandwidth->in)); + if (VIR_ALLOC(bandwidth->out)< 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->out, 0, sizeof(*bandwidth->out)); + + for (i = 0; i< nparams; i++) { + virTypedParameterPtr param =¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->burst = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->burst = params[i].value.ui; + } else { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), + param->field); + goto cleanup; + } + } + + /* average is mandatory, peak and burst is optional. So if no + * average is given, we free inbound/outbound here which causes + * inbound/outbound won't be set. */ + if (!bandwidth->in->average) + VIR_FREE(bandwidth->in); + if (!bandwidth->out->average) + VIR_FREE(bandwidth->out); + + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + ret = 0; + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (isMac) { + for (i = 0; i< vm->def->nnets; i++) { + if (memcmp(mac, vm->def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = vm->def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< vm->def->nnets; i++) { + if (STREQ(device, vm->def->nets[i]->ifname)) { + net = vm->def->nets[i]; + break; + } + } + }
Should we have a helper function to find the net device? accepting both MAC and ifname. Per it's used twice in this function, and once in qemuDomainGetInterfaceParameters. And also it's very likely be used in future.
+ if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("cannt find device %s"), + device); + goto cleanup; + }
And it's better to quit ealier before allocating memory for bandwidth and parsing params if the device can't be found, it's just waste to do those work if the device even doesn't exist.
+ if (virNetDevBandwidthSet(net->ifname, bandwidth)< 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + device); + ret = -1;
Is this intended? to me it's wrong to change the net config below, since it bandwidth setting already failed. "goto cleanup;" makes sense.
+ } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + } + if (ret< 0) + goto cleanup;
Again, failed on setting the bandwidth, but net config is changed.
+ if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (isMac) { + for (i = 0; i< persistentDef->nnets; i++) { + if (memcmp(mac, persistentDef->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = persistentDef->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< persistentDef->nnets; i++) { + if (STREQ(device, persistentDef->nets[i]->ifname)) { + net = persistentDef->nets[i]; + break; + } + } + } + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + + if (virDomainSaveConfig(driver->configDir, persistentDef)< 0) + ret = -1; + } + +cleanup: + if (bandwidth) { + VIR_FREE(bandwidth->in); + VIR_FREE(bandwidth->out); + VIR_FREE(bandwidth); + } + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; + virDomainNetDefPtr net = NULL; + bool isMac = false; + unsigned char mac[VIR_MAC_BUFLEN]; + int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + flags&= ~VIR_TYPED_PARAM_STRING_OKAY; + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + def = vm->def; + } + + if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + }
Likewise,
+ + if ((*nparams) == 0) { + *nparams = QEMU_NB_BANDWIDTH_PARAM; + ret = 0; + goto cleanup; + } + + if ((*nparams)< QEMU_NB_BANDWIDTH_PARAM) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + }
Should we force *nparams >= QEMU_NB_BANDWIDTH_PARAM? IMO it's not neccessary.
+ + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + if (isMac) { + for (i = 0; i< def->nnets; i++) { + if (memcmp(mac, def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< def->nnets; i++) { + if (STREQ(device, def->nets[i]->ifname)) { + net = def->nets[i]; + break; + } + } + } + + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + for (i = 0; i< *nparams&& i< QEMU_NB_BANDWIDTH_PARAM; i++) { + params[i].value.ui = 0; + params[i].type = VIR_TYPED_PARAM_UINT; + + switch(i) { + case 0: /* inbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->average; + break; + case 1: /* inbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_PEAK); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->peak; + break; + case 2: /* inbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_BURST); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->burst; + break; + case 3: /* outbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->average; + break; + case 4: /* outbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_PEAK); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->peak; + break; + case 5: /* outbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_BURST); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->burst; + break; + default: + break; + /* should not hit here */ + } + } + + *nparams = QEMU_NB_BANDWIDTH_PARAM;
If we allow *nprams < QEMU_NB_BANDWIDTH_PARAM, this should be updated.
+ ret = 0; + +cleanup: + if (group) + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemudDomainMemoryStats (virDomainPtr dom, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -11636,6 +12068,8 @@ static virDriver qemuDriver = { .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ + .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ + .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ };
Others looks fine.

On Wed, Dec 28, 2011 at 03:23:39PM +0800, Osier Yang wrote:
On 2011年12月23日 15:09, Hu Tao wrote:
* src/qemu/qemu_driver.c: implement the qemu driver support --- src/qemu/qemu_driver.c | 434 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 434 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c908135..2fab489 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -119,6 +119,8 @@
#define QEMU_NB_BLKIO_PARAM 2
+#define QEMU_NB_BANDWIDTH_PARAM 6 + static void processWatchdogEvent(void *data, void *opaque);
static int qemudShutdown(void); @@ -7846,6 +7848,436 @@ qemudDomainInterfaceStats (virDomainPtr dom, #endif
static int +qemuDomainSetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; + int ret = -1; + virDomainNetDefPtr net = NULL; + bool isMac = false; + virNetDevBandwidthPtr bandwidth; + unsigned char mac[VIR_MAC_BUFLEN]; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + }
== From
+ + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + } + + if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + }
== To
The above block can be shortened with using helper function virDomainLiveConfigHelperMethod. See commit ae52342.
+ + if (VIR_ALLOC(bandwidth)< 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC(bandwidth->in)< 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->in, 0, sizeof(*bandwidth->in)); + if (VIR_ALLOC(bandwidth->out)< 0) { + virReportOOMError(); + goto cleanup; + } + memset(bandwidth->out, 0, sizeof(*bandwidth->out)); + + for (i = 0; i< nparams; i++) { + virTypedParameterPtr param =¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->in->burst = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth average tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->average = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth peak tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->peak = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { + if (param->type != VIR_TYPED_PARAM_UINT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for bandwidth burst tunable, expected a 'unsigned int'")); + goto cleanup; + } + + bandwidth->out->burst = params[i].value.ui; + } else { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), + param->field); + goto cleanup; + } + } + + /* average is mandatory, peak and burst is optional. So if no + * average is given, we free inbound/outbound here which causes + * inbound/outbound won't be set. */ + if (!bandwidth->in->average) + VIR_FREE(bandwidth->in); + if (!bandwidth->out->average) + VIR_FREE(bandwidth->out); + + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + ret = 0; + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (isMac) { + for (i = 0; i< vm->def->nnets; i++) { + if (memcmp(mac, vm->def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = vm->def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< vm->def->nnets; i++) { + if (STREQ(device, vm->def->nets[i]->ifname)) { + net = vm->def->nets[i]; + break; + } + } + }
Should we have a helper function to find the net device? accepting both MAC and ifname. Per it's used twice in this function, and once in qemuDomainGetInterfaceParameters. And also it's very likely be used in future.
+ if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("cannt find device %s"), + device); + goto cleanup; + }
And it's better to quit ealier before allocating memory for bandwidth and parsing params if the device can't be found, it's just waste to do those work if the device even doesn't exist.
+ if (virNetDevBandwidthSet(net->ifname, bandwidth)< 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + device); + ret = -1;
Is this intended? to me it's wrong to change the net config below, since it bandwidth setting already failed. "goto cleanup;" makes sense.
+ } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + } + if (ret< 0) + goto cleanup;
Again, failed on setting the bandwidth, but net config is changed.
+ if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (isMac) { + for (i = 0; i< persistentDef->nnets; i++) { + if (memcmp(mac, persistentDef->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = persistentDef->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< persistentDef->nnets; i++) { + if (STREQ(device, persistentDef->nets[i]->ifname)) { + net = persistentDef->nets[i]; + break; + } + } + } + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + if (!net->bandwidth) { + net->bandwidth = bandwidth; + bandwidth = NULL; + } else { + if (bandwidth->in) { + VIR_FREE(net->bandwidth->in); + net->bandwidth->in = bandwidth->in; + bandwidth->in = NULL; + } + if (bandwidth->out) { + VIR_FREE(net->bandwidth->out); + net->bandwidth->out = bandwidth->out; + bandwidth->out = NULL; + } + } + + if (virDomainSaveConfig(driver->configDir, persistentDef)< 0) + ret = -1; + } + +cleanup: + if (bandwidth) { + VIR_FREE(bandwidth->in); + VIR_FREE(bandwidth->out); + VIR_FREE(bandwidth); + } + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetInterfaceParameters(virDomainPtr dom, + const char *device, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; + virDomainNetDefPtr net = NULL; + bool isMac = false; + unsigned char mac[VIR_MAC_BUFLEN]; + int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + flags&= ~VIR_TYPED_PARAM_STRING_OKAY; + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags& VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + def = vm->def; + } + + if (flags& VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + }
Likewise,
+ + if ((*nparams) == 0) { + *nparams = QEMU_NB_BANDWIDTH_PARAM; + ret = 0; + goto cleanup; + } + + if ((*nparams)< QEMU_NB_BANDWIDTH_PARAM) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + }
Should we force *nparams >= QEMU_NB_BANDWIDTH_PARAM? IMO it's not neccessary.
+ + if (virParseMacAddr(device, mac) == 0) + isMac = true; + + if (isMac) { + for (i = 0; i< def->nnets; i++) { + if (memcmp(mac, def->nets[i]->mac, VIR_MAC_BUFLEN) == 0) { + net = def->nets[i]; + break; + } + } + } else { /* ifname */ + for (i = 0; i< def->nnets; i++) { + if (STREQ(device, def->nets[i]->ifname)) { + net = def->nets[i]; + break; + } + } + } + + if (!net) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't find device %s"), device); + ret = -1; + goto cleanup; + } + + for (i = 0; i< *nparams&& i< QEMU_NB_BANDWIDTH_PARAM; i++) { + params[i].value.ui = 0; + params[i].type = VIR_TYPED_PARAM_UINT; + + switch(i) { + case 0: /* inbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->average; + break; + case 1: /* inbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_PEAK); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->peak; + break; + case 2: /* inbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_IN_BURST); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->in) + params[i].value.ui = net->bandwidth->in->burst; + break; + case 3: /* outbound.average */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->average; + break; + case 4: /* outbound.peak */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_PEAK); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->peak; + break; + case 5: /* outbound.burst */ + if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BANDWIDTH_OUT_BURST); + goto cleanup; + } + if (net->bandwidth&& net->bandwidth->out) + params[i].value.ui = net->bandwidth->out->burst; + break; + default: + break; + /* should not hit here */ + } + } + + *nparams = QEMU_NB_BANDWIDTH_PARAM;
If we allow *nprams < QEMU_NB_BANDWIDTH_PARAM, this should be updated.
+ ret = 0; + +cleanup: + if (group) + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemudDomainMemoryStats (virDomainPtr dom, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -11636,6 +12068,8 @@ static virDriver qemuDriver = { .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ + .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ + .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ };
Others looks fine.
Thanks for your review, I posted a v3 to address the problems you pointed out. -- Thanks, Hu Tao

Add a new command domiftune to get/set interface parameters. * tools/virsh.c: implement the new command * tools/virsh.pod: documentation of the new command --- tools/virsh.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 21 ++++++ 2 files changed, 219 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 02f2e0d..d88fa95 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -427,6 +427,8 @@ static void *_vshCalloc(vshControl *ctl, size_t nmemb, size_t sz, const char *fi static char *_vshStrdup(vshControl *ctl, const char *s, const char *filename, int line); #define vshStrdup(_ctl, _s) _vshStrdup(_ctl, _s, __FILE__, __LINE__) +static int parseRateStr(const char *rateStr, virNetDevBandwidthRatePtr rate); + static void * _vshMalloc(vshControl *ctl, size_t size, const char *filename, int line) { @@ -1601,6 +1603,201 @@ cleanup: return ret; } +/* "domiftune" command + */ +static const vshCmdInfo info_domiftune[] = { + {"help", N_("get link state of a virtual interface")}, + {"desc", N_("Get link state of a domain's virtual interface.")}, + {NULL,NULL} +}; + +static const vshCmdOptDef opts_domiftune[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device")}, + {"inbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's incoming traffics")}, + {"outbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's outgoing traffics")}, + {"config", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect next boot")}, + {"live", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect running domain")}, + {"current", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIftune(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name = NULL, *device = NULL, + *inboundStr = NULL, *outboundStr = NULL; + unsigned int flags = 0; + int nparams = 0; + virTypedParameterPtr params = NULL; + bool ret = false; + int current = vshCommandOptBool(cmd, "current"); + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + virNetDevBandwidthRate inbound, outbound; + int i; + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + } + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + if (vshCommandOptString(cmd, "interface", &device) <= 0) { + virDomainFree(dom); + return false; + } + + if (vshCommandOptString(cmd, "inbound", &inboundStr) < 0 || + vshCommandOptString(cmd, "outbound", &outboundStr) < 0) { + vshError(ctl, "missing argument"); + goto cleanup; + } + + memset(&inbound, 0, sizeof(inbound)); + memset(&outbound, 0, sizeof(outbound)); + + if (inboundStr) { + if (parseRateStr(inboundStr, &inbound) < 0) { + vshError(ctl, _("inbound format is incorrect")); + goto cleanup; + } + if (inbound.average == 0) { + vshError(ctl, _("inbound average is mandatory")); + goto cleanup; + } + nparams++; /* average */ + if (inbound.peak) nparams++; + if (inbound.burst) nparams++; + } + if (outboundStr) { + if (parseRateStr(outboundStr, &outbound) < 0) { + vshError(ctl, _("outbound format is incorrect")); + goto cleanup; + } + if (outbound.average == 0) { + vshError(ctl, _("outbound average is mandatory")); + goto cleanup; + } + nparams++; /* average */ + if (outbound.peak) nparams++; + if (outbound.burst) nparams++; + } + + if (nparams == 0) { + /* get the number of interface parameters */ + if (virDomainGetInterfaceParameters(dom, device, NULL, &nparams, flags) != 0) { + vshError(ctl, "%s", + _("Unable to get number of interface parameters")); + goto cleanup; + } + + if (nparams == 0) { + /* nothing to output */ + ret = true; + goto cleanup; + } + + /* get all interface parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + if (!params) { + virReportOOMError(); + goto cleanup; + } + if (virDomainGetInterfaceParameters(dom, device, params, &nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to get interface parameters")); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + } else { + /* set the interface parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + if (!params) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < nparams; i++) + params[i].type = VIR_TYPED_PARAM_UINT; + + i = 0; + if (inbound.average && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.average; + i++; + } + if (inbound.peak && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.peak; + i++; + } + if (inbound.burst && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.burst; + i++; + } + if (outbound.average && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.average; + i++; + } + if (outbound.peak && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.peak; + i++; + } + if (outbound.burst && i < nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.burst; + i++; + } + + if (virDomainSetInterfaceParameters(dom, device, params, nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to set interface parameters")); + goto cleanup; + } + } + + ret = true; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + virDomainFree(dom); + return ret; +} + /* * "dommemstats" command */ @@ -15589,6 +15786,7 @@ static const vshCmdDef domManagementCmds[] = { info_detach_interface, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, + {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, {"domjobabort", cmdDomjobabort, opts_domjobabort, info_domjobabort, 0}, {"domjobinfo", cmdDomjobinfo, opts_domjobinfo, info_domjobinfo, 0}, {"domname", cmdDomname, opts_domname, info_domname, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 6c10245..d1c5cbe 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -554,6 +554,27 @@ configuration of the domain is modified. Query link state of the domain's virtual interface. If --persistent is specified, query the persistent configuration. +=item B<domiftune> I<domain> I<interface-device> +[[I<--config>] [I<--live>] | [I<--current>]] +[I<--inbound average,peak,burst>] +[I<--outbound average,peak,burst>] + +Set or query the domain's network interface's bandwidth parameters. +I<interface-device> can be the interface name (<target dev='name'/>), +or the mac address if the name is not specified in xml. + +If no I<--inbound> or I<--outbound> is specified, this command will +query and show the bandwidth settings. Otherwise, it will set the +inbound or outbound bandwidth. I<average,peak,burst> is the same as +in command I<attach-interface>. + +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--current> flags may be given, but I<--current> is +exclusive. If no flag is specified, behavior is different depending +on hypervisor. + =item B<dommemstat> I<domain> Get memory stats for a running domain. -- 1.7.4.4

On 2011年12月23日 15:09, Hu Tao wrote:
Add a new command domiftune to get/set interface parameters.
* tools/virsh.c: implement the new command * tools/virsh.pod: documentation of the new command --- tools/virsh.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 21 ++++++ 2 files changed, 219 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 02f2e0d..d88fa95 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -427,6 +427,8 @@ static void *_vshCalloc(vshControl *ctl, size_t nmemb, size_t sz, const char *fi static char *_vshStrdup(vshControl *ctl, const char *s, const char *filename, int line); #define vshStrdup(_ctl, _s) _vshStrdup(_ctl, _s, __FILE__, __LINE__)
+static int parseRateStr(const char *rateStr, virNetDevBandwidthRatePtr rate); + static void * _vshMalloc(vshControl *ctl, size_t size, const char *filename, int line) { @@ -1601,6 +1603,201 @@ cleanup: return ret; }
+/* "domiftune" command + */ +static const vshCmdInfo info_domiftune[] = { + {"help", N_("get link state of a virtual interface")}, + {"desc", N_("Get link state of a domain's virtual interface.")}, + {NULL,NULL} +};
Improper help strings, should be from Copy & Paste. :-)
+ +static const vshCmdOptDef opts_domiftune[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("interface device")}, + {"inbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's incoming traffics")}, + {"outbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's outgoing traffics")}, + {"config", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect next boot")}, + {"live", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect running domain")}, + {"current", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIftune(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name = NULL, *device = NULL, + *inboundStr = NULL, *outboundStr = NULL; + unsigned int flags = 0; + int nparams = 0; + virTypedParameterPtr params = NULL; + bool ret = false; + int current = vshCommandOptBool(cmd, "current"); + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + virNetDevBandwidthRate inbound, outbound; + int i; + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + } + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd,&name))) + return false; + + if (vshCommandOptString(cmd, "interface",&device)<= 0) { + virDomainFree(dom); + return false; + } + + if (vshCommandOptString(cmd, "inbound",&inboundStr)< 0 || + vshCommandOptString(cmd, "outbound",&outboundStr)< 0) { + vshError(ctl, "missing argument"); + goto cleanup; + } + + memset(&inbound, 0, sizeof(inbound)); + memset(&outbound, 0, sizeof(outbound)); + + if (inboundStr) { + if (parseRateStr(inboundStr,&inbound)< 0) { + vshError(ctl, _("inbound format is incorrect")); + goto cleanup; + } + if (inbound.average == 0) { + vshError(ctl, _("inbound average is mandatory")); + goto cleanup; + } + nparams++; /* average */ + if (inbound.peak) nparams++; + if (inbound.burst) nparams++; + } + if (outboundStr) { + if (parseRateStr(outboundStr,&outbound)< 0) { + vshError(ctl, _("outbound format is incorrect")); + goto cleanup; + } + if (outbound.average == 0) { + vshError(ctl, _("outbound average is mandatory")); + goto cleanup; + } + nparams++; /* average */ + if (outbound.peak) nparams++; + if (outbound.burst) nparams++; + } + + if (nparams == 0) { + /* get the number of interface parameters */ + if (virDomainGetInterfaceParameters(dom, device, NULL,&nparams, flags) != 0) { + vshError(ctl, "%s", + _("Unable to get number of interface parameters")); + goto cleanup; + } + + if (nparams == 0) { + /* nothing to output */ + ret = true; + goto cleanup; + } + + /* get all interface parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + if (!params) { + virReportOOMError(); + goto cleanup; + } + if (virDomainGetInterfaceParameters(dom, device, params,&nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to get interface parameters")); + goto cleanup; + } + + for (i = 0; i< nparams; i++) { + char *str = vshGetTypedParamValue(ctl,¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + } else { + /* set the interface parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + if (!params) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i< nparams; i++) + params[i].type = VIR_TYPED_PARAM_UINT; + + i = 0; + if (inbound.average&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.average; + i++; + } + if (inbound.peak&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.peak; + i++; + } + if (inbound.burst&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = inbound.burst; + i++; + } + if (outbound.average&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.average; + i++; + } + if (outbound.peak&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.peak; + i++; + } + if (outbound.burst&& i< nparams) { + if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST, + sizeof(params[i].field))) + goto cleanup; + params[i].value.ui = outbound.burst; + i++; + } + + if (virDomainSetInterfaceParameters(dom, device, params, nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to set interface parameters")); + goto cleanup; + } + } + + ret = true; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + virDomainFree(dom); + return ret; +} + /* * "dommemstats" command */ @@ -15589,6 +15786,7 @@ static const vshCmdDef domManagementCmds[] = { info_detach_interface, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, + {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, {"domjobabort", cmdDomjobabort, opts_domjobabort, info_domjobabort, 0}, {"domjobinfo", cmdDomjobinfo, opts_domjobinfo, info_domjobinfo, 0}, {"domname", cmdDomname, opts_domname, info_domname, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 6c10245..d1c5cbe 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -554,6 +554,27 @@ configuration of the domain is modified. Query link state of the domain's virtual interface. If --persistent is specified, query the persistent configuration.
+=item B<domiftune> I<domain> I<interface-device> +[[I<--config>] [I<--live>] | [I<--current>]] +[I<--inbound average,peak,burst>] +[I<--outbound average,peak,burst>] + +Set or query the domain's network interface's bandwidth parameters. +I<interface-device> can be the interface name (<target dev='name'/>),
It might be better to document as: interface => interface's target name
+or the mac address if the name is not specified in xml.
s/mac/MAC/ "if the name is not specified in xml" should be removed, as it can cause one think MAC address could be only used when the target name is not specified.
+ +If no I<--inbound> or I<--outbound> is specified, this command will +query and show the bandwidth settings. Otherwise, it will set the +inbound or outbound bandwidth. I<average,peak,burst> is the same as +in command I<attach-interface>. + +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--current> flags may be given, but I<--current> is +exclusive. If no flag is specified, behavior is different depending +on hypervisor. + =item B<dommemstat> I<domain>
Get memory stats for a running domain.
ACK with those nits fixed. Regards, Osier

On 2011年12月23日 15:09, Hu Tao wrote:
This series adds a new command domiftune to get/set parameters of domain's network interfaces. Supported parameters are bandwidth settings.
Currently the network interface bandwidth can only be set:
- in domain's xml before the domain is up - when attaching an interface by attach-interface
With this series, users can change network interface bandwidth settings using virsh, whether the domain is running or not.
changes:
- rebase - use vshGetTypedParamValue in patch 5
Hu Tao (5): Add API virDomain{S,G}etInterfaceParameters virDomain{S,G}etInterfaceParameters: the main entry points Add virDomain{S,G}etInterfaceparameters support to the remote driver Add virDomain{S,G}etInterfaceParameters support to qemu driver Enable the virDomain{S,G}etInterfaceParameters in virsh
daemon/remote.c | 64 ++++++ include/libvirt/libvirt.h.in | 50 +++++ python/generator.py | 2 + src/driver.h | 12 ++ src/libvirt.c | 118 ++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 434 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 52 +++++ src/remote/remote_protocol.x | 28 +++- src/remote_protocol-structs | 24 +++ tools/virsh.c | 198 +++++++++++++++++++ tools/virsh.pod | 21 ++ 12 files changed, 1004 insertions(+), 1 deletions(-)
Not neccessary, but adding the support for LXC driver will be better. Regards, Osier
participants (2)
-
Hu Tao
-
Osier Yang