[PATCH v2v v3 REBASE] rhv-upload: Validate UUIDs passed to -oo rhv-disk-uuid (RHBZ#1789279)
by Martin Kletzander
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.
On top of that also check that the UUID is not already taken. 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>
---
v4:
- https://listman.redhat.com/archives/libguestfs/2021-May/msg00095.html
- Merge the two patches together
- Things mentioned in the review are already in, so there is no other change
v3:
- https://listman.redhat.com/archives/libguestfs/2020-March/msg00084.html
- Do the check in precheck
- Fix for Lazy evaluation of regexp UUID
v2:
- https://www.redhat.com/archives/libguestfs/2020-January/msg00221.html
- Use EEXIST instead of EINVAL
- Put the colliding UUID into the error
- Do not evaluate the PCRE needlessly multiple times
v1:
- https://www.redhat.com/archives/libguestfs/2020-January/msg00184.html
v2v/output_rhv_upload.ml | 18 ++++++++++++++++++
v2v/rhv-upload-precheck.py | 10 ++++++++++
2 files changed, 28 insertions(+)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index dbef7011b04b..15ba1078019e 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -49,6 +49,16 @@ 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 rex_uuid = lazy (
+ let hex = "[a-fA-F0-9]" in
+ let str = sprintf "^%s{8}-%s{4}-%s{4}-%s{4}-%s{12}$" hex hex hex hex hex in
+ PCRE.compile str
+ ) in
+ if uuid = nil_uuid then false
+ else PCRE.matches (Lazy.force rex_uuid) uuid
+
let parse_output_options options =
let rhv_cafile = ref None in
let rhv_cluster = ref None in
@@ -71,6 +81,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
@@ -256,6 +268,12 @@ object
error_unless_output_alloc_sparse output_alloc;
(* Python code prechecks. *)
+ let json_params = match rhv_options.rhv_disk_uuids with
+ | None -> json_params
+ | Some uuids ->
+ let ids = List.map (fun uuid -> JSON.String uuid) uuids in
+ ("rhv_disk_uuids", JSON.List ids) :: json_params
+ in
let precheck_fn = tmpdir // "v2vprecheck.json" in
let fd = Unix.openfile precheck_fn [O_WRONLY; O_CREAT] 0o600 in
if Python_script.run_command ~stdout_fd:fd
diff --git a/v2v/rhv-upload-precheck.py b/v2v/rhv-upload-precheck.py
index 2180ea1043ab..c3f74df35f65 100644
--- a/v2v/rhv-upload-precheck.py
+++ b/v2v/rhv-upload-precheck.py
@@ -97,6 +97,16 @@ if cpu.architecture == types.Architecture.UNDEFINED:
raise RuntimeError("The cluster ‘%s’ has an unknown architecture" %
(params['rhv_cluster']))
+# Find if any disk already exists with specified UUID.
+disks_service = system_service.disks_service()
+
+for uuid in params.get('rhv_disk_uuids', []):
+ try:
+ disk_service = disks_service.disk_service(uuid).get()
+ raise RuntimeError("Disk with the UUID '%s' already exists" % uuid)
+ except sdk.NotFoundError:
+ pass
+
# Otherwise everything is OK, print a JSON with the results.
results = {
"rhv_storagedomain_uuid": storage_domain.id,
--
2.31.1
2 years, 11 months
[PATCH nbdkit] Set visibility to default for exported symbols
by Richard W.M. Jones
In plugins we expect the plugin_init function to be exported as well
as *_debug_* variables. In the server various nbdkit_* functions
should be exported.
If you compile nbdkit with -fvisibility=hidden some of these symbols
are not exported at all even though the linker script says they should
be. This option can be useful as it allows the compiler to generate
simpler and hence faster code, and it's required for Clang CFI.
Set the visibility to "default" for all symbols we expect to be
exported. In theory this should also be required for all the nbdkit_*
functions exported by the server, but I found that it is not needed by
either GCC or Clang.
---
include/nbdkit-common.h | 6 +++++-
include/nbdkit-filter.h | 1 +
include/nbdkit-plugin.h | 1 +
tests/Makefile.am | 1 +
plugins/ocaml/bindings.c | 26 +++++++++++++-------------
plugins/ocaml/plugin.c | 14 +++++++-------
tests/dummy-vddk.c | 30 ++++++++++++++++--------------
7 files changed, 44 insertions(+), 35 deletions(-)
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 51a6264c..f43ebe59 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -83,10 +83,14 @@ extern "C" {
#define NBDKIT_EXTENT_ZERO (1<<1) /* Same as NBD_STATE_ZERO */
#ifndef WIN32
-#define NBDKIT_EXTERN_DECL(ret, fn, args) extern ret fn args
+#define NBDKIT_EXTERN_DECL(ret, fn, args) \
+ __attribute__((__visibility__("default"))) \
+ extern ret fn args
+#define NBDKIT_DLL_PUBLIC __attribute__((__visibility__("default")))
#else
#define NBDKIT_EXTERN_DECL(ret, fn, args) \
extern __declspec(dllexport) ret fn args
+#define NBDKIT_DLL_PUBLIC __declspec(dllimport)
#endif
NBDKIT_EXTERN_DECL (void, nbdkit_error,
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 633d34e7..8b587d70 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -263,6 +263,7 @@ struct nbdkit_filter {
#define NBDKIT_REGISTER_FILTER(filter) \
NBDKIT_CXX_LANG_C \
+ NBDKIT_DLL_PUBLIC \
struct nbdkit_filter * \
filter_init (void) \
{ \
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index 611987c7..59ec11b6 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -154,6 +154,7 @@ NBDKIT_EXTERN_DECL (int, nbdkit_is_tls, (void));
#define NBDKIT_REGISTER_PLUGIN(plugin) \
NBDKIT_CXX_LANG_C \
+ NBDKIT_DLL_PUBLIC \
struct nbdkit_plugin * \
plugin_init (void) \
{ \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ae801290..acb74fa8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1027,6 +1027,7 @@ libvixDiskLib_la_SOURCES = \
$(NULL)
libvixDiskLib_la_CPPFLAGS = \
-I$(top_srcdir)/plugins/vddk \
+ -I$(top_srcdir)/include \
$(NULL)
libvixDiskLib_la_CXXFLAGS = $(WARNINGS_CFLAGS)
# For use of the -rpath option, see:
diff --git a/plugins/ocaml/bindings.c b/plugins/ocaml/bindings.c
index 493b147b..a6d57084 100644
--- a/plugins/ocaml/bindings.c
+++ b/plugins/ocaml/bindings.c
@@ -51,7 +51,7 @@
/* Bindings for miscellaneous nbdkit_* utility functions. */
/* NB: noalloc function. */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_error (value nv)
{
int err;
@@ -77,7 +77,7 @@ ocaml_nbdkit_set_error (value nv)
return Val_unit;
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_parse_size (value strv)
{
CAMLparam1 (strv);
@@ -92,7 +92,7 @@ ocaml_nbdkit_parse_size (value strv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_parse_bool (value strv)
{
CAMLparam1 (strv);
@@ -107,7 +107,7 @@ ocaml_nbdkit_parse_bool (value strv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_read_password (value strv)
{
CAMLparam1 (strv);
@@ -124,7 +124,7 @@ ocaml_nbdkit_read_password (value strv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_realpath (value strv)
{
CAMLparam1 (strv);
@@ -140,7 +140,7 @@ ocaml_nbdkit_realpath (value strv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_nanosleep (value secv, value nsecv)
{
CAMLparam2 (secv, nsecv);
@@ -155,7 +155,7 @@ ocaml_nbdkit_nanosleep (value secv, value nsecv)
CAMLreturn (Val_unit);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_export_name (value unitv)
{
CAMLparam1 (unitv);
@@ -174,7 +174,7 @@ ocaml_nbdkit_export_name (value unitv)
}
/* NB: noalloc function. */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_shutdown (value unitv)
{
CAMLparam1 (unitv);
@@ -184,7 +184,7 @@ ocaml_nbdkit_shutdown (value unitv)
}
/* NB: noalloc function. */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_debug (value strv)
{
nbdkit_debug ("%s", String_val (strv));
@@ -192,7 +192,7 @@ ocaml_nbdkit_debug (value strv)
return Val_unit;
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_version (value unitv)
{
CAMLparam1 (unitv);
@@ -202,7 +202,7 @@ ocaml_nbdkit_version (value unitv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_peer_pid (value unitv)
{
CAMLparam1 (unitv);
@@ -213,7 +213,7 @@ ocaml_nbdkit_peer_pid (value unitv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_peer_uid (value unitv)
{
CAMLparam1 (unitv);
@@ -224,7 +224,7 @@ ocaml_nbdkit_peer_uid (value unitv)
CAMLreturn (rv);
}
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_peer_gid (value unitv)
{
CAMLparam1 (unitv);
diff --git a/plugins/ocaml/plugin.c b/plugins/ocaml/plugin.c
index dc2559b5..00959cb6 100644
--- a/plugins/ocaml/plugin.c
+++ b/plugins/ocaml/plugin.c
@@ -85,7 +85,7 @@ static struct nbdkit_plugin plugin = {
.unload = unload_wrapper,
};
-struct nbdkit_plugin *
+NBDKIT_DLL_PUBLIC struct nbdkit_plugin *
plugin_init (void)
{
if (plugin.name == NULL) {
@@ -743,7 +743,7 @@ cache_wrapper (void *h, uint32_t count, uint64_t offset, uint32_t flags)
*/
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_name (value namev)
{
plugin.name = strdup (String_val (namev));
@@ -751,7 +751,7 @@ ocaml_nbdkit_set_name (value namev)
}
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_longname (value longnamev)
{
plugin.longname = strdup (String_val (longnamev));
@@ -759,7 +759,7 @@ ocaml_nbdkit_set_longname (value longnamev)
}
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_version (value versionv)
{
plugin.version = strdup (String_val (versionv));
@@ -767,7 +767,7 @@ ocaml_nbdkit_set_version (value versionv)
}
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_description (value descriptionv)
{
plugin.description = strdup (String_val (descriptionv));
@@ -775,7 +775,7 @@ ocaml_nbdkit_set_description (value descriptionv)
}
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_config_help (value helpv)
{
plugin.config_help = strdup (String_val (helpv));
@@ -783,7 +783,7 @@ ocaml_nbdkit_set_config_help (value helpv)
}
/* NB: noalloc function */
-value
+NBDKIT_DLL_PUBLIC value
ocaml_nbdkit_set_field (value fieldv, value fv)
{
const char *field = String_val (fieldv);
diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c
index c100fc8f..9b5ae0a2 100644
--- a/tests/dummy-vddk.c
+++ b/tests/dummy-vddk.c
@@ -42,6 +42,8 @@
#include <pthread.h>
+#include "nbdkit-plugin.h" /* only for NBDKIT_DLL_PUBLIC */
+
#include "vddk-structs.h"
#define STUB(fn,ret,args) extern ret fn args;
@@ -64,7 +66,7 @@ bg_thread (void *datav)
return NULL;
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_InitEx (uint32_t major, uint32_t minor,
VixDiskLibGenericLogFunc *log_function,
VixDiskLibGenericLogFunc *warn_function,
@@ -88,32 +90,32 @@ VixDiskLib_InitEx (uint32_t major, uint32_t minor,
return VIX_OK;
}
-void
+NBDKIT_DLL_PUBLIC void
VixDiskLib_Exit (void)
{
/* Do nothing. */
}
-char *
+NBDKIT_DLL_PUBLIC char *
VixDiskLib_GetErrorText (VixError err, const char *unused)
{
return strdup ("dummy-vddk: error message");
}
-void
+NBDKIT_DLL_PUBLIC void
VixDiskLib_FreeErrorText (char *text)
{
free (text);
}
-void
+NBDKIT_DLL_PUBLIC void
VixDiskLib_FreeConnectParams (VixDiskLibConnectParams *params)
{
/* never called since we don't define optional AllocateConnectParams */
abort ();
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
char read_only,
const char *snapshot_ref,
@@ -129,7 +131,7 @@ VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
return VIX_OK;
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_Open (const VixDiskLibConnection connection,
const char *path,
uint32_t flags,
@@ -142,25 +144,25 @@ VixDiskLib_Open (const VixDiskLibConnection connection,
return VIX_OK;
}
-const char *
+NBDKIT_DLL_PUBLIC const char *
VixDiskLib_GetTransportMode (VixDiskLibHandle handle)
{
return "file";
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_Close (VixDiskLibHandle handle)
{
return VIX_OK;
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_Disconnect (VixDiskLibConnection connection)
{
return VIX_OK;
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_GetInfo (VixDiskLibHandle handle,
VixDiskLibInfo **info)
{
@@ -169,13 +171,13 @@ VixDiskLib_GetInfo (VixDiskLibHandle handle,
return VIX_OK;
}
-void
+NBDKIT_DLL_PUBLIC void
VixDiskLib_FreeInfo (VixDiskLibInfo *info)
{
free (info);
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_Read (VixDiskLibHandle handle,
uint64_t start_sector, uint64_t nr_sectors,
unsigned char *buf)
@@ -186,7 +188,7 @@ VixDiskLib_Read (VixDiskLibHandle handle,
return VIX_OK;
}
-VixError
+NBDKIT_DLL_PUBLIC VixError
VixDiskLib_Write (VixDiskLibHandle handle,
uint64_t start_sector, uint64_t nr_sectors,
const unsigned char *buf)
--
2.31.1
2 years, 11 months
[PATCH INCOMPLETE] daemon: Reimplement LVM filters using lvmdevices command
by Richard W.M. Jones
LVM2 managed to break device filters. This patch attempts a fix.
https://bugzilla.redhat.com/show_bug.cgi?id=1965941
However it is not working, because the new "lvmdevices" command which
is supposed to be used to set the new filters does not work for
partitioned whole devices. As far as I can tell there is no way to
emulate the old behaviour of filtering such a device. (Adding filters
for each partition in the device is one possible workaround, but the
filters will be out of date as soon as a new partition is created on
the device.)
While I investigate this, I'm posting the patch FYI.
Rich.
2 years, 11 months
Libguestfs / virt-v2v hiring
by Richard W.M. Jones
https://global-redhat.icims.com/jobs/86245/software-engineer---virtualiza...
The work is on libguestfs, guestfs-tools, virt-v2v (primarily),
virt-p2v, and related projects. Upstream development and bug fixing,
dealing with customer bugs, downstream (RHEL) packaging. You will
need excellent C and Linux skills. OCaml is not required, but you'll
need to pick it up.
It says that the job is based in Brno, Czech Republic, but that's not
strictly true. We'd consider hiring world wide (but note that Red Hat
*must* have an office in your country of residence, sorry DPRK). Also
remote working will be considered for the right, self-motivated
candidate.
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
2 years, 11 months
[PATCH] build: Don't use non-POSIX tests
by Martin Kletzander
The `test` builtin/binary usually accepts `==` for string comparison, it is
mostly accepted for typos and people being used to double equals, but is not
documented and not always accepted either. Since autoconf uses the default
shell, it might just fail in some cases with:
./configure: 29986: test: xrustc: unexpected operator
./configure: 29990: test: xcargo: unexpected operator
Just change it to single equals as it is done everywhere else.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
m4/guestfs-rust.m4 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/guestfs-rust.m4 b/m4/guestfs-rust.m4
index aa12a9ef5e5d..1dffd8118874 100644
--- a/m4/guestfs-rust.m4
+++ b/m4/guestfs-rust.m4
@@ -24,8 +24,8 @@ AS_IF([test "x$enable_rust" != "xno"],[
AC_CHECK_PROG([RUSTC],[rustc],[rustc],[no])
AC_CHECK_PROG([CARGO],[cargo],[cargo],[no])
- AS_IF([test "x$RUSTC" == "xno"], [AC_MSG_WARN([rustc not found])])
- AS_IF([test "x$CARGO" == "xno"], [AC_MSG_WARN([cargo not found])])
+ AS_IF([test "x$RUSTC" = "xno"], [AC_MSG_WARN([rustc not found])])
+ AS_IF([test "x$CARGO" = "xno"], [AC_MSG_WARN([cargo not found])])
],[
RUSTC=no
CARGO=no
--
2.31.1
2 years, 11 months