Subject: [Netconf] partial-lock feedback

From: Phil Shafer <[email-masked]>
Date: Thu, 13 Nov 2008 21:46:40 -0500

Here are some comments on the partial lock draft (-04).

Thanks,
 Phil

-----------

>1.  Introduction
>
>   The NETCONF protocol [NETCONF] describes the lock and unlock RPCs

s/the//
s/RPCs/operations/  ?

>   that operate on entire configuration datastores.  Often, multiple
>   management sessions need to be able to modify the configuration of a
>   managed device in parallel.  In these cases, locking only parts of a
>   configuration datastore is needed.  This document defines an
>   extension to the NETCONF protocol to allow this.

s/allow this/support partial locking of NETCONF datastores/

>   The mechanism for partial locking is based on the existing XPath
>   filtering mechanisms.

Or even:

  This document defines an extension to the NETCONF protocol to
  support partial locking of NETCONF datastores using a mechanism
  based on the existing XPath filtering mechanisms.

>   Partial locking is defined as a capability to NETCONF.

Make this part of the the first paragraph, not out on its own.

>2.1.  Overview
>
>   The :partial-lock capability indicates that the device supports the
>   locking of its configuration with a more limited scope than a
>   complete configuration datastore.  The scope to be locked is
>   specified by using restricted or full XPath expressions.  Partial
>   locking covers configuration data, but not state data.

"covers" is a fuzzy word.

You call out state data; should you also call out notification and
rpc data?

>   The duration of the partial lock begins when the partial lock is
>   granted and lasts until (1) either the corresponding <partial-unlock>
>   operation succeeds or (2) the NETCONF session terminates.

It seems odd to start using "<>"s here, and you are referring to an
operation that you haven't introduced.  I'd drop them and refer to the
partial unlock operation in a more generic way.

Or consider adding a paragraph describing the basic operation, how
they work, and what they are used for.  An overview like this would be
helpful to the reader.

In NETCONF, the locks are short duration, meant to allow the client to
grab it, change the config, and release it.  Are partial lock similar?
Do you need to say something about the operating model for partial
locks?

Did you consider simply extending the arguments to the existing
lock/unlock operations rather than creating new ones?  As NETCONF
matures, adding new ops will be less attractive, since it means
increasing the work required to add new features.  For example, if I
supported a capability that added a new <timeout> argument to
<lock-config>, I would now have to introduce another new capability to
add this to <partial-lock>.  Better to :partial add <select> and
multiple <target>s to the existing RPC.

>   A NETCONF session MAY have multiple parts of one or more datastores
>   (running, candidate, startup) locked using partial lock operations.
>   The <partial-lock> operation returns a lock-id to identify each
>   successfully acquired lock.

These two sentences are unrelated.

>2.2.  Dependencies
>
>   The device MUST support restricted XPath expressions in the select
>   element, as described in Section 2.4.1.  Optionally, if the :xpath
>   capability is also supported, the device MUST also support using any
>   XPath 1.0 expression in the select element.

Put a reference on :xpath so we know you are talking about the base
NETCONF :xpath capability.  Would it work better of "xpath" were a
"feature" (in the YANG sense) of this model, rather than forcing folks
that want :xpath in the base netconf ops to support it here?  Having
this implicit behavior is a bad thing.

>2.4.1.  <partial-lock>
>
>   The <partial-lock> operation allows the client to lock a portion of
>   one or more datastores.  The portion to lock is specified with XPath
>   expressions in the select elements and the list of datastores in the
>   target element in the <partial-lock> operation.

s/select/<select>/  or /"select"/
Otherwise is reads like "the select elements" means some special
elements.  Same for <target>.

You have multiple <select> elements and one <target> element
containing multiple values.  This is inconsistent.  Change
target to a leaf-list of enums.

>   Each XPath
>   expression MUST return a node set.

This is an odd sentence.  What are you trying to say?  Are you worried
that someone will think you mean predicate expressions?  Do you want
to explicit reference the XPath spec'c production name (LocationPath
in section 2.0 of http://www.w3.org/TR/xpath.html).

>   Locking a node also protects the
>   complete subtree under the node from modification by others.

This is a very important concept that needs to be brought out.  When
you lock a node, you lock it and its children.  No one can modify any
child of a node you lock.  This may be obvious, but it needs to be
said clearly.

>   In some situations it is desirable that the same set of nodes are
>   locked in more than one datastore.  As an example: if an interface is

s/As an example:/For example,/

>   configured in the candidate datastore, it is dangerous if it is

s/if it is/for it to be/

>   configured by someone else in a possibly conflicting manner in the
>   running datastore.  For this reason <partial-lock> allows the locking
>   of the same sections of the management data in one or multiple
>   datastores.

s/one or//

Locking multiple datastores requires maximally pessimistic lock use,
right?  I MUST lock it everywhere to protect myself.  Seems
pointless.  There's some logic that says the intersection of
boxes with :writable-running and :candidate is (or should be) nil.
So you should only need to lock one or the other.

>   The select XPath expressions are evaluated only once at lock time.

"select XPath expressions" is an odd phrase.

    NETCONF servers evaluate each <select> expression only once, at
    the time the lock is granted.  Once the lock is granted, changes
    to the database to not affect the scope of the lock.

Adding a concrete example would be good.  In general, this draft needs
more examples.  People love examples.  And parfait.

>   Thereafter, the scope of the lock is maintained as a set of nodes.

Do you ever define "scope"?  It's a fuzzy term, so if you want to use
it, please define it in crisp terms.

>   If the configuration data is later altered in a way that would make
>   the original select XPath expressions evaluate to a different set of
>   nodes, this does not affect the scope of the partial lock.
>
>   XPath is only used for the creation of the partial lock.
>   Conceptually, the scope of the lock is defined by the returned node
>   set and not by the XPath expression.

These paragraphs seem to dance around some point that's not clear.
What to you mean "conceptually ... not by the xpath"?  Why doesn't the
XPath expression tell you want you're trying to lock?

>   A <partial-lock> operation MUST be handled atomically by the NETCONF
>   server.  The server either locks all requested parts of the
>   datastore(s) or none.  That is, if during the <partial-lock>

s/That is, if/If/

>   operation one of the requested parts cannot be locked, the server
>   MUST unlock all parts that were previously locked during that

