On Mon, Oct 31, 2011 at 05:33:24PM -0600, 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.
There's a compile problem with this patch
CC libvirt_driver_qemu_la-qemu_driver.lo
qemu/qemu_driver.c: In function 'qemuDomainBlockStatsFlags':
qemu/qemu_driver.c:7325:24: error: 'param' may be used uninitialized in this
function [-Werror=uninitialized]
cc1: all warnings being treated as errors
@@ -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"));
+ 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;
It is easier to see without the diff, only the first if {} block
initializes 'param':
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,
_("Field name '%s' too long"),
VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES);
goto endjob;
}
param->type = VIR_TYPED_PARAM_LLONG;
param->value.l = wr_bytes;
tmp++;
}
if (tmp < *nparams && wr_req != -1) {
if (virStrcpyStatic(param->field,
VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("Field name '%s' too long"),
VIR_DOMAIN_BLOCK_STATS_WRITE_REQ);
goto endjob;
}
param->type = VIR_TYPED_PARAM_LLONG;
param->value.l = wr_req;
tmp++;
}
if (tmp < *nparams && rd_bytes != -1) {
if (virStrcpyStatic(param->field,
VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("Field name '%s' too long"),
VIR_DOMAIN_BLOCK_STATS_READ_BYTES);
goto endjob;
}
param->type = VIR_TYPED_PARAM_LLONG;
param->value.l = rd_bytes;
tmp++;
}
Regards,
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 :|