Re: [Libguestfs] [v2v PATCH v2] RHV outputs: limit copied disk count to 23

We currently support virtio-blk (commonly) or IDE (unusually) for exposing disks to the converted guest; refer to "guestcaps.gcaps_block_bus" in "lib/create_ovf.ml". When using virtio-blk (i.e., in the common case), RHV can deal with at most 23 disks, as it plugs each virtio-blk device in a separate slot on the PCI(e) root bus; and the other slots are reserved for various purposes. When a domain has too many disks, the problem only becomes apparent once the copying finishes and an import is attempted. Modify the RHV outputs to fail relatively early when a domain has more than 23 disks that need to be copied. Notes: - With IDE, the theoretical limit may even be as low as 4. However, in the "Output_module.setup" function, we don't have access to "guestcaps.gcaps_block_bus", and in practice the IDE limitation has not caused surprises. So for now stick with 23, assuming virtio-blk. Modifying the "Output_module.setup" parameter list just for this seems overkill. - We could move the new check to an even earlier step, namely "Output_module.parse_options", due to the v2v directory deliberately existing (and having been populated with input sockets) at that time. However, even discounting the fact that "parse_options" is not a good name for including this kind of step, "parse_options" does not have access to the v2v directory name, and modifying the signature just for this is (again) overkill. - By adding the check to "Output_module.setup", we waste *some* effort (namely, the conversion occurs between "parse_options" and "setup"), but: (a) the "rhv-disk-uuid" count check (against the disk count) is already being done in the rhv-upload module's "setup" function, (b) in practice the slowest step ought to be the copying, and placing the new check in "setup" is early enough to prevent that. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2051564 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - rename "bail_if_disk_count_gt" to "error_if_disk_count_gt" [Rich] v1: This patch can be tested easily by replacing 23 with 0 in all three affected output modules -- then the following test cases all fail in "make check": FAIL: test-v2v-o-rhv.sh FAIL: test-v2v-o-vdsm-options.sh FAIL: test-v2v-o-rhv-upload.sh > [ 11.3] Setting up the destination: -o rhv > virt-v2v: error: this output module doesn't support copying more than 0 disks > [ 9.0] Setting up the destination: -o vdsm > virt-v2v: error: this output module doesn't support copying more than 0 disks > [ 11.2] Setting up the destination: -o rhv-upload -oc https://example.com/ovirt-engine/api -os Storage > virt-v2v: error: this output module doesn't support copying more than 0 disks output/output.mli | 7 +++++++ output/output.ml | 5 +++++ output/output_rhv.ml | 1 + output/output_rhv_upload.ml | 1 + output/output_vdsm.ml | 1 + 5 files changed, 15 insertions(+) diff --git a/output/output.mli b/output/output.mli index 533a0c51d31c..8d3d6865945f 100644 --- a/output/output.mli +++ b/output/output.mli @@ -76,6 +76,13 @@ val get_disks : string -> (int * int64) list (** Examines the v2v directory and opens each input socket (in0 etc), returning a list of input disk index and size. *) +val error_if_disk_count_gt : string -> int -> unit +(** This function lets an output module enforce a maximum disk count. + [error_if_disk_count_gt dir n] checks whether the domain has more than [n] + disks that need to be copied, by examining the existence of input NBD socket + "in[n]" in the v2v directory [dir]. If the socket exists, [error] is + called. *) + val output_to_local_file : ?changeuid:((unit -> unit) -> unit) -> Types.output_allocation -> string -> string -> int64 -> string -> diff --git a/output/output.ml b/output/output.ml index 10e685c46926..5c6670b99c69 100644 --- a/output/output.ml +++ b/output/output.ml @@ -64,6 +64,11 @@ let get_disks dir = in loop [] 0 +let error_if_disk_count_gt dir n = + let socket = sprintf "%s/in%d" dir n in + if Sys.file_exists socket then + error (f_"this output module doesn't support copying more than %d disks") n + let output_to_local_file ?(changeuid = fun f -> f ()) output_alloc output_format filename size socket = (* Check nbdkit is installed and has the required plugin. *) diff --git a/output/output_rhv.ml b/output/output_rhv.ml index 119207fdc065..8571e07b0cc3 100644 --- a/output/output_rhv.ml +++ b/output/output_rhv.ml @@ -56,6 +56,7 @@ module RHV = struct (options.output_alloc, options.output_format, output_name, output_storage) let rec setup dir options source = + error_if_disk_count_gt dir 23; let disks = get_disks dir in let output_alloc, output_format, output_name, output_storage = options in diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml index 828996b36261..f2ced4f4e98e 100644 --- a/output/output_rhv_upload.ml +++ b/output/output_rhv_upload.ml @@ -133,6 +133,7 @@ after their uploads (if you do, you must supply one for each disk): else PCRE.matches (Lazy.force rex_uuid) uuid let rec setup dir options source = + error_if_disk_count_gt dir 23; let disks = get_disks dir in let output_conn, output_format, output_password, output_name, output_storage, diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml index a1e8c2465810..23d1b9cd25b4 100644 --- a/output/output_vdsm.ml +++ b/output/output_vdsm.ml @@ -119,6 +119,7 @@ For each disk you must supply one of each of these options: compat, ovf_flavour) let setup dir options source = + error_if_disk_count_gt dir 23; let disks = get_disks dir in let output_alloc, output_format, output_name, output_storage,