[libvirt] [PATCH 0/2] Increase bound limit for virDomainGetBlockIoTune
Related to my onlist (and ACK'd) series for adding 6 new parameters for adding length (duration) parameters to/for iotune throttling: http://www.redhat.com/archives/libvir-list/2016-September/msg01090.html As described in a response to the cover: http://www.redhat.com/archives/libvir-list/2016-October/msg00053.html for some reason I had a brain cramp and neglected to check the virsh blkdeviotune command. These patches could then be applied before the other (already ACK'd) series once the current release is out. In reality they just need to go in some time before patch 8 which adds the 6 new parameters to qemuMonitorGetBlockIoThrottle. A followup patch to that series will enable setting the new parameters (IOW: changes to virsh-domain.c cmdBlkdeviotune(). John Ferlan (2): remote: Fix erroneous usage of constant remote: Increase bound limit for virDomainGetBlockIoTune src/remote/remote_driver.c | 4 ++-- src/remote/remote_protocol.x | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.7.4
The REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX was erroneously used in the remoteDomainBlockStatsFlags and remoteDomainGetBlockIoTune calls. Change the constant to be the right one. Fortunately, all 3 are defined as 16. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/remote/remote_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 3b8b796..f6c6940 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1762,7 +1762,7 @@ remoteDomainBlockStatsFlags(virDomainPtr domain, /* Deserialize the result. */ if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, ret.params.params_len, - REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX, ¶ms, nparams) < 0) goto cleanup; @@ -2869,7 +2869,7 @@ static int remoteDomainGetBlockIoTune(virDomainPtr domain, if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, ret.params.params_len, - REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX, ¶ms, nparams) < 0) goto cleanup; -- 2.7.4
We are about to add 6 new values to fetch. This will put us over the current limit of 16 (we're at 13 now). Once there are more than 16 parameters, this will affect existing clients that attempt to fetch blockiotune config values for the domain from the remote host since the server side has no mechanism to determine whether the capability for the emulator exists and thus would attempt to return all known values from the persistentDef. If attempting to fetch the blockiotune values from a running domain, the code will check the emulator capabilities and set maxparams (in qemuDomainGetBlockIoTune) appropriately. On the client side of the remote connection, it uses this constant in xdr_remote_domain_get_block_io_tune_ret and virTypedParamsDeserialize calls, so if a remote server returns more than 16 parameters, then the client will fail with "Unable to decode message payload". Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/remote/remote_protocol.x | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8a8e263..e8382dc 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -104,7 +104,7 @@ const REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX = 16; const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16; /* Upper limit on list of blockio tuning parameters. */ -const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16; +const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 32; /* Upper limit on list of numa parameters. */ const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16; -- 2.7.4
On 04.10.2016 15:22, John Ferlan wrote:
Related to my onlist (and ACK'd) series for adding 6 new parameters for adding length (duration) parameters to/for iotune throttling:
http://www.redhat.com/archives/libvir-list/2016-September/msg01090.html
As described in a response to the cover:
http://www.redhat.com/archives/libvir-list/2016-October/msg00053.html
for some reason I had a brain cramp and neglected to check the virsh blkdeviotune command. These patches could then be applied before the other (already ACK'd) series once the current release is out.
Agreed. this is not something that needs fixing in this release.
In reality they just need to go in some time before patch 8 which adds the 6 new parameters to qemuMonitorGetBlockIoThrottle. A followup patch to that series will enable setting the new parameters (IOW: changes to virsh-domain.c cmdBlkdeviotune().
John Ferlan (2): remote: Fix erroneous usage of constant remote: Increase bound limit for virDomainGetBlockIoTune
src/remote/remote_driver.c | 4 ++-- src/remote/remote_protocol.x | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
ACK series, I think that raising the limits has already proven to be safe over the time. Michal
participants (2)
-
John Ferlan -
Michal Privoznik