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

[Pino Toscano]

On Monday, 24 February 2020 12:05:55 CET Martin Kletzander wrote:
> 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 at redhat.com>
> >> ---
> >>  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).

We can always add new parameters to the scripts - they take a JSON with
the parameters already.

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

My thought is that, since we already have these parameters, checking
them earlier avoids issues later on.

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

Yes, this is a possibility, albeit IMHO very remote (and it will error
out anyway, even if with the current semi-criptic message).

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200224/f0c71e87/attachment.sig>