On Mon, Nov 21, 2016 at 06:35:48PM -0500, John Ferlan wrote:
Rather than using negative logic and setting the maxparams to a lesser value based on which capabilities exist, alter the logic to modify the maxparams based on a base value plus the found capabilities. Reduces the chance that some backported feature produces an incorrect value.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d039255..87d219f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -112,9 +112,12 @@ VIR_LOG_INIT("qemu.qemu_driver");
#define QEMU_NB_MEM_PARAM 3
-#define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 -#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX 13 -#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH 19 +#define QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS 6 +#define QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS 7 +#define QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS 6 +#define QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS (QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS + \ + QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS + \ + QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS)
#define QEMU_NB_NUMA_PARAM 2
@@ -17719,7 +17722,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virDomainBlockIoTuneInfo reply; char *device = NULL; int ret = -1; - int maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH; + int maxparams;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -17753,11 +17756,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; }
- if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) - maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; - else if (!virQEMUCapsGet(priv->qemuCaps, - QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) - maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX; + maxparams = QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) + maxparams += QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) + maxparams += QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS; + } else { + maxparams = QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS; }
I'm curious about the else branch. Can you elaborate more on why do you assume qemu supports all the features when you retrieve a persistentdef vs livedef? From what I understand, this is one of the APIs that you have to call twice, since the RPC layer does not allocate a structure of an appropriate size automatically. So with the first run, you say, please allocate a structure of size maxparams equal _ALL_PARAMS (without knowing that qemu actually supports all these features) and rerun. Then during the second run, you run BLOCK_IOTUNE_ASSIGN on all of them, what am I missing? Erik
if (*nparams == 0) { -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list