Michal Privoznik <mprivozn@xxxxxxxxxx> [2019-04-01, 02:08PM +0200]:
> On 4/1/19 1:14 PM, Bjoern Walk wrote:
> > Michal Privoznik <mprivozn@xxxxxxxxxx> [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?
>
> Yes, it would.
>
> > 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.
>
> Well, to be fair we use something like this. For instance when listing
> domains/networks/... But there is a clear advantage of using 'new'
> virConnectListAll*() over 'old' GetNumDomains() + ListDomains() +
> GetDefinedDomains() + ListDefinedDomains() - they are atomic. But what is
> the advantage of using the new API in case of plain 'virsh domstate'?
>
> BTW: We don't really have notion of deprecating an API. We can merely say
> 'don't use this, use that because reasons' in the docs. But we still have to
> fix all APIs / maitain them.
>
> This is the reason I am advocating for using new API only if neccessary.
Ok, I am convinced now :)
Thanks for your advice, I will send v3 after the release.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
