On 8/29/19 2:10 PM, Markus Armbruster wrote: > Michal Privoznik <mpriv...@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 <mpriv...@redhat.com> Michal