On 10/12/2011 03:26 AM, Hu Tao wrote: Apologies on the delayed review, but I'm finally getting to this one.

This makes string can be transported between client and server. For compatibility,

o new server should not send strings to old client if it doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY. o new client that wants to be able to send/receive strings should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY; if it is rejected by a old server that doesn't understand VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have a second try with out flag VIR_DOMAIN_TYPED_STRING_OKAY to cope with an old server.

Ideas for compatibility are coming from Eric, thanks. --- daemon/remote.c | 32 +++++++++++++++++++++++++++++--- include/libvirt/libvirt.h.in | 25 ++++++++++++++++++++++++- src/libvirt.c | 38 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 30 ++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 2 ++ src/remote_protocol-structs | 2 ++ src/rpc/gendispatch.pl | 2 +- src/util/util.c | 15 +++++++++++++++ src/util/util.h | 2 ++ 9 files changed, 141 insertions(+), 7 deletions(-)

This patch did not compile for out of the box: libvirt.c: In function 'virDomainSetMemory': libvirt.c:3482:5: error: implicit declaration of function 'virDomainCheckTypedStringFlag' [-Wimplicit-function-declaration] but I think that is how git (mis-)applied the patch based on all the changes that have occurred in the meantime, and the fact that the hunks were contextually ambiguous. Git tried to patch virDomainSetMemoryFlags, which does not use typed parameters, instead of virDomainSetBlkioParameters ;( That just goes to show that a lot has happened in libvirt.c since this patch was proposed.

+++ b/include/libvirt/libvirt.h.in @@ -208,6 +208,27 @@ typedef enum { } virDomainModificationImpact;

/** + * virDomainFlags:

Given that this enum only affects the use of typed parameters, I think it fits slightly better if listed near typed parameters, with a slightly different name.

+ * + * Flags that can be used with some libvirt APIs. + * + * These enums should not confilict with those of virDomainModificationImpact.

s/confilict/conflict/

+ */ +typedef enum { + VIR_DOMAIN_TYPED_STRING_OKAY = 1<< 2, /* Usage of this flag:

virTypedParameters are not domain-specific - it is feasible that we will reuse the type even on the node or network level in the future. I'm dropping _DOMAIN out of the name (and yes, that means I have to rebase a lot of the patch, accordingly - but better to get a good name out of things).

+ * o new server should not send strings to old client if it

mention 0.9.7 as what constitutes new (a year from now, 0.9.7 will feel old :)

+ * doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY. + * o new client that wants to be able to send/receive strings + * should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY; + * if it is rejected by a old server that doesn't understand

s/a old/an old/

+ * VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have + * a second try without flag VIR_DOMAIN_TYPED_STRING_OKAY to + * cope with an old server. + */

I reformatted this, for a better 80 column fit.

+++ b/src/libvirt.c @@ -3447,6 +3447,23 @@ error: return -1; }

+static int virDomainCheckTypedStringFlag(virTypedParameterPtr params, + int nparams, + unsigned int flags)

Again, I went with a more generic name: virTypedParameterStringCheck

+++ b/src/util/util.c @@ -2509,3 +2509,18 @@ or other application using the libvirt API.\n\

return 0; } + +void virTypedParameterFree(virTypedParameterPtr params, int nparams) +{

Needs an export in libvirt_private.syms. And unfortunately, it violates the typical vir*Free convention of taking a single parameter, so it's not free-like, but I don't know how to fix that. Here's what I'm squashing in: diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index 26a3306..38d1dfb 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -200,6 +200,8 @@ typedef virDomainControlInfo *virDomainControlInfoPtr; * current domain state. VIR_DOMAIN_AFFECT_LIVE requires a running * domain, and VIR_DOMAIN_AFFECT_CONFIG requires a persistent domain * (whether or not it is running). + * + * These enums should not conflict with those of virTypedParameterFlags. */ typedef enum { VIR_DOMAIN_AFFECT_CURRENT = 0, /* Affect current domain state. */ @@ -208,27 +210,6 @@ typedef enum { } virDomainModificationImpact; /** - * virDomainFlags: - * - * Flags that can be used with some libvirt APIs. - * - * These enums should not confilict with those of virDomainModificationImpact. - */ -typedef enum { - VIR_DOMAIN_TYPED_STRING_OKAY = 1 << 2, /* Usage of this flag: - * o new server should not send strings to old client if it - * doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY. - * o new client that wants to be able to send/receive strings - * should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY; - * if it is rejected by a old server that doesn't understand - * VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have - * a second try without flag VIR_DOMAIN_TYPED_STRING_OKAY to - * cope with an old server. - */ - -} virDomainFlags; - -/** * virDomainInfoPtr: * * a virDomainInfo is a structure filled by virDomainGetInfo() and extracting @@ -511,10 +492,31 @@ typedef enum { VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ - VIR_TYPED_PARAM_STRING = 7, /* string case */ + VIR_TYPED_PARAM_STRING = 7, /* string case; see virTypedParameterFlags */ } virTypedParameterType; /** + * virTypedParameterFlags: + * + * Flags related to libvirt APIs that use virTypedParameter. + * + * These enums should not conflict with those of virDomainModificationImpact. + */ +typedef enum { + /* Usage of this flag: + * - servers before 0.9.7 do not understand typed strings. + * - servers 0.9.7 and newer will not send or receive strings + * without the flag VIR_TYPED_PARAM_STRING_OKAY. + * - clients that want to be able to send/receive strings should + * first try to set the flag VIR_TYPED_PARAM_STRING_OKAY; if + * it is rejected by an old server, then the client can try + * again without the flag, to get only non-string parameters. + */ + VIR_TYPED_PARAM_STRING_OKAY = 1 << 2, + +} virTypedParameterFlags; + +/** * VIR_TYPED_PARAM_FIELD_LENGTH: * * Macro providing the field length of virTypedParameter name diff --git i/src/libvirt.c w/src/libvirt.c index 6e524c3..379bb20 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -3479,9 +3479,6 @@ virDomainSetMemory(virDomainPtr domain, unsigned long memory) virResetLastError(); - if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) - return -1; - if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3549,9 +3546,6 @@ virDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, virResetLastError(); - if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) - return -1; - if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3585,11 +3579,12 @@ error: return -1; } -static int virDomainCheckTypedStringFlag(virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +virTypedParameterStringCheck(virTypedParameterPtr params, + int nparams, + unsigned int flags) { - if (!(flags & VIR_DOMAIN_TYPED_STRING_OKAY)) { + if (!(flags & VIR_TYPED_PARAM_STRING_OKAY)) { int i; for (i = 0; i < nparams; i++) { if (params[i].type == VIR_TYPED_PARAM_STRING) { @@ -3608,7 +3603,7 @@ static int virDomainCheckTypedStringFlag(virTypedParameterPtr params, * @params: pointer to memory parameter objects * @nparams: number of memory parameter (this value can be the same or * less than the number of parameters supported) - * @flags: bitwise-OR of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Change all or a subset of the memory tunables. * This function may require privileged access to the hypervisor. @@ -3627,7 +3622,7 @@ virDomainSetMemoryParameters(virDomainPtr domain, virResetLastError(); - if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + if (virTypedParameterStringCheck(params, nparams, flags) < 0) return -1; if (!VIR_IS_CONNECTED_DOMAIN(domain)) { @@ -3666,7 +3661,7 @@ error: * @params: pointer to memory parameter object * (return value, allocated by the caller) * @nparams: pointer to number of memory parameters - * @flags: one of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Get all memory parameters, the @params array will be filled with the values * equal to the number of parameters suggested by @nparams @@ -3705,7 +3700,7 @@ virDomainGetMemoryParameters(virDomainPtr domain, virResetLastError(); - if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + if (virTypedParameterStringCheck(params, *nparams, flags) < 0) return -1; if (!VIR_IS_CONNECTED_DOMAIN(domain)) { @@ -3740,7 +3735,7 @@ error: * @params: pointer to blkio parameter objects * @nparams: number of blkio parameters (this value can be the same or * less than the number of parameters supported) - * @flags: an OR'ed set of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Change all or a subset of the blkio tunables. * This function may require privileged access to the hypervisor. @@ -3759,6 +3754,9 @@ virDomainSetBlkioParameters(virDomainPtr domain, virResetLastError(); + if (virTypedParameterStringCheck(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3795,7 +3793,7 @@ error: * @params: pointer to blkio parameter object * (return value, allocated by the caller) * @nparams: pointer to number of blkio parameters - * @flags: an OR'ed set of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Get all blkio parameters, the @params array will be filled with the values * equal to the number of parameters suggested by @nparams. @@ -3818,6 +3816,9 @@ virDomainGetBlkioParameters(virDomainPtr domain, virResetLastError(); + if (virTypedParameterStringCheck(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6336,9 +6337,6 @@ virDomainGetSchedulerType(virDomainPtr domain, int *nparams) virResetLastError(); - if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) - return -1; - if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6388,6 +6386,9 @@ virDomainGetSchedulerParameters(virDomainPtr domain, virResetLastError(); + if (virTypedParameterStringCheck(params, *nparams, 0) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6424,7 +6425,7 @@ error: * @nparams: pointer to number of scheduler parameter * (this value should be same than the returned value * nparams of virDomainGetSchedulerType) - * @flags: one of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Get the scheduler parameters, the @params array will be filled with the * values. @@ -6446,7 +6447,7 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain, virResetLastError(); - if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + if (virTypedParameterStringCheck(params, *nparams, flags) < 0) return -1; if (!VIR_IS_CONNECTED_DOMAIN(domain)) { @@ -6503,6 +6504,9 @@ virDomainSetSchedulerParameters(virDomainPtr domain, virResetLastError(); + if (virTypedParameterStringCheck(params, nparams, 0) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6543,7 +6547,7 @@ error: * @nparams: number of scheduler parameter objects * (this value can be the same or less than the returned value * nparams of virDomainGetSchedulerType) - * @flags: bitwise-OR of virDomainModificationImpact + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags * * Change a subset or all scheduler parameters. The value of @flags * should be either VIR_DOMAIN_AFFECT_CURRENT, or a bitwise-or of @@ -6566,6 +6570,9 @@ virDomainSetSchedulerParametersFlags(virDomainPtr domain, virResetLastError(); + if (virTypedParameterStringCheck(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6636,9 +6643,6 @@ virDomainBlockStats (virDomainPtr dom, const char *path, virResetLastError(); - if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) - return -1; - if (!VIR_IS_CONNECTED_DOMAIN (dom)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6672,7 +6676,7 @@ error: * @params: pointer to block stats parameter object * (return value) * @nparams: pointer to number of block stats - * @flags: unused, always passes 0 + * @flags: bitwise-OR of virTypedParameterFlags * * This function is to get block stats parameters for block * devices attached to the domain. @@ -6710,6 +6714,9 @@ int virDomainBlockStatsFlags (virDomainPtr dom, virResetLastError(); + if (virTypedParameterStringCheck(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); diff --git i/src/util/util.c w/src/util/util.c index 895e6b1..506d355 100644 --- i/src/util/util.c +++ w/src/util/util.c @@ -2571,7 +2571,8 @@ or other application using the libvirt API.\n\ return 0; } -void virTypedParameterFree(virTypedParameterPtr params, int nparams) +void +virTypedParameterFree(virTypedParameterPtr params, int nparams) { int i; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Oct 12, 2011 at 05:26:34PM +0800, Hu Tao wrote:

This makes string can be transported between client and server. For compatibility,

o new server should not send strings to old client if it doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY. o new client that wants to be able to send/receive strings should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY; if it is rejected by a old server that doesn't understand VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have a second try with out flag VIR_DOMAIN_TYPED_STRING_OKAY to cope with an old server.

Ideas for compatibility are coming from Eric, thanks. --- daemon/remote.c | 32 +++++++++++++++++++++++++++++--- include/libvirt/libvirt.h.in | 25 ++++++++++++++++++++++++- src/libvirt.c | 38 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 30 ++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 2 ++ src/remote_protocol-structs | 2 ++ src/rpc/gendispatch.pl | 2 +- src/util/util.c | 15 +++++++++++++++ src/util/util.h | 2 ++ 9 files changed, 141 insertions(+), 7 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c991dfc..fa53147 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -208,6 +208,27 @@ typedef enum { } virDomainModificationImpact;

/** + * virDomainFlags: + * + * Flags that can be used with some libvirt APIs. + * + * These enums should not confilict with those of virDomainModificationImpact. + */ +typedef enum { + VIR_DOMAIN_TYPED_STRING_OKAY = 1 << 2, /* Usage of this flag: + * o new server should not send strings to old client if it + * doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY. + * o new client that wants to be able to send/receive strings + * should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY; + * if it is rejected by a old server that doesn't understand + * VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have + * a second try without flag VIR_DOMAIN_TYPED_STRING_OKAY to + * cope with an old server. + */ + +} virDomainFlags;

I'm not really a fan of this - it is exposing details of the RPC implementation to the applications. IMHO we should be using the internal RPC features capability, and the VIR_DRV_SUPPORTS_FEATURE to detect support internally, and then return 'VIR_ERR_NO_SUPPORT' if string parameters are not supported.

diff --git a/src/libvirt.c b/src/libvirt.c index f07c720..59d6d26 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3472,6 +3489,9 @@ virDomainSetMemoryParameters(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3547,6 +3567,9 @@ virDomainGetMemoryParameters(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3598,6 +3621,9 @@ virDomainSetBlkioParameters(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3657,6 +3683,9 @@ virDomainGetBlkioParameters(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6279,6 +6308,9 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6396,6 +6428,9 @@ virDomainSetSchedulerParametersFlags(virDomainPtr domain,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6537,6 +6572,9 @@ int virDomainBlockStatsFlags (virDomainPtr dom,

virResetLastError();

+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL);

So IMHO all these should be VIR_DRV_SUPPORTS_FEATURE(VIR_DRV_FEATURE_TYPED_STRING) and if the app gets back an error, it can re-try without the string parameter.

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f95253e..994c339 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -317,6 +317,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s; };

struct remote_typed_param {

I'm kind of suprised this actually works, but I presume you've tested old client with new server, and vica-verca so I guess its OK. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This patch adds a parameter --weight-device to virsh command blkiotune for setting/getting blkio.weight_device.

--- daemon/remote.c | 2 +- include/libvirt/libvirt.h.in | 9 ++ src/conf/domain_conf.c | 129 ++++++++++++++++++++++++++- src/conf/domain_conf.h | 16 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c | 22 +++++ src/qemu/qemu_driver.c | 199 +++++++++++++++++++++++++++++++++++++++-- src/util/cgroup.c | 33 +++++++ src/util/cgroup.h | 3 + tools/virsh.c | 69 +++++++++++++-- tools/virsh.pod | 5 +- 11 files changed, 467 insertions(+), 22 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c index 520fef2..7691b08 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1619,7 +1619,7 @@ success: cleanup: if (rv< 0) virNetMessageSaveError(rerr); - VIR_FREE(params); + virTypedParameterFree(params, nparams); if (dom) virDomainFree(dom); return rv; diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index fa53147..84178c2 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1162,6 +1162,15 @@ char * virDomainGetSchedulerType(virDomainPtr domain,

#define VIR_DOMAIN_BLKIO_WEIGHT "weight"

+/** + * VIR_DOMAIN_BLKIO_WEIGHT_DEVICE: + * + * Macro for the blkio tunable weight_device: it represents the + * per-device weight. This name refers to a VIR_TYPED_PARAM_STRING. + */ + +#define VIR_DOMAIN_BLKIO_WEIGHT_DEVICE "weight_device" + /* Set Blkio tunables for the domain*/ int virDomainSetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b9ddf26..96723ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -580,6 +580,97 @@ VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST, #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE

+ +void virBlkioWeightDeviceFree(virBlkioWeightDevicePtr deviceWeights, + int ndevices) +{ + int i; + + for (i = 0; i< ndevices; i++) + VIR_FREE(deviceWeights[i].path); + VIR_FREE(deviceWeights); +} + +/** + * virBlkioWeightDeviceToStr: + * + * This function returns a string representing device weights that is + * suitable for writing to /cgroup/blkio/blkio.weight_device, given + * a list of weight devices. + */ +#if defined(major)&& defined(minor) +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr weightdevices, + int ndevices, + char **result) +{ + int i; + struct stat s; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + for (i = 0; i< ndevices; i++) { + if (stat(weightdevices[i].path,&s) == -1) + return -1; + if ((s.st_mode& S_IFMT) != S_IFBLK) + return -1; + virBufferAsprintf(&buf, "%d:%d %d\n", + major(s.st_rdev), + minor(s.st_rdev), + weightdevices[i].weight); + } + + *result = virBufferContentAndReset(&buf); + return 0; +} +#else +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr weightdevices ATTRIBUTE_UNUSED, + int ndevices ATTRIBUTE_UNUSED, + char **result ATTRIBUTE_UNUSED) +{ + return -1; +} +#endif + +/** + * virDomainBlkioWeightDeviceParseXML + * + * this function parses a XML node: + * + *<device> + *<path>/fully/qualified/device/path</path> + *<weight>weight</weight> + *</device> + * + * and fills a virBlkioWeightDevice struct. + */ +static int virDomainBlkioWeightDeviceParseXML(xmlNodePtr root, + virBlkioWeightDevicePtr dw) +{ + char *c; + xmlNodePtr node; + + if (!dw) + return -1; + + node = root->children; + while (node) { + if (node->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(node->name, BAD_CAST "path")) { + dw->path = (char *)xmlNodeGetContent(node); + } else if (xmlStrEqual(node->name, BAD_CAST "weight")) { + c = (char *)xmlNodeGetContent(node); + if (virStrToLong_i(c, NULL, 10,&dw->weight)< 0) + return -1; + VIR_FREE(c); + } + } + node = node->next; + } + + return 0; +} + + + static void virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { @@ -1244,6 +1335,8 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->emulator); VIR_FREE(def->description);

+ virBlkioWeightDeviceFree(def->blkio.weight_devices, def->blkio.ndevices); + virDomainWatchdogDefFree(def->watchdog);

virDomainMemballoonDefFree(def->memballoon); @@ -6520,6 +6613,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->blkio.weight)< 0) def->blkio.weight = 0;

+ n = virXPathNodeSet("./blkiotune/device", ctxt,&nodes); + if (n> 0) { + if (VIR_ALLOC_N(def->blkio.weight_devices, n)< 0) { + virReportOOMError(); + goto error; + } + + for (i = 0; i< n; i++) { + virDomainBlkioWeightDeviceParseXML(nodes[i],&def->blkio.weight_devices[i]); + } + def->blkio.ndevices = n; + VIR_FREE(nodes); + } + /* Extract other memory tunables */ if (virXPathULong("string(./memtune/hard_limit)", ctxt, &def->mem.hard_limit)< 0) @@ -10572,10 +10679,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->mem.cur_balloon);

/* add blkiotune only if there are any */ - if (def->blkio.weight) { + if (def->blkio.weight || def->blkio.weight_devices) { virBufferAsprintf(buf, "<blkiotune>\n"); - virBufferAsprintf(buf, "<weight>%u</weight>\n", - def->blkio.weight); + + if (def->blkio.weight) + virBufferAsprintf(buf, "<weight>%u</weight>\n", + def->blkio.weight); + + if (def->blkio.weight_devices) { + int i; + + for (i = 0; i< def->blkio.ndevices; i++) { + virBufferAsprintf(buf, "<device>\n"); + virBufferAsprintf(buf, "<path>%s</path>\n", + def->blkio.weight_devices[i].path); + virBufferAsprintf(buf, "<weight>%d</weight>\n", + def->blkio.weight_devices[i].weight); + virBufferAsprintf(buf, "</device>\n"); + } + } + virBufferAsprintf(buf, "</blkiotune>\n"); }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8765f32..140369c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1297,6 +1297,20 @@ struct _virDomainNumatuneDef { /* Future NUMA tuning related stuff should go here. */ };

+typedef struct _virBlkioWeightDevice virBlkioWeightDevice; +typedef virBlkioWeightDevice *virBlkioWeightDevicePtr; +struct _virBlkioWeightDevice { + char *path; + int weight; +}; + +void virBlkioWeightDeviceFree(virBlkioWeightDevicePtr deviceWeights, + int ndevices); +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr deviceWeights, + int ndevices, + char **result); + + /* * Guest VM main configuration * @@ -1314,6 +1328,8 @@ struct _virDomainDef {

struct { unsigned int weight; + int ndevices; + virBlkioWeightDevicePtr weight_devices; } blkio;

struct { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d79f65c..00099c0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -227,6 +227,8 @@ virDomainAuditVcpu;

# domain_conf.h +virBlkioWeightDeviceFree; +virBlkioWeightDeviceToStr; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2a10bd2..ff13965 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -312,6 +312,28 @@ int qemuSetupCgroup(struct qemud_driver *driver, } }

+ if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + char *tmp; + if (virBlkioWeightDeviceToStr(vm->def->blkio.weight_devices, + vm->def->blkio.ndevices, +&tmp)< 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set io device weight for domain %s"), + vm->def->name); + goto cleanup; + } + if (tmp) { + rc = virCgroupSetBlkioWeightDevice(cgroup, tmp); + VIR_FREE(tmp); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io device weight for domain %s"), + vm->def->name); + goto cleanup; + } + } + } + if (vm->def->mem.hard_limit != 0 || vm->def->mem.soft_limit != 0 || vm->def->mem.swap_hard_limit != 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ea3236..179ecb4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -89,6 +89,7 @@ #include "locking/lock_manager.h" #include "locking/domain_lock.h" #include "virkeycode.h" +#include "c-ctype.h"

#define VIR_FROM_THIS VIR_FROM_QEMU

@@ -112,7 +113,7 @@ # define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ #endif

-#define QEMU_NB_BLKIO_PARAM 1 +#define QEMU_NB_BLKIO_PARAM 2

static void processWatchdogEvent(void *data, void *opaque);

@@ -5847,6 +5848,73 @@ cleanup: return ret; }

+/* weightDeviceStr in the form of /device/path,weight;/device/path,weight + * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 + */ +static int parseBlkioWeightDeviceStr(const char *weightDeviceStr, virBlkioWeightDevicePtr *dw, int *size) +{ + char *temp; + int nDevice = 0; + int i; + virBlkioWeightDevicePtr result = NULL; + + temp = (char *)weightDeviceStr; + while (temp) { + temp = strchr(temp, ';'); + if (temp) { + temp++; + if (*temp == '\0') + break; + nDevice++; + } + } + nDevice++; + + if (VIR_ALLOC_N(result, nDevice)< 0) { + virReportOOMError(); + return -1; + } + + i = 0; + temp = (char *)weightDeviceStr; + while (temp&& i< nDevice) { + char *p = temp; + + /* device path */ + + p = strchr(p, ','); + if (!p) + goto fail; + + result[i].path = strndup(temp, p - temp); + + /* weight */ + + temp = p + 1; + if (!temp) + goto fail; + + if (virStrToLong_i(temp,&p, 10,&result[i].weight)< 0) + goto fail; + + i++; + if (*p == '\0') + break; + else if (*p != ';') + goto fail; + temp = p + 1; + } + + *dw = result; + *size = i; + + return 0; + +fail: + VIR_FREE(result); + return -1; +} + static int qemuDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -5859,9 +5927,11 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; int ret = -1; bool isActive; + bool typed_string;

virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_TYPED_STRING_OKAY, -1); qemuDriverLock(driver);

vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -5874,6 +5944,8 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,

isActive = virDomainObjIsActive(vm);

+ typed_string = (flags& VIR_DOMAIN_TYPED_STRING_OKAY) == VIR_DOMAIN_TYPED_STRING_OKAY; + flags&= ~VIR_DOMAIN_TYPED_STRING_OKAY; if (flags == VIR_DOMAIN_AFFECT_CURRENT) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE; @@ -5913,10 +5985,10 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (flags& VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i< nparams; i++) { + int rc; virTypedParameterPtr param =¶ms[i];

if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - int rc; if (param->type != VIR_TYPED_PARAM_UINT) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid type for blkio weight tunable, expected a 'unsigned int'")); @@ -5937,6 +6009,46 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, _("unable to set blkio weight tunable")); ret = -1; } + } else if(STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE)) { + int ndevices; + virBlkioWeightDevicePtr weightDevices = NULL; + char *tmp; + if (param->type != VIR_TYPED_PARAM_STRING) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for blkio weight_device tunable, expected a 'char *'")); + ret = -1; + continue; + } + + if (parseBlkioWeightDeviceStr(params[i].value.s, +&weightDevices, +&ndevices)< 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid format for blkio weight_device")); + ret = -1; + continue; + } + if (virBlkioWeightDeviceToStr(weightDevices, + ndevices, +&tmp)< 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set blkio weight_device tunable")); + virBlkioWeightDeviceFree(weightDevices, ndevices); + ret = -1; + continue; + } + virBlkioWeightDeviceFree(weightDevices, ndevices); + VIR_FREE(weightDevices); + if (tmp) { + rc = virCgroupSetBlkioWeightDevice(group, tmp); + VIR_FREE(tmp); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set blkio weight_device tunable")); + ret = -1; + continue; + } + } } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); @@ -5966,6 +6078,21 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, }

persistentDef->blkio.weight = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE)) { + virBlkioWeightDevicePtr tmp = NULL; + int ndevices; + if (parseBlkioWeightDeviceStr(params[i].value.s, +&tmp, +&ndevices)< 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid device weight format: %s"), + params[i].value.s); + ret = -1; + continue; + } + virBlkioWeightDeviceFree(persistentDef->blkio.weight_devices, persistentDef->blkio.ndevices); + persistentDef->blkio.weight_devices = tmp; + persistentDef->blkio.ndevices = ndevices; } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); @@ -5991,7 +6118,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; - int i; + int i, j; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; @@ -5999,9 +6126,11 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, int ret = -1; int rc; bool isActive; + bool typed_string;

virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_TYPED_STRING_OKAY, -1); qemuDriverLock(driver);

vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6014,19 +6143,32 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,

if ((*nparams) == 0) { /* Current number of blkio parameters supported by cgroups */ - *nparams = QEMU_NB_BLKIO_PARAM; + if (flags& VIR_DOMAIN_TYPED_STRING_OKAY) + *nparams = QEMU_NB_BLKIO_PARAM; + else + *nparams = QEMU_NB_BLKIO_PARAM - 1; ret = 0; goto cleanup; }

- if ((*nparams) != QEMU_NB_BLKIO_PARAM) { - qemuReportError(VIR_ERR_INVALID_ARG, - "%s", _("Invalid parameter count")); - goto cleanup; + if (flags& VIR_DOMAIN_TYPED_STRING_OKAY) { + if ((*nparams) != QEMU_NB_BLKIO_PARAM) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + } + } else { + if ((*nparams) != QEMU_NB_BLKIO_PARAM - 1) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + } }

