Skip to content

Conversation

robhoes
Copy link
Member

@robhoes robhoes commented May 18, 2023

This adds a mode to vhd-tool to allow sparse VDI exports to a VHD-formatted stream, where the input comes from an NBD block device as created by nbd-client. Sparseness data is taken directly from the NBD server through a helper program.

This is one step towards our goal of switching away from using blktap2 kernel devices, which are not part of the upstream Linux kernel. A future improvement is to no longer rely on nbd-client and kernel block devices at all, but use the NBD protocol directly, in userspace.

This PR "vendors" https://github.com/mirage/ocaml-vhd into our source tree to work on it more efficiently. Patches should be upstreamed later.

I recommend reviewing commit-by-commit.

robhoes added 2 commits May 17, 2023 15:45
git-subtree-dir: ocaml/libs/vhd
git-subtree-split: 6f3f75bc881c6f22117e01aa8520fa70564fde42
This makes it easier to add functionality to this library and use it in
vhd-tool.

This places commit 6f3f75bc8 from github.com:mirage/ocaml-vhd into the
directory ocaml/libs/vhd.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
robhoes added 9 commits May 18, 2023 11:30
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Cstruct.len is deprecated.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
The function `vhd_from_raw` creates a sparse VHD file from an input
stream plus a function `include_block` that indicates which blocks have
data. The current `include_block` is specific to raw file input. We can
now reuse `vhd_from_raw` for input streams that require different ways
to detect sparseness.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
The function `vhd_from_raw` takes a `input_block` argument, which is a
function that takes a VHD block index and returns a Boolean to indicate
whether the block has data or not. It can be impractical and inefficient
for the caller to provide such a "random access" function, where it may
need to either scan the input multiple times, or keep a large
quick-access data structure in memory.

Instead, `vhd_from_raw` is changes to take a function
`find_data_blocks`, which simply returns a list of the VHD block indices
that have data. These indices are expected to be in accending order,
such that they can be iterated over in an efficient way to construct the
VHD stream.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
@psafont
Copy link
Member

psafont commented May 18, 2023

The initial vhd fixes were also available here: mirage/ocaml-vhd#77 :)

@psafont
Copy link
Member

psafont commented May 18, 2023

I heard about the code passing some import/export tests. Are these in place in the repo? I don't see any test being added to exercise the code.

I'm a bit worried that upstream is unresponsive to simple fixes, it will make pushing these refactorings quite difficult

@robhoes
Copy link
Member Author

robhoes commented May 18, 2023

The export/import tests are part of quicktest. In particular:

  • ocaml/quicktest/quicktest_max_vdi_size.ml
  • ocaml/quicktest/quicktest_vdi_ops_data_integrity.ml

This new code is not exercised by xapi right now, but I have to further changes to make it do that (that will be the next PR). I have run quicktest with those changes together, and all tests pass.

@robhoes
Copy link
Member Author

robhoes commented May 18, 2023

The DCO error is due to the "vendoring" using git subtree. I can't easily change the commit for it I think.

@psafont
Copy link
Member

psafont commented May 18, 2023

It's not worth changing the commit, we can override the DCO check before merging

exception Not_sector_aligned of int64

let assert_sector_aligned n =
if Int64.(mul(div n 512L) 512L) <> n then begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to test is to check the lower 9 bits are zero: n land 0b1_1111_1111 = 0

robhoes added 3 commits May 25, 2023 13:03
The function `Nbd_input.vhd` takes a raw data stream, typically from a
`/dev/nbdX` device as provided by nbd-client, plus NBD server and export
details, and outputs a sparse VHD stream.

Just like the existing `Nbd_input.raw`, it uses a Python helper script
to find the file extents with associated flags that indicate whether an
extent has data or not. The extent data is used to construct a list of
VHD data-block indices, for use by the ocaml-vhd library call to create
the actual stream.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
@robhoes robhoes merged commit 31992ab into xapi-project:master May 25, 2023
@robhoes robhoes deleted the nbd-vhd-export branch May 25, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants