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

Re: [libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use



On Tue, Aug 23, 2011 at 05:39:37PM +0800, Osier Yang wrote:
> Error code VIR_ERR_NO_SUPPORT will be translated to "this function
> is not supported by the connection driver", however, it's used
> across the whole projects, this patch is trying to cleanup all
> the improper use in the source tree.
> 
> The modification can be grouped to 3 following groups:
> 
> 1) The error intends to tell user it's invalid operation.
> 
>      s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.

Most of these changes are wrong. Operation invalid is
defined as:

    VIR_ERR_OPERATION_INVALID = 55,     /* operation is not applicable at this
                                           time */

In other words, the object is in the wrong state for the
operation be requested.  For example, attempting to invoke
'virDomainSuspend' on a guest that is not currently running
would imply OPERATION_INVALID.

For arguments that aren't implemented, or are incorrect
or not possible on this platform, then one of the other
codes like ARGUMENT_UNSUPPORTED or CONFIG_UNSUPPORTED
or even just INTERNAL_ERROR

> 2) The error intends to tell the configuration of domain
>    is not supported.
> 
>      s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
> 
> 3) The error intends to tell the function is not implemented
>    on some platform.
> 
>      * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>      * and add error strings

This is not correct. On platforms where a driver method is
registered, but stubbed out to a no-op, then we should still
be reporting NO_SUPPORT.  Ideally we would have not actually
registered the driver method at all on that platform, but it
is simpler to register it and make it a no-op stub and then
raise NO_SUPPORT.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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