isActive = virDomainObjIsActive(vm);

+ typed_string = (flags& VIR_DOMAIN_TYPED_STRING_OKAY) == VIR_DOMAIN_TYPED_STRING_OKAY; + flags&= ~VIR_DOMAIN_TYPED_STRING_OKAY; if (flags == VIR_DOMAIN_AFFECT_CURRENT) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE; @@ -6065,6 +6207,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,

if (flags& VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i< *nparams; i++) { + char *weight_device; virTypedParameterPtr param =¶ms[i]; val = 0; param->value.ui = 0; @@ -6085,6 +6228,23 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, } param->value.ui = val; break; + case 1: /* blkio.weight_device */ + if (typed_string) { + param->type = VIR_TYPED_PARAM_STRING; + rc = virCgroupGetBlkioWeightDevice(group,&weight_device); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get blkio weight_device")); + goto cleanup; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field blkio weight_device too long for destination")); + goto cleanup; + } + param->value.s = weight_device; + } + break;

default: break; @@ -6108,6 +6268,25 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, param->value.ui = persistentDef->blkio.weight; break;

+ case 1: /* blkio.weight_device */ + if (typed_string&& persistentDef->blkio.ndevices> 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (j = 0; j< persistentDef->blkio.ndevices; j++) { + virBufferAsprintf(&buf, "%s %d\n", + persistentDef->blkio.weight_devices[j].path, + persistentDef->blkio.weight_devices[j].weight); + } + + param->value.s = virBufferContentAndReset(&buf); + param->type = VIR_TYPED_PARAM_STRING; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field blkio weight_device too long for destination")); + goto cleanup; + } + break; + default: break; /* should not hit here */ diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c8d1f33..f6e5a08 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -982,6 +982,39 @@ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) }

/** + * virCgroupSetBlkioWeightDevice: + * + * @group: The cgroup to change io weight device for + * @weight_device: The Weight device for this cgroup + * + * Returns: 0 on success + */ +int virCgroupSetBlkioWeightDevice(virCgroupPtr group, + const char *weight_device) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight_device", + weight_device); +} + +/** + * virCgroupGetBlkioWeightDevice: + * + * @group: The cgroup to get weight_device for + * @weight_device: returned weight_device string + * + * Returns: 0 on success + */ +int virCgroupGetBlkioWeightDevice(virCgroupPtr group, + char **weight_device) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight_device", weight_device); +} + +/** * virCgroupSetMemory: * * @group: The cgroup to change memory for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index d190bb3..87e196b 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -55,6 +55,9 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);

+int virCgroupSetBlkioWeightDevice(virCgroupPtr group, const char *weight_device); +int virCgroupGetBlkioWeightDevice(virCgroupPtr group, char **weight_device); + int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb);

diff --git a/tools/virsh.c b/tools/virsh.c index 067d3e5..c85f5b6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4644,6 +4644,8 @@ static const vshCmdOptDef opts_blkiotune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"weight", VSH_OT_INT, VSH_OFLAG_NONE, N_("IO Weight in range [100, 1000]")}, + {"weight-device", VSH_OT_STRING, VSH_OFLAG_NONE, + N_("per-device IO Weight, in the form of /path/to/device,weight")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -4654,6 +4656,7 @@ static bool cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; + const char *weight_device = NULL; int weight = 0; int nparams = 0; int rv = 0; @@ -4698,12 +4701,36 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } }

+ rv = vshCommandOptString(cmd, "weight-device",&weight_device); + if (rv< 0) { + vshError(ctl, "%s", + _("Unable to parse string parameter")); + goto cleanup; + } + if (rv> 0) { + nparams++; + } + if (nparams == 0) { /* get the number of blkio parameters */ + /* old libvirtd doesn't understand VIR_DOMAIN_TYPED_STRING_OKAY, we + * give it a second try with this flag disabled in the case of an + * old libvirtd. */ + flags |= VIR_DOMAIN_TYPED_STRING_OKAY; if (virDomainGetBlkioParameters(dom, NULL,&nparams, flags) != 0) { - vshError(ctl, "%s", - _("Unable to get number of blkio parameters")); - goto cleanup; + if (last_error->code == VIR_ERR_INVALID_ARG) { + flags&= ~VIR_DOMAIN_TYPED_STRING_OKAY; + nparams = 0; + if (virDomainGetBlkioParameters(dom, NULL,&nparams, flags) != 0) { + vshError(ctl, "%s", + _("Unable to get number of blkio parameters")); + goto cleanup; + } + } else { + vshError(ctl, "%s", + _("Unable to get number of blkio parameters")); + goto cleanup; + } }

if (nparams == 0) { @@ -4745,6 +4772,10 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.b); break; + case VIR_TYPED_PARAM_STRING: + vshPrint(ctl, "%-15s: %s\n", params[i].field, + params[i].value.s); + break; default: vshPrint(ctl, "unimplemented blkio parameter type\n"); } @@ -4765,11 +4796,35 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) sizeof(temp->field)); weight = 0; } + + if (weight_device) { + temp->value.s = (char *)weight_device; + temp->type = VIR_TYPED_PARAM_STRING; + strncpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE, + sizeof(temp->field)); + } + } + ret = true; + if (weight_device) { + flags |= VIR_DOMAIN_TYPED_STRING_OKAY; + if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0) { + if (last_error->code == VIR_ERR_INVALID_ARG) { + flags&= ~VIR_DOMAIN_TYPED_STRING_OKAY; + if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to change blkio parameters")); + ret = false; + } + } else { + vshError(ctl, "%s", _("Unable to change blkio parameters")); + ret = false; + } + } + } else { + if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to change blkio parameters")); + ret = false; + } } - if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0) - vshError(ctl, "%s", _("Unable to change blkio parameters")); - else - ret = true; }

cleanup: What happened if use blkiotune --weight-device first, then setting<path>,<weight> of the device in domain.xml? I found

On 10/12/2011 05:26 PM, Hu Tao wrote: the weight number did not change after I restart the guest, did I do something wrong?

diff --git a/tools/virsh.pod b/tools/virsh.pod index 7712db4..8c29759 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1030,12 +1030,15 @@ value are kilobytes (i.e. blocks of 1024 bytes).

Specifying -1 as a value for these limits is interpreted as unlimited.

-=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] [[I<--config>] +=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] +[I<--weight-device> B<weight-device>] [[I<--config>] [I<--live>] | [I<--current>]]

Display or set the blkio parameters. QEMU/KVM supports I<--weight>. I<--weight> is in range [100, 1000].

+B<weight-device> is in the format of /device/path,weight;/device/path,weight. + 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.

-- Lei