Re: [Qemu-devel] [PATCH] qapi: Reintroduce CommandDisabled error class

[Michal Privoznik <mprivozn@redhat.com>]

On 8/29/19 2:10 PM, Markus Armbruster wrote:
> Michal Privoznik <mprivozn@redhat.com> writes:
> 
>> If there was a disabled command, then qemu-ga used to report
>> CommandDisabled error class (among with human readable
>> description). This changed in v1.2.0-rc0~28^2~16 in favor of
>> GenericError class.
> 
> Really?  I believe it was slightly earlier in the same series:
> 
> 93b91c59db qemu-ga: switch to the new error format on the wire
> de253f1491 qmp: switch to the new error format on the wire

Ah, you're right. It's the first commit that you reference.

> 
> The commit you mention (df1e608a01e) is merely follow-up simplification.
> 
>>                      While the change might work for other
>> classes, this one should not have been dropped because it helps
>> callers distinguish the root cause of the error.
>>
>> A bit of background: up until very recently libvirt used qemu-ga
>> in all or nothing way. It didn't care why a qemu-ga command
>> failed. But very recently a new API was introduced which
>> implements 'best effort' approach (in some cases) and thus
>> libvirt must differentiate between: {CommandNotFound,
>> CommandDisabled} and some generic error. While the former classes
>> mean the API can issue some other commands the latter raises a
>> red flag causing the API to fail.
> 
> Why do you need to distinguish CommandNotFound from CommandDisabled?

I don't. That's why I've put them both in curly braces. Perhaps this 
says its better:

switch (klass) {
  case CommandNotFound:
  case CommandDisabled:
        /* okay */
        break;

  default:
        /* bad, error out */
        break;
}

> 
>> This reverts df1e608a01 partially.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Michal