----- Original Message -----
> This API has boundary value problem, if start_cpu is equal to
> the number of cpus, no error infomation will be reported.
>
> This is because the confused meaning of variable max_id,
> so change the comparision and rename the variable max_id to total_num.
>
> Signed-off-by: Jincheng Miao <jmiao@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 18 +++++++++---------
> src/util/vircgroup.c | 18 +++++++++---------
> tools/virsh-domain.c | 12 ++++++------
> 3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c9a865e..54b8e5b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15983,7 +15983,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
> {
> int rv = -1;
> size_t i;
> - int id, max_id;
> + int id, total_num;
> char *pos;
> char *buf = NULL;
> unsigned long long *sum_cpu_time = NULL;
> @@ -15999,19 +15999,19 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
> return QEMU_NB_PER_CPU_STAT_PARAM;
>
> /* To parse account file, we need to know how many cpus are present. */
> - max_id = nodeGetCPUCount();
> - if (max_id < 0)
> + total_num = nodeGetCPUCount();
> + if (total_num < 0)
> return rv;
>
> - if (ncpus == 0) { /* returns max cpu ID */
> - rv = max_id;
> + if (ncpus == 0) { /* returns total number of cpu */
> + rv = total_num;
> goto cleanup;
> }
>
> - if (start_cpu > max_id) {
> + if (start_cpu > total_num - 1) {
This comparison is between max cpu id and start cpu id. But max_id is the value of total
cpu number, got from nodeGetCPUCount(). So it failed to deal with boundary value.
And I file a related bug https://bugzilla.redhat.com/show_bug.cgi?id=1070680
> virReportError(VIR_ERR_INVALID_ARG,
> _("start_cpu %d larger than maximum of %d"),
> - start_cpu, max_id);
> + start_cpu, total_num - 1);
> goto cleanup;
> }
>
> @@ -16024,8 +16024,8 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
> param_idx = 0;
>
> /* number of cpus to compute */
> - if (start_cpu >= max_id - ncpus)
> - id = max_id - 1;
> + if (start_cpu + ncpus >= total_num)
> + id = total_num - 1;
> else
> id = start_cpu + ncpus - 1;
>
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 0f04b4d..2af06af 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -2836,7 +2836,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> {
> int rv = -1;
> size_t i;
> - int id, max_id;
> + int id, total_num;
> char *pos;
> char *buf = NULL;
> virTypedParameterPtr ent;
> @@ -2848,19 +2848,19 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> return CGROUP_NB_PER_CPU_STAT_PARAM;
>
> /* To parse account file, we need to know how many cpus are present. */
> - max_id = nodeGetCPUCount();
> - if (max_id < 0)
> + total_num = nodeGetCPUCount();
> + if (total_num < 0)
> return rv;
>
> - if (ncpus == 0) { /* returns max cpu ID */
> - rv = max_id;
> + if (ncpus == 0) { /* returns total number of cpu */
> + rv = total_num;
> goto cleanup;
> }
>
> - if (start_cpu > max_id) {
> + if (start_cpu > total_num - 1) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("start_cpu %d larger than maximum of %d"),
> - start_cpu, max_id);
> + start_cpu, total_num - 1);
> goto cleanup;
> }
>
> @@ -2873,8 +2873,8 @@ virCgroupGetPercpuStats(virCgroupPtr group,
> param_idx = 0;
>
> /* number of cpus to compute */
> - if (start_cpu >= max_id - ncpus)
> - id = max_id - 1;
> + if (start_cpu + ncpus >= total_num)
> + id = total_num - 1;
> else
> id = start_cpu + ncpus - 1;
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 3e73340..0ead80f 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6331,7 +6331,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
> {
> virDomainPtr dom;
> virTypedParameterPtr params = NULL;
> - int pos, max_id, cpu = 0, show_count = -1, nparams = 0;
> + int pos, total_num, cpu = 0, show_count = -1, nparams = 0;
> size_t i, j;
> bool show_total = false, show_per_cpu = false;
> unsigned int flags = 0;
> @@ -6376,12 +6376,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
> goto do_show_total;
>
> /* get number of cpus on the node */
> - if ((max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, flags)) < 0)
> + if ((total_num = virDomainGetCPUStats(dom, NULL, 0, 0, 0, flags)) < 0)
> goto failed_stats;
> - if (show_count < 0 || show_count > max_id) {
> - if (show_count > max_id)
> - vshPrint(ctl, _("Only %d CPUs available to show\n"), max_id);
> - show_count = max_id;
> + if (show_count < 0 || show_count > total_num) {
> + if (show_count > total_num)
> + vshPrint(ctl, _("Only %d CPUs available to show\n"), total_num);
> + show_count = total_num;
> }
>
> /* get percpu information */
> --
> 1.8.5.3
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list