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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/05/2021 11:10, Edwin Torok wrote:
> 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?

Yes - arm32 is very much a thing, and currently 32bit userspace on x86
is a supported configuration.

>
> 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);

In the case that v is 32 bits wide, it will underflow and fail the v >=
0 check, before the upcast to Int64.

~Andrew