Re: [Libguestfs] [PATCH v2v v2 2/2] rhv-upload: Check that rhv-disk-uuid is not already taken (RHBZ#1789279)

On Mon, Feb 24, 2020 at 10:28:46AM +0100, Pino Toscano wrote: >On Wednesday, 29 January 2020 15:34:49 CET Martin Kletzander wrote: >> This makes the code fail with a sensible error message instead of cryptic error >> from ovirtsdk. >> >> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >> --- >> v2v/rhv-upload-plugin.py | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py >> index d3e6260e97f4..413ad53b05ab 100644 >> --- a/v2v/rhv-upload-plugin.py >> +++ b/v2v/rhv-upload-plugin.py >> @@ -26,6 +26,9 @@ import ssl >> import sys >> import time >> >> +import nbdkit >> +import errno >> + >> from http.client import HTTPSConnection, HTTPConnection >> from urllib.parse import urlparse >> >> @@ -461,6 +464,15 @@ def create_disk(connection): >> system_service = connection.system_service() >> disks_service = system_service.disks_service() >> >> + uuid = params.get('rhv_disk_uuid') >> + if uuid is not None: >> + try: >> + disks_service.disk_service(uuid).get() >> + nbdkit.set_error(errno.EEXIST) >> + raise ValueError("Disk with the UUID '%s' already exists" % uuid) >> + except sdk.NotFoundError: >> + pass > >This check seems correct to me, although it is done too late: IMHO this >is something to do in the precheck script, so we do not even attempt to >connect to the source if any of the specified UUIDs already exists in >oVirt. > Well at that point we are not passing the UUIDs to the script and that's why I selected this one (if I remember correctly).

Given how often this would happen I don't think it would be that big of a deal to reject it slightly slower.

Also the race window (of the UUID being created in the meantime) would become bigger.