-
Notifications
You must be signed in to change notification settings - Fork 292
Support qcow2 format in VDI export/import #6396
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 1 commit
0647d38
c2621a6
9bd40d4
def1cf6
81232c5
05b56c4
2f6c847
ecc0ee1
15f0f6e
d77adf5
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 |
---|---|---|
@@ -0,0 +1,60 @@ | ||
(* | ||
* Copyright (C) 2025 Vates. | ||
* | ||
* This program is free software; you can redistribute it and/or modify | ||
* it under the terms of the GNU Lesser General Public License as published | ||
* by the Free Software Foundation; version 2.1 only. with the special | ||
* exception on linking described in file LICENSE. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Lesser General Public License for more details. | ||
*) | ||
|
||
open Xapi_stdext_std.Xstringext | ||
|
||
(** [find_backend_device path] returns [Some path'] where [path'] is the backend path in | ||
the driver domain corresponding to the frontend device [path] in this domain. *) | ||
let find_backend_device path = | ||
try | ||
let open Ezxenstore_core.Xenstore in | ||
(* If we're looking at a xen frontend device, see if the backend | ||
is in the same domain. If so check if it looks like a .vhd *) | ||
let rdev = (Unix.stat path).Unix.st_rdev in | ||
let major = rdev / 256 and minor = rdev mod 256 in | ||
let link = | ||
Unix.readlink (Printf.sprintf "/sys/dev/block/%d:%d/device" major minor) | ||
in | ||
match List.rev (String.split '/' link) with | ||
| id :: "xen" :: "devices" :: _ | ||
when Astring.String.is_prefix ~affix:"vbd-" id -> | ||
let id = int_of_string (String.sub id 4 (String.length id - 4)) in | ||
with_xs (fun xs -> | ||
let self = xs.Xs.read "domid" in | ||
let backend = | ||
xs.Xs.read (Printf.sprintf "device/vbd/%d/backend" id) | ||
in | ||
let params = xs.Xs.read (Printf.sprintf "%s/params" backend) in | ||
match String.split '/' backend with | ||
| "local" :: "domain" :: bedomid :: _ -> | ||
if not (self = bedomid) then | ||
raise | ||
Api_errors.( | ||
Server_error | ||
( internal_error | ||
, [ | ||
Printf.sprintf | ||
"find_backend_device: Got domid %s but expected \ | ||
%s" | ||
bedomid self | ||
] | ||
) | ||
) ; | ||
Some params | ||
| _ -> | ||
raise Not_found | ||
) | ||
| _ -> | ||
raise Not_found | ||
with _ -> None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -163,7 +163,6 @@ let localhost_handler rpc session_id vdi_opt (req : Request.t) | |
not | ||
(Sm_fs_ops.must_write_zeroes_into_new_vdi ~__context vdi) | ||
in | ||
debug "GTNDEBUG: we are receiving Raw, Vhd or Qcow file" ; | ||
Sm_fs_ops.with_block_attached_device __context rpc | ||
session_id vdi `RW (fun path -> | ||
if chunked then | ||
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. Should there be calls to (some, currently undefined) 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. Yes exactly. In fact I did that so I was able to compile but a better way is to report an error that it is not implemented or an empty Qcow_tool_wrapper.receive. But yes the calls will be to Qcow_tool_wrapper.receive. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,42 +113,6 @@ let receive progress_cb format protocol (s : Unix.file_descr) | |
in | ||
run_vhd_tool progress_cb args s s' path | ||
|
||
(** [find_backend_device path] returns [Some path'] where [path'] is the backend path in | ||
the driver domain corresponding to the frontend device [path] in this domain. *) | ||
let find_backend_device path = | ||
try | ||
let open Ezxenstore_core.Xenstore in | ||
(* If we're looking at a xen frontend device, see if the backend | ||
is in the same domain. If so check if it looks like a .vhd *) | ||
let rdev = (Unix.stat path).Unix.st_rdev in | ||
let major = rdev / 256 and minor = rdev mod 256 in | ||
let link = | ||
Unix.readlink (Printf.sprintf "/sys/dev/block/%d:%d/device" major minor) | ||
in | ||
match List.rev (String.split '/' link) with | ||
| id :: "xen" :: "devices" :: _ | ||
when Astring.String.is_prefix ~affix:"vbd-" id -> | ||
let id = int_of_string (String.sub id 4 (String.length id - 4)) in | ||
with_xs (fun xs -> | ||
let self = xs.Xs.read "domid" in | ||
let backend = | ||
xs.Xs.read (Printf.sprintf "device/vbd/%d/backend" id) | ||
in | ||
let params = xs.Xs.read (Printf.sprintf "%s/params" backend) in | ||
match String.split '/' backend with | ||
| "local" :: "domain" :: bedomid :: _ -> | ||
if not (self = bedomid) then | ||
Helpers.internal_error | ||
"find_backend_device: Got domid %s but expected %s" bedomid | ||
self ; | ||
Some params | ||
| _ -> | ||
raise Not_found | ||
) | ||
| _ -> | ||
raise Not_found | ||
with _ -> None | ||
|
||
Comment on lines
-116
to
-151
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. There is an identical function also in 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. Probably in xcp-idl somewhere. I don't think there's a good place for this currently. It might be worth creating a library package on top of ezxenstore that encapsulates common patterns when interacting with xenstore. Maybe a sublibrary of ezxenstore. 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. Yes I agree, I saw some common patterns. |
||
(** [vhd_of_device path] returns (Some vhd) where 'vhd' is the vhd leaf backing a particular device [path] or None. | ||
[path] may either be a blktap2 device *or* a blkfront device backed by a blktap2 device. If the latter then | ||
the script must be run in the same domain as blkback. *) | ||
|
@@ -178,22 +142,27 @@ let vhd_of_device path = | |
debug "Device %s has an unknown driver" path ; | ||
None | ||
in | ||
find_backend_device path |> Option.value ~default:path |> tapdisk_of_path | ||
Common_tool_wrapper.find_backend_device path | ||
|> Option.value ~default:path | ||
|> tapdisk_of_path | ||
|
||
let send progress_cb ?relative_to (protocol : string) (dest_format : string) | ||
(s : Unix.file_descr) (path : string) (size : Int64.t) (prefix : string) = | ||
let s' = Uuidx.(to_string (make ())) in | ||
debug "GTNDEBUG: path is %s" path ; | ||
debug "GTNDEBUG: prefix is %s" prefix ; | ||
let source_format, source = | ||
debug "GTNDEBUG: get_nbd_device %s" path ; | ||
debug "GTNDEBUG: s' is %s" s' ; | ||
match (Stream_vdi.get_nbd_device path, vhd_of_device path, relative_to) with | ||
| Some (nbd_server, exportname), _, None -> | ||
debug "GTNDEBUG: nbdhybrid %s:%s:%s:%Ld" path nbd_server exportname size ; | ||
( "nbdhybrid" | ||
, Printf.sprintf "%s:%s:%s:%Ld" path nbd_server exportname size | ||
) | ||
| Some _, Some vhd, Some _ | None, Some vhd, _ -> | ||
debug "GTNDEBUG: hybrid %s" (path ^ ":" ^ vhd) ; | ||
("hybrid", path ^ ":" ^ vhd) | ||
| None, None, None -> | ||
debug "GTNDEBUG: raw %s" path ; | ||
("raw", path) | ||
| _, None, Some _ -> | ||
let msg = "Cannot compute differences on non-VHD images" in | ||
|
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.
Avoid this library if possible, instead use Astring.String, or the standard library (String.split_on_char)
Also avoid open, they can be quite evil. Instead use
module Example = This.Is.Some.Example
Christian maintains a useful style guide here: https://github.com/lindig/ocaml-style?tab=readme-ov-file#avoid-opening-modules-globally
let open
s are much better because their scope is contained and are obvious if the function is not too long.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 100% agree with
let open
. I copied the open from another package I guess but I will apply the your suggestion for sure.