| Message ID | 9f3823a494bd512348812355fbfecf6be447aca0.1610748224.git.edvin.torok@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | oxenstored build enhancements | expand |
On 15.01.21 23:28, Edwin Török wrote: > Signed-off-by: Edwin Török <edvin.torok@citrix.com> > --- > 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
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 <edvin.torok@citrix.com> > > --- > > 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
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 |
Signed-off-by: Edwin Török <edvin.torok@citrix.com> --- Changed since V1: * post publicly now that the XSA is out --- docs/designs/xenstore-migration.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)