On Thu, Jun 28, 2018 at 15:45:12 +0200, Michal Privoznik wrote:
> On 06/28/2018 03:29 PM, Jiri Denemark wrote:
> > If we ever fail to properly set jobinfo->statsType,
> > qemuDomainJobInfoToParams would return -1 without setting an error.
>
> Have you actually seen such error? Looks to me like both callers handle
> this case properly.
Yes, when the code for destination migration filled in migration stats,
but failed to set the stats type. As a result no stats were reported
and the programmer's error was silently ignored.
> > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> > ---
> > src/qemu/qemu_domain.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index fee44812c1..9afe705929 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -730,6 +730,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
> > return qemuDomainDumpJobInfoToParams(jobInfo, type, params, nparams);
> >
> > case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("invalid job statistics type"));
> > + break;
> > +
> > + default:
> > + virReportEnumRangeError(qemuDomainJobStatsType, jobInfo->statsType);
> > break;
> > }
> >
> >
>
> Can't we join TYPE_NONE and default together and just report enum range
> error? What is the point in differentiating the two?
I did it in the first version of this patch (a long time ago), but
Daniel explained that virReportEnumRangeError is supposed to be used
only for unknown enum values. A different error should be used for known
values which do not make sense in a specific place in the code.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
