From 0d3bf3232718c605984afd38e87108e9d8678001 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 29 Oct 2021 14:09:21 +0100 Subject: [PATCH] Add asynchronous support in nbdkit-vddk-plugin resolves: rhbz#2018463, rhbz#2011709 --- ...ctor-how-D-vddk.stats-1-is-collected.patch | 117 ++ ...dk.stats-1-to-show-number-of-calls-a.patch | 140 ++ ...d-consolidate-VDDK_CALL_START-END-ma.patch | 320 ++++ ...troubleshooting-performance-problems.patch | 84 ++ ...K-major-library-version-in-dump-plug.patch | 141 ++ ...-and-physical-sector-size-to-D-vddk..patch | 52 + 0007-vddk-Fix-typo-in-debug-message.patch | 27 + ...vddk_library_version-when-we-managed.patch | 55 + ...ine-in-dump-plugin-output-for-each-V.patch | 72 + ...ent-about-VixDiskLib_PrepareForAcces.patch | 31 + ...-what-I-found-about-VixDiskLib_Flush.patch | 32 + ...-Note-that-we-have-tested-VDDK-7.0.3.patch | 26 + ...ddk-Move-minimum-version-to-VDDK-6.5.patch | 147 ++ ...rite-and-wait-asynchronous-functions.patch | 72 + ...art-to-split-VDDK-over-several-files.patch | 259 ++++ ...actor-D-vddk.stats-1-into-a-new-file.patch | 287 ++++ ...vddk-Implement-parallel-thread-model.patch | 1287 +++++++++++++++++ nbdkit.spec | 26 +- sources | 4 +- 19 files changed, 3174 insertions(+), 5 deletions(-) create mode 100644 0001-vddk-Refactor-how-D-vddk.stats-1-is-collected.patch create mode 100644 0002-vddk-Extend-D-vddk.stats-1-to-show-number-of-calls-a.patch create mode 100644 0003-vddk-Simplify-and-consolidate-VDDK_CALL_START-END-ma.patch create mode 100644 0004-vddk-Document-troubleshooting-performance-problems.patch create mode 100644 0005-vddk-Include-VDDK-major-library-version-in-dump-plug.patch create mode 100644 0006-vddk-Add-logical-and-physical-sector-size-to-D-vddk..patch create mode 100644 0007-vddk-Fix-typo-in-debug-message.patch create mode 100644 0008-vddk-Only-print-vddk_library_version-when-we-managed.patch create mode 100644 0009-vddk-Print-one-line-in-dump-plugin-output-for-each-V.patch create mode 100644 0010-vddk-Update-comment-about-VixDiskLib_PrepareForAcces.patch create mode 100644 0011-vddk-Document-what-I-found-about-VixDiskLib_Flush.patch create mode 100644 0012-vddk-Note-that-we-have-tested-VDDK-7.0.3.patch create mode 100644 0013-vddk-Move-minimum-version-to-VDDK-6.5.patch create mode 100644 0014-vddk-Add-read-write-and-wait-asynchronous-functions.patch create mode 100644 0015-vddk-Start-to-split-VDDK-over-several-files.patch create mode 100644 0016-vddk-Refactor-D-vddk.stats-1-into-a-new-file.patch create mode 100644 0017-vddk-Implement-parallel-thread-model.patch diff --git a/0001-vddk-Refactor-how-D-vddk.stats-1-is-collected.patch b/0001-vddk-Refactor-how-D-vddk.stats-1-is-collected.patch new file mode 100644 index 0000000..2d91302 --- /dev/null +++ b/0001-vddk-Refactor-how-D-vddk.stats-1-is-collected.patch @@ -0,0 +1,117 @@ +From 7ab943bbb0ac0dd0094d6771d4cb04374d569eb3 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Thu, 21 Oct 2021 14:49:52 +0100 +Subject: [PATCH] vddk: Refactor how -D vddk.stats=1 is collected + +In order to allow us to collect more per-API stats, introduce a global +struct per API for storing these stats. + +(cherry picked from commit 3d8657f3d9a2c1b59284333566428b4c7ce32a74) +--- + plugins/vddk/vddk.c | 36 ++++++++++++++++++------------------ + 1 file changed, 18 insertions(+), 18 deletions(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index c6e023eb..fc872883 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -1,5 +1,5 @@ + /* nbdkit +- * Copyright (C) 2013-2020 Red Hat Inc. ++ * Copyright (C) 2013-2021 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are +@@ -103,14 +103,23 @@ static bool is_remote; + /* For each VDDK API define a variable to store the time taken (used + * to implement -D vddk.stats=1). + */ ++struct vddk_stat { ++ const char *name; /* function name */ ++ int64_t usecs; /* total number of usecs consumed */ ++}; + static pthread_mutex_t stats_lock = PTHREAD_MUTEX_INITIALIZER; + static void display_stats (void); +-#define STUB(fn,ret,args) static int64_t stats_##fn; +-#define OPTIONAL_STUB(fn,ret,args) static int64_t stats_##fn; ++#define STUB(fn,ret,args) \ ++ static struct vddk_stat stats_##fn = { .name = #fn } ++#define OPTIONAL_STUB(fn,ret,args) \ ++ static struct vddk_stat stats_##fn = { .name = #fn } + #include "vddk-stubs.h" + #undef STUB + #undef OPTIONAL_STUB + ++/* Macros to bracket each VDDK API call, for printing debugging ++ * information and collecting statistics. ++ */ + #define VDDK_CALL_START(fn, fs, ...) \ + do { \ + struct timeval start_t, end_t; \ +@@ -131,10 +140,11 @@ static void display_stats (void); + if (vddk_debug_stats) { \ + gettimeofday (&end_t, NULL); \ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ +- stats_##fn += tvdiff_usec (&start_t, &end_t); \ ++ stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ + } \ + } while (0) + ++/* Print VDDK errors. */ + #define VDDK_ERROR(err, fs, ...) \ + do { \ + char *vddk_err_msg; \ +@@ -167,10 +177,6 @@ vddk_unload (void) + free (password); + } + +-struct vddk_stat { +- const char *fn; +- int64_t usecs; +-}; + DEFINE_VECTOR_TYPE(statlist, struct vddk_stat) + + static int +@@ -179,7 +185,7 @@ stat_compare (const void *vp1, const void *vp2) + const struct vddk_stat *st1 = vp1; + const struct vddk_stat *st2 = vp2; + +- /* Note: sorts in reverse order. */ ++ /* Note: sorts in reverse order of time spent in each API call. */ + if (st1->usecs < st2->usecs) return 1; + else if (st1->usecs > st2->usecs) return -1; + else return 0; +@@ -189,19 +195,13 @@ static void + display_stats (void) + { + statlist stats = empty_vector; +- struct vddk_stat st; + size_t i; + +-#define ADD_ONE_STAT(fn_, usecs_) \ +- st.fn = fn_; \ +- st.usecs = usecs_; \ +- statlist_append (&stats, st) +-#define STUB(fn,ret,args) ADD_ONE_STAT (#fn, stats_##fn); +-#define OPTIONAL_STUB(fn,ret,args) ADD_ONE_STAT (#fn, stats_##fn); ++#define STUB(fn,ret,args) statlist_append (&stats, stats_##fn) ++#define OPTIONAL_STUB(fn,ret,args) statlist_append (&stats, stats_##fn) + #include "vddk-stubs.h" + #undef STUB + #undef OPTIONAL_STUB +-#undef ADD_ONE_STAT + + qsort (stats.ptr, stats.size, sizeof stats.ptr[0], stat_compare); + +@@ -209,7 +209,7 @@ display_stats (void) + nbdkit_debug ("%-40s %9s", "", "µs"); + for (i = 0; i < stats.size; ++i) { + if (stats.ptr[i].usecs) +- nbdkit_debug ("%-40s %9" PRIi64, stats.ptr[i].fn, stats.ptr[i].usecs); ++ nbdkit_debug ("%-40s %9" PRIi64, stats.ptr[i].name, stats.ptr[i].usecs); + } + statlist_reset (&stats); + } +-- +2.31.1 + diff --git a/0002-vddk-Extend-D-vddk.stats-1-to-show-number-of-calls-a.patch b/0002-vddk-Extend-D-vddk.stats-1-to-show-number-of-calls-a.patch new file mode 100644 index 0000000..6eaf301 --- /dev/null +++ b/0002-vddk-Extend-D-vddk.stats-1-to-show-number-of-calls-a.patch @@ -0,0 +1,140 @@ +From 7139553f4008c0d4dfdce1dae4c933713af60548 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Thu, 21 Oct 2021 15:10:00 +0100 +Subject: [PATCH] vddk: Extend -D vddk.stats=1 to show number of calls and + bytes transferred +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The new output looks like this: + +nbdkit: debug: VDDK function stats (-D vddk.stats=1): +nbdkit: debug: VixDiskLib_... µs calls bytes +nbdkit: debug: Exit 1000854 1 +nbdkit: debug: InitEx 79304 1 +nbdkit: debug: Flush 13577 1 +nbdkit: debug: Write 12534 21 10485760 +nbdkit: debug: Open 4753 3 +nbdkit: debug: Read 966 20 5242880 +nbdkit: debug: Close 574 3 +nbdkit: debug: QueryAllocatedBlocks 116 4 +nbdkit: debug: ConnectEx 103 3 +nbdkit: debug: Disconnect 88 3 +nbdkit: debug: GetTransportMode 68 3 +nbdkit: debug: GetInfo 46 3 +nbdkit: debug: FreeConnectParams 36 3 +nbdkit: debug: FreeInfo 36 3 +nbdkit: debug: FreeBlockList 22 4 +nbdkit: debug: AllocateConnectParams 22 3 +(cherry picked from commit 5c80f0d290db45a679d55baf37ff39bacb8ce7ec) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 3 +-- + plugins/vddk/vddk.c | 41 +++++++++++++++++++++++++---- + 2 files changed, 37 insertions(+), 7 deletions(-) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 2363b8fa..88df0ddb 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -517,8 +517,7 @@ Suppress debugging of datapath calls (C and C). + + =item B<-D vddk.stats=1> + +-When the plugin exits print some statistics about the amount of time +-spent waiting on each VDDK call. ++When the plugin exits print some statistics about each VDDK call. + + =back + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index fc872883..b623c25c 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -106,6 +106,8 @@ static bool is_remote; + struct vddk_stat { + const char *name; /* function name */ + int64_t usecs; /* total number of usecs consumed */ ++ uint64_t calls; /* number of times called */ ++ uint64_t bytes; /* bytes transferred, datapath calls only */ + }; + static pthread_mutex_t stats_lock = PTHREAD_MUTEX_INITIALIZER; + static void display_stats (void); +@@ -141,6 +143,17 @@ static void display_stats (void); + gettimeofday (&end_t, NULL); \ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ + stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ ++ stats_##fn.calls++; \ ++ } \ ++ } while (0) ++#define VDDK_CALL_END_DATAPATH(fn, bytes_) \ ++ while (0); \ ++ if (vddk_debug_stats) { \ ++ gettimeofday (&end_t, NULL); \ ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ ++ stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ ++ stats_##fn.calls++; \ ++ stats_##fn.bytes += bytes_; \ + } \ + } while (0) + +@@ -191,6 +204,12 @@ stat_compare (const void *vp1, const void *vp2) + else return 0; + } + ++static const char * ++api_name_without_prefix (const char *name) ++{ ++ return strncmp (name, "VixDiskLib_", 11) == 0 ? name + 11 : name; ++} ++ + static void + display_stats (void) + { +@@ -206,10 +225,22 @@ display_stats (void) + qsort (stats.ptr, stats.size, sizeof stats.ptr[0], stat_compare); + + nbdkit_debug ("VDDK function stats (-D vddk.stats=1):"); +- nbdkit_debug ("%-40s %9s", "", "µs"); ++ nbdkit_debug ("%-24s %15s %5s %15s", ++ "VixDiskLib_...", "µs", "calls", "bytes"); + for (i = 0; i < stats.size; ++i) { +- if (stats.ptr[i].usecs) +- nbdkit_debug ("%-40s %9" PRIi64, stats.ptr[i].name, stats.ptr[i].usecs); ++ if (stats.ptr[i].usecs) { ++ if (stats.ptr[i].bytes > 0) ++ nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64 " %15" PRIu64, ++ api_name_without_prefix (stats.ptr[i].name), ++ stats.ptr[i].usecs, ++ stats.ptr[i].calls, ++ stats.ptr[i].bytes); ++ else ++ nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64, ++ api_name_without_prefix (stats.ptr[i].name), ++ stats.ptr[i].usecs, ++ stats.ptr[i].calls); ++ } + } + statlist_reset (&stats); + } +@@ -832,7 +863,7 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + "%" PRIu32 " sectors, buffer", + offset, count) { + err = VixDiskLib_Read (h->handle, offset, count, buf); +- } VDDK_CALL_END (VixDiskLib_Read); ++ } VDDK_CALL_END_DATAPATH (VixDiskLib_Read, count * VIXDISKLIB_SECTOR_SIZE); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Read"); + return -1; +@@ -872,7 +903,7 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + "%" PRIu32 " sectors, buffer", + offset, count) { + err = VixDiskLib_Write (h->handle, offset, count, buf); +- } VDDK_CALL_END (VixDiskLib_Write); ++ } VDDK_CALL_END_DATAPATH (VixDiskLib_Write, count * VIXDISKLIB_SECTOR_SIZE); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Write"); + return -1; +-- +2.31.1 + diff --git a/0003-vddk-Simplify-and-consolidate-VDDK_CALL_START-END-ma.patch b/0003-vddk-Simplify-and-consolidate-VDDK_CALL_START-END-ma.patch new file mode 100644 index 0000000..1c8eb31 --- /dev/null +++ b/0003-vddk-Simplify-and-consolidate-VDDK_CALL_START-END-ma.patch @@ -0,0 +1,320 @@ +From 8bd83802eb36508c2ca647da9ba8fb8a5dce7a2b Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Thu, 21 Oct 2021 22:55:17 +0100 +Subject: [PATCH] vddk: Simplify and consolidate VDDK_CALL_START/END macros + +We don't need the VDDK_CALL_*_DATAPATH versions of these macros +because the compiler is able to optimize some static strcmps. +Furthermore we can remove extra { .. } when the macros are applied. + +(cherry picked from commit 3ea0ed6582faa8f800b7a2a15d58032917a21bd5) +--- + plugins/vddk/vddk.c | 124 ++++++++++++++++++++------------------------ + 1 file changed, 56 insertions(+), 68 deletions(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index b623c25c..85b0c722 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -125,28 +125,16 @@ static void display_stats (void); + #define VDDK_CALL_START(fn, fs, ...) \ + do { \ + struct timeval start_t, end_t; \ ++ /* GCC can optimize this away at compile time: */ \ ++ const bool datapath = \ ++ strcmp (#fn, "VixDiskLib_Read") == 0 || \ ++ strcmp (#fn, "VixDiskLib_Write") == 0; \ + if (vddk_debug_stats) \ + gettimeofday (&start_t, NULL); \ +- nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ +- do +-#define VDDK_CALL_START_DATAPATH(fn, fs, ...) \ +- do { \ +- struct timeval start_t, end_t; \ +- if (vddk_debug_stats) \ +- gettimeofday (&start_t, NULL); \ +- if (vddk_debug_datapath) \ ++ if (!datapath || vddk_debug_datapath) \ + nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ + do +-#define VDDK_CALL_END(fn) \ +- while (0); \ +- if (vddk_debug_stats) { \ +- gettimeofday (&end_t, NULL); \ +- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ +- stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ +- stats_##fn.calls++; \ +- } \ +- } while (0) +-#define VDDK_CALL_END_DATAPATH(fn, bytes_) \ ++#define VDDK_CALL_END(fn, bytes_) \ + while (0); \ + if (vddk_debug_stats) { \ + gettimeofday (&end_t, NULL); \ +@@ -161,13 +149,13 @@ static void display_stats (void); + #define VDDK_ERROR(err, fs, ...) \ + do { \ + char *vddk_err_msg; \ +- VDDK_CALL_START (VixDiskLib_GetErrorText, "%lu", err) { \ ++ VDDK_CALL_START (VixDiskLib_GetErrorText, "%lu", err) \ + vddk_err_msg = VixDiskLib_GetErrorText ((err), NULL); \ +- } VDDK_CALL_END (VixDiskLib_GetErrorText); \ ++ VDDK_CALL_END (VixDiskLib_GetErrorText, 0); \ + nbdkit_error (fs ": %s", ##__VA_ARGS__, vddk_err_msg); \ +- VDDK_CALL_START (VixDiskLib_FreeErrorText, "") { \ ++ VDDK_CALL_START (VixDiskLib_FreeErrorText, "") \ + VixDiskLib_FreeErrorText (vddk_err_msg); \ +- } VDDK_CALL_END (VixDiskLib_FreeErrorText); \ ++ VDDK_CALL_END (VixDiskLib_FreeErrorText, 0); \ + } while (0) + + /* Unload the plugin. */ +@@ -175,9 +163,9 @@ static void + vddk_unload (void) + { + if (init_called) { +- VDDK_CALL_START (VixDiskLib_Exit, "") { ++ VDDK_CALL_START (VixDiskLib_Exit, "") + VixDiskLib_Exit (); +- } VDDK_CALL_END (VixDiskLib_Exit); ++ VDDK_CALL_END (VixDiskLib_Exit, 0); + } + if (dl) + dlclose (dl); +@@ -572,13 +560,13 @@ vddk_after_fork (void) + VDDK_CALL_START (VixDiskLib_InitEx, + "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", + VDDK_MAJOR, VDDK_MINOR, +- libdir, config ? : "NULL") { ++ libdir, config ? : "NULL") + err = VixDiskLib_InitEx (VDDK_MAJOR, VDDK_MINOR, + &debug_function, /* log function */ + &error_function, /* warn function */ + &error_function, /* panic function */ + libdir, config); +- } VDDK_CALL_END (VixDiskLib_InitEx); ++ VDDK_CALL_END (VixDiskLib_InitEx, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_InitEx"); + exit (EXIT_FAILURE); +@@ -640,9 +628,9 @@ allocate_connect_params (void) + VixDiskLibConnectParams *ret; + + if (VixDiskLib_AllocateConnectParams != NULL) { +- VDDK_CALL_START (VixDiskLib_AllocateConnectParams, "") { ++ VDDK_CALL_START (VixDiskLib_AllocateConnectParams, "") + ret = VixDiskLib_AllocateConnectParams (); +- } VDDK_CALL_END (VixDiskLib_AllocateConnectParams); ++ VDDK_CALL_END (VixDiskLib_AllocateConnectParams, 0); + } + else + ret = calloc (1, sizeof (VixDiskLibConnectParams)); +@@ -657,9 +645,9 @@ free_connect_params (VixDiskLibConnectParams *params) + * originally called. Otherwise use free. + */ + if (VixDiskLib_AllocateConnectParams != NULL) { +- VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params") { ++ VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params") + VixDiskLib_FreeConnectParams (params); +- } VDDK_CALL_END (VixDiskLib_FreeConnectParams); ++ VDDK_CALL_END (VixDiskLib_FreeConnectParams, 0); + } + else + free (params); +@@ -717,13 +705,13 @@ vddk_open (int readonly) + "h->params, %d, %s, %s, &connection", + readonly, + snapshot_moref ? : "NULL", +- transport_modes ? : "NULL") { ++ transport_modes ? : "NULL") + err = VixDiskLib_ConnectEx (h->params, + readonly, + snapshot_moref, + transport_modes, + &h->connection); +- } VDDK_CALL_END (VixDiskLib_ConnectEx); ++ VDDK_CALL_END (VixDiskLib_ConnectEx, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_ConnectEx"); + goto err1; +@@ -744,25 +732,25 @@ vddk_open (int readonly) + } + + VDDK_CALL_START (VixDiskLib_Open, +- "connection, %s, %d, &handle", filename, flags) { ++ "connection, %s, %d, &handle", filename, flags) + err = VixDiskLib_Open (h->connection, filename, flags, &h->handle); +- } VDDK_CALL_END (VixDiskLib_Open); ++ VDDK_CALL_END (VixDiskLib_Open, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Open: %s", filename); + goto err2; + } + +- VDDK_CALL_START (VixDiskLib_GetTransportMode, "handle") { ++ VDDK_CALL_START (VixDiskLib_GetTransportMode, "handle") + transport_mode = VixDiskLib_GetTransportMode (h->handle); +- } VDDK_CALL_END (VixDiskLib_GetTransportMode); ++ VDDK_CALL_END (VixDiskLib_GetTransportMode, 0); + nbdkit_debug ("transport mode: %s", transport_mode); + + return h; + + err2: +- VDDK_CALL_START (VixDiskLib_Disconnect, "connection") { ++ VDDK_CALL_START (VixDiskLib_Disconnect, "connection") + VixDiskLib_Disconnect (h->connection); +- } VDDK_CALL_END (VixDiskLib_Disconnect); ++ VDDK_CALL_END (VixDiskLib_Disconnect, 0); + err1: + free_connect_params (h->params); + err0: +@@ -777,12 +765,12 @@ vddk_close (void *handle) + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); + struct vddk_handle *h = handle; + +- VDDK_CALL_START (VixDiskLib_Close, "handle") { ++ VDDK_CALL_START (VixDiskLib_Close, "handle") + VixDiskLib_Close (h->handle); +- } VDDK_CALL_END (VixDiskLib_Close); +- VDDK_CALL_START (VixDiskLib_Disconnect, "connection") { ++ VDDK_CALL_END (VixDiskLib_Close, 0); ++ VDDK_CALL_START (VixDiskLib_Disconnect, "connection") + VixDiskLib_Disconnect (h->connection); +- } VDDK_CALL_END (VixDiskLib_Disconnect); ++ VDDK_CALL_END (VixDiskLib_Disconnect, 0); + + free_connect_params (h->params); + free (h); +@@ -797,9 +785,9 @@ vddk_get_size (void *handle) + VixError err; + uint64_t size; + +- VDDK_CALL_START (VixDiskLib_GetInfo, "handle, &info") { ++ VDDK_CALL_START (VixDiskLib_GetInfo, "handle, &info") + err = VixDiskLib_GetInfo (h->handle, &info); +- } VDDK_CALL_END (VixDiskLib_GetInfo); ++ VDDK_CALL_END (VixDiskLib_GetInfo, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_GetInfo"); + return -1; +@@ -828,9 +816,9 @@ vddk_get_size (void *handle) + info->uuid ? : "NULL"); + } + +- VDDK_CALL_START (VixDiskLib_FreeInfo, "info") { ++ VDDK_CALL_START (VixDiskLib_FreeInfo, "info") + VixDiskLib_FreeInfo (info); +- } VDDK_CALL_END (VixDiskLib_FreeInfo); ++ VDDK_CALL_END (VixDiskLib_FreeInfo, 0); + + return (int64_t) size; + } +@@ -858,12 +846,12 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + offset /= VIXDISKLIB_SECTOR_SIZE; + count /= VIXDISKLIB_SECTOR_SIZE; + +- VDDK_CALL_START_DATAPATH (VixDiskLib_Read, +- "handle, %" PRIu64 " sectors, " +- "%" PRIu32 " sectors, buffer", +- offset, count) { ++ VDDK_CALL_START (VixDiskLib_Read, ++ "handle, %" PRIu64 " sectors, " ++ "%" PRIu32 " sectors, buffer", ++ offset, count) + err = VixDiskLib_Read (h->handle, offset, count, buf); +- } VDDK_CALL_END_DATAPATH (VixDiskLib_Read, count * VIXDISKLIB_SECTOR_SIZE); ++ VDDK_CALL_END (VixDiskLib_Read, count * VIXDISKLIB_SECTOR_SIZE); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Read"); + return -1; +@@ -898,12 +886,12 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + offset /= VIXDISKLIB_SECTOR_SIZE; + count /= VIXDISKLIB_SECTOR_SIZE; + +- VDDK_CALL_START_DATAPATH (VixDiskLib_Write, +- "handle, %" PRIu64 " sectors, " +- "%" PRIu32 " sectors, buffer", +- offset, count) { ++ VDDK_CALL_START (VixDiskLib_Write, ++ "handle, %" PRIu64 " sectors, " ++ "%" PRIu32 " sectors, buffer", ++ offset, count) + err = VixDiskLib_Write (h->handle, offset, count, buf); +- } VDDK_CALL_END_DATAPATH (VixDiskLib_Write, count * VIXDISKLIB_SECTOR_SIZE); ++ VDDK_CALL_END (VixDiskLib_Write, count * VIXDISKLIB_SECTOR_SIZE); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Write"); + return -1; +@@ -938,9 +926,9 @@ vddk_flush (void *handle, uint32_t flags) + struct vddk_handle *h = handle; + VixError err; + +- VDDK_CALL_START (VixDiskLib_Flush, "handle") { ++ VDDK_CALL_START (VixDiskLib_Flush, "handle") + err = VixDiskLib_Flush (h->handle); +- } VDDK_CALL_END (VixDiskLib_Flush); ++ VDDK_CALL_END (VixDiskLib_Flush, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Flush"); + return -1; +@@ -976,17 +964,17 @@ vddk_can_extents (void *handle) + */ + VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, + "handle, 0, %d sectors, %d sectors", +- VIXDISKLIB_MIN_CHUNK_SIZE, VIXDISKLIB_MIN_CHUNK_SIZE) { ++ VIXDISKLIB_MIN_CHUNK_SIZE, VIXDISKLIB_MIN_CHUNK_SIZE) + err = VixDiskLib_QueryAllocatedBlocks (h->handle, + 0, VIXDISKLIB_MIN_CHUNK_SIZE, + VIXDISKLIB_MIN_CHUNK_SIZE, + &block_list); +- } VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks); ++ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); + error_suppression = 0; + if (err == VIX_OK) { +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") + VixDiskLib_FreeBlockList (block_list); +- } VDDK_CALL_END (VixDiskLib_FreeBlockList); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); + } + if (err != VIX_OK) { + char *errmsg = VixDiskLib_GetErrorText (err, NULL); +@@ -1066,12 +1054,12 @@ vddk_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, + VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, + "handle, %" PRIu64 " sectors, %" PRIu64 " sectors, " + "%d sectors", +- start_sector, nr_sectors, VIXDISKLIB_MIN_CHUNK_SIZE) { ++ start_sector, nr_sectors, VIXDISKLIB_MIN_CHUNK_SIZE) + err = VixDiskLib_QueryAllocatedBlocks (h->handle, + start_sector, nr_sectors, + VIXDISKLIB_MIN_CHUNK_SIZE, + &block_list); +- } VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks); ++ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks"); + return -1; +@@ -1090,15 +1078,15 @@ vddk_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, + add_extent (extents, &position, blk_offset, true) == -1) || + (add_extent (extents, + &position, blk_offset + blk_length, false) == -1)) { +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") + VixDiskLib_FreeBlockList (block_list); +- } VDDK_CALL_END (VixDiskLib_FreeBlockList); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); + return -1; + } + } +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") + VixDiskLib_FreeBlockList (block_list); +- } VDDK_CALL_END (VixDiskLib_FreeBlockList); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); + + /* There's an implicit hole after the returned list of blocks, up + * to the end of the QueryAllocatedBlocks request. +-- +2.31.1 + diff --git a/0004-vddk-Document-troubleshooting-performance-problems.patch b/0004-vddk-Document-troubleshooting-performance-problems.patch new file mode 100644 index 0000000..190b740 --- /dev/null +++ b/0004-vddk-Document-troubleshooting-performance-problems.patch @@ -0,0 +1,84 @@ +From 5bd343fff4a8bb7a115e4b39555a727b33ba52dd Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Fri, 22 Oct 2021 18:00:27 +0100 +Subject: [PATCH] vddk: Document troubleshooting performance problems + +Document how to use -D vddk.stats=1 to diagnose performance problems +with VDDK. + +(cherry picked from commit e491978c193f49010cc28ad344d0fb3c1b5ede35) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 57 +++++++++++++++++++++++++++++ + 1 file changed, 57 insertions(+) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 88df0ddb..6c50877e 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -475,6 +475,63 @@ and restarting the C service: + + For more information see L. + ++=head2 Troubleshooting performance problems ++ ++VDDK has very uneven performance with some operations being very slow. ++This plugin has options to allow you to debug performance issues. If ++your application has a debug or diagnostic setting, add the following ++nbdkit command line options: ++ ++ -v -D nbdkit.backend.datapath=0 -D vddk.datapath=0 -D vddk.stats=1 ++ ++C<-v> enables verbose messages and the two datapath options I ++the very verbose per-read/-write messages. C<-D vddk.stats=1> enables ++a summary when nbdkit exits of the cumulative time taken in each VDDK ++function, the number of times each function was called, and (for read ++and write) the number of bytes transferred. An example of what those ++stats look like can be found here: ++L ++ ++You can interpret the stats as follows: ++ ++=over 4 ++ ++=item C ++ ++The cumulative time spent waiting for VDDK to return from ++C calls, the number of times this function was ++called, and the total bytes read. You can use this to determine the ++read bandwidth to the VMware server. ++ ++=item C ++ ++=item C ++ ++Same as above, but for writing and flushing writes. ++ ++=item C ++ ++This call is used to query information about the sparseness of the ++remote disk. It is only available in VDDK E 6.7. The call is ++notably very slow in all versions of VMware we have tested. ++ ++=item C ++ ++=item C ++ ++=item C ++ ++=item C ++ ++=item C ++ ++=item C ++ ++The cumulative time spent connecting and disconnecting from the VMware ++server, which can also be very slow. ++ ++=back ++ + =head1 SUPPORTED VERSIONS OF VDDK + + This plugin requires VDDK E 5.5.5, which in turn means that it +-- +2.31.1 + diff --git a/0005-vddk-Include-VDDK-major-library-version-in-dump-plug.patch b/0005-vddk-Include-VDDK-major-library-version-in-dump-plug.patch new file mode 100644 index 0000000..7631c13 --- /dev/null +++ b/0005-vddk-Include-VDDK-major-library-version-in-dump-plug.patch @@ -0,0 +1,141 @@ +From 7777f1a86f692a9e9bc720e29272321f124208b8 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Sat, 23 Oct 2021 16:16:39 +0100 +Subject: [PATCH] vddk: Include VDDK major library version in --dump-plugin + output + +Although it doesn't seem to be possible to get the precise VDDK +version, With a relatively simple change we can at least return the +VDDK major version. Currently this can be 5, 6 or 7. + +(cherry picked from commit 8700649d147948897f3b97810a1dff37924bdd6e) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 4 ++++ + plugins/vddk/vddk.c | 29 +++++++++++++++++++---------- + tests/test-vddk-real-dump-plugin.sh | 2 ++ + 3 files changed, 25 insertions(+), 10 deletions(-) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 6c50877e..5c356cc4 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -422,6 +422,10 @@ at runtime. + If this is printed then the C parameter is supported + by this build. + ++=item C ++ ++The VDDK major library version: 5, 6, 7, ... ++ + =item C + + Prints the full path to the VDDK shared library. Since this requires +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index 85b0c722..14e7ada9 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -81,6 +81,7 @@ NBDKIT_DLL_PUBLIC int vddk_debug_stats; + static void *dl; /* dlopen handle */ + static bool init_called; /* was InitEx called */ + static __thread int error_suppression; /* threadlocal error suppression */ ++static int library_version; /* VDDK major: 5, 6, 7, ... */ + + static enum { NONE = 0, ZLIB, FASTLZ, SKIPZ } compression; /* compression */ + static char *config; /* config */ +@@ -405,7 +406,10 @@ vddk_config (const char *key, const char *value) + static void + load_library (bool load_error_is_fatal) + { +- static const char *sonames[] = { ++ static struct { ++ const char *soname; ++ int library_version; ++ } libs[] = { + /* Prefer the newest library in case multiple exist. Check two + * possible directories: the usual VDDK installation puts .so + * files in an arch-specific subdirectory of $libdir (our minimum +@@ -413,12 +417,13 @@ load_library (bool load_error_is_fatal) + * but our testsuite is easier to write if we point libdir + * directly to a stub .so. + */ +- "lib64/libvixDiskLib.so.7", +- "libvixDiskLib.so.7", +- "lib64/libvixDiskLib.so.6", +- "libvixDiskLib.so.6", +- "lib64/libvixDiskLib.so.5", +- "libvixDiskLib.so.5", ++ { "lib64/libvixDiskLib.so.7", 7 }, ++ { "libvixDiskLib.so.7", 7 }, ++ { "lib64/libvixDiskLib.so.6", 6 }, ++ { "libvixDiskLib.so.6", 6 }, ++ { "lib64/libvixDiskLib.so.5", 5 }, ++ { "libvixDiskLib.so.5", 5 }, ++ { NULL } + }; + size_t i; + CLEANUP_FREE char *orig_error = NULL; +@@ -431,19 +436,20 @@ load_library (bool load_error_is_fatal) + } + } + +- for (i = 0; i < sizeof sonames / sizeof sonames[0]; ++i) { ++ for (i = 0; libs[i].soname != NULL; ++i) { + CLEANUP_FREE char *path; + + /* Set the full path so that dlopen will preferentially load the + * system libraries from the same directory. + */ +- if (asprintf (&path, "%s/%s", libdir, sonames[i]) == -1) { ++ if (asprintf (&path, "%s/%s", libdir, libs[i].soname) == -1) { + nbdkit_error ("asprintf: %m"); + exit (EXIT_FAILURE); + } + + dl = dlopen (path, RTLD_NOW); + if (dl != NULL) { ++ library_version = libs[i].library_version; + /* Now that we found the library, ensure that LD_LIBRARY_PATH + * includes its directory for all future loads. This may modify + * path in-place and/or re-exec nbdkit, but that's okay. +@@ -464,10 +470,12 @@ load_library (bool load_error_is_fatal) + "If '%s' is located on a non-standard path you may need to\n" + "set libdir=/path/to/vmware-vix-disklib-distrib.\n\n" + "See nbdkit-vddk-plugin(1) man page section \"LIBRARY LOCATION\" for details.", +- orig_error ? : "(unknown error)", sonames[0]); ++ orig_error ? : "(unknown error)", libs[0].soname); + exit (EXIT_FAILURE); + } + ++ assert (library_version >= 5); ++ + /* Load symbols. */ + #define STUB(fn,ret,args) \ + do { \ +@@ -583,6 +591,7 @@ vddk_dump_plugin (void) + + printf ("vddk_default_libdir=%s\n", VDDK_LIBDIR); + printf ("vddk_has_nfchostport=1\n"); ++ printf ("vddk_library_version=%d\n", library_version); + + #if defined(HAVE_DLADDR) + /* It would be nice to print the version of VDDK from the shared +diff --git a/tests/test-vddk-real-dump-plugin.sh b/tests/test-vddk-real-dump-plugin.sh +index 1479e416..59c79693 100755 +--- a/tests/test-vddk-real-dump-plugin.sh ++++ b/tests/test-vddk-real-dump-plugin.sh +@@ -51,10 +51,12 @@ rm -f $files + cleanup_fn rm -f $files + + nbdkit -f -v vddk libdir="$vddkdir" --dump-plugin > $out ++cat $out + + # Check the vddk_* entries are set. + grep ^vddk_default_libdir= $out + grep ^vddk_has_nfchostport= $out ++grep ^vddk_library_version= $out + grep ^vddk_dll= $out + + dll="$(grep ^vddk_dll $out | cut -d= -f2)" +-- +2.31.1 + diff --git a/0006-vddk-Add-logical-and-physical-sector-size-to-D-vddk..patch b/0006-vddk-Add-logical-and-physical-sector-size-to-D-vddk..patch new file mode 100644 index 0000000..c8a377d --- /dev/null +++ b/0006-vddk-Add-logical-and-physical-sector-size-to-D-vddk..patch @@ -0,0 +1,52 @@ +From b4a6854a45eed920ed6631c8c38e979b95a52470 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Sat, 23 Oct 2021 16:24:27 +0100 +Subject: [PATCH] vddk: Add logical and physical sector size to -D + vddk.diskinfo output + +In VDDK >= 7 it is possible to display the logical and physical sector +size in debug output. + +This commit also extends the test since this flag was not tested +before. + +(cherry picked from commit 5bb8f0586e1faabcbf4f43d722a3b3cb5b352e33) +--- + plugins/vddk/vddk.c | 6 ++++++ + tests/test-vddk-real.sh | 3 ++- + 2 files changed, 8 insertions(+), 1 deletion(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index 14e7ada9..290a99a8 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -823,6 +823,12 @@ vddk_get_size (void *handle) + info->parentFileNameHint ? : "NULL"); + nbdkit_debug ("disk info: uuid: %s", + info->uuid ? : "NULL"); ++ if (library_version >= 7) { ++ nbdkit_debug ("disk info: sectory size: " ++ "logical %" PRIu32 " physical %" PRIu32, ++ info->logicalSectorSize, ++ info->physicalSectorSize); ++ } + } + + VDDK_CALL_START (VixDiskLib_FreeInfo, "info") +diff --git a/tests/test-vddk-real.sh b/tests/test-vddk-real.sh +index 08f6f8d8..b5f85067 100755 +--- a/tests/test-vddk-real.sh ++++ b/tests/test-vddk-real.sh +@@ -82,7 +82,8 @@ if grep 'cannot open shared object file' test-vddk-real.log; then + fi + + # Now run nbdkit for the test. +-start_nbdkit -P $pid -U $sock -D vddk.stats=1 vddk libdir="$vddkdir" $vmdk ++start_nbdkit -P $pid -U $sock -D vddk.stats=1 -D vddk.diskinfo=1 \ ++ vddk libdir="$vddkdir" $vmdk + uri="nbd+unix:///?socket=$sock" + + # VDDK < 6.0 did not support flush, so disable flush test there. Also +-- +2.31.1 + diff --git a/0007-vddk-Fix-typo-in-debug-message.patch b/0007-vddk-Fix-typo-in-debug-message.patch new file mode 100644 index 0000000..81ca52d --- /dev/null +++ b/0007-vddk-Fix-typo-in-debug-message.patch @@ -0,0 +1,27 @@ +From a11757f193bc86f74cb67b52b727bd9b762bf573 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Sat, 23 Oct 2021 19:41:07 +0100 +Subject: [PATCH] vddk: Fix typo in debug message + +Fixes: commit 5bb8f0586e1faabcbf4f43d722a3b3cb5b352e33 +(cherry picked from commit 343dadeb7340d7b8c5730e2bbab33c829b569122) +--- + plugins/vddk/vddk.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index 290a99a8..e206a47f 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -824,7 +824,7 @@ vddk_get_size (void *handle) + nbdkit_debug ("disk info: uuid: %s", + info->uuid ? : "NULL"); + if (library_version >= 7) { +- nbdkit_debug ("disk info: sectory size: " ++ nbdkit_debug ("disk info: sector size: " + "logical %" PRIu32 " physical %" PRIu32, + info->logicalSectorSize, + info->physicalSectorSize); +-- +2.31.1 + diff --git a/0008-vddk-Only-print-vddk_library_version-when-we-managed.patch b/0008-vddk-Only-print-vddk_library_version-when-we-managed.patch new file mode 100644 index 0000000..2441173 --- /dev/null +++ b/0008-vddk-Only-print-vddk_library_version-when-we-managed.patch @@ -0,0 +1,55 @@ +From 574d356f6c55b8f0873586a20b07f7a898e955b0 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Sat, 23 Oct 2021 19:50:52 +0100 +Subject: [PATCH] vddk: Only print vddk_library_version when we managed to load + the library + +Because --dump-plugin calls load_library (false) it won't fail if we +didn't manage to load the library. This results in library_version +being 0, which we printed incorrectly. + +Resolve this problem by not printing the vddk_library_version entry in +this case. + +Fixes: commit 8700649d147948897f3b97810a1dff37924bdd6e +(cherry picked from commit a3fba12c3e9c2113009f556360ae0bd04c45f6bb) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 1 + + plugins/vddk/vddk.c | 9 ++++++++- + 2 files changed, 9 insertions(+), 1 deletion(-) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 5c356cc4..a5534f2e 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -425,6 +425,7 @@ by this build. + =item C + + The VDDK major library version: 5, 6, 7, ... ++If this is omitted it means the library could not be loaded. + + =item C + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index e206a47f..e54eb9a8 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -591,7 +591,14 @@ vddk_dump_plugin (void) + + printf ("vddk_default_libdir=%s\n", VDDK_LIBDIR); + printf ("vddk_has_nfchostport=1\n"); +- printf ("vddk_library_version=%d\n", library_version); ++ ++ /* Because load_library (false) we might not have loaded VDDK, in ++ * which case we didn't set library_version. Note this cannot ++ * happen in the normal (non-debug-plugin) path because there we use ++ * load_library (true). ++ */ ++ if (library_version > 0) ++ printf ("vddk_library_version=%d\n", library_version); + + #if defined(HAVE_DLADDR) + /* It would be nice to print the version of VDDK from the shared +-- +2.31.1 + diff --git a/0009-vddk-Print-one-line-in-dump-plugin-output-for-each-V.patch b/0009-vddk-Print-one-line-in-dump-plugin-output-for-each-V.patch new file mode 100644 index 0000000..a429ecd --- /dev/null +++ b/0009-vddk-Print-one-line-in-dump-plugin-output-for-each-V.patch @@ -0,0 +1,72 @@ +From bb1c6bc5db8169498e05adce22b01ac51eaaae47 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Mon, 25 Oct 2021 08:36:53 +0100 +Subject: [PATCH] vddk: Print one line in --dump-plugin output for each VDDK + API + +Helps when detecting if certain optional features are being used, such +as flush and extents. + +(cherry picked from commit 4ee13559e46cf622410d0bdd7db29bb00908b40a) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 9 +++++++++ + plugins/vddk/vddk.c | 10 ++++++++++ + tests/test-vddk-real-dump-plugin.sh | 1 + + 3 files changed, 20 insertions(+) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index a5534f2e..fd8950c2 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -432,6 +432,15 @@ If this is omitted it means the library could not be loaded. + Prints the full path to the VDDK shared library. Since this requires + a glibc extension it may not be available in all builds of the plugin. + ++=item C ++ ++For each VDDK API that the plugin uses I which is present in the ++VDDK library that was loaded, we print the name of the API ++(eg. C). This lets you see which optional APIs are ++available, such as C and ++C. If the library could not be ++loaded then these lines are not printed. ++ + =back + + =head1 NOTES +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index e54eb9a8..8955bc51 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -616,6 +616,16 @@ vddk_dump_plugin (void) + printf ("vddk_dll=%s\n", p); + } + #endif ++ ++ /* Note we print all VDDK APIs found here, not just the optional ++ * ones. That is so if we update the baseline VDDK in future and ++ * make optional into required APIs, the output doesn't change. ++ */ ++#define STUB(fn,ret,args) if (fn != NULL) printf ("%s=1\n", #fn); ++#define OPTIONAL_STUB(fn,ret,args) STUB(fn,ret,args) ++#include "vddk-stubs.h" ++#undef STUB ++#undef OPTIONAL_STUB + } + + /* The rules on threads and VDDK are here: +diff --git a/tests/test-vddk-real-dump-plugin.sh b/tests/test-vddk-real-dump-plugin.sh +index 59c79693..92e1f3d2 100755 +--- a/tests/test-vddk-real-dump-plugin.sh ++++ b/tests/test-vddk-real-dump-plugin.sh +@@ -58,6 +58,7 @@ grep ^vddk_default_libdir= $out + grep ^vddk_has_nfchostport= $out + grep ^vddk_library_version= $out + grep ^vddk_dll= $out ++grep ^VixDiskLib_Open=1 $out + + dll="$(grep ^vddk_dll $out | cut -d= -f2)" + test -f "$dll" +-- +2.31.1 + diff --git a/0010-vddk-Update-comment-about-VixDiskLib_PrepareForAcces.patch b/0010-vddk-Update-comment-about-VixDiskLib_PrepareForAcces.patch new file mode 100644 index 0000000..1d9042d --- /dev/null +++ b/0010-vddk-Update-comment-about-VixDiskLib_PrepareForAcces.patch @@ -0,0 +1,31 @@ +From 72f0a3ce7635c4a94135cb037ae9d36e82bd5f73 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 26 Oct 2021 12:55:14 +0100 +Subject: [PATCH] vddk: Update comment about VixDiskLib_PrepareForAccess + +(cherry picked from commit 5875b3d93c8c913aa96ac5d5a605d081d6a21393) +--- + plugins/vddk/vddk.c | 7 +++---- + 1 file changed, 3 insertions(+), 4 deletions(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index 8955bc51..fc377a29 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -721,10 +721,9 @@ vddk_open (int readonly) + h->params->specType = VIXDISKLIB_SPEC_VMX; + } + +- /* XXX Some documentation suggests we should call +- * VixDiskLib_PrepareForAccess here. It may be required for +- * Advanced Transport modes, but I could not make it work with +- * either ESXi or vCenter servers. ++ /* XXX We should call VixDiskLib_PrepareForAccess here. It disables ++ * live storage migration (Storage VMotion) of the VM while we are ++ * accessing it, and may be required for "Advanced Transport modes". + */ + + VDDK_CALL_START (VixDiskLib_ConnectEx, +-- +2.31.1 + diff --git a/0011-vddk-Document-what-I-found-about-VixDiskLib_Flush.patch b/0011-vddk-Document-what-I-found-about-VixDiskLib_Flush.patch new file mode 100644 index 0000000..6a365e7 --- /dev/null +++ b/0011-vddk-Document-what-I-found-about-VixDiskLib_Flush.patch @@ -0,0 +1,32 @@ +From 6c8f9fada5d50dc3ee5a1304420fdee3ed023d9f Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 26 Oct 2021 14:30:43 +0100 +Subject: [PATCH] vddk: Document what I found about VixDiskLib_Flush + +(cherry picked from commit b79a59a81255bfd64ba403994bdf3ef4581e6535) +--- + plugins/vddk/vddk.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index fc377a29..c506b5a1 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -957,6 +957,14 @@ vddk_flush (void *handle, uint32_t flags) + struct vddk_handle *h = handle; + VixError err; + ++ /* The documentation for Flush is missing, but the comment in the ++ * header file seems to indicate that it waits for WriteAsync ++ * commands to finish. We don't use WriteAsync, and in any case ++ * there's a new function Wait to wait for those. However I ++ * verified using strace that in fact Flush does call fsync on the ++ * file so it appears to be the correct call to use here. ++ */ ++ + VDDK_CALL_START (VixDiskLib_Flush, "handle") + err = VixDiskLib_Flush (h->handle); + VDDK_CALL_END (VixDiskLib_Flush, 0); +-- +2.31.1 + diff --git a/0012-vddk-Note-that-we-have-tested-VDDK-7.0.3.patch b/0012-vddk-Note-that-we-have-tested-VDDK-7.0.3.patch new file mode 100644 index 0000000..dd999ae --- /dev/null +++ b/0012-vddk-Note-that-we-have-tested-VDDK-7.0.3.patch @@ -0,0 +1,26 @@ +From 21bfaf35b4a0eb825a3db5736792dac97d1dd7c2 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 26 Oct 2021 19:44:46 +0100 +Subject: [PATCH] vddk: Note that we have tested VDDK 7.0.3 + +(cherry picked from commit 189e9d52a4d6ea1bcb6a9368c954a1a80802262d) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index fd8950c2..0702aa75 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -551,7 +551,7 @@ server, which can also be very slow. + This plugin requires VDDK E 5.5.5, which in turn means that it + is only supported on x64-64 platforms. + +-It has been tested with all versions up to 7.0.2 (but should work with ++It has been tested with all versions up to 7.0.3 (but should work with + future versions). + + VDDK E 6.0 should be used if possible. This is the first version +-- +2.31.1 + diff --git a/0013-vddk-Move-minimum-version-to-VDDK-6.5.patch b/0013-vddk-Move-minimum-version-to-VDDK-6.5.patch new file mode 100644 index 0000000..799205c --- /dev/null +++ b/0013-vddk-Move-minimum-version-to-VDDK-6.5.patch @@ -0,0 +1,147 @@ +From a50111258e315abe3f8f2bf89cb72a1d6711f454 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 26 Oct 2021 19:46:32 +0100 +Subject: [PATCH] vddk: Move minimum version to VDDK 6.5 + +Drop support for VDDK 5.5.5 (released in 2015) and 6.0 (released the +same year). Move minimum supported version to 6.5 (released Nov +2016). This is so we can use asynchronous operations. + +Acked-by: Laszlo Ersek +(cherry picked from commit 5ed23616762a72e039531a9a7cd81353cd4f436e) +--- + plugins/vddk/nbdkit-vddk-plugin.pod | 10 +++------- + plugins/vddk/vddk-stubs.h | 3 +-- + plugins/vddk/vddk.c | 24 ++++++++++++++++-------- + tests/dummy-vddk.c | 6 ++++++ + 4 files changed, 26 insertions(+), 17 deletions(-) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 0702aa75..1c16d096 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -424,7 +424,7 @@ by this build. + + =item C + +-The VDDK major library version: 5, 6, 7, ... ++The VDDK major library version: 6, 7, ... + If this is omitted it means the library could not be loaded. + + =item C +@@ -548,16 +548,12 @@ server, which can also be very slow. + + =head1 SUPPORTED VERSIONS OF VDDK + +-This plugin requires VDDK E 5.5.5, which in turn means that it +-is only supported on x64-64 platforms. ++This plugin requires VDDK E 6.5 (released Nov 2016). It is only ++supported on the x64-64 archtecture. + + It has been tested with all versions up to 7.0.3 (but should work with + future versions). + +-VDDK E 6.0 should be used if possible. This is the first version +-which added Flush support which is crucial for data integrity when +-writing. +- + VDDK 6.7 was the first version that supported the + C API, required to provide extent + information over NBD. +diff --git a/plugins/vddk/vddk-stubs.h b/plugins/vddk/vddk-stubs.h +index 5e70238d..a94df9cd 100644 +--- a/plugins/vddk/vddk-stubs.h ++++ b/plugins/vddk/vddk-stubs.h +@@ -40,8 +40,7 @@ + */ + + /* Required stubs, present in all versions of VDDK that we support. I +- * have checked that all these exist in at least VDDK 5.5.5 (2015) +- * which is the earliest version of VDDK that we support. ++ * have checked that all these exist in at least VDDK 5.5.5 (2015). + */ + + STUB (VixDiskLib_InitEx, +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index c506b5a1..bf12c3c9 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -75,13 +75,13 @@ NBDKIT_DLL_PUBLIC int vddk_debug_stats; + #undef OPTIONAL_STUB + + /* Parameters passed to InitEx. */ +-#define VDDK_MAJOR 5 ++#define VDDK_MAJOR 6 + #define VDDK_MINOR 5 + + static void *dl; /* dlopen handle */ + static bool init_called; /* was InitEx called */ + static __thread int error_suppression; /* threadlocal error suppression */ +-static int library_version; /* VDDK major: 5, 6, 7, ... */ ++static int library_version; /* VDDK major: 6, 7, ... */ + + static enum { NONE = 0, ZLIB, FASTLZ, SKIPZ } compression; /* compression */ + static char *config; /* config */ +@@ -413,16 +413,14 @@ load_library (bool load_error_is_fatal) + /* Prefer the newest library in case multiple exist. Check two + * possible directories: the usual VDDK installation puts .so + * files in an arch-specific subdirectory of $libdir (our minimum +- * supported version is VDDK 5.5.5, which only supports x64-64); +- * but our testsuite is easier to write if we point libdir +- * directly to a stub .so. ++ * supported version is VDDK 6.5, which only supports x64-64); but ++ * our testsuite is easier to write if we point libdir directly to ++ * a stub .so. + */ + { "lib64/libvixDiskLib.so.7", 7 }, + { "libvixDiskLib.so.7", 7 }, + { "lib64/libvixDiskLib.so.6", 6 }, + { "libvixDiskLib.so.6", 6 }, +- { "lib64/libvixDiskLib.so.5", 5 }, +- { "libvixDiskLib.so.5", 5 }, + { NULL } + }; + size_t i; +@@ -474,7 +472,7 @@ load_library (bool load_error_is_fatal) + exit (EXIT_FAILURE); + } + +- assert (library_version >= 5); ++ assert (library_version >= 6); + + /* Load symbols. */ + #define STUB(fn,ret,args) \ +@@ -490,6 +488,16 @@ load_library (bool load_error_is_fatal) + #include "vddk-stubs.h" + #undef STUB + #undef OPTIONAL_STUB ++ ++ /* Additionally, VDDK version must be >= 6.5. This was the first ++ * version which introduced VixDiskLib_Wait symbol so we can check ++ * for that. ++ */ ++ if (VixDiskLib_Wait == NULL) { ++ nbdkit_error ("VDDK version must be >= 6.5. " ++ "See nbdkit-vddk-plugin(1) man page section \"SUPPORTED VERSIONS OF VDDK\"."); ++ exit (EXIT_FAILURE); ++ } + } + + static int +diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c +index 9b5ae0a2..cb88380c 100644 +--- a/tests/dummy-vddk.c ++++ b/tests/dummy-vddk.c +@@ -198,3 +198,9 @@ VixDiskLib_Write (VixDiskLibHandle handle, + memcpy (disk + offset, buf, nr_sectors * VIXDISKLIB_SECTOR_SIZE); + return VIX_OK; + } ++ ++NBDKIT_DLL_PUBLIC VixError ++VixDiskLib_Wait (VixDiskLibHandle handle) ++{ ++ return VIX_OK; ++} +-- +2.31.1 + diff --git a/0014-vddk-Add-read-write-and-wait-asynchronous-functions.patch b/0014-vddk-Add-read-write-and-wait-asynchronous-functions.patch new file mode 100644 index 0000000..494e697 --- /dev/null +++ b/0014-vddk-Add-read-write-and-wait-asynchronous-functions.patch @@ -0,0 +1,72 @@ +From cf60f0e3a2f6fb8efe79ec41ca284050990b4d79 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 27 Oct 2021 11:57:35 +0100 +Subject: [PATCH] vddk: Add read, write and wait asynchronous functions + +These functions added in VDDK 6.0 - 6.5 implement asynchronous read +and write. + +Acked-by: Laszlo Ersek +(cherry picked from commit ad53e7becafed6ca3573795a79c534281fe9c274) +--- + plugins/vddk/vddk-structs.h | 3 +++ + plugins/vddk/vddk-stubs.h | 19 ++++++++++++++++++- + 2 files changed, 21 insertions(+), 1 deletion(-) + +diff --git a/plugins/vddk/vddk-structs.h b/plugins/vddk/vddk-structs.h +index aeb5bfd0..e97f017c 100644 +--- a/plugins/vddk/vddk-structs.h ++++ b/plugins/vddk/vddk-structs.h +@@ -43,6 +43,7 @@ + + typedef uint64_t VixError; + #define VIX_OK 0 ++#define VIX_ASYNC 25000 + + #define VIXDISKLIB_FLAG_OPEN_UNBUFFERED 1 + #define VIXDISKLIB_FLAG_OPEN_SINGLE_LINK 2 +@@ -61,6 +62,8 @@ typedef void *VixDiskLibHandle; + + typedef void VixDiskLibGenericLogFunc (const char *fmt, va_list args); + ++typedef void (*VixDiskLibCompletionCB) (void *data, VixError result); ++ + enum VixDiskLibCredType { + VIXDISKLIB_CRED_UID = 1, + VIXDISKLIB_CRED_SESSIONID = 2, +diff --git a/plugins/vddk/vddk-stubs.h b/plugins/vddk/vddk-stubs.h +index a94df9cd..66353691 100644 +--- a/plugins/vddk/vddk-stubs.h ++++ b/plugins/vddk/vddk-stubs.h +@@ -103,10 +103,27 @@ STUB (VixDiskLib_Write, + uint64_t start_sector, uint64_t nr_sectors, + const unsigned char *buf)); + +-/* Added in VDDK 6.0, this will be NULL in earlier versions. */ ++/* Added in VDDK 6.0, these will be NULL in earlier versions. */ + OPTIONAL_STUB (VixDiskLib_Flush, + VixError, + (VixDiskLibHandle handle)); ++OPTIONAL_STUB (VixDiskLib_ReadAsync, ++ VixError, ++ (VixDiskLibHandle handle, ++ uint64_t start_sector, uint64_t nr_sectors, ++ unsigned char *buf, ++ VixDiskLibCompletionCB callback, void *data)); ++OPTIONAL_STUB (VixDiskLib_WriteAsync, ++ VixError, ++ (VixDiskLibHandle handle, ++ uint64_t start_sector, uint64_t nr_sectors, ++ const unsigned char *buf, ++ VixDiskLibCompletionCB callback, void *data)); ++ ++/* Added in VDDK 6.5, this will be NULL in earlier versions. */ ++OPTIONAL_STUB (VixDiskLib_Wait, ++ VixError, ++ (VixDiskLibHandle handle)); + + /* Added in VDDK 6.7, these will be NULL for earlier versions: */ + OPTIONAL_STUB (VixDiskLib_QueryAllocatedBlocks, +-- +2.31.1 + diff --git a/0015-vddk-Start-to-split-VDDK-over-several-files.patch b/0015-vddk-Start-to-split-VDDK-over-several-files.patch new file mode 100644 index 0000000..25109c1 --- /dev/null +++ b/0015-vddk-Start-to-split-VDDK-over-several-files.patch @@ -0,0 +1,259 @@ +From da7c163452d2a1f073a6d5c6d4d6fda824aa9a6f Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 27 Oct 2021 12:20:31 +0100 +Subject: [PATCH] vddk: Start to split VDDK over several files + +This change doesn't do anything except move some definitions into the +header file vddk.h, but it allows future commits to split up the very +large vddk.c file. + +Acked-by: Laszlo Ersek +(cherry picked from commit 117634dccf4e29394e8718a8d62e93a9edf0a39c) +--- + plugins/vddk/vddk.c | 91 +++++++++++++-------------------------------- + plugins/vddk/vddk.h | 89 +++++++++++++++++++++++++++++++++++++++++++- + 2 files changed, 112 insertions(+), 68 deletions(-) + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index bf12c3c9..0167aa2f 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -50,14 +50,12 @@ + #include + + #include "cleanup.h" +-#include "isaligned.h" + #include "minmax.h" + #include "rounding.h" + #include "tvdiff.h" + #include "vector.h" + + #include "vddk.h" +-#include "vddk-structs.h" + + /* Debug flags. */ + NBDKIT_DLL_PUBLIC int vddk_debug_diskinfo; +@@ -65,11 +63,11 @@ NBDKIT_DLL_PUBLIC int vddk_debug_extents; + NBDKIT_DLL_PUBLIC int vddk_debug_datapath = 1; + NBDKIT_DLL_PUBLIC int vddk_debug_stats; + +-/* For each VDDK API define a static global variable. These globals +- * are initialized when the plugin is loaded (by vddk_get_ready). ++/* For each VDDK API define a global variable. These globals are ++ * initialized when the plugin is loaded (by vddk_get_ready). + */ +-#define STUB(fn,ret,args) static ret (*fn) args +-#define OPTIONAL_STUB(fn,ret,args) static ret (*fn) args ++#define STUB(fn,ret,args) ret (*fn) args ++#define OPTIONAL_STUB(fn,ret,args) ret (*fn) args + #include "vddk-stubs.h" + #undef STUB + #undef OPTIONAL_STUB +@@ -78,28 +76,28 @@ NBDKIT_DLL_PUBLIC int vddk_debug_stats; + #define VDDK_MAJOR 6 + #define VDDK_MINOR 5 + +-static void *dl; /* dlopen handle */ +-static bool init_called; /* was InitEx called */ +-static __thread int error_suppression; /* threadlocal error suppression */ +-static int library_version; /* VDDK major: 6, 7, ... */ ++void *dl; /* dlopen handle */ ++bool init_called; /* was InitEx called */ ++__thread int error_suppression; /* threadlocal error suppression */ ++int library_version; /* VDDK major: 6, 7, ... */ ++bool is_remote; /* true if remote connection */ + +-static enum { NONE = 0, ZLIB, FASTLZ, SKIPZ } compression; /* compression */ +-static char *config; /* config */ +-static const char *cookie; /* cookie */ +-static const char *filename; /* file */ +-char *libdir; /* libdir */ +-static uint16_t nfc_host_port; /* nfchostport */ +-char *password; /* password */ +-static uint16_t port; /* port */ +-static const char *server_name; /* server */ +-static bool single_link; /* single-link */ +-static const char *snapshot_moref; /* snapshot */ +-static const char *thumb_print; /* thumbprint */ +-static const char *transport_modes; /* transports */ +-static bool unbuffered; /* unbuffered */ +-static const char *username; /* user */ +-static const char *vmx_spec; /* vm */ +-static bool is_remote; ++enum compression_type compression; /* compression */ ++char *config; /* config */ ++const char *cookie; /* cookie */ ++const char *filename; /* file */ ++char *libdir; /* libdir */ ++uint16_t nfc_host_port; /* nfchostport */ ++char *password; /* password */ ++uint16_t port; /* port */ ++const char *server_name; /* server */ ++bool single_link; /* single-link */ ++const char *snapshot_moref; /* snapshot */ ++const char *thumb_print; /* thumbprint */ ++const char *transport_modes; /* transports */ ++bool unbuffered; /* unbuffered */ ++const char *username; /* user */ ++const char *vmx_spec; /* vm */ + + /* For each VDDK API define a variable to store the time taken (used + * to implement -D vddk.stats=1). +@@ -120,45 +118,6 @@ static void display_stats (void); + #undef STUB + #undef OPTIONAL_STUB + +-/* Macros to bracket each VDDK API call, for printing debugging +- * information and collecting statistics. +- */ +-#define VDDK_CALL_START(fn, fs, ...) \ +- do { \ +- struct timeval start_t, end_t; \ +- /* GCC can optimize this away at compile time: */ \ +- const bool datapath = \ +- strcmp (#fn, "VixDiskLib_Read") == 0 || \ +- strcmp (#fn, "VixDiskLib_Write") == 0; \ +- if (vddk_debug_stats) \ +- gettimeofday (&start_t, NULL); \ +- if (!datapath || vddk_debug_datapath) \ +- nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ +- do +-#define VDDK_CALL_END(fn, bytes_) \ +- while (0); \ +- if (vddk_debug_stats) { \ +- gettimeofday (&end_t, NULL); \ +- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ +- stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ +- stats_##fn.calls++; \ +- stats_##fn.bytes += bytes_; \ +- } \ +- } while (0) +- +-/* Print VDDK errors. */ +-#define VDDK_ERROR(err, fs, ...) \ +- do { \ +- char *vddk_err_msg; \ +- VDDK_CALL_START (VixDiskLib_GetErrorText, "%lu", err) \ +- vddk_err_msg = VixDiskLib_GetErrorText ((err), NULL); \ +- VDDK_CALL_END (VixDiskLib_GetErrorText, 0); \ +- nbdkit_error (fs ": %s", ##__VA_ARGS__, vddk_err_msg); \ +- VDDK_CALL_START (VixDiskLib_FreeErrorText, "") \ +- VixDiskLib_FreeErrorText (vddk_err_msg); \ +- VDDK_CALL_END (VixDiskLib_FreeErrorText, 0); \ +- } while (0) +- + /* Unload the plugin. */ + static void + vddk_unload (void) +diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h +index 8c63b4ee..29775eb4 100644 +--- a/plugins/vddk/vddk.h ++++ b/plugins/vddk/vddk.h +@@ -1,5 +1,5 @@ + /* nbdkit +- * Copyright (C) 2013-2020 Red Hat Inc. ++ * Copyright (C) 2013-2021 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are +@@ -33,11 +33,96 @@ + #ifndef NBDKIT_VDDK_H + #define NBDKIT_VDDK_H + ++#include ++#include ++#include ++ ++#include ++ ++#include "isaligned.h" ++#include "tvdiff.h" ++#include "vector.h" ++ ++#include "vddk-structs.h" ++ ++enum compression_type { NONE = 0, ZLIB, FASTLZ, SKIPZ }; ++ ++extern void *dl; ++extern bool init_called; ++extern __thread int error_suppression; ++extern int library_version; ++extern bool is_remote; ++ ++extern enum compression_type compression; ++extern char *config; ++extern const char *cookie; ++extern const char *filename; + extern char *libdir; ++extern uint16_t nfc_host_port; + extern char *password; ++extern uint16_t port; ++extern const char *server_name; ++extern bool single_link; ++extern const char *snapshot_moref; ++extern const char *thumb_print; ++extern const char *transport_modes; ++extern bool unbuffered; ++extern const char *username; ++extern const char *vmx_spec; ++ ++extern int vddk_debug_diskinfo; ++extern int vddk_debug_extents; ++extern int vddk_debug_datapath; ++extern int vddk_debug_stats; ++ ++#define STUB(fn,ret,args) extern ret (*fn) args ++#define OPTIONAL_STUB(fn,ret,args) extern ret (*fn) args ++#include "vddk-stubs.h" ++#undef STUB ++#undef OPTIONAL_STUB ++ ++/* Macros to bracket each VDDK API call, for printing debugging ++ * information and collecting statistics. ++ */ ++#define VDDK_CALL_START(fn, fs, ...) \ ++ do { \ ++ struct timeval start_t, end_t; \ ++ /* GCC can optimize this away at compile time: */ \ ++ const bool datapath = \ ++ strcmp (#fn, "VixDiskLib_Read") == 0 || \ ++ strcmp (#fn, "VixDiskLib_Write") == 0; \ ++ if (vddk_debug_stats) \ ++ gettimeofday (&start_t, NULL); \ ++ if (!datapath || vddk_debug_datapath) \ ++ nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ ++ do ++#define VDDK_CALL_END(fn, bytes_) \ ++ while (0); \ ++ if (vddk_debug_stats) { \ ++ gettimeofday (&end_t, NULL); \ ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \ ++ stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \ ++ stats_##fn.calls++; \ ++ stats_##fn.bytes += bytes_; \ ++ } \ ++ } while (0) ++ ++/* Print VDDK errors. */ ++#define VDDK_ERROR(err, fs, ...) \ ++ do { \ ++ char *vddk_err_msg; \ ++ VDDK_CALL_START (VixDiskLib_GetErrorText, "%lu", err) \ ++ vddk_err_msg = VixDiskLib_GetErrorText ((err), NULL); \ ++ VDDK_CALL_END (VixDiskLib_GetErrorText, 0); \ ++ nbdkit_error (fs ": %s", ##__VA_ARGS__, vddk_err_msg); \ ++ VDDK_CALL_START (VixDiskLib_FreeErrorText, "") \ ++ VixDiskLib_FreeErrorText (vddk_err_msg); \ ++ VDDK_CALL_END (VixDiskLib_FreeErrorText, 0); \ ++ } while (0) ++ ++/* reexec.c */ + extern bool noreexec; + extern char *reexeced; +- + extern void reexec_if_needed (const char *prepend); + extern int restore_ld_library_path (void); + +-- +2.31.1 + diff --git a/0016-vddk-Refactor-D-vddk.stats-1-into-a-new-file.patch b/0016-vddk-Refactor-D-vddk.stats-1-into-a-new-file.patch new file mode 100644 index 0000000..c8460e3 --- /dev/null +++ b/0016-vddk-Refactor-D-vddk.stats-1-into-a-new-file.patch @@ -0,0 +1,287 @@ +From 837407bebdf1d746e047d3664156d2ad09784e31 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 27 Oct 2021 12:30:41 +0100 +Subject: [PATCH] vddk: Refactor -D vddk.stats=1 into a new file + +Acked-by: Laszlo Ersek +(cherry picked from commit dcd5bc51ed7710c32d956345ea8da14ba15ef8f5) +--- + plugins/vddk/Makefile.am | 1 + + plugins/vddk/stats.c | 118 +++++++++++++++++++++++++++++++++++++++ + plugins/vddk/vddk.c | 78 +------------------------- + plugins/vddk/vddk.h | 15 +++++ + 4 files changed, 135 insertions(+), 77 deletions(-) + create mode 100644 plugins/vddk/stats.c + +diff --git a/plugins/vddk/Makefile.am b/plugins/vddk/Makefile.am +index 232aaedd..4f470ff9 100644 +--- a/plugins/vddk/Makefile.am ++++ b/plugins/vddk/Makefile.am +@@ -46,6 +46,7 @@ nbdkit_vddk_plugin_la_SOURCES = \ + vddk.c \ + vddk.h \ + reexec.c \ ++ stats.c \ + vddk-structs.h \ + vddk-stubs.h \ + $(top_srcdir)/include/nbdkit-plugin.h \ +diff --git a/plugins/vddk/stats.c b/plugins/vddk/stats.c +new file mode 100644 +index 00000000..18a42714 +--- /dev/null ++++ b/plugins/vddk/stats.c +@@ -0,0 +1,118 @@ ++/* nbdkit ++ * Copyright (C) 2013-2021 Red Hat Inc. ++ * ++ * Redistribution and use in source and binary forms, with or without ++ * modification, are permitted provided that the following conditions are ++ * met: ++ * ++ * * Redistributions of source code must retain the above copyright ++ * notice, this list of conditions and the following disclaimer. ++ * ++ * * Redistributions in binary form must reproduce the above copyright ++ * notice, this list of conditions and the following disclaimer in the ++ * documentation and/or other materials provided with the distribution. ++ * ++ * * Neither the name of Red Hat nor the names of its contributors may be ++ * used to endorse or promote products derived from this software without ++ * specific prior written permission. ++ * ++ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND ++ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, ++ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A ++ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR ++ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, ++ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT ++ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF ++ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ++ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, ++ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT ++ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF ++ * SUCH DAMAGE. ++ */ ++ ++#include ++ ++#include ++#include ++#include ++#include ++ ++#include ++ ++#define NBDKIT_API_VERSION 2 ++#include ++ ++#include "vector.h" ++ ++#include "vddk.h" ++ ++/* Debug flags. */ ++NBDKIT_DLL_PUBLIC int vddk_debug_stats; ++ ++pthread_mutex_t stats_lock = PTHREAD_MUTEX_INITIALIZER; ++ ++/* For each VDDK API define a variable to store the time taken (used ++ * to implement -D vddk.stats=1). ++ */ ++#define STUB(fn,ret,args) struct vddk_stat stats_##fn = { .name = #fn } ++#define OPTIONAL_STUB(fn,ret,args) STUB(fn,ret,args) ++#include "vddk-stubs.h" ++#undef STUB ++#undef OPTIONAL_STUB ++ ++DEFINE_VECTOR_TYPE(statlist, struct vddk_stat) ++ ++static int ++stat_compare (const void *vp1, const void *vp2) ++{ ++ const struct vddk_stat *st1 = vp1; ++ const struct vddk_stat *st2 = vp2; ++ ++ /* Note: sorts in reverse order of time spent in each API call. */ ++ if (st1->usecs < st2->usecs) return 1; ++ else if (st1->usecs > st2->usecs) return -1; ++ else return 0; ++} ++ ++static const char * ++api_name_without_prefix (const char *name) ++{ ++ return strncmp (name, "VixDiskLib_", 11) == 0 ? name + 11 : name; ++} ++ ++void ++display_stats (void) ++{ ++ statlist stats = empty_vector; ++ size_t i; ++ ++ if (!vddk_debug_stats) return; ++ ++#define STUB(fn,ret,args) statlist_append (&stats, stats_##fn) ++#define OPTIONAL_STUB(fn,ret,args) statlist_append (&stats, stats_##fn) ++#include "vddk-stubs.h" ++#undef STUB ++#undef OPTIONAL_STUB ++ ++ qsort (stats.ptr, stats.size, sizeof stats.ptr[0], stat_compare); ++ ++ nbdkit_debug ("VDDK function stats (-D vddk.stats=1):"); ++ nbdkit_debug ("%-24s %15s %5s %15s", ++ "VixDiskLib_...", "µs", "calls", "bytes"); ++ for (i = 0; i < stats.size; ++i) { ++ if (stats.ptr[i].usecs) { ++ if (stats.ptr[i].bytes > 0) ++ nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64 " %15" PRIu64, ++ api_name_without_prefix (stats.ptr[i].name), ++ stats.ptr[i].usecs, ++ stats.ptr[i].calls, ++ stats.ptr[i].bytes); ++ else ++ nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64, ++ api_name_without_prefix (stats.ptr[i].name), ++ stats.ptr[i].usecs, ++ stats.ptr[i].calls); ++ } ++ } ++ statlist_reset (&stats); ++} +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index 0167aa2f..c05dbfcc 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -61,7 +61,6 @@ + NBDKIT_DLL_PUBLIC int vddk_debug_diskinfo; + NBDKIT_DLL_PUBLIC int vddk_debug_extents; + NBDKIT_DLL_PUBLIC int vddk_debug_datapath = 1; +-NBDKIT_DLL_PUBLIC int vddk_debug_stats; + + /* For each VDDK API define a global variable. These globals are + * initialized when the plugin is loaded (by vddk_get_ready). +@@ -99,25 +98,6 @@ bool unbuffered; /* unbuffered */ + const char *username; /* user */ + const char *vmx_spec; /* vm */ + +-/* For each VDDK API define a variable to store the time taken (used +- * to implement -D vddk.stats=1). +- */ +-struct vddk_stat { +- const char *name; /* function name */ +- int64_t usecs; /* total number of usecs consumed */ +- uint64_t calls; /* number of times called */ +- uint64_t bytes; /* bytes transferred, datapath calls only */ +-}; +-static pthread_mutex_t stats_lock = PTHREAD_MUTEX_INITIALIZER; +-static void display_stats (void); +-#define STUB(fn,ret,args) \ +- static struct vddk_stat stats_##fn = { .name = #fn } +-#define OPTIONAL_STUB(fn,ret,args) \ +- static struct vddk_stat stats_##fn = { .name = #fn } +-#include "vddk-stubs.h" +-#undef STUB +-#undef OPTIONAL_STUB +- + /* Unload the plugin. */ + static void + vddk_unload (void) +@@ -130,69 +110,13 @@ vddk_unload (void) + if (dl) + dlclose (dl); + +- if (vddk_debug_stats) +- display_stats (); ++ display_stats (); + + free (config); + free (libdir); + free (password); + } + +-DEFINE_VECTOR_TYPE(statlist, struct vddk_stat) +- +-static int +-stat_compare (const void *vp1, const void *vp2) +-{ +- const struct vddk_stat *st1 = vp1; +- const struct vddk_stat *st2 = vp2; +- +- /* Note: sorts in reverse order of time spent in each API call. */ +- if (st1->usecs < st2->usecs) return 1; +- else if (st1->usecs > st2->usecs) return -1; +- else return 0; +-} +- +-static const char * +-api_name_without_prefix (const char *name) +-{ +- return strncmp (name, "VixDiskLib_", 11) == 0 ? name + 11 : name; +-} +- +-static void +-display_stats (void) +-{ +- statlist stats = empty_vector; +- size_t i; +- +-#define STUB(fn,ret,args) statlist_append (&stats, stats_##fn) +-#define OPTIONAL_STUB(fn,ret,args) statlist_append (&stats, stats_##fn) +-#include "vddk-stubs.h" +-#undef STUB +-#undef OPTIONAL_STUB +- +- qsort (stats.ptr, stats.size, sizeof stats.ptr[0], stat_compare); +- +- nbdkit_debug ("VDDK function stats (-D vddk.stats=1):"); +- nbdkit_debug ("%-24s %15s %5s %15s", +- "VixDiskLib_...", "µs", "calls", "bytes"); +- for (i = 0; i < stats.size; ++i) { +- if (stats.ptr[i].usecs) { +- if (stats.ptr[i].bytes > 0) +- nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64 " %15" PRIu64, +- api_name_without_prefix (stats.ptr[i].name), +- stats.ptr[i].usecs, +- stats.ptr[i].calls, +- stats.ptr[i].bytes); +- else +- nbdkit_debug (" %-22s %15" PRIi64 " %5" PRIu64, +- api_name_without_prefix (stats.ptr[i].name), +- stats.ptr[i].usecs, +- stats.ptr[i].calls); +- } +- } +- statlist_reset (&stats); +-} +- + static void + trim (char *str) + { +diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h +index 29775eb4..1400589d 100644 +--- a/plugins/vddk/vddk.h ++++ b/plugins/vddk/vddk.h +@@ -126,4 +126,19 @@ extern char *reexeced; + extern void reexec_if_needed (const char *prepend); + extern int restore_ld_library_path (void); + ++/* stats.c */ ++struct vddk_stat { ++ const char *name; /* function name */ ++ int64_t usecs; /* total number of usecs consumed */ ++ uint64_t calls; /* number of times called */ ++ uint64_t bytes; /* bytes transferred, datapath calls only */ ++}; ++extern pthread_mutex_t stats_lock; ++#define STUB(fn,ret,args) extern struct vddk_stat stats_##fn; ++#define OPTIONAL_STUB(fn,ret,args) STUB(fn,ret,args) ++#include "vddk-stubs.h" ++#undef STUB ++#undef OPTIONAL_STUB ++extern void display_stats (void); ++ + #endif /* NBDKIT_VDDK_H */ +-- +2.31.1 + diff --git a/0017-vddk-Implement-parallel-thread-model.patch b/0017-vddk-Implement-parallel-thread-model.patch new file mode 100644 index 0000000..263d304 --- /dev/null +++ b/0017-vddk-Implement-parallel-thread-model.patch @@ -0,0 +1,1287 @@ +From 43e4f0cba34e2b96579339c2c0293c085a591877 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 27 Oct 2021 10:17:22 +0100 +Subject: [PATCH] vddk: Implement parallel thread model + +Since VDDK 6.0, asynchronous read and write operations are available. +This commit makes use of these, allowing us to use the parallel thread +model for increased performance. + +Note that at least VDDK 6.5 is required because VDDK 6.0 had a +different and incompatible signature for VixDiskLibCompletionCB. + +Also note at least vSphere 6.7 is required for asynch calls to make +any performance difference. In older versions they work +synchronously. + +In the parallel thread model, nbdkit will be calling us in parallel +from multiple nbdkit threads. VDDK does not allow multiple threads to +simultaneously call VDDK operations on the same handle. So we create +a background thread per handle (== connection). + +Only the background thread makes VDDK calls[1]. The background thread +handles a mix of synchronous (like extents, flush) and asynchronous +(like read, write) operations, but all from one thread. + +Parallel nbdkit threads issue commands to the background thread +associated with each handle, and wait until they are retired. + +[1] All VDDK calls except for connecting and disconnecting which for +different reasons are protected by a global lock, so I did not need to +change those. + +(cherry picked from commit 1eecf15fc3d8ea253ccec4f5883fdbb9aa6f8c2b) +--- + plugins/vddk/Makefile.am | 1 + + plugins/vddk/nbdkit-vddk-plugin.pod | 11 +- + plugins/vddk/vddk.c | 380 +++++-------------- + plugins/vddk/vddk.h | 49 ++- + plugins/vddk/worker.c | 567 ++++++++++++++++++++++++++++ + tests/dummy-vddk.c | 32 ++ + 6 files changed, 745 insertions(+), 295 deletions(-) + create mode 100644 plugins/vddk/worker.c + +diff --git a/plugins/vddk/Makefile.am b/plugins/vddk/Makefile.am +index 4f470ff9..f8382fc9 100644 +--- a/plugins/vddk/Makefile.am ++++ b/plugins/vddk/Makefile.am +@@ -49,6 +49,7 @@ nbdkit_vddk_plugin_la_SOURCES = \ + stats.c \ + vddk-structs.h \ + vddk-stubs.h \ ++ worker.c \ + $(top_srcdir)/include/nbdkit-plugin.h \ + $(NULL) + +diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod +index 1c16d096..ce82a734 100644 +--- a/plugins/vddk/nbdkit-vddk-plugin.pod ++++ b/plugins/vddk/nbdkit-vddk-plugin.pod +@@ -523,6 +523,14 @@ read bandwidth to the VMware server. + + Same as above, but for writing and flushing writes. + ++=item C ++ ++=item C ++ ++Same as above, but for asynchronous read and write calls introduced in ++nbdkit 1.30. Unfortunately at the moment the amount of time spent in ++these calls is not accounted for correctly. ++ + =item C + + This call is used to query information about the sparseness of the +@@ -580,7 +588,8 @@ Debug extents returned by C. + + =item B<-D vddk.datapath=0> + +-Suppress debugging of datapath calls (C and C). ++Suppress debugging of datapath calls (C, C, C ++and C). + + =item B<-D vddk.stats=1> + +diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c +index c05dbfcc..cd3c3134 100644 +--- a/plugins/vddk/vddk.c ++++ b/plugins/vddk/vddk.c +@@ -50,9 +50,6 @@ + #include + + #include "cleanup.h" +-#include "minmax.h" +-#include "rounding.h" +-#include "tvdiff.h" + #include "vector.h" + + #include "vddk.h" +@@ -522,23 +519,18 @@ vddk_dump_plugin (void) + /* The rules on threads and VDDK are here: + * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html + * +- * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit +- * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around +- * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite +- * within the letter of the rules, but is within the spirit. ++ * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. In nbdkit ++ * 1.22-1.28 we changed this to SERIALIZE_REQUESTS and added a mutex ++ * around calls to VixDiskLib_Open and VixDiskLib_Close. In nbdkit ++ * 1.30 and above we assign a background thread per connection to do ++ * asynch operations and use the PARALLEL model. We still need the ++ * lock around Open and Close. + */ +-#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS ++#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL + + /* Lock protecting open/close calls - see above. */ + static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; + +-/* The per-connection handle. */ +-struct vddk_handle { +- VixDiskLibConnectParams *params; /* connection parameters */ +- VixDiskLibConnection connection; /* connection */ +- VixDiskLibHandle handle; /* disk handle */ +-}; +- + static inline VixDiskLibConnectParams * + allocate_connect_params (void) + { +@@ -579,12 +571,16 @@ vddk_open (int readonly) + VixError err; + uint32_t flags; + const char *transport_mode; ++ int pterr; + +- h = malloc (sizeof *h); ++ h = calloc (1, sizeof *h); + if (h == NULL) { +- nbdkit_error ("malloc: %m"); ++ nbdkit_error ("calloc: %m"); + return NULL; + } ++ h->commands = (command_queue) empty_vector; ++ pthread_mutex_init (&h->commands_lock, NULL); ++ pthread_cond_init (&h->commands_cond, NULL); + + h->params = allocate_connect_params (); + if (h->params == NULL) { +@@ -661,8 +657,22 @@ vddk_open (int readonly) + VDDK_CALL_END (VixDiskLib_GetTransportMode, 0); + nbdkit_debug ("transport mode: %s", transport_mode); + ++ /* Start the background thread which actually does the asynchronous ++ * work. ++ */ ++ pterr = pthread_create (&h->thread, NULL, vddk_worker_thread, h); ++ if (pterr != 0) { ++ errno = pterr; ++ nbdkit_error ("pthread_create: %m"); ++ goto err3; ++ } ++ + return h; + ++ err3: ++ VDDK_CALL_START (VixDiskLib_Close, "handle") ++ VixDiskLib_Close (h->handle); ++ VDDK_CALL_END (VixDiskLib_Close, 0); + err2: + VDDK_CALL_START (VixDiskLib_Disconnect, "connection") + VixDiskLib_Disconnect (h->connection); +@@ -670,6 +680,8 @@ vddk_open (int readonly) + err1: + free_connect_params (h->params); + err0: ++ pthread_mutex_destroy (&h->commands_lock); ++ pthread_cond_destroy (&h->commands_cond); + free (h); + return NULL; + } +@@ -680,6 +692,10 @@ vddk_close (void *handle) + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); + struct vddk_handle *h = handle; ++ struct command stop_cmd = { .type = STOP }; ++ ++ send_command_and_wait (h, &stop_cmd); ++ pthread_join (h->thread, NULL); + + VDDK_CALL_START (VixDiskLib_Close, "handle") + VixDiskLib_Close (h->handle); +@@ -689,6 +705,9 @@ vddk_close (void *handle) + VDDK_CALL_END (VixDiskLib_Disconnect, 0); + + free_connect_params (h->params); ++ pthread_mutex_destroy (&h->commands_lock); ++ pthread_cond_destroy (&h->commands_cond); ++ command_queue_reset (&h->commands); + free (h); + } + +@@ -697,54 +716,29 @@ static int64_t + vddk_get_size (void *handle) + { + struct vddk_handle *h = handle; +- VixDiskLibInfo *info; +- VixError err; + uint64_t size; ++ struct command get_size_cmd = { .type = GET_SIZE, .ptr = &size }; + +- VDDK_CALL_START (VixDiskLib_GetInfo, "handle, &info") +- err = VixDiskLib_GetInfo (h->handle, &info); +- VDDK_CALL_END (VixDiskLib_GetInfo, 0); +- if (err != VIX_OK) { +- VDDK_ERROR (err, "VixDiskLib_GetInfo"); ++ if (send_command_and_wait (h, &get_size_cmd) == -1) + return -1; +- } +- +- size = info->capacity * (uint64_t)VIXDISKLIB_SECTOR_SIZE; +- +- if (vddk_debug_diskinfo) { +- nbdkit_debug ("disk info: capacity: %" PRIu64 " sectors " +- "(%" PRIi64 " bytes)", +- info->capacity, size); +- nbdkit_debug ("disk info: biosGeo: C:%" PRIu32 " H:%" PRIu32 " S:%" PRIu32, +- info->biosGeo.cylinders, +- info->biosGeo.heads, +- info->biosGeo.sectors); +- nbdkit_debug ("disk info: physGeo: C:%" PRIu32 " H:%" PRIu32 " S:%" PRIu32, +- info->physGeo.cylinders, +- info->physGeo.heads, +- info->physGeo.sectors); +- nbdkit_debug ("disk info: adapter type: %d", +- (int) info->adapterType); +- nbdkit_debug ("disk info: num links: %d", info->numLinks); +- nbdkit_debug ("disk info: parent filename hint: %s", +- info->parentFileNameHint ? : "NULL"); +- nbdkit_debug ("disk info: uuid: %s", +- info->uuid ? : "NULL"); +- if (library_version >= 7) { +- nbdkit_debug ("disk info: sector size: " +- "logical %" PRIu32 " physical %" PRIu32, +- info->logicalSectorSize, +- info->physicalSectorSize); +- } +- } +- +- VDDK_CALL_START (VixDiskLib_FreeInfo, "info") +- VixDiskLib_FreeInfo (info); +- VDDK_CALL_END (VixDiskLib_FreeInfo, 0); + + return (int64_t) size; + } + ++static int ++vddk_can_fua (void *handle) ++{ ++ /* The Flush call was not available in VDDK < 6.0. */ ++ return VixDiskLib_Flush != NULL ? NBDKIT_FUA_NATIVE : NBDKIT_FUA_NONE; ++} ++ ++static int ++vddk_can_flush (void *handle) ++{ ++ /* The Flush call was not available in VDDK < 6.0. */ ++ return VixDiskLib_Flush != NULL; ++} ++ + /* Read data from the file. + * + * Note that reads have to be aligned to sectors (XXX). +@@ -754,32 +748,14 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + uint32_t flags) + { + struct vddk_handle *h = handle; +- VixError err; ++ struct command read_cmd = { ++ .type = READ, ++ .ptr = buf, ++ .count = count, ++ .offset = offset, ++ }; + +- /* Align to sectors. */ +- if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { +- nbdkit_error ("%s is not aligned to sectors", "read"); +- return -1; +- } +- if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { +- nbdkit_error ("%s is not aligned to sectors", "read"); +- return -1; +- } +- offset /= VIXDISKLIB_SECTOR_SIZE; +- count /= VIXDISKLIB_SECTOR_SIZE; +- +- VDDK_CALL_START (VixDiskLib_Read, +- "handle, %" PRIu64 " sectors, " +- "%" PRIu32 " sectors, buffer", +- offset, count) +- err = VixDiskLib_Read (h->handle, offset, count, buf); +- VDDK_CALL_END (VixDiskLib_Read, count * VIXDISKLIB_SECTOR_SIZE); +- if (err != VIX_OK) { +- VDDK_ERROR (err, "VixDiskLib_Read"); +- return -1; +- } +- +- return 0; ++ return send_command_and_wait (h, &read_cmd); + } + + static int vddk_flush (void *handle, uint32_t flags); +@@ -792,32 +768,17 @@ static int + vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + uint32_t flags) + { ++ struct vddk_handle *h = handle; + const bool fua = flags & NBDKIT_FLAG_FUA; +- struct vddk_handle *h = handle; +- VixError err; ++ struct command write_cmd = { ++ .type = WRITE, ++ .ptr = (void *) buf, ++ .count = count, ++ .offset = offset, ++ }; + +- /* Align to sectors. */ +- if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { +- nbdkit_error ("%s is not aligned to sectors", "write"); ++ if (send_command_and_wait (h, &write_cmd) == -1) + return -1; +- } +- if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { +- nbdkit_error ("%s is not aligned to sectors", "write"); +- return -1; +- } +- offset /= VIXDISKLIB_SECTOR_SIZE; +- count /= VIXDISKLIB_SECTOR_SIZE; +- +- VDDK_CALL_START (VixDiskLib_Write, +- "handle, %" PRIu64 " sectors, " +- "%" PRIu32 " sectors, buffer", +- offset, count) +- err = VixDiskLib_Write (h->handle, offset, count, buf); +- VDDK_CALL_END (VixDiskLib_Write, count * VIXDISKLIB_SECTOR_SIZE); +- if (err != VIX_OK) { +- VDDK_ERROR (err, "VixDiskLib_Write"); +- return -1; +- } + + if (fua) { + if (vddk_flush (handle, 0) == -1) +@@ -827,126 +788,32 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + return 0; + } + +-static int +-vddk_can_fua (void *handle) +-{ +- /* The Flush call was not available in VDDK < 6.0. */ +- return VixDiskLib_Flush != NULL ? NBDKIT_FUA_NATIVE : NBDKIT_FUA_NONE; +-} +- +-static int +-vddk_can_flush (void *handle) +-{ +- /* The Flush call was not available in VDDK < 6.0. */ +- return VixDiskLib_Flush != NULL; +-} +- + /* Flush data to the file. */ + static int + vddk_flush (void *handle, uint32_t flags) + { + struct vddk_handle *h = handle; +- VixError err; ++ struct command flush_cmd = { ++ .type = FLUSH, ++ }; + +- /* The documentation for Flush is missing, but the comment in the +- * header file seems to indicate that it waits for WriteAsync +- * commands to finish. We don't use WriteAsync, and in any case +- * there's a new function Wait to wait for those. However I +- * verified using strace that in fact Flush does call fsync on the +- * file so it appears to be the correct call to use here. +- */ +- +- VDDK_CALL_START (VixDiskLib_Flush, "handle") +- err = VixDiskLib_Flush (h->handle); +- VDDK_CALL_END (VixDiskLib_Flush, 0); +- if (err != VIX_OK) { +- VDDK_ERROR (err, "VixDiskLib_Flush"); +- return -1; +- } +- +- return 0; ++ return send_command_and_wait (h, &flush_cmd); + } + + static int + vddk_can_extents (void *handle) + { + struct vddk_handle *h = handle; +- VixError err; +- VixDiskLibBlockList *block_list; ++ int ret; ++ struct command can_extents_cmd = { ++ .type = CAN_EXTENTS, ++ .ptr = &ret, ++ }; + +- /* This call was added in VDDK 6.7. In earlier versions the +- * function pointer will be NULL and we cannot query extents. +- */ +- if (VixDiskLib_QueryAllocatedBlocks == NULL) { +- nbdkit_debug ("can_extents: VixDiskLib_QueryAllocatedBlocks == NULL, " +- "probably this is VDDK < 6.7"); +- return 0; +- } +- +- /* Suppress errors around this call. See: +- * https://bugzilla.redhat.com/show_bug.cgi?id=1709211#c7 +- */ +- error_suppression = 1; +- +- /* However even when the call is available it rarely works well so +- * the best thing we can do here is to try the call and if it's +- * non-functional return false. +- */ +- VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, +- "handle, 0, %d sectors, %d sectors", +- VIXDISKLIB_MIN_CHUNK_SIZE, VIXDISKLIB_MIN_CHUNK_SIZE) +- err = VixDiskLib_QueryAllocatedBlocks (h->handle, +- 0, VIXDISKLIB_MIN_CHUNK_SIZE, +- VIXDISKLIB_MIN_CHUNK_SIZE, +- &block_list); +- VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); +- error_suppression = 0; +- if (err == VIX_OK) { +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") +- VixDiskLib_FreeBlockList (block_list); +- VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); +- } +- if (err != VIX_OK) { +- char *errmsg = VixDiskLib_GetErrorText (err, NULL); +- nbdkit_debug ("can_extents: VixDiskLib_QueryAllocatedBlocks test failed, " +- "extents support will be disabled: " +- "original error: %s", +- errmsg); +- VixDiskLib_FreeErrorText (errmsg); +- return 0; +- } +- +- return 1; +-} +- +-static int +-add_extent (struct nbdkit_extents *extents, +- uint64_t *position, uint64_t next_position, bool is_hole) +-{ +- uint32_t type = 0; +- const uint64_t length = next_position - *position; +- +- if (is_hole) { +- type = NBDKIT_EXTENT_HOLE; +- /* Images opened as single link might be backed by another file in the +- chain, so the holes are not guaranteed to be zeroes. */ +- if (!single_link) +- type |= NBDKIT_EXTENT_ZERO; +- } +- +- assert (*position <= next_position); +- if (*position == next_position) +- return 0; +- +- if (vddk_debug_extents) +- nbdkit_debug ("adding extent type %s at [%" PRIu64 "...%" PRIu64 "]", +- is_hole ? "hole" : "allocated data", +- *position, next_position-1); +- if (nbdkit_add_extent (extents, *position, length, type) == -1) ++ if (send_command_and_wait (h, &can_extents_cmd) == -1) + return -1; + +- *position = next_position; +- return 0; ++ return ret; + } + + static int +@@ -955,88 +822,15 @@ vddk_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, + { + struct vddk_handle *h = handle; + bool req_one = flags & NBDKIT_FLAG_REQ_ONE; +- uint64_t position, end, start_sector; +- +- position = offset; +- end = offset + count; +- +- /* We can only query whole chunks. Therefore start with the first +- * chunk before offset. +- */ +- start_sector = +- ROUND_DOWN (offset, VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE) +- / VIXDISKLIB_SECTOR_SIZE; +- while (start_sector * VIXDISKLIB_SECTOR_SIZE < end) { +- VixError err; +- uint32_t i; +- uint64_t nr_chunks, nr_sectors; +- VixDiskLibBlockList *block_list; +- +- assert (IS_ALIGNED (start_sector, VIXDISKLIB_MIN_CHUNK_SIZE)); +- +- nr_chunks = +- ROUND_UP (end - start_sector * VIXDISKLIB_SECTOR_SIZE, +- VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE) +- / (VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE); +- nr_chunks = MIN (nr_chunks, VIXDISKLIB_MAX_CHUNK_NUMBER); +- nr_sectors = nr_chunks * VIXDISKLIB_MIN_CHUNK_SIZE; +- +- VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, +- "handle, %" PRIu64 " sectors, %" PRIu64 " sectors, " +- "%d sectors", +- start_sector, nr_sectors, VIXDISKLIB_MIN_CHUNK_SIZE) +- err = VixDiskLib_QueryAllocatedBlocks (h->handle, +- start_sector, nr_sectors, +- VIXDISKLIB_MIN_CHUNK_SIZE, +- &block_list); +- VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); +- if (err != VIX_OK) { +- VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks"); +- return -1; +- } +- +- for (i = 0; i < block_list->numBlocks; ++i) { +- uint64_t blk_offset, blk_length; +- +- blk_offset = block_list->blocks[i].offset * VIXDISKLIB_SECTOR_SIZE; +- blk_length = block_list->blocks[i].length * VIXDISKLIB_SECTOR_SIZE; +- +- /* The query returns allocated blocks. We must insert holes +- * between the blocks as necessary. +- */ +- if ((position < blk_offset && +- add_extent (extents, &position, blk_offset, true) == -1) || +- (add_extent (extents, +- &position, blk_offset + blk_length, false) == -1)) { +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") +- VixDiskLib_FreeBlockList (block_list); +- VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); +- return -1; +- } +- } +- VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") +- VixDiskLib_FreeBlockList (block_list); +- VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); +- +- /* There's an implicit hole after the returned list of blocks, up +- * to the end of the QueryAllocatedBlocks request. +- */ +- if (add_extent (extents, +- &position, +- (start_sector + nr_sectors) * VIXDISKLIB_SECTOR_SIZE, +- true) == -1) +- return -1; +- +- start_sector += nr_sectors; +- +- /* If one extent was requested, as long as we've added an extent +- * overlapping the original offset we're done. +- */ +- if (req_one && position > offset) +- break; +- } +- +- return 0; ++ struct command extents_cmd = { ++ .type = EXTENTS, ++ .ptr = extents, ++ .count = count, ++ .offset = offset, ++ .req_one = req_one, ++ }; ++ ++ return send_command_and_wait (h, &extents_cmd); + } + + static struct nbdkit_plugin plugin = { +diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h +index 1400589d..be0b3492 100644 +--- a/plugins/vddk/vddk.h ++++ b/plugins/vddk/vddk.h +@@ -90,7 +90,9 @@ extern int vddk_debug_stats; + /* GCC can optimize this away at compile time: */ \ + const bool datapath = \ + strcmp (#fn, "VixDiskLib_Read") == 0 || \ +- strcmp (#fn, "VixDiskLib_Write") == 0; \ ++ strcmp (#fn, "VixDiskLib_ReadAsync") == 0 || \ ++ strcmp (#fn, "VixDiskLib_Write") == 0 || \ ++ strcmp (#fn, "VixDiskLib_WriteAsync") == 0; \ + if (vddk_debug_stats) \ + gettimeofday (&start_t, NULL); \ + if (!datapath || vddk_debug_datapath) \ +@@ -120,6 +122,46 @@ extern int vddk_debug_stats; + VDDK_CALL_END (VixDiskLib_FreeErrorText, 0); \ + } while (0) + ++/* Queue of asynchronous commands sent to the background thread. */ ++enum command_type { GET_SIZE, READ, WRITE, FLUSH, CAN_EXTENTS, EXTENTS, STOP }; ++struct command { ++ /* These fields are set by the caller. */ ++ enum command_type type; /* command */ ++ void *ptr; /* buffer, extents list, return values */ ++ uint32_t count; /* READ, WRITE, EXTENTS */ ++ uint64_t offset; /* READ, WRITE, EXTENTS */ ++ bool req_one; /* EXTENTS NBDKIT_FLAG_REQ_ONE */ ++ ++ /* This field is set to a unique value by send_command_and_wait. */ ++ uint64_t id; /* serial number */ ++ ++ /* These fields are used by the internal implementation. */ ++ pthread_mutex_t mutex; /* completion mutex */ ++ pthread_cond_t cond; /* completion condition */ ++ enum { SUBMITTED, SUCCEEDED, FAILED } status; ++}; ++ ++DEFINE_VECTOR_TYPE(command_queue, struct command *) ++ ++/* The per-connection handle. */ ++struct vddk_handle { ++ VixDiskLibConnectParams *params; /* connection parameters */ ++ VixDiskLibConnection connection; /* connection */ ++ VixDiskLibHandle handle; /* disk handle */ ++ ++ pthread_t thread; /* background thread for asynch work */ ++ ++ /* Command queue of commands sent to the background thread. Use ++ * send_command_and_wait to add a command. Only the background ++ * thread must make VDDK API calls (apart from opening and closing). ++ * The lock protects all of these fields. ++ */ ++ pthread_mutex_t commands_lock; /* lock */ ++ command_queue commands; /* command queue */ ++ pthread_cond_t commands_cond; /* condition (queue size 0 -> 1) */ ++ uint64_t id; /* next command ID */ ++}; ++ + /* reexec.c */ + extern bool noreexec; + extern char *reexeced; +@@ -141,4 +183,9 @@ extern pthread_mutex_t stats_lock; + #undef OPTIONAL_STUB + extern void display_stats (void); + ++/* worker.c */ ++extern const char *command_type_string (enum command_type type); ++extern int send_command_and_wait (struct vddk_handle *h, struct command *cmd); ++extern void *vddk_worker_thread (void *handle); ++ + #endif /* NBDKIT_VDDK_H */ +diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c +new file mode 100644 +index 00000000..2a1d4f26 +--- /dev/null ++++ b/plugins/vddk/worker.c +@@ -0,0 +1,567 @@ ++/* nbdkit ++ * Copyright (C) 2013-2021 Red Hat Inc. ++ * ++ * Redistribution and use in source and binary forms, with or without ++ * modification, are permitted provided that the following conditions are ++ * met: ++ * ++ * * Redistributions of source code must retain the above copyright ++ * notice, this list of conditions and the following disclaimer. ++ * ++ * * Redistributions in binary form must reproduce the above copyright ++ * notice, this list of conditions and the following disclaimer in the ++ * documentation and/or other materials provided with the distribution. ++ * ++ * * Neither the name of Red Hat nor the names of its contributors may be ++ * used to endorse or promote products derived from this software without ++ * specific prior written permission. ++ * ++ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND ++ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, ++ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A ++ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR ++ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, ++ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT ++ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF ++ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ++ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, ++ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT ++ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF ++ * SUCH DAMAGE. ++ */ ++ ++#include ++ ++#include ++#include ++#include ++#include ++ ++#include ++ ++#define NBDKIT_API_VERSION 2 ++#include ++ ++#include "cleanup.h" ++#include "minmax.h" ++#include "rounding.h" ++#include "vector.h" ++ ++#include "vddk.h" ++ ++const char * ++command_type_string (enum command_type type) ++{ ++ switch (type) { ++ case GET_SIZE: return "get_size"; ++ case READ: return "read"; ++ case WRITE: return "write"; ++ case FLUSH: return "flush"; ++ case CAN_EXTENTS: return "can_extents"; ++ case EXTENTS: return "extents"; ++ case STOP: return "stop"; ++ default: abort (); ++ } ++} ++ ++/* Send command to the background thread and wait for completion. ++ * ++ * Returns 0 for OK ++ * On error, calls nbdkit_error and returns -1. ++ */ ++int ++send_command_and_wait (struct vddk_handle *h, struct command *cmd) ++{ ++ /* Add the command to the command queue. */ ++ { ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->commands_lock); ++ cmd->id = h->id++; ++ ++ if (command_queue_append (&h->commands, cmd) == -1) ++ /* On error command_queue_append will call nbdkit_error. */ ++ return -1; ++ ++ /* Signal the caller if it could be sleeping on an empty queue. */ ++ if (h->commands.size == 1) ++ pthread_cond_signal (&h->commands_cond); ++ ++ /* This will be used to signal command completion back to us. */ ++ pthread_mutex_init (&cmd->mutex, NULL); ++ pthread_cond_init (&cmd->cond, NULL); ++ } ++ ++ /* Wait for the command to be completed by the background thread. */ ++ { ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&cmd->mutex); ++ while (cmd->status == SUBMITTED) ++ pthread_cond_wait (&cmd->cond, &cmd->mutex); ++ } ++ ++ pthread_mutex_destroy (&cmd->mutex); ++ pthread_cond_destroy (&cmd->cond); ++ ++ /* On error the background thread will call nbdkit_error. */ ++ switch (cmd->status) { ++ case SUCCEEDED: return 0; ++ case FAILED: return -1; ++ default: abort (); ++ } ++} ++ ++/* Asynchronous commands are completed when this function is called. */ ++static void ++complete_command (void *vp, VixError result) ++{ ++ struct command *cmd = vp; ++ ++ if (vddk_debug_datapath) ++ nbdkit_debug ("command %" PRIu64 " completed", cmd->id); ++ ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&cmd->mutex); ++ ++ if (result == VIX_OK) { ++ cmd->status = SUCCEEDED; ++ } else { ++ VDDK_ERROR (result, "command %" PRIu64 ": asynchronous %s failed", ++ cmd->id, command_type_string (cmd->type)); ++ cmd->status = FAILED; ++ } ++ ++ pthread_cond_signal (&cmd->cond); ++} ++ ++/* Wait for any asynchronous commands to complete. */ ++static int ++do_stop (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ ++ /* Because we assume VDDK >= 6.5, VixDiskLib_Wait must exist. */ ++ VDDK_CALL_START (VixDiskLib_Wait, "handle") ++ err = VixDiskLib_Wait (h->handle); ++ VDDK_CALL_END (VixDiskLib_Wait, 0); ++ if (err != VIX_OK) { ++ VDDK_ERROR (err, "VixDiskLib_Wait"); ++ /* In the end this error indication is ignored because it only ++ * happens on the close path when we cannot handle errors. ++ */ ++ return -1; ++ } ++ return 0; ++} ++ ++/* Get size command. */ ++static int64_t ++do_get_size (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ VixDiskLibInfo *info; ++ uint64_t size; ++ ++ VDDK_CALL_START (VixDiskLib_GetInfo, "handle, &info") ++ err = VixDiskLib_GetInfo (h->handle, &info); ++ VDDK_CALL_END (VixDiskLib_GetInfo, 0); ++ if (err != VIX_OK) { ++ VDDK_ERROR (err, "VixDiskLib_GetInfo"); ++ return -1; ++ } ++ ++ size = info->capacity * (uint64_t)VIXDISKLIB_SECTOR_SIZE; ++ ++ if (vddk_debug_diskinfo) { ++ nbdkit_debug ("disk info: capacity: %" PRIu64 " sectors " ++ "(%" PRIi64 " bytes)", ++ info->capacity, size); ++ nbdkit_debug ("disk info: biosGeo: C:%" PRIu32 " H:%" PRIu32 " S:%" PRIu32, ++ info->biosGeo.cylinders, ++ info->biosGeo.heads, ++ info->biosGeo.sectors); ++ nbdkit_debug ("disk info: physGeo: C:%" PRIu32 " H:%" PRIu32 " S:%" PRIu32, ++ info->physGeo.cylinders, ++ info->physGeo.heads, ++ info->physGeo.sectors); ++ nbdkit_debug ("disk info: adapter type: %d", ++ (int) info->adapterType); ++ nbdkit_debug ("disk info: num links: %d", info->numLinks); ++ nbdkit_debug ("disk info: parent filename hint: %s", ++ info->parentFileNameHint ? : "NULL"); ++ nbdkit_debug ("disk info: uuid: %s", ++ info->uuid ? : "NULL"); ++ if (library_version >= 7) { ++ nbdkit_debug ("disk info: sector size: " ++ "logical %" PRIu32 " physical %" PRIu32, ++ info->logicalSectorSize, ++ info->physicalSectorSize); ++ } ++ } ++ ++ VDDK_CALL_START (VixDiskLib_FreeInfo, "info") ++ VixDiskLib_FreeInfo (info); ++ VDDK_CALL_END (VixDiskLib_FreeInfo, 0); ++ ++ return (int64_t) size; ++} ++ ++static int ++do_read (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ uint32_t count = cmd->count; ++ uint64_t offset = cmd->offset; ++ void *buf = cmd->ptr; ++ ++ /* Align to sectors. */ ++ if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { ++ nbdkit_error ("%s is not aligned to sectors", "read"); ++ return -1; ++ } ++ if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { ++ nbdkit_error ("%s is not aligned to sectors", "read"); ++ return -1; ++ } ++ offset /= VIXDISKLIB_SECTOR_SIZE; ++ count /= VIXDISKLIB_SECTOR_SIZE; ++ ++ VDDK_CALL_START (VixDiskLib_ReadAsync, ++ "handle, %" PRIu64 " sectors, " ++ "%" PRIu32 " sectors, buffer, callback, %" PRIu64, ++ offset, count, cmd->id) ++ err = VixDiskLib_ReadAsync (h->handle, offset, count, buf, ++ complete_command, cmd); ++ VDDK_CALL_END (VixDiskLib_ReadAsync, count * VIXDISKLIB_SECTOR_SIZE); ++ if (err != VIX_ASYNC) { ++ VDDK_ERROR (err, "VixDiskLib_ReadAsync"); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static int ++do_write (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ uint32_t count = cmd->count; ++ uint64_t offset = cmd->offset; ++ const void *buf = cmd->ptr; ++ ++ /* Align to sectors. */ ++ if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { ++ nbdkit_error ("%s is not aligned to sectors", "write"); ++ return -1; ++ } ++ if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { ++ nbdkit_error ("%s is not aligned to sectors", "write"); ++ return -1; ++ } ++ offset /= VIXDISKLIB_SECTOR_SIZE; ++ count /= VIXDISKLIB_SECTOR_SIZE; ++ ++ VDDK_CALL_START (VixDiskLib_WriteAsync, ++ "handle, %" PRIu64 " sectors, " ++ "%" PRIu32 " sectors, buffer, callback, %" PRIu64, ++ offset, count, cmd->id) ++ err = VixDiskLib_WriteAsync (h->handle, offset, count, buf, ++ complete_command, cmd); ++ VDDK_CALL_END (VixDiskLib_WriteAsync, count * VIXDISKLIB_SECTOR_SIZE); ++ if (err != VIX_ASYNC) { ++ VDDK_ERROR (err, "VixDiskLib_WriteAsync"); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static int ++do_flush (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ ++ /* It seems safer to wait for outstanding asynchronous commands to ++ * complete before doing a flush, so do this but ignore errors ++ * except to print them. ++ */ ++ VDDK_CALL_START (VixDiskLib_Wait, "handle") ++ err = VixDiskLib_Wait (h->handle); ++ VDDK_CALL_END (VixDiskLib_Wait, 0); ++ if (err != VIX_OK) ++ VDDK_ERROR (err, "VixDiskLib_Wait"); ++ ++ /* The documentation for Flush is missing, but the comment in the ++ * header file seems to indicate that it waits for WriteAsync ++ * commands to finish. There's a new function Wait to wait for ++ * those. However I verified using strace that in fact Flush calls ++ * fsync on the file so it appears to be the correct call to use ++ * here. ++ */ ++ VDDK_CALL_START (VixDiskLib_Flush, "handle") ++ err = VixDiskLib_Flush (h->handle); ++ VDDK_CALL_END (VixDiskLib_Flush, 0); ++ if (err != VIX_OK) { ++ VDDK_ERROR (err, "VixDiskLib_Flush"); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static int ++do_can_extents (struct command *cmd, struct vddk_handle *h) ++{ ++ VixError err; ++ VixDiskLibBlockList *block_list; ++ ++ /* This call was added in VDDK 6.7. In earlier versions the ++ * function pointer will be NULL and we cannot query extents. ++ */ ++ if (VixDiskLib_QueryAllocatedBlocks == NULL) { ++ nbdkit_debug ("can_extents: VixDiskLib_QueryAllocatedBlocks == NULL, " ++ "probably this is VDDK < 6.7"); ++ return 0; ++ } ++ ++ /* Suppress errors around this call. See: ++ * https://bugzilla.redhat.com/show_bug.cgi?id=1709211#c7 ++ */ ++ error_suppression = 1; ++ ++ /* However even when the call is available it rarely works well so ++ * the best thing we can do here is to try the call and if it's ++ * non-functional return false. ++ */ ++ VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, ++ "handle, 0, %d sectors, %d sectors", ++ VIXDISKLIB_MIN_CHUNK_SIZE, VIXDISKLIB_MIN_CHUNK_SIZE) ++ err = VixDiskLib_QueryAllocatedBlocks (h->handle, ++ 0, VIXDISKLIB_MIN_CHUNK_SIZE, ++ VIXDISKLIB_MIN_CHUNK_SIZE, ++ &block_list); ++ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); ++ error_suppression = 0; ++ if (err == VIX_OK) { ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") ++ VixDiskLib_FreeBlockList (block_list); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); ++ } ++ if (err != VIX_OK) { ++ char *errmsg = VixDiskLib_GetErrorText (err, NULL); ++ nbdkit_debug ("can_extents: " ++ "VixDiskLib_QueryAllocatedBlocks test failed, " ++ "extents support will be disabled: " ++ "original error: %s", ++ errmsg); ++ VixDiskLib_FreeErrorText (errmsg); ++ return 0; ++ } ++ ++ return 1; ++} ++ ++/* Add an extent to the list of extents. */ ++static int ++add_extent (struct nbdkit_extents *extents, ++ uint64_t *position, uint64_t next_position, bool is_hole) ++{ ++ uint32_t type = 0; ++ const uint64_t length = next_position - *position; ++ ++ if (is_hole) { ++ type = NBDKIT_EXTENT_HOLE; ++ /* Images opened as single link might be backed by another file in the ++ chain, so the holes are not guaranteed to be zeroes. */ ++ if (!single_link) ++ type |= NBDKIT_EXTENT_ZERO; ++ } ++ ++ assert (*position <= next_position); ++ if (*position == next_position) ++ return 0; ++ ++ if (vddk_debug_extents) ++ nbdkit_debug ("adding extent type %s at [%" PRIu64 "...%" PRIu64 "]", ++ is_hole ? "hole" : "allocated data", ++ *position, next_position-1); ++ if (nbdkit_add_extent (extents, *position, length, type) == -1) ++ return -1; ++ ++ *position = next_position; ++ return 0; ++} ++ ++static int ++do_extents (struct command *cmd, struct vddk_handle *h) ++{ ++ uint32_t count = cmd->count; ++ uint64_t offset = cmd->offset; ++ bool req_one = cmd->req_one; ++ struct nbdkit_extents *extents = cmd->ptr; ++ uint64_t position, end, start_sector; ++ ++ position = offset; ++ end = offset + count; ++ ++ /* We can only query whole chunks. Therefore start with the ++ * first chunk before offset. ++ */ ++ start_sector = ++ ROUND_DOWN (offset, VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE) ++ / VIXDISKLIB_SECTOR_SIZE; ++ while (start_sector * VIXDISKLIB_SECTOR_SIZE < end) { ++ VixError err; ++ uint32_t i; ++ uint64_t nr_chunks, nr_sectors; ++ VixDiskLibBlockList *block_list; ++ ++ assert (IS_ALIGNED (start_sector, VIXDISKLIB_MIN_CHUNK_SIZE)); ++ ++ nr_chunks = ++ ROUND_UP (end - start_sector * VIXDISKLIB_SECTOR_SIZE, ++ VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE) ++ / (VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE); ++ nr_chunks = MIN (nr_chunks, VIXDISKLIB_MAX_CHUNK_NUMBER); ++ nr_sectors = nr_chunks * VIXDISKLIB_MIN_CHUNK_SIZE; ++ ++ VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks, ++ "handle, %" PRIu64 " sectors, %" PRIu64 " sectors, " ++ "%d sectors", ++ start_sector, nr_sectors, VIXDISKLIB_MIN_CHUNK_SIZE) ++ err = VixDiskLib_QueryAllocatedBlocks (h->handle, ++ start_sector, nr_sectors, ++ VIXDISKLIB_MIN_CHUNK_SIZE, ++ &block_list); ++ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0); ++ if (err != VIX_OK) { ++ VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks"); ++ return -1; ++ } ++ ++ for (i = 0; i < block_list->numBlocks; ++i) { ++ uint64_t blk_offset, blk_length; ++ ++ blk_offset = block_list->blocks[i].offset * VIXDISKLIB_SECTOR_SIZE; ++ blk_length = block_list->blocks[i].length * VIXDISKLIB_SECTOR_SIZE; ++ ++ /* The query returns allocated blocks. We must insert holes ++ * between the blocks as necessary. ++ */ ++ if ((position < blk_offset && ++ add_extent (extents, &position, blk_offset, true) == -1) || ++ (add_extent (extents, ++ &position, blk_offset + blk_length, false) == -1)) { ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") ++ VixDiskLib_FreeBlockList (block_list); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); ++ return -1; ++ } ++ } ++ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") ++ VixDiskLib_FreeBlockList (block_list); ++ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0); ++ ++ /* There's an implicit hole after the returned list of blocks, ++ * up to the end of the QueryAllocatedBlocks request. ++ */ ++ if (add_extent (extents, ++ &position, ++ (start_sector + nr_sectors) * VIXDISKLIB_SECTOR_SIZE, ++ true) == -1) { ++ return -1; ++ } ++ ++ start_sector += nr_sectors; ++ ++ /* If one extent was requested, as long as we've added an extent ++ * overlapping the original offset we're done. ++ */ ++ if (req_one && position > offset) ++ break; ++ } ++ ++ return 0; ++} ++ ++/* Background worker thread, one per connection, which is where the ++ * VDDK commands are issued. ++ */ ++void * ++vddk_worker_thread (void *handle) ++{ ++ struct vddk_handle *h = handle; ++ bool stop = false; ++ ++ while (!stop) { ++ struct command *cmd; ++ int r; ++ bool async = false; ++ ++ /* Wait until we are sent at least one command. */ ++ { ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->commands_lock); ++ while (h->commands.size == 0) ++ pthread_cond_wait (&h->commands_cond, &h->commands_lock); ++ cmd = h->commands.ptr[0]; ++ command_queue_remove (&h->commands, 0); ++ } ++ ++ switch (cmd->type) { ++ case STOP: ++ r = do_stop (cmd, h); ++ stop = true; ++ break; ++ ++ case GET_SIZE: { ++ int64_t size = do_get_size (cmd, h); ++ if (size == -1) ++ r = -1; ++ else { ++ r = 0; ++ *(uint64_t *)cmd->ptr = size; ++ } ++ break; ++ } ++ ++ case READ: ++ r = do_read (cmd, h); ++ /* If async is true, don't retire this command now. */ ++ async = r == 0; ++ break; ++ ++ case WRITE: ++ r = do_write (cmd, h); ++ /* If async is true, don't retire this command now. */ ++ async = r == 0; ++ break; ++ ++ case FLUSH: ++ r = do_flush (cmd, h); ++ break; ++ ++ case CAN_EXTENTS: ++ r = do_can_extents (cmd, h); ++ if (r >= 0) ++ *(int *)cmd->ptr = r; ++ break; ++ ++ case EXTENTS: ++ r = do_extents (cmd, h); ++ break; ++ ++ default: abort (); /* impossible, but keeps GCC happy */ ++ } /* switch */ ++ ++ if (!async) { ++ /* Update the command status. */ ++ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&cmd->mutex); ++ cmd->status = r >= 0 ? SUCCEEDED : FAILED; ++ ++ /* For synchronous commands signal the caller thread that the ++ * command has completed. (Asynchronous commands are completed in ++ * the callback handler). ++ */ ++ pthread_cond_signal (&cmd->cond); ++ } ++ } /* while (!stop) */ ++ ++ /* Exit the worker thread. */ ++ return NULL; ++} +diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c +index cb88380c..b6f12042 100644 +--- a/tests/dummy-vddk.c ++++ b/tests/dummy-vddk.c +@@ -188,6 +188,19 @@ VixDiskLib_Read (VixDiskLibHandle handle, + return VIX_OK; + } + ++NBDKIT_DLL_PUBLIC VixError ++VixDiskLib_ReadAsync (VixDiskLibHandle handle, ++ uint64_t start_sector, uint64_t nr_sectors, ++ unsigned char *buf, ++ VixDiskLibCompletionCB callback, void *data) ++{ ++ size_t offset = start_sector * VIXDISKLIB_SECTOR_SIZE; ++ ++ memcpy (buf, disk + offset, nr_sectors * VIXDISKLIB_SECTOR_SIZE); ++ callback (data, VIX_OK); ++ return VIX_ASYNC; ++} ++ + NBDKIT_DLL_PUBLIC VixError + VixDiskLib_Write (VixDiskLibHandle handle, + uint64_t start_sector, uint64_t nr_sectors, +@@ -199,6 +212,25 @@ VixDiskLib_Write (VixDiskLibHandle handle, + return VIX_OK; + } + ++NBDKIT_DLL_PUBLIC VixError ++VixDiskLib_WriteAsync (VixDiskLibHandle handle, ++ uint64_t start_sector, uint64_t nr_sectors, ++ const unsigned char *buf, ++ VixDiskLibCompletionCB callback, void *data) ++{ ++ size_t offset = start_sector * VIXDISKLIB_SECTOR_SIZE; ++ ++ memcpy (disk + offset, buf, nr_sectors * VIXDISKLIB_SECTOR_SIZE); ++ callback (data, VIX_OK); ++ return VIX_ASYNC; ++} ++ ++NBDKIT_DLL_PUBLIC VixError ++VixDiskLib_Flush (VixDiskLibHandle handle) ++{ ++ return VIX_OK; ++} ++ + NBDKIT_DLL_PUBLIC VixError + VixDiskLib_Wait (VixDiskLibHandle handle) + { +-- +2.31.1 + diff --git a/nbdkit.spec b/nbdkit.spec index 4ee8698..cccf805 100644 --- a/nbdkit.spec +++ b/nbdkit.spec @@ -44,13 +44,13 @@ ExclusiveArch: x86_64 %global verify_tarball_signature 1 # If there are patches which touch autotools files, set this to 1. -%global patches_touch_autotools %{nil} +%global patches_touch_autotools 1 # The source directory. %global source_directory 1.28-stable Name: nbdkit -Version: 1.28.0 +Version: 1.28.1 Release: 1%{?dist} Summary: NBD server @@ -76,7 +76,23 @@ Source3: copy-patches.sh # https://gitlab.com/nbdkit/nbdkit/-/commits/rhel-9.0/ # Patches. -# (no patches) +Patch0001: 0001-vddk-Refactor-how-D-vddk.stats-1-is-collected.patch +Patch0002: 0002-vddk-Extend-D-vddk.stats-1-to-show-number-of-calls-a.patch +Patch0003: 0003-vddk-Simplify-and-consolidate-VDDK_CALL_START-END-ma.patch +Patch0004: 0004-vddk-Document-troubleshooting-performance-problems.patch +Patch0005: 0005-vddk-Include-VDDK-major-library-version-in-dump-plug.patch +Patch0006: 0006-vddk-Add-logical-and-physical-sector-size-to-D-vddk..patch +Patch0007: 0007-vddk-Fix-typo-in-debug-message.patch +Patch0008: 0008-vddk-Only-print-vddk_library_version-when-we-managed.patch +Patch0009: 0009-vddk-Print-one-line-in-dump-plugin-output-for-each-V.patch +Patch0010: 0010-vddk-Update-comment-about-VixDiskLib_PrepareForAcces.patch +Patch0011: 0011-vddk-Document-what-I-found-about-VixDiskLib_Flush.patch +Patch0012: 0012-vddk-Note-that-we-have-tested-VDDK-7.0.3.patch +Patch0013: 0013-vddk-Move-minimum-version-to-VDDK-6.5.patch +Patch0014: 0014-vddk-Add-read-write-and-wait-asynchronous-functions.patch +Patch0015: 0015-vddk-Start-to-split-VDDK-over-several-files.patch +Patch0016: 0016-vddk-Refactor-D-vddk.stats-1-into-a-new-file.patch +Patch0017: 0017-vddk-Implement-parallel-thread-model.patch BuildRequires: make %if 0%{patches_touch_autotools} @@ -1247,6 +1263,10 @@ export LIBGUESTFS_TRACE=1 %changelog +* Fri Oct 29 2021 Richard W.M. Jones - 1.28.1-1 +- Add asynchronous support in nbdkit-vddk-plugin + resolves: rhbz#2018463, rhbz#2011709 + * Fri Oct 08 2021 Richard W.M. Jones - 1.28.0-1 - Rebase to new stable branch version 1.28.0 resolves: rhbz#2011709 diff --git a/sources b/sources index e4ed4c7..40fcfb6 100644 --- a/sources +++ b/sources @@ -1,2 +1,2 @@ -SHA512 (nbdkit-1.28.0.tar.gz) = 904b69847fc7fc3c3a394391657f52794e9a0d6d2a96df748ae74c6657ca07de33dd99b83888410d240031451927ead3a4776d0bdd433c2bf1e2acd36c3c8403 -SHA512 (nbdkit-1.28.0.tar.gz.sig) = 2b6d5c3c54766e1401d5c0b9192c46cf413aa586f0f33fed909c2056ca205a2bd8bb1a73077054d594ad9a963b6a07364f1e60e067437c0d333c4a0b202ebfb1 +SHA512 (nbdkit-1.28.1.tar.gz) = 058b50b6674cb6af0ca24b200919423852eacdf80d04913f2d2680575292078644b39c8bdf2f9f3c7fa31f4aefbcc31f3c736d3e9fc4753e74e6701321bc4cab +SHA512 (nbdkit-1.28.1.tar.gz.sig) = b69409503dd3b40e1f160ec98b599207158b42549fae4be9f1db79d512633b34b41e7229249a4c6d09b2b9bee38d57c232e78564fc43d0d6e37fd786c3ddda0d