[libvirt] [PATCH v2 6/7] virsh: domstate: report detailed state if available

[Bjoern Walk]

Michal Privoznik <mprivozn at redhat.com> [2019-04-01, 10:49AM +0200]:
> On 4/1/19 10:17 AM, Bjoern Walk wrote:
> > I intent to have the new API function superseed the existing one, so I
> > actually want to call virDomainGetStateParams() unconditionally and only
> > fall back to virshDomainState() when the new API is not there (so that
> > hopefully at some distant point in the future, we could deprecate the
> > old stuff).
> 
> I understand that but consider the following scenario: a shell script that
> calls 'virsh domstate $dom' every so often. If we switch to new API then
> this will still fetch expected info for the client but also it will produce
> an error message logged at the server side (if it is old and doesn't support
> the new API).
> 
> That's the main reason I think we should use new API only if necessary, i.e.
> if --info was requested. But if you still want to prefer the new API we
> should do it the proper way. That is something among these lines:
> 
> if (virDomainGetStateParams() < 0) {
>   if (!last_error || last_error->code != VIR_ERR_NO_SUPPORT)
>     goto error;
> 
>   /* fallback */
>   if (virshDomainState() < 0)
>     goto error;
> }
> 
> Note that we have to check for the reason why vurDomainGetStateParams()
> failed and fallback on old API if and only if the new API is not supported.

Which would still generate the server-side error message, right? If this
is an issue, it's an issue and I will use your initial suggestion.
Otherwise, something like the above seems cleaner. I will have to think
about this.

> > > Also, printing out some error message would be nice:
> > 
> > If the new API is not available, I wanted to just silently have the old
> > behaviour. The domain state and reason can always be retrieve (by means
> > of the old API) only additional information could be missing. I am not
> > sure if we should error out if this is the case.
> 
> I think we should. I mean, if I'd run 'virsh domstate --info' that means I
> am expecting some extended info on domain state. If I don't get it, isn't
> that a failure?

There can also be no additional information available, think different
domain states or other hypervisors, for which also silently no further
information is printed. Should we do something about this scenario too?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 902 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190401/d21b2096/attachment-0001.sig>