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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

-- 
Andrea Bolognani / Red Hat / Virtualization





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux