[PATCH v1 0/1] oxenstored: fix ABI breakage in reset watches

[PATCH v1 0/1] oxenstored: fix ABI breakage in reset watches

[Edwin Török]

dbc84d2983969bb47d294131ed9e6bbbdc2aec49 (Xen >= 4.9.0) deleted XS_RESTRICT
from oxenstored, which caused all the following opcodes to be shifted by 1,
breaking the ABI compared to the C version and guests.

The affected opcode is 'reset watches', e.g. Linux uses this during kexec if a suitable
control/platform-feature-xs_reset_watches  field is present in xenstore.

Edwin Török (1):
  oxenstored: fix ABI breakage introduced in Xen 4.9.0

 tools/ocaml/libs/xb/op.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1

[PATCH v1 1/1] oxenstored: fix ABI breakage introduced in Xen 4.9.0

[Edwin Török]

dbc84d2983969bb47d294131ed9e6bbbdc2aec49 (Xen >= 4.9.0) deleted XS_RESTRICT
from oxenstored, which caused all the following opcodes to be shifted by 1:
reset_watches became off-by-one compared to the C version of xenstored.

Looking at the C code the opcode for reset watches needs:
XS_RESET_WATCHES = XS_SET_TARGET + 2

So add the placeholder `Invalid` in the OCaml<->C mapping list.
(Note that the code here doesn't simply convert the OCaml constructor to
 an integer, so we don't need to introduce a dummy constructor).

Igor says that with a suitably patched xenopsd to enable watch reset,
we now see `reset watches` during kdump of a guest in xenstored-access.log.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Tested-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
 tools/ocaml/libs/xb/op.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
index d4f1f08185..9bcab0f38c 100644
--- a/tools/ocaml/libs/xb/op.ml
+++ b/tools/ocaml/libs/xb/op.ml
@@ -28,7 +28,7 @@ let operation_c_mapping =
            Transaction_end; Introduce; Release;
            Getdomainpath; Write; Mkdir; Rm;
            Setperms; Watchevent; Error; Isintroduced;
-           Resume; Set_target; Reset_watches |]
+           Resume; Set_target; Invalid; Reset_watches |]
 let size = Array.length operation_c_mapping
 
 let array_search el a =
-- 
2.25.1

Re: [PATCH v1 1/1] oxenstored: fix ABI breakage introduced in Xen 4.9.0

[Christian Lindig]

________________________________________
From: Edwin Török <edvin.torok@citrix.com>
Sent: 15 July 2020 16:10
To: xen-devel@lists.xenproject.org
Cc: Edwin Torok; Christian Lindig; David Scott; Ian Jackson; Wei Liu; Igor Druzhinin
Subject: [PATCH v1 1/1] oxenstored: fix ABI breakage introduced in Xen 4.9.0

dbc84d2983969bb47d294131ed9e6bbbdc2aec49 (Xen >= 4.9.0) deleted XS_RESTRICT
from oxenstored, which caused all the following opcodes to be shifted by 1:
reset_watches became off-by-one compared to the C version of xenstored.

Looking at the C code the opcode for reset watches needs:
XS_RESET_WATCHES = XS_SET_TARGET + 2

So add the placeholder `Invalid` in the OCaml<->C mapping list.
(Note that the code here doesn't simply convert the OCaml constructor to
 an integer, so we don't need to introduce a dummy constructor).

Igor says that with a suitably patched xenopsd to enable watch reset,
we now see `reset watches` during kdump of a guest in xenstored-access.log.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Tested-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
 tools/ocaml/libs/xb/op.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
index d4f1f08185..9bcab0f38c 100644
--- a/tools/ocaml/libs/xb/op.ml
+++ b/tools/ocaml/libs/xb/op.ml
@@ -28,7 +28,7 @@ let operation_c_mapping =
            Transaction_end; Introduce; Release;
            Getdomainpath; Write; Mkdir; Rm;
            Setperms; Watchevent; Error; Isintroduced;
-           Resume; Set_target; Reset_watches |]
+           Resume; Set_target; Invalid; Reset_watches |]
 let size = Array.length operation_c_mapping

 let array_search el a =
--
2.25.1

-- 
Acked-by: Christian Lindig <christian.lindig@citrix.com>

Re: [PATCH v1 1/1] oxenstored: fix ABI breakage introduced in Xen 4.9.0

[Wei Liu]

On Wed, Jul 15, 2020 at 03:21:50PM +0000, Christian Lindig wrote:
> 
> ________________________________________
> From: Edwin Török <edvin.torok@citrix.com>
> Sent: 15 July 2020 16:10
> To: xen-devel@lists.xenproject.org
> Cc: Edwin Torok; Christian Lindig; David Scott; Ian Jackson; Wei Liu; Igor Druzhinin
> Subject: [PATCH v1 1/1] oxenstored: fix ABI breakage introduced in Xen 4.9.0
> 
> dbc84d2983969bb47d294131ed9e6bbbdc2aec49 (Xen >= 4.9.0) deleted XS_RESTRICT
> from oxenstored, which caused all the following opcodes to be shifted by 1:
> reset_watches became off-by-one compared to the C version of xenstored.
> 

I guess this needs

Backport: 4.9+

(Ian FYI)

> Looking at the C code the opcode for reset watches needs:
> XS_RESET_WATCHES = XS_SET_TARGET + 2
> 
> So add the placeholder `Invalid` in the OCaml<->C mapping list.
> (Note that the code here doesn't simply convert the OCaml constructor to
>  an integer, so we don't need to introduce a dummy constructor).
> 
> Igor says that with a suitably patched xenopsd to enable watch reset,
> we now see `reset watches` during kdump of a guest in xenstored-access.log.
> 
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>
> Tested-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
>  tools/ocaml/libs/xb/op.ml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
> index d4f1f08185..9bcab0f38c 100644
> --- a/tools/ocaml/libs/xb/op.ml
> +++ b/tools/ocaml/libs/xb/op.ml
> @@ -28,7 +28,7 @@ let operation_c_mapping =
>             Transaction_end; Introduce; Release;
>             Getdomainpath; Write; Mkdir; Rm;
>             Setperms; Watchevent; Error; Isintroduced;
> -           Resume; Set_target; Reset_watches |]
> +           Resume; Set_target; Invalid; Reset_watches |]
>  let size = Array.length operation_c_mapping
> 
>  let array_search el a =
> --
> 2.25.1
> 
> -- 
> Acked-by: Christian Lindig <christian.lindig@citrix.com>