CESNET / libnetconf

C NETCONF library
Other
113 stars 83 forks source link

Why first to lock file datastore before to handle edit-config operation #239

Closed chenlilong84 closed 7 years ago

chenlilong84 commented 7 years ago

In libnetconf, _ncds_fileeditconfig function is used to handle edit-config file datastore, I doubt why first to lock datastore, the aim is to confirm only has one session to edit same file datastore at the same time?

> int ncds_file_editconfig( ... )
{
 ...
    LOCK(file_ds, ret); 
...
}

If like this, i think the logic is repetitive, as we know, before to handle edit-config operation, pthread mutex lock in each datastore also confirm it, code is as follows:

> static nc_reply* ncds_apply_rpc( ... ) 
{
 ...
pthread_mutex_lock(&ds->lock);
...
pthread_mutex_unlock(&ds->lock);
}

BTW, the logical flow is : ncds_apply_rpc2all->ncds_apply_rpc->ncds_file_editconfig

rkrejci commented 7 years ago

I don't understand your question. ncds_apply_rpc2all() calls ncds_apply_rpc() which is calling ncds_file_editconfig() (via ds->func.editconfig() on datastore.c:6025). ncds_apply_rpc() locks the internal representation of the datastore to avoid other NETCONF session to access it (all parts of it, to avoid interleaving of different operations). When it gets into the specific implementation (FILE) of the datastore part, it locks the file where the data are stored to avoid concurrent access to the file from other processes. Note that the last part is specific for implementation of the datastore parts. If there would be some DB implementation, it would be probably necessary to lock the DB (or some part of it). What is wrong on this workflow?

chenlilong84 commented 7 years ago

Thanks rkrejci! I get it. Your meaning is that: 1) Lock in ncds_apply_rpc() is designed to avoid concurrent access to datastore from different NETCONF sessions, but why to design like this, RFC requires? Maybe we can support a little concurrent access, such as one session reads, one session writes and use "stop-and-waiting" mechanism to confirm it; 2) Lock file datastore is to avoid concurrent access from other processes, before I always thought that is to avoid access from other sessions.

rkrejci commented 7 years ago

1) someone can reimplement it, but current libnetconf implementation does not allow concurrent access. 2) yes