s/all/any/?

>   operation.

This last sentence is unclear (or evil).  "previously locked during"
includes anything I had locked from previous lock requests, which
isn't what you want.

This whole issue makes me wonder why you need to support multiple
<select> expressions, instead of making the client "OR" them
together.  Where's the win?

>   If a node is locked by a session, only that same session is able to
>   modify that node or any node in the subtree underneath it.

  When a NETCONF session holds a lock on a node, no other session or
  non-NETCONF mechanism of the system can change that node or any node
  in the hierarchy of nodes beneath it.

Merge this with the related text above (where I said this point needs
to be more strongly made).

>   If a top level node of a locked subtree is deleted, any other session
>   can recreate it, as it is no longer covered by the lock.  If all top
>   level nodes are deleted, the lock will still be present.  However,
>   its scope will become nil (i.e., the lock will not cover any nodes).

s/ nil (i.e., the/empty (since/

"cover" and "scope" are fuzzy.  Define them and how they relate.  (But
I think with better wording you can avoid these terms).

   Nodes are released from the lock when they are deleted, at which
   point other clients or non-NETCONF mechanisms are able to recreate them.

>   A NETCONF server MUST be able to grant multiple simultaneous partial
>   locks to a single NETCONF session.  If the scope of the individual
>   locks overlaps, nodes in the common area MUST be locked until all of
>   the locks are released.

   Nodes locked under multiple overlapping locks remain locked until
   all such locks are released.

>   A partial lock operation MUST fail if:
>
>   o  Any NETCONF session (including the current session) owns the
>      global lock on any target datastore.
>
>   o  Any part of the scope to be locked is already locked by another
>      management session/protocol, including other NETCONF sessions
>      using the <partial-lock> or any other non-NETCONF management
>      method.
>
>   o  The NETCONF server implements access control, and the locking user
>      does not have sufficient rights to all parts of a datastore
>      section to be locked.

  The NETCONF server's access control mechanism does not give the
  locking user sufficient rights to any nodes to be locked.

>      The exact handling of access rights is
>      outside the scope of this document, but it is assumed that there
>      is an access control system that MAY deny or allow the partial
>      lock operation.
>
>

Two blank lines?

>   As with most locking systems, it is possible that two management
>   sessions trying to lock different parts of the configuration could
>   become dead-locked.  To avoid this situation, clients SHOULD lock
>   everything they need in one operation.  If that operation fails, the
>   client SHOULD back down, release any previously acquired locks, and
>   retry the procedure after waiting some time interval.  The length of
>   the interval SHOULD be random to avoid repeated dead-locks when both
>   (or all) clients back down and then retry the partial lock operation.

Need a "Deadlock Avoidance" section.

"back down" is a fuzzy concept.  If you want exponential back-off, say so.

s/dead-lock/deadlock/

>   The <partial-lock> operation is designed for simplicity, so when a
>   partial lock is executed you get what you asked for: a set of nodes
>   that are locked for writing.  There are some other issues that are
>   intentionally not addressed:

"not addressed"?  Hogwash.  Address them.

>   o  Locking does not affect read operations.

Address it:  Say "Locking does not affect read operations".

>   o  If part of a datastore is locked, this has no effect on any
>      unlocked parts of the datastore.  If this is a problem (e.g.,
>      changes depend on data values in the unlocked part of the
>      datastore), these values SHOULD be included in the scope of the
>      lock.

s/values/nodes/

s/SHOULD/should/ since this is not a requirement of the specification.

Address it: If the client should not attempting operations that rely
on configuration that is not locked.

>   o  Configuration data can be edited both inside and outside the scope
>      of a lock.  It is the responsibility of the NETCONF client
>      application to lock all relevant parts of a datastore that are
>      crucial for a specific management action.

Address it.  This is a big point and should be in a list of "things we
didn't address".

>   Note: The <partial-lock> operation does not modify the global <lock>
>   operation defined in the base NETCONF Protocol [NETCONF].  If part of
>   a datastore is already locked by <partial-lock>, then a global lock
>   for that datastore MUST fail even if the global lock is requested by
>   the NETCONF session that owns the partial lock.

Make a section on "Interactions with <lock> and <unlock>", so this is
more visible to the reader (and in the table of contents).

>2.4.1.1.  Parameters, Result, Examples
>
>   Parameters:
>
>   target:  Name of one or more configuration datastores of which a part
>       shall be locked.  If multiple datastores are specified the same
>       select parameter(s) are used for all of them.  For each datastore
>       locking of the same set of nodes will be requested.

"same set of nodes": it won't be the same set of nodes if the
datastores have different contents.

I mentioned before that having one <target> will multiple contents and
multiple <select>s is inconsistent.  You also need to think about what
it would look like if we make an identity called "datastore" in
4741-bis and folks extend it with <url> or <file> or <datastore>.
When they want to lock multiple files, how will it work?

>   select:  One or more 'select' elements, each containing an XPath
>       expression....

>       Each select expression is evaluated for each targeted datastore.

Seems like this paragraph needs more words.

>       The nodes that matched the select expression and are locked are
>       reported in the rpc-reply message.

Make one paragraph on locked and a different one on reporting.  Mixing
different topics in one paragraph is hard for the reader (and worse
for the skimmer).

I strongly dislike the idea of reporting what was locked in the
reply.  This is mostly useless information and will be annoying to
generate with proper namespace mappings.  What is the win in reporting
to the client the information the client just sent to you?

>       Note that if some of the
>       requested nodes exist only in one of the datastores, the lock is
>       granted on different nodes in different datastores.

Don't hide "Note"s in unrelated paragraphs.  Remember that
most readers skim, reading the topic sentence and then skipping
the rest of the paragraph if they (a) assume they know what you
are going to say, or (b) thing this paragraph doesn't interest
them.  Allow the same lines, there are placing when you can
consolidate all the text about an issue in one spot instead of
interlacing the text with other topics.

>       Each select expression MUST return a node set, and at least one
>       of the node sets for one of the specified datastores MUST be non-
>       empty.

Is this done to catch errors?  I don't understand how this helps.

>       If the device supports the :xpath capability, any valid XPath 1.0
>       expression can be used.  If the device does not support the
>       :xpath capability, the XPath expression MUST be limited to an
>       Instance Identifier expression.  An Instance Identifier is an
>       absolute path expression in abbreviated syntax, where predicates
>       are used only to specify values for nodes defined as keys to
>       distinguish multiple instances.

Is the term "Instance Identifier" local to this draft?  If so, maybe
it's time to make a "Terminology" section.  At a minimum, it needs a
more complete definition.

Is it worthwhile to put XPath expressions into a distinct argument?
<identifier> .vs. <select>?

>   Positive Response:
>   If the device was able to satisfy the request, an <rpc-reply> is sent
>   with a <lock-id> element (lock identifier) in the <rpc-reply>
>   element.  A list of locked nodes per datastore is also returned in
>   Instance Identifier format.

So if I lock all sonet interfaces, I get a list of all sonet
interfaces?  When do I need this?

>   Negative Response:
>   If a lock is already held by another session on any node within the
>   subtrees to be locked, the <error-tag> element is 'lock-denied' and
>   the <error-info> element includes the <session-id> of the lock owner.
>   If the lock is held by a non-NETCONF session, a <session-id> of 0
>   (zero) is included.  If needed the returned session-id may be used to
>   <kill-session> the NETCONF session holding the lock.  The same error
>   response is returned if the requesting session already holds the
>   (global) lock for the same datastore.

If global locks are the same, merge their handling into the text.

   If a partial lock is already held by another session on any node
   within the subtrees to be locked or a global lock is held by any
   NETCONF client, the <error-tag> element is 'lock-denied' and...

>   If all the select expressions return an empty node set, the <error-
>   tag> is 'operation-failed', and the <error-app-tag> is 'no-matches'.

Why is this app-tag have dashes but the other's don't?  "no matches".

>   If any select expression returns something other than a node set, the
>   <error-tag> is 'invalid-value', the <error-app-tag> is 'XPath does
>   not return a node set'.

How can the expression return something other than a node set?

>   If the :xpath capability is not supported and the XPath expression is
>   not an Instance Identifier, the <error-tag> is 'invalid-value', the
>   <error-app-tag> is ':xpath capability not supported'.

Show some love to your users: "invalid lock specification"

>   If access control denies the partial lock, the <error-tag> is
>   'access-denied'.

"access denied".

>2.4.1.2.  Reserving model sections for future work

This is a poor title.  "Locking nodes before they are created"?

>   Partial lock cannot be used to lock non-existent nodes, which would
>   effectively attempt to reserve them for future use.  To guarantee
>   that a node cannot be created by some other session, the parent node
>   SHOULD be locked, the top level node of the new section created, and
>   then locked with another <partial-lock> operation.  After this, the
>   lock on the parent node SHOULD be removed.

  Since partial locks cannot be taken out for nodes that do not exist,
  special handling is required on the client to handle scenarios the
  locking of newly created nodes.  To guarantee ...

>2.4.2.  <partial-unlock>
>
>   The operation unlocks the parts of the datastores that were
>   previously locked using <partial-lock> during the same session.
>
>   Parameters:
>
>   lock-id:  Lock identifier to unlock, taken from a reply to a previous
>       <partial-lock> operation.

  This lock-id MUST have been received as a response to a lock request
  by the NETCONF server during the current session, and MUST NOT have
  been sent in a previous unlock request.

>2.5.  Modifications to Existing Operations
>
>   A successful partial lock will cause a subsequent operation to fail
>   if it attempts to modify the locked area and is executed in a NETCONF
>   session other than the session that owns the lock.

Fail with what error?

s/owns/has been granted/

"locked area" is a new term.

>   All operations
>   that modify the datastore are affected, including: <edit-config>,
>   <copy-config>, <delete-config>, <commit> and <discard-changes>.  A
>   successful partial lock will also cause the (global) <lock> operation
>   to fail.  All of these operations are affected only if they are
>   targeting the same datastore.

"successful" here is confusing.  You make it sound like the
partial-lock operation will cause a <lock> to fail, rather
than the existence of locked nodes.

<commit>?  You can't commit it someone else has a partial lock?

I think you need to put more words into what operations are not
allowed.  I know it sounds trivial to say it, but are things
like insert allowed?

>2.6.2.  Confirmed Commit Capability
>
>   If:
>
>
>
>
>Lengyel & Bjorklund        Expires May 2, 2009                  [Page 9]
>
>Internet-Draft        Partial Lock RPC for NETCONF          October 2008

This "If:" is a _horrible_ case of hanging text.  I think this can be
avoided completely if you rephrase the text a bit:

If the NETCONF server supports the :confirmed-commit capability and a
confirmed commit is outstanding, partial lock requests for the
<running> database MUST be denied, since the running datastore must be
rolled back if a confirming commit is not performed within the
appropriate time window.

>   In this case the <error-tag> 'in-use' and the <error-app-tag> 'Lock
>   denied, Outstanding confirmed commit' is returned.

Can we please use lower case for app-tag values?

>2.6.3.  Distinct Startup Capability
>   Partial locking of the startup datastore can only be done if the
>   :startup capability is supported by the device.

:startup is not a valid target for <edit-config>, so it should not be
a target for partial locks.

>3.  Security Considerations
>...
>   Partial locking is NOT an authorization mechanism; it SHOULD NOT be
>   used to provide security or access control.  Partial locking SHOULD
>   only be used as a mechanism for providing consistency when multiple
>   managers are trying to configure the node.  It is vital that users
>   easily understand the exact scope of a lock.  This is why the scope
>   is determined when granting a lock and is not modified thereafter.

"users"?  Users understand XPaths expressions?  "exact scope"?  I
don't get this argument.  The way you've got this working, not one
can understand the exact scope of a lock.  If I lock all sonet
interfaces, no one without a log file and a crystal ball will be able
to tell me which sonet interfaces I have locked.  Any interfaces
created after I got my lock aren't locked.  Anything I deleted and
someone (even me) recreated isn't locked.  If you are looking for
exactness and understandability, you need to rethink this.

>5.  Appendix A  -  XML Schema for Partial Locking (normative)

For the record, I didn't read this section.

>6.  Appendix B  - YANG Module for Partial Locking (non-normative)
> module netconf-partial-lock {

s/netconf/ietf-netconf/

IETF YANG modules should all start with "ietf-".

>   grouping configNames {

Should this be an identify?

>     container target {
>         description
>             "A list of one or more datastore names.";
>         leaf running   { type empty; }
>         leaf candidate { type empty; }
>         leaf startup   { type empty; }

Add if-features for :writable-running, :candidate, and :startup.

>       leaf-list select {
>         description
>           "XPath expression that specifies the scope of the lock.
>           An Instance Identifier expression MUST be used unless the
>           :xpath capability is supported, in which case any XPath 1.0
>           expression is allowed.";
>         type string;

We should have a typedef for XPath expressions (locationPath).

>     output {
>       leaf lock-id {
>         description
>           "Identifies the lock, if granted. The lock-id MUST be
>            used in the partial-unlock rpc.";

MUST? I can do my change and then terminate my connection without
doing a partial-unlock.

>         type uint32;

This should be a typedef.

-------------------
_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: Balazs Lengyel <[email-masked]>
Date: Mon, 17 Nov 2008 21:26:43 +0100

Hello Phil,
Thanks for the comments. See my replies below. Tell me if I missed anything.
Balazs

> Here are some comments on the partial lock draft (-04).

OK

>>   filFrom [email-masked]  Mon Nov 17 12:26:46 2008
Date: Mon, 17 Nov 2008 21:26:43 +0100
From: Balazs Lengyel <[email-masked]>
To: [email-masked]
Subject: Re: [Netconf] partial-lock feedback
Reply-To: [email-masked]
Sender: [email-masked]

Hello Phil,
Thanks for the comments. See my replies below. Tell me if I missed anything.
Balazs

> Here are some comments on the partial lock draft (-04).

OK

>>tering mechanisms.

OK

> Make this part of the the first paragraph, not out on its own.
OK
> "covers" is a fuzzy word.

OK, will reword: Partial locking affects configuration data but not state data.

> You call out state data; should you also call out notification and

What is the meaning of locking rpc or a notification? It does not exist for a prolonged
time, so what does it mean that it doesn't changed. I don't think this is needed.

> partial unlock operation in a more generic way.

OK

> Do you need to say something about the operating model for partial

No we want to minimize describing intended usage.

> multiple <target>s to the existing RPC.

We do not want some of the logic connected to global
locking to apply for partial-lock: e.g. special rules for handling the candidate
datastore.
As an aside:
I think there are not so many operations that we need to worry about their number.
RC4741 does not allow extending the lock requirement (We might be argument about this, but
this is the current situation.).

> These two sentences are unrelated.
OK, put them in separate paragraphs.
> this implicit behavior is a bad thing.

OK, will include reference.
NO for separate feature, because
- it is more simple and consistent if you have just one switch per box that describes the
XPath support by an agent: the XPath capability.
- notification uses the capability for the same purpose, it would be confusing to use
something differen   filtering mechanisms.
>   based on the existing XPath filtering mechanisms.

OK

> Make this part of the the first paragraph, not out on its own.
OK
> "covers" is a fuzzy word.

OK, will reword: Partial locking affects configuration data but not state data.

> You call out state data; should you also call out notification and

What is the meaning of locking rpc or a notification? It does not exist for a prolonged
time, so what does it mean that it doesn't changed. I don't think this is needed.

> partial unlock operation in a more generic way.

OK

> Do you need to say something about the operating model for partial

No we want to minimize describing intended usage.

> multiple <target>s to the existing RPC.

We do not want some of the logic connected to global
locking to apply for partial-lock: e.g. special rules for handling the candidate
datastore.
As an aside:
I think there are not so many operations that we need to worry about their number.
RC4741 does not allow extending the lock requirement (We might be argument about this, but
this is the current situation.).

> These two sentences are unrelated.
OK, put them in separate paragraphs.
> this implicit behavior is a bad thing.

OK, will include reference.
NO for separate feature, because
- it is more simple and consistent if you have just one switch per box that describes the
XPath support by an agent: the XPath capability.
- notification uses the capability for the same purpose, it would be confusing to use
something different here
- want to avoid to many options and features
- feature is a YANG construct that is not yet ready

> elements.  Same for <target>.

OK

> target to a leaf-list of enums.

We must settle on the encoding. I think both solutions are OK. Some people wanted the
target wrapper. We will ask the group.

> in section 2.0 of http://www.w3.org/TR/xpath.html).

An XPath expression can return a node set, a boolean, a
number or a string (see section 1. in http://www.w3.org/TR/xpath).
> child of a node you lock.  This may be obvious, but it needs to be

OK, make it a separte paragraph.

> s/As an example:/For example,/

OK

>>   configured in the candidate datastore, it is dangerous if it is

OK

>>   of the same sections of the management data in one or multiple

OK

> So you should only need to lock one or the other.

It is not stated anywhere that you can't have candidate and writable-running together. So
give pessimists some tools.

> "select XPath expressions" is an odd phrase.

OK I will change it to "The XPath expressions".
> more examples.  People love examples.  And parfait.

????

> it, please define it in crisp terms.

OK. Add to terms:

Scope of the lock: set of nodes returned by the XPath expressions in a successful
partial-lock operation.

> XPath expression tell you want you're trying to lock?
--------------------------------------------------------------------------------------------------------------------
>>   datastore(s) or none.  That is, if during the <partial-lock>

OK

>>   MUST unlock all parts that were previously locked during that

Keep all.

> includes anything I had locked from previous lock requests, which

Misunderstanding. It is stated that:
unlock all parts that were previously LOCKED DURING THAT OPERATION. This indicates that
only nodes locked during that same operation are involved.

> together.  

OR would return a boolean
We believe it is more clear to use multiple smaller expressions then one big one. (We had
the same argument for tang-types.)

>   in the hierarchy of nodes beneath it.

OK

> to be more strongly made).

Its needed here as well.

> s/ nil (i.e., the/empty (since/

OK

> I think with better wording you can avoid these terms).

OK rephrase.

>    all such locks are released.

Less clear, it does not state that partial locks MAY overlap.

>   locking user sufficient rights to any nodes to be locked.

OK, but is this really better?

>>      is an access control system that MAY deny or allow the partial

OK, remove one.

> Need a "Deadlock Avoidance" section.

No with don't want to make this into a tutorial.

> 
> "back down" is a fuzzy concept.  If you want want exponential back-off, say so.

OK, reword.

> s/dead-lock/deadlock/

OK

> "not addressed"?  Hogwash.  Address them.

Misunderstanding. We will rephrase. The issues ARE addressed in the following paragraphs. 
These paragraphs rather describe items that are out of scope.

> Address it:  Say "Locking does not affect read operations".

It is stated.

>>      datastore), these values SHOULD be included in the scope of the

OK

> s/SHOULD/should/ since this is not a requirement of the specification.

OK

> on configuration that is not locked.

We don't want to state more, this is not a tutorial.

> more visible to the reader (and in the table of contents).

It would be unusual to have a separate section on all the things we do not modify.

> datastores have different contents.

OK remove last sentence.

> When they want to lock multiple files, how will it work?

Take it to workgroup meeting. I don't think we want to go into (or worry about) file
locking. We will cross that bridge, when we get there.

> different topics in one paragraph is hard for the reader (and worse

I think the current text is OK

> 
> I strongly dislike the idea of reporting what was locked it here
- want to avoid to many options and features
- feature is a YANG construct that is not yet ready

> elements.  Same for <target>.

OK

> target to a leaf-list of enums.

We must settle on the encoding. I think both solutions are OK. Some people wanted the
target wrapper. We will ask the group.

> in section 2.0 of http://www.w3.org/TR/xpath.html).

An XPath expression can return a node set, a boolean, a
number or a string (see section 1. in http://www.w3.org/TR/xpath).
> child of a node you lock.  This may be obvious, but it needs to be

OK, make it a separte paragraph.

> s/As an example:/For example,/

OK

>>   configured in the candidate datastore, it is dangerous if it is

OK

>>   of the same sections of the management data in one or multiple

OK

> So you should only need to lock one or the other.

It is not stated anywhere that you can't have candidate and writable-running together. So
give pessimists some tools.

> "select XPath expressions" is an odd phrase.

OK I will change it to "The XPath expressions".
> more examples.  People love examples.  And parfait.

????

> it, please define it in crisp terms.

OK. Add to terms:

Scope of the lock: set of nodes returned by the XPath expressions in a successful
partial-lock operation.

> XPath expression tell you want you're trying to lock?
--------n the
> to the client the information the client just sent to you?

We I don't agree with these statements.  It is useful and it is handled
by the same code that already generates the error-path XPath
expressions with namespaces etc. It was asked for by other reviewers.
The actual set of nodes locked by an XpAth is
dependent on the content of the datastore. It is nice to immediately
know what did you exactly locked.

> interlacing the text with other topics.

OK separate paragraph.

> Is this done to catch errors?  I don't understand how this helps.

An expression might return a boolean which is an error. If all selects return an empty
nodeset, and we don't actually lock anything this is not success either.

> more complete definition.

OK make it a definition.
Incidentally, a YANG instance-identifier is an "Instance Identifier"
as defined in this document.

> <identifier> .vs. <select>?

No, because select usually selects a group of nodes, while identifiers must always point
out one individual node. Will be described in the definition.

> interfaces?  When do I need this?

Because it might not be trivial for all XPath expressions which nodes are exactly in their
scope.

> If global locks are the same, merge their handling into the text.

They are not the same. multiple partial locks for the same session are OK, but you can not
have a partial and a global lock side by side.

>    NETCONF client, the <error-tag> element is 'lock-denied' and..------------------------------------------------------------------------------------------------------------

OK

>>   MUST unlock all parts that were previously locked during that

Keep all.

> includes anything I had locked from previous lock requests, which

Misunderstanding. It is stated that:
unlock all parts that were previously LOCKED DURING THAT OPERATION. This indicates that
only nodes locked during that same operation are involved.

> together.  

OR would return a boolean
We believe it is more clear to use multiple smaller expressions then one big one. (We had
the same argument for tang-types.)

>   in the hierarchy of nodes beneath it.

OK

> to be more strongly made).

Its needed here as well.

> s/ nil (i.e., the/empty (since/

OK

> I think with better wording you can avoid these terms).

OK rephrase.

>    all such locks are released.

Less clear, it does not state that partial locks MAY overlap.

>   locking user sufficient rights to any nodes to be locked.

OK, but is this really better?

>>      is an access control system that MAY deny or allow the partial

OK, remove one.

>>   retry the proced.

OK will be corrected. All app-tags will have dashes.

> How can the expression return something other than a node set?

Take the xpath expression 1=1

> Show some love to your users: "invalid lock specification"

I love users, but ???????????????

>>   If access control denies the partial lock, the <error-tag> is

access-denied is used in RFC4741. We should not redefine it.

> This is a poor title.  "Locking nodes before they are created"?

OK will be changed to "Reserving nodes for future editing"
"Locking nodes before they are created" is not good because that is exactly what is not 
possible.

>   locking of newly created nodes.  To guarantee ...

No, I like this better. The content seems to be the same.

>   been sent in a previous unlock request.

OK

>>   session other than the session that owns the lock.

Same as in the case in global lock. It will be included.

> s/owns/has been granted/

OK

> "locked area" is a new term.

Is it not understandable? OK, scope of the lock will be used.

> than the existence of locked nodes.

OK, reword.

> allowed.  I know it sounds trivial to say it, but are things

What words are missing. If insert modifies data it is not allowed. We can't give a full
specification of all operations, as we will have more operations comming up e.g. YANG
defined RPCs.

>> Internet-Draft        ure after waiting some time interval.  The length of

No with don't want to make this into a tutorial.

> 
> "back down" is a fuzzy concept.  If you want want exponential back-off, say so.

OK, reword.

> s/dead-lock/deadlock/

OK

> "not addressed"?  Hogwash.  Address them.

Misunderstanding. We will rephrase. The issues ARE addressed in the following paragraphs. 
These paragraphs rather describe items that are out of scope.

> Address it:  Say "Locking does not affect read operations".

It is stated.

>>      datastore), these values SHOULD be included in the scope of the

OK

> s/SHOULD/should/ since this is not a requirement of the specification.

OK

> on configuration that is not locked.

We don't want to state more, this is not a tutorial.

> more visible to the reader (and in the table of contents).

It would be unusual to have a separate section on all the things we do not modify.

> datastores have different contents.

OK remove last sentence.

> When they want to lock multiple files, how will it work?

Take it to workgroup meeting. I don't think we want to go into (or worry about) file
locking. We will cross that bridge, when we get there.

> different topics in one paragraph is hard for the reader (and worse

I think the current text is OK

> I strongly dislike the idea of reporting what was locked in the
Partial Lock RPC for NETCONF          October 2008
> rolled back if a confirming commit is not performed within the

It might not be the best but it is very clear. A native English speaker has composed these
sentences.

> Can we please use lower case for app-tag values?

OK

> a target for partial locks.

Copy-config is valid for startup so locking is needed. If I am only interested in locking 
part of the datastore using partial lock is enough, no need for a global lock.
(It is not really stated in 4741 that startup can not be the target of edit-config.)

> exactness and understandability, you need to rethink this.

The scope of the lock is exactly specified as a list of nodes in the rpc-reply. If we
would use the xpath as the definition instead of the list of nodes, the scope could change
due to unrelated edit operations.

>> module netconf-partial-lock {

Ask work group !!!

> Should this be an identify?

I don't see the need for it. It is not handled as identity in other Netconf RFCs. It would 
be inconsistent to use identities here.
Identities don't yet exist, as yang is not complete. I was told that as yet the XSD must 
be the normative, where identities are not primary constructs.

> Add if-features for :writable-running, :candidate, and :startup.

I don't see the need to have features to duplicate the information already contained in
capabilities.

> We should have a typedef for XPath expressions (locationPath).

I would rather leave that to yang-types. Might be a good idea.

> MUST? I can do my change and then terminate my connection without

OK reword.

> This should be a typedef.

OK.

> to the client the information the client just sent to you?

We I don't agree with these statements.  It is useful and it is handled
by the same code that already generates the error-path XPath
expressions with namespaces etc. It was asked for by other reviewers.
The actual set of nodes locked by an XpAth is
dependent on the content of the datastore. It is nice to immediately
know what did you exactly locked.

> interlacing the text with other topics.

OK separate paragraph.

> Is this done to catch errors?  I don't understand how this helps.

An expression might return a boolean which is an error. If all selects return an empty
nodeset, and we don't actually lock anything this is not success either.

> more complete definition.

OK make it a definition.
Incidentally, a YANG instance-identifier is an "Instance Identifier"
as defined in this document.

> <identifier> .vs. <select>?

No, because select usually selects a group of nodes, while identifiers must always point
out one individual node. Will be described in the definition.

> interfaces?  When do I need this?

Because it might not be trivial for all XPath expressions which nodes are exactly in their
scope.

> If global locks are the same, merge their handling into the text.

They are not the same. multiple partial locks for the same session are OK, but you can not
have a partial and a global lock side by side.

> Why is this app-tag have dashes but the other's don't?  "no matches".

OK will be corrected. All app-tags will have dashes.

> How can the expression return something other than a node set?

Take the xpath expression 1=1

> Show some love to your users: "invalid lock specification"

I love users, but ???????????????

>>   If access control denies the partial lock, the <error-tag> is

access-denied is used in RFC4741. We should not redefine it.

> This is a poor title.  "Locking nodes before they are created"?

OK will be changed to "Reserving nodes for future editing"
"Locking nodes before they are created" is not good because that is exactly what is not 
possible.

>   locking of newly created nodes.  To guarantee ...

No, I like this better. The content seems to be the same.

>   been sent in a previous unlock request.

OK

>>   session other than the session that owns the lock.

Same as in the case in global lock. It will be included.

> s/owns/has been granted/

OK

> "locked area" is a new term.

Is it not understandable? OK, scope of the lock will be used.

> than the existence of locked nodes.

OK, reword.

> allowed.  I know it sounds trivial to say it, but are things

What words are missing. If insert modifies data it is not allowed. We can't give a full
specification of all operations, as we will have more operations comming up e.g. YANG
defined RPCs.

> rolled back if a confirming commit is not performed within the

It might not be the best but it is very clear. A native English speaker has composed these
sentences.

> Can we please use lower case for app-tag values?

OK

> a target for partial locks.

Copy-config is valid for startup so locking is needed. If I am only interested in locking 
part of the datastore using partial lock is enough, no need for a global lock.
(It is not really stated in 4741 that startup can not be the target of edit-config.)

> exactness and understandability, you need to rethink this.

The scope of the lock is exactly specified as a list of nodes in the rpc-reply. If we
would use the xpath as the definition instead of the list of nodes, the scope could change
due to unrelated edit operations.

>> module netconf-partial-lock {

Ask work group !!!

> Should this be an identify?

I don't see the need for it. It is not handled as identity in other Netconf RFCs. It would 
be inconsistent to use identities here.
Identities don't yet exist, as yang is not complete. I was told that as yet the XSD must 
be the normative, where identities are not primary constructs.

> Add if-features for :writable-running, :candidate, and :startup.

I don't see the need to have features to duplicate the information already contained in
capabilities.

> We should have a typedef for XPath expressions (locationPath).

I would rather leave that to yang-types. Might be a good idea.

> MUST? I can do my change and then terminate my connection without

OK reword.

> This should be a typedef.

OK.

---

From: Phil Shafer <[email-masked]>
Date: Mon, 17 Nov 2008 23:20:01 -0500

>time, so what does it mean that it doesn't changed. I don't think this is need

What is the meaning of locking state data?  Why call it out
specifically without calling out the other odd data contexts
that are not lockable?  Perhaps you should just state "Only
configuration data can be locked".

>No we want to minimize describing intenFrom [email-masked]  Mon Nov 17 20:25:44 2008
To: [email-masked]
Date: Mon, 17 Nov 2008 23:20:01 -0500
From: Phil Shafer <[email-masked]>
Cc: [email-masked]
Subject: Re: [Netconf] partial-lock feedback
Sender: [email-masked]

>time, so what does it mean that it doesn't changed. I don't think this is need

What is the meaning of locking state data?  Why call it out
specifically without calling out the other odd data contexts
that are not lockable?  Perhaps you should just state "Only
configuration data can be locked".

>No we want to minimize describingded usage.

Can you explain this?  Minimizing describing intended usage
doesn't sound like something that will help create interoperable
implementations.

>this is the current situation.).

We can certainly debate whether that is the current situation.

>number or a string (see section 1. in http://www.w3.org/TR/xpath).

True, which is why I'm suggesting you choose a more specific production
name, such as LocationPath or UnionExpr (see section 3.3 of xpath).

>give pessimists some tools.

Tools yes, but very expensive ones.

????

>only nodes locked during that same operation are involved.

That's the part that's unclear.  "previously" and "during" are in
conflict.  "previously locked" can't apply to node while the lock
operation is going on, since those nodes aren't locked until the
(atomic) lock operation completes.

>the same argument for tang-types.)

My bad.  I meant "union" not "OR".  Was thinking of the "|" operator,
which does both in XPath.

    <select> a </select>
    <select> b </select>

is identical to:

    <select> a | b </select>

>Less clear, it does not state that partial locks MAY overlap.

Keep the first sentence.  Repair the second one.

>>>   o  Any part of the scope intended usage.

Can you explain this?  Minimizing describing intended usage
doesn't sound like something that will help create interoperable
implementations.

>this is the current situation.).

We can certainly debate whether that is the current situation.

>number or a string (see section 1. in http://www.w3.org/TR/xpath).

True, which is why I'm suggesting you choose a more specific production
name, such as LocationPath or UnionExpr (see section 3.3 of xpath).

>give pessimists some tools.

Tools yes, but very expensive ones.

????

>only nodes locked during that same operation are involved.

That's the part that's unclear.  "previously" and "during" are in
conflict.  "previously locked" can't apply to node while the lock
operation is going on, since those nodes aren't locked until the
(atomic) lock operation completes.

>the same argument for tang-types.)

My bad.  I meant "union" not "OR".  Was thinking of the "|" operator,
which does both in XPath.

    <select> a </select>
    <select> b </select>

is identical to:

    <select> a | b </select>

>Less clear, it does not state that partial locks MAY overlap.

Keep the first sentence.  Repair the second one.

>>>   o  Any part of the to be locked is already locked by another

Dismisses the unrealistic case of the server not implementing
access control.

>No with don't want to make this into a tutorial.

You don't need a tutorial, but having "deadlock" show up in the
table of contents of an RFC on locking seems appropriate.  Covering
it in a section on New Operations isn't.

>These paragraphs rather describe items that are out of scope.

Please rephrase this, since it's very confusing.  Even your
"items that are out of scope" comment here seems to say that
you are not addressing these points.

>We don't want to state more, this is not a tutorial.

I don't think this turns your draft into a tutorial.

>It would be unusual to have a separate section on all the things we do not mod

But you _did_ change it.  Global locks do not operate the same way.
If some party has a partial lock, requests for the global lock will
fail.

>locking. We will cross that bridge, when we get there.

I'm clearly not saying you need to cross that bridge, just that
you should spend some time thinking about future evolution of
netconf.

>I think the current text is OK

OK, but not good.  In technical writing, you want the topic sentence
to contain a paragraph's main idea.  The rest of the paragraph
should support and develop that idea with related details.  If the
reader sees a topic sentence of "The nodes that matched the select
expression and are locked are reported in the rpc-reply message.",
they will expect the rest of the paragraph to be related, but "Note
that if some of the requested nodes exist only in one of the
datastores, the lock is granted on different nodes in different
datastores." isn't related.

>know what did you exactly locked.

"nice" depends on the database.  If you lock 90,000 things, having
someone tell you is definitely not nice.  Even if you lock one or
two things, you know what you locked and don't need someone to tell
you.

>> Is it worthwhile to put XPath expressions into a distinct argume scope to be locked is already locked by another

Dismisses the unrealistic case of the server not implementing
access control.

>No with don't want to make this into a tutorial.

You don't need a tutorial, but having "deadlock" show up in the
table of contents of an RFC on locking seems appropriate.  Covering
it in a section on New Operations isn't.

>These paragraphs rather describe items that are out of scope.

Please rephrase this, since it's very confusing.  Even your
"items that are out of scope" comment here seems to say that
you are not addressing these points.

>We don't want to state more, this is not a tutorial.

I don't think this turns your draft into a tutorial.

>It would be unusual to have a separate section on all the things we do not mod

But you _did_ change it.  Global locks do not operate the same way.
If some party has a partial lock, requests for the global lock will
fail.

>locking. We will cross that bridge, when we get there.

I'm clearly not saying you need to cross that bridge, just that
you should spend some time thinking about future evolution of
netconf.

>I think the current text is OK

OK, but not good.  In technical writing, you want the topic sentence
to contain a paragraph's main idea.  The rest of the paragraph
should support and develop that idea with related details.  If the
reader sees a topic sentence of "The nodes that matched the select
expression and are locked are reported in the rpc-reply message.",
they will expect the rest of the paragraph to be related, but "Note
that if some of the requested nodes exist only in one of the
datastores, the lock is granted on different nodes in different
datastores." isn't related.

>know what did you exactly locked.

"nice" depends on the database.  If you lock 90,000 things, having
someone tell you is definitely not nice.  Even if you lock one or
two things, you know what you locked and don't need someone to tell
you.

>> Is it worthwhile to put XPath expressions into a distinct nt?

This is more to my point.  The XPath select and the non-XPath
instance identifier are different datatypes.  In YANG, one
would be w3c:UnionExpr and the other yang:instance-identifier
(or something like that).  Where's the win of putting them in
the same argument?

>Because it might not be trivial for all XPath expressions which nodes are exac

If you wanted this, could you add another RPC, like

   rpc get-partial-lock-information {
       input {
           leaf lockId { ... }
       }
       output {
           container partial-lock-information {
               list lockedNode {
                    ...
               }
           }
       }
   }

Don't put the price of this not-so-nice behavior on every locker.

>I love users, but ???????????????

Sorry, I didn't make my point clearly.  I make the assumption
that error messages will most likely be displayed directly to
the users of applications.  Choosing text that will be lucid
to non-developers reduces the shock to the user.

>No, I like this better. The content seems to be the same.

I find "reserve them for future use" confusing.  I was trying to
suggest an alternative, but anything that avoids this phrase
would be better.

>It might not be the best but it is very clear. A native English speaker has co

I do not think this is an English issue but one of text organization.
Bulleting here isn't necessary.

>part of the datastore using partial lock is enough, no need for a global lock.

But the startup datastore isn't required to be parsable/addressable.
It's a blob with no internal visibility.  It's data in a SRAM, not
something you can run XPaths expression on.

>(It is not really stated in 4741 that startup can not be the target of edit-co

8.7.5.1 lists the set of operations for which <startup/> is an
allowed target.  <edit-config> is not among them.  This choice was
intentional, since startup config is not required to be in memory,
addressable, parsed, open, searchable, or available for XPath
expressions.  It's typically a section of SRAM or flash; you can
read it or write it, but that's about it.

>Aargument?

This is more to my point.  The XPath select and the non-XPath
instance identifier are different datatypes.  In YANG, one
would be w3c:UnionExpr and the other yang:instance-identifier
(or something like that).  Where's the win of putting them in
the same argument?

>Because it might not be trivial for all XPath expressions which nodes are exac

If you wanted this, could you add another RPC, like

   rpc get-partial-lock-information {
       input {
           leaf lockId { ... }
       }
       output {
           container partial-lock-information {
               list lockedNode {
                    ...
               }
           }
       }
   }

Don't put the price of this not-so-nice behavior on every locker.

>I love users, but ???????????????

Sorry, I didn't make my point clearly.  I make the assumption
that error messages will most likely be displayed directly to
the users of applications.  Choosing text that will be lucid
to non-developers reduces the shock to the user.

>No, I like this better. The content seems to be the same.

I find "reserve them for future use" confusing.  I was trying to
suggest an alternative, but anything that avoids this phrase
would be better.

>It might not be the best but it is very clear. A native English speaker has co

I do not think this is an English issue but one of text organization.
Bulleting here isn't necessary.

>part of the datastore using partial lock is enough, no need for a global lock.

But the startup datastore isn't required to be parsable/addressable.
It's a blob with no internal visibility.  It's data in a SRAM, not
something you can run XPaths expression on.

>(It is not really stated in 4741 that startup can not be the target of edit-co

8.7.5.1 lists the set of operations for which <startup/> is an
allowed target.  <edit-config> is not among them.  This choice was
intentional, since startup config is not required to be in memory,
addressable, parsed, open, searchable, or available for XPath
expressions.  It's typically a section of SRAM or flash; you can
read it or write it, but that's about it.

>> s/netconf/ietf-netcosk work group !!!

Isn't that what review comments do?  Do I need to open formal issues
for items that need to be addressed?

Thanks,
 Phil

nf/
>Ask work group !!!

Isn't that what review comments do?  Do I need to open formal issues
for items that need to be addressed?

Thanks,
 Phil

---

From: Balazs Lengyel <[email-masked]>
Date: Thu, 20 Nov 2008 21:29:42 +0100

Hello Phil,
Some further answers. See below!
Balazs

OK.

> doesn't sound like something that will help create interoperable

OK, will try to add usage scenarios. 
---------------------------------------------------------------------------

>>> XPath expression tell you want you're trying to lock?

OK, merged the two related paragraphs and added an example.

> (atomic) lock operation completes.

OK, reworded

> which does both in XPath.

Yes, but it is often easier to break potentially long expressions into multiple smaller
ones. However the user can still use the union if he prefers to.

> Keep the first sentence.  Repair the second one.

Reworded paragraph.

> Dismisses the unrealistic case of the server not implementing

I personally agree with you, but I was explicitly told by more then one person, not to
assume that access control is always present.

>> No with don't want to make this into a tutorial.

OK, made it a separate chapter.

> you are not addressing these points.

OK rephrased, hope its good.

> If some party has a partial lock, requests for the global lock will

That is already stated in 4741:
"An attempt to lock the configuration MUST fail if an existing
       session or other entity holds a lock on any portion of the lock
       target."

> you should spend some time thinking about future evolution of

OK, done

> that if some of the requested nodes exist only in From [email-masked]  Thu Nov 20 12:29:35 2008
Date: Thu, 20 Nov 2008 21:29:42 +0100
From: Balazs Lengyel <[email-masked]>
To: Phil Shafer <[email-masked]>
Cc: [email-masked]
Subject: Re: [Netconf] partial-lock feedback
Reply-To: [email-masked]
Sender: [email-masked]

Hello Phil,
Some further answers. See below!
Balazs

OK.

>> No we want to minimize one of the

OK reworded.

> two things, you know what you locked and don't need someone to tell

As only the top node of each subtree is locked, locking 90.000 things would be extreme. 
That needs a list with 90.000 entries without a containing container/list.

> suggest an alternative, but anything that avoids this phrase

OK. reworded.

> 
>>> If the NETCONF server supports the :confirmed-commit capabidescribing intended usage.
> 

OK, will try to add usage scenarios. 
---------------------------------------------------------------------------

>>> XPath expression tell you want you're trying to lock?

OK, merged the two related paragraphs and added an example.

> (atomic) lock operation completes.

OK, reworded

> which does both in XPath.

Yes, but it is often easier to break potentially long expressions into multiple smaller
ones. However the user can still use the union if he prefers to.

>>>>   locks overlaps, nodes in the common arelity and a

- even if only copy-config is supported locking is relevant.
- the text of 4741 states what must be supported, but it says nothing about edit-config.

> for items that need to be addressed?

OK, will have ietf

a MUST be locked until all of
> Keep the first sentence.  Repair the second one.

Reworded paragraph.

> Dismisses the unrealistic case of the server not implementing

I personally agree with you, but I was explicitly told by more then one person, not to
assume that access control is always present.

>> No with don't want to make this into a tutorial.

OK, made it a separate chapter.

> you are not addressing these points.

OK rephrased, hope its good.

> If some party has a partial lock, requests for the global lock will

That is already stated in 4741:
"An attempt to lock the configuration MUST fail if an existing
       session or other entity holds a lock on any portion of the lock
       target."

> you should spend some time thinking about future evolution of

OK, done

> datastores." isn't related.

OK reworded.

> two things, you know what you locked and don't need someone to tell

As only the top node of each subtree is locked, locking 90.000 things would be extreme. 
That needs a list with 90.000 entries without a containing container/list.

> suggest an alternative, but anything that avoids this phrase

OK. reworded.

> read it or write it, but that's about it.

- even if only copy-config is supported locking is relevant.
- the text of 4741 states what must be supported, but it says nothing about edit-config.

> for items that need to be addressed?

OK, will have ietf