[libvirt PATCH] drivers: Group global feature together

[Andrea Bolognani]

On Thu, Apr 07, 2022 at 02:24:05PM +0200, Ján Tomko wrote:
> On a Thursday in 2022, Andrea Bolognani wrote:
> > On Wed, Feb 16, 2022 at 09:26:21PM +0100, Ján Tomko wrote:
> > > >     switch ((virDrvFeature) feature) {
> > > > +        case VIR_DRV_FEATURE_REMOTE:
> > > > +        case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
> > > > +        case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
> > > > +        case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
> > > >         case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
> > > >         case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:
> > > > -            return 1;
> > > > +        case VIR_DRV_FEATURE_FD_PASSING:
> > > > +            /* Should have already been handled by virDriverFeatureIsGlobal() */
> > > > +            return -1;
> > >
> > > Here you return an error without reporting an error.
> > >
> > > Would virReportEnumRangeError be reasonable to use here?
> >
> > Mh. While the error message ("Unexpected enum value N for virFoo") is
> > not too specific about this, virReportEnumRangeError() is usually
> > called when the provided value cannot be translated back to one of
> > the enum values; in this case it *can* be converted, it's just not
> > supposed to be encountered in the specific situation. Given the
> > subtly different semantics, I'm leaning towards not using that
> > helper.
> >
> > Would something like
> >
> >  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >                 _("Global feature %d should have already been handled"),
> >                 feature);
> >
> > work for you? I'd drop the comment of course.
>
> Reviewed-by: Ján Tomko <jtomko at redhat.com>

Thanks! Pushed after squashing in the virReportError() calls and
tweaking the error message slightly.

-- 
Andrea Bolognani / Red Hat / Virtualization