[PATCH v2 3/8] docs/designs/xenstore-migration.md: clarify that deletes are recursive

2021-01-15 Thread Edwin Török
Signed-off-by: Edwin Török 
---
Changed since V1:
* post publicly now that the XSA is out
---
 docs/designs/xenstore-migration.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/designs/xenstore-migration.md 
b/docs/designs/xenstore-migration.md
index 2ce2c836f5..f44bc0c61d 100644
--- a/docs/designs/xenstore-migration.md
+++ b/docs/designs/xenstore-migration.md
@@ -365,7 +365,8 @@ record previously present).
 |  | 0x0001: read   |
 |  | 0x0002: written|
 |  ||
-|  | The value will be zero for a deleted node  |
+|  | The value will be zero for a recursively   |
+|  | deleted node   |
 |  ||
 | `perm-count` | The number (N) of node permission specifiers   |
 |  | (which will be 0 for a node deleted in a   |
-- 
2.29.2




Re: [PATCH v2 3/8] docs/designs/xenstore-migration.md: clarify that deletes are recursive

2021-01-22 Thread Jürgen Groß

On 15.01.21 23:28, Edwin Török wrote:

Signed-off-by: Edwin Török 
---
Changed since V1:
* post publicly now that the XSA is out
---
  docs/designs/xenstore-migration.md | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/designs/xenstore-migration.md 
b/docs/designs/xenstore-migration.md
index 2ce2c836f5..f44bc0c61d 100644
--- a/docs/designs/xenstore-migration.md
+++ b/docs/designs/xenstore-migration.md
@@ -365,7 +365,8 @@ record previously present).
  |  | 0x0001: read   |
  |  | 0x0002: written|
  |  ||
-|  | The value will be zero for a deleted node  |
+|  | The value will be zero for a recursively   |
+|  | deleted node   |


I don't see the value in this modification.

The wording is ambiguous: is the value zero only for nodes which were
deleted due to recursion, or do you mean deletes are recursive?

Per docs/misc/xenstore.txt all deletes are recursive, so there is no
need to repeat that here. And a zero value only for the recursions makes
no sense at all.

So I'd nack this patch.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH v2 3/8] docs/designs/xenstore-migration.md: clarify that deletes are recursive

2021-01-22 Thread Edwin Torok
On Fri, 2021-01-22 at 14:04 +0100, Jürgen Groß wrote:
> On 15.01.21 23:28, Edwin Török wrote:
> > Signed-off-by: Edwin Török 
> > ---
> > Changed since V1:
> > * post publicly now that the XSA is out
> > ---
> >   docs/designs/xenstore-migration.md | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/designs/xenstore-migration.md
> > b/docs/designs/xenstore-migration.md
> > index 2ce2c836f5..f44bc0c61d 100644
> > --- a/docs/designs/xenstore-migration.md
> > +++ b/docs/designs/xenstore-migration.md
> > @@ -365,7 +365,8 @@ record previously present).
> >   |  | 0x0001: read   |
> >   |  | 0x0002: written    |
> >   |  |    |
> > -|  | The value will be zero for a deleted node  |
> > +|  | The value will be zero for a recursively   |
> > +|  | deleted node   |
> 
> I don't see the value in this modification.
> 
> The wording is ambiguous: is the value zero only for nodes which were
> deleted due to recursion, or do you mean deletes are recursive?

I was looking at this from the point of view of generating the diff,
where you can optimize and reduce the size of the diff if you notice
that it is sufficient to add a record only for the topmost entry when
the entire subtree is deleted.

You are right that looking at it from the point of view of applying the
transaction record you would reuse the existing delete implementation
which is already recursive.

> 
> Per docs/misc/xenstore.txt all deletes are recursive, so there is no
> need to repeat that here. And a zero value only for the recursions
> makes
> no sense at all.
> 
> So I'd nack this patch.

We can drop it.
--Edwin
> 
> Juergen