Split the VM existance check out of the precheck script to a new vmcheck script, and invoke that in #prepare_targets. Invoke the precheck script in #precheck, as now it can be run with only values of command line options. This does not change which checks are performed; however, an invalid cluster name will make virt-v2v fail way earlier (even before connecting to the source). --- v2v/Makefile.am | 8 ++- v2v/output_rhv_upload.ml | 14 +++-- v2v/output_rhv_upload_vmcheck_source.mli | 19 ++++++ v2v/rhv-upload-precheck.py | 10 ---- v2v/rhv-upload-vmcheck.py | 73 ++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 v2v/output_rhv_upload_vmcheck_source.mli create mode 100644 v2v/rhv-upload-vmcheck.py diff --git a/v2v/Makefile.am b/v2v/Makefile.am index 2b939368e..067e11049 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -26,7 +26,8 @@ BUILT_SOURCES = \ config.ml \ output_rhv_upload_createvm_source.ml \ output_rhv_upload_plugin_source.ml \ - output_rhv_upload_precheck_source.ml + output_rhv_upload_precheck_source.ml \ + output_rhv_upload_vmcheck_source.ml EXTRA_DIST = \ $(SOURCES_MLI) $(SOURCES_ML) $(SOURCES_C) \ @@ -36,6 +37,7 @@ EXTRA_DIST = \ rhv-upload-createvm.py \ rhv-upload-plugin.py \ rhv-upload-precheck.py \ + rhv-upload-vmcheck.py \ var_expander_tests.ml \ v2v_unit_tests.ml \ virt-v2v.pod \ @@ -88,6 +90,7 @@ SOURCES_MLI = \ output_rhv_upload_createvm_source.mli \ output_rhv_upload_plugin_source.mli \ output_rhv_upload_precheck_source.mli \ + output_rhv_upload_vmcheck_source.mli \ output_vdsm.mli \ parse_ova.mli \ parse_ovf_from_ova.mli \ @@ -153,6 +156,7 @@ SOURCES_ML = \ output_rhv_upload_createvm_source.ml \ output_rhv_upload_plugin_source.ml \ output_rhv_upload_precheck_source.ml \ + output_rhv_upload_vmcheck_source.ml \ output_rhv_upload.ml \ output_vdsm.ml \ output_openstack.ml \ @@ -173,6 +177,8 @@ output_rhv_upload_plugin_source.ml: $(srcdir)/rhv-upload-plugin.py $(srcdir)/embed.sh code $^ $@ output_rhv_upload_precheck_source.ml: $(srcdir)/rhv-upload-precheck.py $(srcdir)/embed.sh code $^ $@ +output_rhv_upload_vmcheck_source.ml: $(srcdir)/rhv-upload-vmcheck.py + $(srcdir)/embed.sh code $^ $@ if HAVE_OCAML diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml index 5bc6a4007..91552536a 100644 --- a/v2v/output_rhv_upload.ml +++ b/v2v/output_rhv_upload.ml @@ -94,10 +94,13 @@ class output_rhv_upload output_alloc output_conn let diskid_file_of_id id = tmpdir // sprintf "diskid.%d" id in - (* Create Python scripts for precheck, plugin and create VM. *) + (* Create Python scripts for precheck, vmcheck, plugin and create VM. *) let precheck_script = Python_script.create ~name:"rhv-upload-precheck.py" Output_rhv_upload_precheck_source.code in + let vmcheck_script = + Python_script.create ~name:"rhv-upload-vmcheck.py" + Output_rhv_upload_vmcheck_source.code in let plugin_script = Python_script.create ~name:"rhv-upload-plugin.py" Output_rhv_upload_plugin_source.code in @@ -226,6 +229,9 @@ object error_unless_nbdkit_working (); error_unless_nbdkit_python_plugin_working (); error_unless_output_alloc_sparse (); + (* Python code prechecks. *) + if Python_script.run_command precheck_script json_params [] <> 0 then + error (f_"failed server prechecks, see earlier errors"); if have_selinux then error_unless_nbdkit_compiled_with_selinux () @@ -247,11 +253,11 @@ object let json_params = ("output_name", JSON.String output_name) :: json_params in - (* Python code prechecks. These can't run in #precheck because + (* Check that the VM does not exist. This can't run in #precheck because * we need to know the name of the virtual machine. *) - if Python_script.run_command precheck_script json_params [] <> 0 then - error (f_"failed server prechecks, see earlier errors"); + if Python_script.run_command vmcheck_script json_params [] <> 0 then + error (f_"failed vmchecks, see earlier errors"); (* Create an nbdkit instance for each disk and set the * target URI to point to the NBD socket. diff --git a/v2v/output_rhv_upload_vmcheck_source.mli b/v2v/output_rhv_upload_vmcheck_source.mli new file mode 100644 index 000000000..c1bafa15b --- /dev/null +++ b/v2v/output_rhv_upload_vmcheck_source.mli @@ -0,0 +1,19 @@ +(* virt-v2v + * Copyright (C) 2018 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +val code : string diff --git a/v2v/rhv-upload-precheck.py b/v2v/rhv-upload-precheck.py index b79f91b4a..6253c26ac 100644 --- a/v2v/rhv-upload-precheck.py +++ b/v2v/rhv-upload-precheck.py @@ -60,16 +60,6 @@ connection = sdk.Connection( system_service = connection.system_service() -# Find if a virtual machine already exists with that name. -vms_service = system_service.vms_service() -vms = vms_service.list( - search = ("name=%s" % params['output_name']), -) -if len(vms) > 0: - vm = vms[0] - raise RuntimeError("VM already exists with name ‘%s’, id ‘%s’" % - (params['output_name'], vm.id)) - # Check whether the specified cluster exists. clusters_service = system_service.clusters_service() clusters = clusters_service.list( diff --git a/v2v/rhv-upload-vmcheck.py b/v2v/rhv-upload-vmcheck.py new file mode 100644 index 000000000..76a1a23c1 --- /dev/null +++ b/v2v/rhv-upload-vmcheck.py @@ -0,0 +1,73 @@ +# -*- python -*- +# oVirt or RHV pre-upload VM checks used by ‘virt-v2v -o rhv-upload’ +# Copyright (C) 2018 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +import json +import logging +import sys +import time + +from http.client import HTTPSConnection +from urllib.parse import urlparse + +import ovirtsdk4 as sdk +import ovirtsdk4.types as types + +# Parameters are passed in via a JSON doc from the OCaml code. +# Because this Python code ships embedded inside virt-v2v there +# is no formal API here. +params = None + +if len(sys.argv) != 2: + raise RuntimeError("incorrect number of parameters") + +# Parameters are passed in via a JSON document. +with open(sys.argv[1], 'r') as fp: + params = json.load(fp) + +# What is passed in is a password file, read the actual password. +with open(params['output_password'], 'r') as fp: + output_password = fp.read() +output_password = output_password.rstrip() + +# Parse out the username from the output_conn URL. +parsed = urlparse(params['output_conn']) +username = parsed.username or "admin@internal" + +# Connect to the server. +connection = sdk.Connection( + url = params['output_conn'], + username = username, + password = output_password, + ca_file = params['rhv_cafile'], + log = logging.getLogger(), + insecure = params['insecure'], +) + +system_service = connection.system_service() + +# Find if a virtual machine already exists with that name. +vms_service = system_service.vms_service() +vms = vms_service.list( + search = ("name=%s" % params['output_name']), +) +if len(vms) > 0: + vm = vms[0] + raise RuntimeError("VM already exists with name ‘%s’, id ‘%s’" % + (params['output_name'], vm.id)) + +# Otherwise everything is OK, exit with no error. -- 2.21.0

This way it is possible to communicate data from the precheck script back to virt-v2v. For now there are no results, so the resulting JSON is discarded. --- v2v/output_rhv_upload.ml | 7 ++++++- v2v/rhv-upload-precheck.py | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml index 91552536a..e74a897b9 100644 --- a/v2v/output_rhv_upload.ml +++ b/v2v/output_rhv_upload.ml @@ -230,8 +230,13 @@ object error_unless_nbdkit_python_plugin_working (); error_unless_output_alloc_sparse (); (* Python code prechecks. *) - if Python_script.run_command precheck_script json_params [] <> 0 then + let precheck_fn = tmpdir // "v2vprecheck.json" in + let fd = Unix.openfile precheck_fn [O_WRONLY; O_CREAT] 0o600 in + if Python_script.run_command ~stdout_fd:fd precheck_script json_params [] <> 0 then error (f_"failed server prechecks, see earlier errors"); + let json = JSON_parser.json_parser_tree_parse_file precheck_fn in + debug "precheck output parsed as: %s" + (JSON.string_of_doc ~fmt:JSON.Indented ["", json]); if have_selinux then error_unless_nbdkit_compiled_with_selinux () diff --git a/v2v/rhv-upload-precheck.py b/v2v/rhv-upload-precheck.py index 6253c26ac..a317d997c 100644 --- a/v2v/rhv-upload-precheck.py +++ b/v2v/rhv-upload-precheck.py @@ -70,4 +70,8 @@ if len(clusters) == 0: raise RuntimeError("The cluster ‘%s’ does not exist" % (params['rhv_cluster'])) -# Otherwise everything is OK, exit with no error. +# Otherwise everything is OK, print a JSON with the results. +results = { +} + +json.dump(results, sys.stdout) -- 2.21.0

On Mon, Sep 16, 2019 at 07:13:47PM +0200, Pino Toscano wrote: Looks good, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

On Mon, Sep 16, 2019 at 07:13:48PM +0200, Pino Toscano wrote: There's a Unicode-art diagram in v2v/types.mli which also should be updated: https://github.com/libguestfs/libguestfs/blob/dea9636c596acd030c995505786... ACK if you also update this diagram. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

On Mon, Sep 16, 2019 at 07:13:49PM +0200, Pino Toscano wrote: This is certainly cleaner than the existing code, so the change in behaviour for --no-copy isn't important (and it's still an error in either case), therefore: ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

I think we should do this slightly differently for consistency with other classes. See here for example: https://github.com/libguestfs/libguestfs/blob/dea9636c596acd030c995505786... (1) Declare a val mutable in the object: val mutable delete_disks_on_exit = true (2) Change this check to: if delete_disks_on_exit then (3) Remove this hunk. I would be happy with this patch if it was change in the way described above. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

Make sure to cancel the trasfer in RHV in case of failure during the copying of a disk: this way, the disk can be actually removed by RHV itself. --- v2v/rhv-upload-plugin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index 4d61a089b..57e90484f 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -485,6 +485,8 @@ def flush(h): r.read() def delete_disk_on_failure(h): + transfer_service = h['transfer_service'] + transfer_service.cancel() disk_service = h['disk_service'] disk_service.remove() -- 2.21.0