Peter Memishian wrote:
> > For the Link ID management API:
> >
> > dladm_create_linkid() /* get_new_linkid */
> > dladm_destroy_linkid() /* delete_linkid */
> > dladm_linkid2name() /* id2name */
> > dladm_name2linkid() /* name2id */
> > dladm_change_linkid_mapping() /* rename */
> > dladm_walk_linkid() /* The walker mentioned in my reply to Cathy */
> >
> > For the persistent link configuration API:
> >
> > dladm_create_conf() /* get_new_conf */
> > dladm_set_conf_field() /* set_conf_field */
> > dladm_get_conf_field() /* get_conf_field */
> > dladm_read_conf() /* get_conf */
> > dladm_write_conf() /* commit_conf */
> > dladm_delete_conf() /* delete_conf */
> > dladm_destroy_conf() /* free_conf */
> > dladm_walk_conf() /* walk_confs */
>
> I like those names -- except dladm_change_linkid_mapping(), which feels
> a bit bulky. Maybe:
>
> dladm_status_t dladm_remap_linkid(dladm_linkid_t id, const char *linkname)
>
OK, sounds good.
Also, I noticed the other mails between you and Cathy about the
dladm_delete_conf and dladm_destroy_conf names. Let's rename
dladm_delete_conf dladm_remove_conf.
> > I'd prefer to not require class, ID, and name for the create call,
> > because what if in the future we need more required fields in the
> > future, but I see your point.
>
> Do you have any other specific required fields in mind? Or is more of a
> future-proofing of the API?
>
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'll also make the ID and class field read-only so that attempts to set
> > either with dladm_set_conf_field will fail.
>
> Sounds good.
>
> > > Why couldn't that be detected when the persistence is attempted -- e.g.,
> > > fail the commit/write()?
> >
> > I could add some checks. We could have the commit check to make sure
> > the name to ID mapping in the configuration structure matches what the
> > link ID management API shows. However, I'm not sure how the
> > configuration API would tell the difference between this situation and a
> > rename operation.
> >
> > For example, we search using an ID to see if there is already an
> > existing persistent configuration for this ID. Since net10 has net6's
> > ID, there is a configuration. We can tell the names are different, but
> > how does the API tell the difference between this operation and an
> > attempt to rename the interface?
>
> My thinking was that the API should never allow a configuration to be
> written out that would result in two persisted configurations with the
> same linkid. In the scenario you gave before:
>
> delete_linkid("net6");
> id = get_new_linkid("net10", class); /* Could return net6's link ID! */
> create_conf("net10", id, ...)
> write_conf("net10");
>
> ... the write_conf() for "net10" would fail because "net6" already has
> its persistent linkid. If someone wanted to do a rename, they'd need to
> delete_conf("net6") first.
>
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.
> > Another problem I thought of as I typed this up is what happens if
> > devfsadmd crashes and is restarted? Then we lose all the temporary link
> > ID to name mappings, unless devfsadmd has some way of storing the
> > temporary link ID to name mappings. If we persist them with the
> > persistence API, the mapping is restored on reboot, which is not what
> > we'd want.
> >
> > I think my use of the term persistence could be problematic elsewhere.
>
> 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.
> > Even if we have two processes (or even two threads in the same process)
> > trying to change the same link's configuration?
>
> There are two separate notions of safety here. One is that the
> configuration is self-consistent and not corrupt. That is, the API must
> not allow a given persistent configuration to appear to be a mongrel
> consisting of state that was written by multiple threads competing to
> update it. That is, from the point of view of a thread using the API to
> read the persistent configuration, it should always see a single,
> consistent view of the persistent configuration.
>
OK, agreed.
> The other notion of safety is to protect against update collisions (i.e.,
> from multiple updaters stomping on one-another) -- e.g., one application
> tries to name a link one thing, and the other tries to name a link another
> thing. I think as long as the resulting configuration is self-consistent
> (that is, the final link name reflects either one name or the other), this
> is not required.
>
> 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.
Dan
