4:12 p.m.
My stab at fixing this: https://bugzilla.redhat.com/show_bug.cgi?id=1789279
It took me quite some time to go through the whole rfc 4122 just to realize we
do not need to do anything with the versions.
Martin Kletzander (2):
rhv-upload: Validate UUIDs passed to -oo rhv-disk-uuid (RHBZ#1789279)
rhv-upload: Check that rhv-disk-uuid is not already taken
(RHBZ#1789279)
v2v/output_rhv_upload.ml | 10 ++++++++++
v2v/rhv-upload-plugin.py | 12 ++++++++++++
2 files changed, 22 insertions(+)
--
2.25.0
4:12 p.m.
New subject: [v2v PATCH 1/2] rhv-upload: Validate UUIDs passed to -oo rhv-disk-uuid (RHBZ#1789279)
The validation helps us fail early and with a sensible error message. The NIL
UUID is not valid for oVirt, but other than that there is no other logic in
there merely because the UUID types are a matter of the generator and they are
just forwarded in this partucular case.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
v2v/output_rhv_upload.ml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index 14153db36897..923ef56d4ca5 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -49,6 +49,14 @@ after their uploads (if you do, you must supply one for each disk):
-oo rhv-disk-uuid=UUID Disk UUID
")
+let is_nonnil_uuid uuid =
+ let nil_uuid = "00000000-0000-0000-0000-000000000000" in
+ let hex = "[a-fA-F0-9]" in
+ let rex_uuid =
"^"^hex^"{8}-"^hex^"{4}-"^hex^"{4}-"^hex^"{4}-"^hex^"{12}$"
in
+ let rex_uuid = PCRE.compile rex_uuid in
+ if uuid = nil_uuid then false
+ else PCRE.matches rex_uuid uuid
+
let parse_output_options options =
let rhv_cafile = ref None in
let rhv_cluster = ref None in
@@ -71,6 +79,8 @@ let parse_output_options options =
| "rhv-verifypeer", "" -> rhv_verifypeer := true
| "rhv-verifypeer", v -> rhv_verifypeer := bool_of_string v
| "rhv-disk-uuid", v ->
+ if not (is_nonnil_uuid v) then
+ error (f_"-o rhv-upload: invalid UUID for -oo rhv-disk-uuid");
rhv_disk_uuids := Some (v :: (Option.default [] !rhv_disk_uuids))
| k, _ ->
error (f_"-o rhv-upload: unknown output option ‘-oo %s’") k
--
2.25.0
3:34 a.m.
New subject: [v2v PATCH 1/2] rhv-upload: Validate UUIDs passed to -oo rhv-disk-uuid (RHBZ#1789279)
On Thu, Jan 23, 2020 at 11:12:22PM +0100, Martin Kletzander wrote:
The validation helps us fail early and with a sensible error message.
The NIL
UUID is not valid for oVirt, but other than that there is no other logic in
there merely because the UUID types are a matter of the generator and they are
just forwarded in this partucular case.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
v2v/output_rhv_upload.ml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index 14153db36897..923ef56d4ca5 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -49,6 +49,14 @@ after their uploads (if you do, you must supply one for each disk):
-oo rhv-disk-uuid=UUID Disk UUID
")
+let is_nonnil_uuid uuid =
+ let nil_uuid = "00000000-0000-0000-0000-000000000000" in
+ let hex = "[a-fA-F0-9]" in
+ let rex_uuid =
"^"^hex^"{8}-"^hex^"{4}-"^hex^"{4}-"^hex^"{4}-"^hex^"{12}$"
in
Use sprintf?
+ let rex_uuid = PCRE.compile rex_uuid in
+ if uuid = nil_uuid then false
+ else PCRE.matches rex_uuid uuid
This could compile the regexp only once if you write this function as:
let is_nonnil_uuid =
let nil_uuid = "00000000-0000-0000-0000-000000000000" in
let hex = "[a-fA-F0-9]" in
let rex_uuid = sprintf "^%s{8}...." etc... in
let rex_uuid = PCRE.compile rex_uuid in
function uuid ->
if uuid = nil_uuid then false
else PCRE.matches rex_uuid uuid
Or even better if you lazily evaluate the regexp so it is
only compiled once when the function is first called:
let is_nonnil_uuid =
let rex_uuid = lazy (
let nil_uuid = "00000000-0000-0000-0000-000000000000" in
let hex = "[a-fA-F0-9]" in
let rex_uuid = sprintf "^%s{8}...." ... in
PCRE.compile rex_uuid
) in
function uuid ->
if uuid = nil_uuid then false
else PCRE.matches (Lazy.force rex_uuid) uuid
But basically the idea is fine so ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
3:38 a.m.
New subject: [v2v PATCH 1/2] rhv-upload: Validate UUIDs passed to -oo rhv-disk-uuid (RHBZ#1789279)
On Fri, Jan 24, 2020 at 09:34:04AM +0000, Richard W.M. Jones wrote:
Or even better if you lazily evaluate the regexp so it is
only compiled once when the function is first called:
let is_nonnil_uuid =
let rex_uuid = lazy (
let nil_uuid = "00000000-0000-0000-0000-000000000000" in
nil_uuid needs to be defined outside the lazy () clause, but you get the idea.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
4:12 p.m.
New subject: [v2v PATCH 2/2] rhv-upload: Check that rhv-disk-uuid is not already taken (RHBZ#1789279)
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>
---
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..edb2699214c2 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.EINVAL)
+ raise ValueError("Disk with the specified UUID already exists")
+ except sdk.NotFoundError:
+ pass
+
if params['disk_format'] == "raw":
disk_format = types.DiskFormat.RAW
else:
--
2.25.0
3:34 a.m.
New subject: [v2v PATCH 2/2] rhv-upload: Check that rhv-disk-uuid is not already taken (RHBZ#1789279)
On Thu, Jan 23, 2020 at 11:12:23PM +0100, 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>
---
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..edb2699214c2 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.EINVAL)
+ raise ValueError("Disk with the specified UUID already exists")
+ except sdk.NotFoundError:
+ pass
+
if params['disk_format'] == "raw":
disk_format = types.DiskFormat.RAW
else:
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
5:01 a.m.
New subject: [v2v PATCH 2/2] rhv-upload: Check that rhv-disk-uuid is not already taken (RHBZ#1789279)
On Thursday, 23 January 2020 23:12:23 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>
---
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..edb2699214c2 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:
'is not None' can be avoided. (Yes, there are few occurrences of that,
should be changed too IMHO.)
+ try:
+ disks_service.disk_service(uuid).get()
+ nbdkit.set_error(errno.EINVAL)
Maybe EEXIST rather than EINVAL?
+ raise ValueError("Disk with the specified UUID
already exists")
Print also the UUID, so it is easier to debug a failure even without
the virt-v2v command line.
With the above fixes, LGTM.
--
Pino Toscano
1763
days inactive
1767
days old
6 comments
3 participants
participants (3)
-
Martin Kletzander -
Pino Toscano -
Richard W.M. Jones