Re: [libvirt PATCH] drivers: Group global feature together

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.