6:26 a.m.
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