[libvirt] [PATCH 2/2] qemu: allow getting < max typed parameters
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Nov 1 10:47:06 UTC 2011
On 10/31/2011 07:33 PM, Eric Blake wrote:
> Since all virTypedParameter APIs allow us to return the number
> of slots we actually populated, we should allow the user to
> call with nparams too small (without overrunning their array)
> or too large (ignoring the tail of the array that we can't fill),
> rather than requiring that they get things exactly right.
>
> * src/qemu/qemu_driver.c (qemuDomainGetBlkioParameters)
> (qemuDomainGetMemoryParameters): Allow variable nparams on entry.
> (qemuGetSchedulerParametersFlags): Drop redundant check.
> (qemudDomainBlockStats, qemudDomainBlockStatsFlags): Rename...
> (qemuDomainBlockStats, qemuDomainBlockStatsFlags): ...to this.
> Don't return unavailable stats.
> ---
>
> Making this change will make it easier to introduce VIR_TYPED_PARAM_STRING,
> with libvirt.c doing the filtering to further strip off string arguments,
> rather than having to teach every driver to honor a new flag.
>
> src/qemu/qemu_driver.c | 231 ++++++++++++++++++++++++------------------------
> 1 files changed, 116 insertions(+), 115 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 02cbf2d..d70ab7a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6059,12 +6059,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
> goto cleanup;
> }
>
> - if ((*nparams) != QEMU_NB_BLKIO_PARAM) {
> - qemuReportError(VIR_ERR_INVALID_ARG,
> - "%s", _("Invalid parameter count"));
> - goto cleanup;
> - }
> -
> isActive = virDomainObjIsActive(vm);
>
> if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
> @@ -6104,7 +6098,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
> }
>
> if (flags& VIR_DOMAIN_AFFECT_LIVE) {
> - for (i = 0; i< *nparams; i++) {
> + for (i = 0; i< *nparams&& i< QEMU_NB_BLKIO_PARAM; i++) {
> virTypedParameterPtr param =¶ms[i];
> val = 0;
> param->value.ui = 0;
> @@ -6132,7 +6126,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
> }
> }
> } else if (flags& VIR_DOMAIN_AFFECT_CONFIG) {
> - for (i = 0; i< *nparams; i++) {
> + for (i = 0; i< *nparams&& i< QEMU_NB_BLKIO_PARAM; i++) {
> virTypedParameterPtr param =¶ms[i];
> val = 0;
> param->value.ui = 0;
> @@ -6155,6 +6149,8 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
> }
> }
>
> + if (QEMU_NB_BLKIO_PARAM< *nparams)
> + *nparams = QEMU_NB_BLKIO_PARAM;
> ret = 0;
>
> cleanup:
> @@ -6395,14 +6391,8 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom,
> goto cleanup;
> }
>
> - if ((*nparams)< QEMU_NB_MEM_PARAM) {
> - qemuReportError(VIR_ERR_INVALID_ARG,
> - "%s", _("Invalid parameter count"));
> - goto cleanup;
> - }
> -
> if (flags& VIR_DOMAIN_AFFECT_CONFIG) {
> - for (i = 0; i< *nparams; i++) {
> + for (i = 0; i< *nparams&& i< QEMU_NB_MEM_PARAM; i++) {
> virMemoryParameterPtr param =¶ms[i];
> val = 0;
> param->value.ul = 0;
> @@ -6444,7 +6434,7 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom,
> goto out;
> }
>
> - for (i = 0; i< QEMU_NB_MEM_PARAM; i++) {
> + for (i = 0; i< *nparams&& i< QEMU_NB_MEM_PARAM; i++) {
> virTypedParameterPtr param =¶ms[i];
> val = 0;
> param->value.ul = 0;
> @@ -6506,7 +6496,8 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom,
> }
>
> out:
> - *nparams = QEMU_NB_MEM_PARAM;
> + if (QEMU_NB_MEM_PARAM< *nparams)
> + *nparams = QEMU_NB_MEM_PARAM;
> ret = 0;
>
> cleanup:
> @@ -6894,12 +6885,6 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom,
> goto cleanup;
> }
>
> - if (*nparams< 1) {
> - qemuReportError(VIR_ERR_INVALID_ARG,
> - "%s", _("Invalid parameter count"));
> - goto cleanup;
> - }
> -
> if (*nparams> 1) {
> rc = qemuGetCpuBWStatus(driver->cgroup);
> if (rc< 0)
> @@ -7048,9 +7033,9 @@ qemuGetSchedulerParameters(virDomainPtr dom,
> * not supported we detect this and return the appropriate error.
> */
> static int
> -qemudDomainBlockStats (virDomainPtr dom,
> - const char *path,
> - struct _virDomainBlockStats *stats)
> +qemuDomainBlockStats(virDomainPtr dom,
> + const char *path,
> + struct _virDomainBlockStats *stats)
> {
> struct qemud_driver *driver = dom->conn->privateData;
> int i, ret = -1;
> @@ -7129,11 +7114,11 @@ cleanup:
> }
>
> static int
> -qemudDomainBlockStatsFlags (virDomainPtr dom,
> - const char *path,
> - virTypedParameterPtr params,
> - int *nparams,
> - unsigned int flags)
> +qemuDomainBlockStatsFlags(virDomainPtr dom,
> + const char *path,
> + virTypedParameterPtr params,
> + int *nparams,
> + unsigned int flags)
> {
> struct qemud_driver *driver = dom->conn->privateData;
> int i, tmp, ret = -1;
> @@ -7142,6 +7127,7 @@ qemudDomainBlockStatsFlags (virDomainPtr dom,
> qemuDomainObjPrivatePtr priv;
> long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times;
> long long wr_total_times, flush_req, flush_total_times, errs;
> + virTypedParameterPtr param;
>
> virCheckFlags(0, -1);
>
> @@ -7178,7 +7164,8 @@ qemudDomainBlockStatsFlags (virDomainPtr dom,
>
> if (!disk->info.alias) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - _("missing disk device alias name for %s"), disk->dst);
> + _("missing disk device alias name for %s"),
> + disk->dst);
> goto cleanup;
> }
> }
> @@ -7199,7 +7186,7 @@ qemudDomainBlockStatsFlags (virDomainPtr dom,
> tmp = *nparams;
> ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams);
>
> - if (tmp == 0) {
> + if (tmp == 0 || ret< 0) {
> qemuDomainObjExitMonitor(driver, vm);
> goto endjob;
> }
> @@ -7221,97 +7208,111 @@ qemudDomainBlockStatsFlags (virDomainPtr dom,
> if (ret< 0)
> goto endjob;
>
> - /* Field 'errs' is meaningless for QEMU, won't set it. */
> - for (i = 0; i< *nparams; i++) {
> - virTypedParameterPtr param =¶ms[i];
> -
> - switch (i) {
> - case 0: /* fill write_bytes here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field write bytes too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = wr_bytes;
> - break;
> + tmp = 0;
> + ret = -1;
>
> - case 1: /* fill wr_operations here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field write requests too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = wr_req;
> - break;
> + if (tmp< *nparams&& wr_bytes != -1) {
> + param =¶ms[tmp];
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field write bytes too long for destination"));
Existing but odd error message... "Field 'write bytes' is too long for
..." (?)
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = wr_bytes;
> + tmp++;
> + }
>
> - case 2: /* fill read_bytes here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field read bytes too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = rd_bytes;
> - break;
> + if (tmp< *nparams&& wr_req != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field write requests too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = wr_req;
> + tmp++;
> + }
>
> - case 3: /* fill rd_operations here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_REQ) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field read requests too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = rd_req;
> - break;
> + if (tmp< *nparams&& rd_bytes != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field read bytes too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = rd_bytes;
> + tmp++;
> + }
>
> - case 4: /* fill flush_operations here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field flush requests too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = flush_req;
> - break;
> + if (tmp< *nparams&& rd_req != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_READ_REQ) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field read requests too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = rd_req;
> + tmp++;
> + }
>
> - case 5: /* fill wr_total_times_ns here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field write total times too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = wr_total_times;
> - break;
> + if (tmp< *nparams&& flush_req != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field flush requests too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = flush_req;
> + tmp++;
> + }
>
> - case 6: /* fill rd_total_times_ns here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field read total times too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = rd_total_times;
> - break;
> + if (tmp< *nparams&& wr_total_times != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field write total times too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = wr_total_times;
> + tmp++;
> + }
>
> - case 7: /* fill flush_total_times_ns here */
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) {
> - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field flush total times too long for destination"));
> - goto endjob;
> - }
> - param->type = VIR_TYPED_PARAM_LLONG;
> - param->value.l = flush_total_times;
> - break;
> + if (tmp< *nparams&& rd_total_times != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field read total times too long for destination"));
> + goto endjob;
> + }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = rd_total_times;
> + tmp++;
> + }
>
> - default:
> - break;
> - /* should not hit here */
> + if (tmp< *nparams&& flush_total_times != -1) {
> + if (virStrcpyStatic(param->field,
> + VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Field flush total times too long for destination"));
> + goto endjob;
> }
> + param->type = VIR_TYPED_PARAM_LLONG;
> + param->value.l = flush_total_times;
> + tmp++;
> }
>
> + /* Field 'errs' is meaningless for QEMU, won't set it. */
> +
> + ret = 0;
> + *nparams = tmp;
> +
> endjob:
> if (qemuDomainObjEndJob(driver, vm) == 0)
> vm = NULL;
> @@ -10811,8 +10812,8 @@ static virDriver qemuDriver = {
> .domainSetSchedulerParameters = qemuSetSchedulerParameters, /* 0.7.0 */
> .domainSetSchedulerParametersFlags = qemuSetSchedulerParametersFlags, /* 0.9.2 */
> .domainMigratePerform = qemudDomainMigratePerform, /* 0.5.0 */
> - .domainBlockStats = qemudDomainBlockStats, /* 0.4.1 */
> - .domainBlockStatsFlags = qemudDomainBlockStatsFlags, /* 0.9.5 */
> + .domainBlockStats = qemuDomainBlockStats, /* 0.4.1 */
> + .domainBlockStatsFlags = qemuDomainBlockStatsFlags, /* 0.9.5 */
> .domainInterfaceStats = qemudDomainInterfaceStats, /* 0.4.1 */
> .domainMemoryStats = qemudDomainMemoryStats, /* 0.7.5 */
> .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */
ACK
More information about the libvir-list
mailing list