Peter Memishian wrote:
>  > It is more future proofing.  How about I do something like:
>  > 
>  > dladm_create_conf(const char *name, const char *class, dladm_linkid_t 
>  > id, dladm_conf_t *conf, void *args);
>  > 
>  > Where args is marked for future growth?
> 
> I could see the need if we plan to make this API public now.  Is that
> the plan?  If not, I'd defer adding `args' until such a time.
> 

I remember when we went to PSARC for the inception review, someone (I 
think Erik Nordmark, but I my memory might have failed me) expressed an 
interest in having the API used by consumers outside of Clearview.  I 
don't remember if the intent was to have the API public.  So for now, 
I'll drop the args argument.  If we need to make it public, it should be 
an easy change to add it.

>  > We could do this.  This does make renaming a little more complicated 
>  > since there's a particular order things have to happen.  If we went this 
>  > route I would want to add Cathy's suggested conf_rename function, and we 
>  > might also want to make name a read only field of the configuration.
> 
> That sounds fine to me.
>

I added the rename function.  I called it dladm_rename_conf.

>  > > This is certainly something worth some thought, but my gut reaction is
>  > > that recovery of temporary state across it crashing is no more a
>  > > requirement than recovery of temporary state across a kernel crash.
>  > 
>  > I think we might have situations where we would have undesirable 
>  > behavior.  I'd need to think about this.  Do you think this problem 
>  > should be solved before we continue?  I think what we have is pretty 
>  > high level, and we can begin work on the internals.
> 
> I think it's OK to proceed with what we have.
> 

OK, I'll do that.

>  > > That said, I *think* we'd need to make some changes to the API to satisfy
>  > > the first notion of safety.  Specifically, since some changes require
>  > > atomically applying changes to several links (e.g., creating an
>  > > aggregation with two links), I think read_conf()/write_conf() would need
>  > > to operate on the entire persistent configuration, rather than just a
>  > > given link.  With that approach, I think it would also be possible to
>  > > detect/prevent update collisions -- e.g., you could store a generation
>  > > number associated with the configuration that is returned when it is 
> read,
>  > > and then bump that number when it's written out.  If the number doesn't
>  > > match across the read()/write(), then the writer is forced to go re-read
>  > > to pull in the latest changes (and generation number), and then retry the
>  > > write() if appropriate.
>  > 
>  > I see what you're saying.  I think it would work, and I think we could 
>  > hide the details of how it worked, and the only API change would be a 
>  > new error value.  Let me think about that.
> 
> Yes, I think this could (and ideally should) be completely hidden from the
> API user other than an "update collision" error code.
> 

OK.  I mentioned a collision error in the new version of the document 
(coming soon).

Dan



Reply via email to