05/11/2018 09:25, Stojaczyk, Dariusz:
> Hi Thomas,
>
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 05/11/2018 08:04, Darek Stojaczyk:
> > > -int __rte_experimental
> > > -rte_devargs_insert(struct rte_devargs *da)
> > > +void __rte_experimental
> > > +rte_devargs_insert(struct rte_devargs *da, struct rte_devargs
> > **prev_da)
> >
> > You should update the API section of the release notes.
>
> Even for experimental API? OK, I didn't know it's needed.
Yes, even for experimental API, the API changes must documented.
> > > {
> > > - int ret;
> > > + struct rte_devargs *d;
> > > + void *tmp;
> > > +
> > > + *prev_da = NULL;
> > > + TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
> > > + if (strcmp(d->bus->name, da->bus->name) == 0 &&
> > > + strcmp(d->name, da->name) == 0) {
> > > + TAILQ_REMOVE(&devargs_list, d, next);
> > > + *prev_da = d;
> > > + break;
> > > + }
> > > + }
> > >
> > > - ret = rte_devargs_remove(da);
> > > - if (ret < 0)
> > > - return ret;
> >
> > Why not updating rte_devargs_remove instead of duplicating its code?
>
> We still want to preserve the functionality of rte_devargs_remove.
> rte_devargs_remove does TAILQ_REMOVE + free; rte_devargs_insert does just
> TAILQ_REMOVE. (I think I also forgot to update rte_devargs_insert
> documentation, I'll do that in V2)
Yes, because of the rollback, OK.
Please mention in devargs_insert doc that the old devargs
can be used for rollback.
> Since you've mentioned it:
> Eventually I'd see rte_devargs_remove to accept the exact same devargs
> parameter that was passed to rte_devargs_insert. Then rte_devargs_remove
> wouldn't do any sort of lookup. Maybe additional rte_devargs_find(const char
> *name) could be added for existing cases where the original devargs struct is
> not available. However, I'm not familiar enough with this code to perform the
> refactor and am just trying to fix the stuff. Still, how does it sound?
I think we can keep it as is.
We can re-think the whole thing in the next release.
I think we should not play with devargs list as we do.
There should be only lists for scanned devices of each bus and that's all.
