-
Notifications
You must be signed in to change notification settings - Fork 301
Optimize data cluster format for VHD and QCOW #6895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1aa52ef
627f321
fa4f516
6a31727
5457512
1fc8ee7
a52b126
f9db34b
3f6f6dd
829c018
15afce2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -506,6 +506,7 @@ | |
| cohttp | ||
| cohttp-lwt | ||
| conf-libssl | ||
| (conf-qemu-img :with-test) | ||
| (cstruct | ||
| (>= "3.0.0")) | ||
| (ezxenstore | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2903,24 +2903,10 @@ functor | |
|
|
||
| let raw ?from (vhd : fd Vhd.t) = raw_common ?from vhd | ||
|
|
||
| let vhd_blocks_to_json (t : fd Vhd.t) = | ||
| let vhd_blocks_to_json_aux (t : fd Vhd.t) blocks = | ||
| let block_size_sectors_shift = | ||
| t.Vhd.header.Header.block_size_sectors_shift | ||
| in | ||
| let max_table_entries = Vhd.used_max_table_entries t in | ||
|
|
||
| let include_block = include_block None t in | ||
|
|
||
| let blocks = | ||
| Seq.init max_table_entries Fun.id | ||
| |> Seq.filter_map (fun i -> | ||
| if include_block i then | ||
| Some (`Int i) | ||
| else | ||
| None | ||
| ) | ||
| |> List.of_seq | ||
| in | ||
| let json = | ||
| `Assoc | ||
| [ | ||
|
|
@@ -2934,6 +2920,52 @@ functor | |
| let json_string = Yojson.to_string json in | ||
| print_string json_string ; return () | ||
|
|
||
| let vhd_blocks_to_json (t : fd Vhd.t) = | ||
| let max_table_entries = Vhd.used_max_table_entries t in | ||
| let blocks = | ||
| Seq.init max_table_entries Fun.id | ||
| |> Seq.filter_map (fun i -> | ||
| if include_block None t i then | ||
| Some (`Int i) | ||
| else | ||
| None | ||
| ) | ||
| |> List.of_seq | ||
| in | ||
| vhd_blocks_to_json_aux t blocks | ||
|
|
||
| let vhd_blocks_to_json_interval (t : fd Vhd.t) = | ||
| let max_table_entries = Vhd.used_max_table_entries t in | ||
| let blocks, last_block = | ||
| Seq.init max_table_entries Fun.id | ||
| |> Seq.fold_left | ||
| (fun (acc, left_block) i -> | ||
| if include_block None t i then | ||
| match left_block with | ||
| | Some _ -> | ||
| (acc, left_block) | ||
| | None -> | ||
| (acc, Some i) | ||
| else | ||
| match left_block with | ||
| | Some x -> | ||
| (`List [`Int x; `Int (i - 1)] :: acc, None) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the JSON our own design or do we need to conform to something? If it's our own choice, I would prefer an object with two entries over a list if this JSON is consumed by software. If it's just for display and never parsed it's fine to do it this way because it is more compact.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is our design, it's parsed in |
||
| | None -> | ||
| (acc, None) | ||
| ) | ||
| ([], None) | ||
| in | ||
| (* Close off the interval we were tracking we ran off the end of the seq *) | ||
| let blocks = | ||
| match last_block with | ||
| | Some x -> | ||
| `List [`Int x; `Int (max_table_entries - 1)] :: blocks | ||
| | None -> | ||
| blocks | ||
| in | ||
| let blocks = List.rev blocks in | ||
| vhd_blocks_to_json_aux t blocks | ||
|
|
||
| let vhd_common ?from ?raw ?(emit_batmap = false) (t : fd Vhd.t) = | ||
| let block_size_sectors_shift = | ||
| t.Vhd.header.Header.block_size_sectors_shift | ||
|
|
@@ -3173,6 +3205,8 @@ functor | |
| Vhd_input.vhd_common ?from ~raw vhd | ||
|
|
||
| let blocks_json = Vhd_input.vhd_blocks_to_json | ||
|
|
||
| let blocks_json_interval = Vhd_input.vhd_blocks_to_json_interval | ||
| end | ||
|
|
||
| (* Create a VHD stream from data on t, using `include_block` guide us which blocks have data *) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| Create a sparse raw file | ||
| $ dd if=/dev/random of=test.raw bs=2097152 count=16 > /dev/null 2>&1 | ||
| $ dd if=/dev/zero of=test.raw bs=2097152 count=10 seek=3 conv=notrunc > /dev/null 2>&1 | ||
|
|
||
| Convert to .vhd | ||
| $ qemu-img convert -f raw -O vpc test.raw test.vhd | ||
|
psafont marked this conversation as resolved.
|
||
|
|
||
| Check if the right clusters are found to be allocated (legacy JSON format) | ||
| $ ./main.exe read_headers test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[0,1,2,13,14,15]} | ||
|
|
||
| Check if the right clusters are found to be allocated (interval-based JSON format) | ||
| $ ./main.exe read_headers_interval test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[[0,2],[13,15]]} | ||
|
|
||
| Check fully allocated file | ||
| $ dd if=/dev/random of=test.raw bs=2097152 count=16 > /dev/null 2>&1 | ||
| $ qemu-img convert -f raw -O vpc test.raw test.vhd | ||
| $ ./main.exe read_headers test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]} | ||
| $ ./main.exe read_headers_interval test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[[0,15]]} | ||
|
|
||
| Check empty file | ||
| $ dd if=/dev/zero of=test.raw bs=2097152 count=16 > /dev/null 2>&1 | ||
| $ qemu-img convert -f raw -O vpc test.raw test.vhd | ||
| $ ./main.exe read_headers test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[]} | ||
| $ ./main.exe read_headers_interval test.vhd | ||
| {"virtual_size":33562624,"cluster_bits":21,"data_clusters":[]} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no conf package for qemu-img, even though for some reason I thought it existed.
Another way of configuring it is:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it last year, seems to still be there: https://github.com/ocaml/opam-repository/blob/master/packages/conf-qemu-img/conf-qemu-img.1/opam
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it in xs-opam, though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it in xapi-project/xs-opam#755
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this waiting for a review from Edwin, then? I think it was him that raised concerns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, CC @edwintorok