For -o rhv-upload, the -os parameter specifies the storage domain. Because the RHV API allows globs when searching for a domain, if you used a parameter like -os 'data*' then this would confuse the Python code, since it can glob to the name of a storage domain, but then later fail when we try to exact match the storage domain we found. The result of this was a confusing error in the precheck script: IndexError: list index out of range This fix validates the output storage parameter before trying to use it. Since valid storage domain names cannot contain glob characters or spaces, it avoids the problems above and improves the error message that the user sees: $ virt-v2v [...] -o rhv-upload -os '' ... RuntimeError: The storage domain (-os) parameter ‘’ is not valid virt-v2v: error: failed server prechecks, see earlier errors $ virt-v2v [...] -o rhv-upload -os 'data*' ... RuntimeError: The storage domain (-os) parameter ‘data*’ is not valid virt-v2v: error: failed server prechecks, see earlier errors Although the IndexError should no longer happen, I also added a try...catch around that code to improve the error in case it still happens. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1986386 Reported-by: Junqin Zhou Thanks: Nir Soffer --- output/rhv-upload-precheck.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/output/rhv-upload-precheck.py b/output/rhv-upload-precheck.py index 1dc1b8498a..ba125611ba 100644 --- a/output/rhv-upload-precheck.py +++ b/output/rhv-upload-precheck.py @@ -18,6 +18,7 @@ import json import logging +import re import sys from urllib.parse import urlparse @@ -46,6 +47,15 @@ output_password = output_password.rstrip() parsed = urlparse(params['output_conn']) username = parsed.username or "admin@internal" +# Check the storage domain name is valid +# (https://bugzilla.redhat.com/show_bug.cgi?id=1986386#c1) +# Also this means it cannot contain spaces or glob symbols, so +# the search below is valid. +output_storage = params['output_storage'] +if not re.match('^[-a-zA-Z0-9_]+$', output_storage): + raise RuntimeError("The storage domain (-os) parameter ‘%s’ is not valid" % + output_storage) + # Connect to the server. connection = sdk.Connection( url=params['output_conn'], @@ -60,28 +70,33 @@ system_service = connection.system_service() # Check whether there is a datacenter for the specified storage. data_centers = system_service.data_centers_service().list( - search='storage.name=%s' % params['output_storage'], + search='storage.name=%s' % output_storage, case_sensitive=True, ) if len(data_centers) == 0: storage_domains = system_service.storage_domains_service().list( - search='name=%s' % params['output_storage'], + search='name=%s' % output_storage, case_sensitive=True, ) if len(storage_domains) == 0: # The storage domain does not even exist. raise RuntimeError("The storage domain ‘%s’ does not exist" % - (params['output_storage'])) + output_storage) # The storage domain is not attached to a datacenter # (shouldn't happen, would fail on disk creation). raise RuntimeError("The storage domain ‘%s’ is not attached to a DC" % - (params['output_storage'])) + output_storage) datacenter = data_centers[0] # Get the storage domain. storage_domains = connection.follow_link(datacenter.storage_domains) -storage_domain = [sd for sd in storage_domains if sd.name == params['output_storage']][0] +try: + storage_domain = [sd for sd in storage_domains + if sd.name == output_storage][0] +except IndexError: + raise RuntimeError("The storage domain ‘%s’ does not exist" % + output_storage) # Get the cluster. clusters = connection.follow_link(datacenter.clusters) @@ -90,7 +105,7 @@ if len(clusters) == 0: raise RuntimeError("The cluster ‘%s’ is not part of the DC ‘%s’, " "where the storage domain ‘%s’ is" % (params['rhv_cluster'], datacenter.name, - params['output_storage'])) + output_storage)) cluster = clusters[0] cpu = cluster.cpu if cpu.architecture == types.Architecture.UNDEFINED: -- 2.39.0