Re: [PATCH v2 00/17] live update and gnttab patches

[Edwin Torok <edvin.torok@xxxxxxxxxx>]

On Tue, 2021-05-11 at 21:05 +0100, Andrew Cooper wrote:
> On 11/05/2021 19:05, Edwin Török wrote:
> > These patches have been posted previously.
> > The gnttab patches (tools/ocaml/libs/mmap) were not applied at the
> > time
> > to avoid conflicts with an in-progress XSA.
> > The binary format live-update and fuzzing patches were not applied
> > because it was too close to the next Xen release freeze.
> > 
> > The patches depend on each-other: live-update only works correctly
> > when the gnttab
> > patches are taken too (MFN is not part of the binary live-update
> > stream),
> > so they are included here as a single series.
> > The gnttab patches replaces one use of libxenctrl with stable
> > interfaces, leaving one unstable
> > libxenctrl interface used by oxenstored.
> > 
> > The 'vendor external dependencies' may be optional, it is useful to
> > be part
> > of a patchqueue in a specfile so that you can build everything
> > without external dependencies,
> > but might as well commit it so everyone has it easily available not
> > just XenServer.
> > 
> > Note that the live-update fuzz test doesn't yet pass, it is still
> > able to find bugs.
> > However the reduced version with a fixed seed used as a unit test
> > does pass,
> > so it is useful to have it committed, and further improvements can
> > be made later
> > as more bugs are discovered and fixed.
> > 
> > Edwin Török (17):
> >   docs/designs/xenstore-migration.md: clarify that deletes are
> > recursive
> >   tools/ocaml: add unit test skeleton with Dune build system
> >   tools/ocaml: vendor external dependencies for convenience
> >   tools/ocaml/xenstored: implement the live migration binary format
> >   tools/ocaml/xenstored: add binary dump format support
> >   tools/ocaml/xenstored: add support for binary format
> >   tools/ocaml/xenstored: validate config file before live update
> >   Add structured fuzzing unit test
> >   tools/ocaml: use common macros for manipulating mmap_interface
> >   tools/ocaml/libs/mmap: allocate correct number of bytes
> >   tools/ocaml/libs/mmap: Expose stub_mmap_alloc
> >   tools/ocaml/libs/mmap: mark mmap/munmap as blocking
> >   tools/ocaml/libs/xb: import gnttab stubs from mirage
> >   tools/ocaml: safer Xenmmap interface
> >   tools/ocaml/xenstored: use gnttab instead of xenctrl's
> >     foreign_map_range
> >   tools/ocaml/xenstored: don't store domU's mfn of ring page
> >   tools/ocaml/libs/mmap: Clean up unused read/write
> 
> Gitlab CI reports failures across the board in Debian Stretch 32-bit
> builds.  All logs
> https://gitlab.com/xen-project/patchew/xen/-/pipelines/301146112 but
> the
> tl;dr seems to be:
> 
> File "disk.ml", line 179, characters 26-37:
> Error: Integer literal exceeds the range of representable integers of
> type int

Thanks, this should fix it, I refreshed my git tree (there is also a
fix there for the older version of Make):
https://gitlab.com/xen-project/patchew/xen/-/pipelines/301146112

Not sure whether it is worth continuing to support 32-bit i686 builds,
any modern Intel/AMD CPU would be 64-bit capable, but perhaps 32-bit is
still popular in the ARM world and keeping 32-bit Intel supported is
the easiest way to build-test it?

diff --git a/tools/ocaml/xenstored/disk.ml
b/tools/ocaml/xenstored/disk.ml
index 59794324e1..b7678af87f 100644
--- a/tools/ocaml/xenstored/disk.ml
+++ b/tools/ocaml/xenstored/disk.ml
@@ -176,7 +176,7 @@ let write store =
            output_byte ch i
 
        let w32 ch v =
-           assert (v >= 0 && v <= 0xFFFF_FFFF);
+           assert (v >= 0 && Int64.of_int v <= 0xFFFF_FFFFL);
            output_binary_int ch v
 
        let pos = pos_out
@@ -213,7 +213,7 @@ let write store =
 
        let r32 t =
            (* read unsigned 32-bit int *)
-           let r = input_binary_int t land 0xFFFF_FFFF in
+           let r = Int64.logand (Int64.of_int (input_binary_int t))
0xFFFF_FFFFL |> Int64.to_int in
            assert (r >= 0);
            r
 
@@ -293,7 +293,7 @@ module LiveRecord = struct
        write_record t Type.global_data 8 @@ fun b ->
        O.w32 b (FD.to_int rw_sock);
                 (* TODO: this needs a unit test/live update test too!
*)
-       O.w32 b 0xFFFF_FFFF
+       O.w32 b 0x3FFF_FFFF
 
    let read_global_data t ~len f =
        read_expect t "global_data" 8 len;
> 
> ~Andrew