From 50a74177b3577952159d87335cf40c0ad3e51b4d Mon Sep 17 00:00:00 2001

From: Laszlo Ersek <lersek@redhat.com>

Date: Fri, 17 Jun 2022 11:53:37 +0200

Subject: [PATCH] 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@redhat.com>

Message-Id: <20220617095337.9122-1-lersek@redhat.com>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

(cherry picked from commit e186cc2bea99a077990f192953e1bf6c9ba70e79)

---

 output/output.ml            | 5 +++++

 output/output.mli           | 7 +++++++

 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.ml b/output/output.ml

index 10e685c4..5c6670b9 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.mli b/output/output.mli

index 533a0c51..8d3d6865 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_rhv.ml b/output/output_rhv.ml

index 119207fd..8571e07b 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 828996b3..f2ced4f4 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 a1e8c246..23d1b9cd 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,