[libvirt] [PATCH] qemu: Report error on unexpected job stats type

[Jiri Denemark]

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 at redhat.com>
> > ---
> >  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