From 51713e7702d389fd55d5721c4773fca40e3e89f6 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Wed, 18 Aug 2021 14:26:30 +0100 Subject: [PATCH] vddk: Replace DEBUG_CALL with bracketed VDDK_CALL_START/END macros This is neutral refactoring, but allows us in the next commit to collect statistics about the amount of time spent in these calls. (cherry picked from commit 1335ebfb5637bf5a44403d0b152da7272fdd3e54) --- plugins/vddk/vddk.c | 175 +++++++++++++++++++++++++------------------- 1 file changed, 99 insertions(+), 76 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index b5bce9a0..888009ab 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -104,19 +104,23 @@ static bool is_remote; VixDiskLib_FreeErrorText (vddk_err_msg); \ } while (0) -#define DEBUG_CALL(fn, fs, ...) \ - nbdkit_debug ("VDDK call: %s (" fs ")", fn, ##__VA_ARGS__) -#define DEBUG_CALL_DATAPATH(fn, fs, ...) \ - if (vddk_debug_datapath) \ - nbdkit_debug ("VDDK call: %s (" fs ")", fn, ##__VA_ARGS__) +#define VDDK_CALL_START(fn, fs, ...) \ + nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ + do +#define VDDK_CALL_START_DATAPATH(fn, fs, ...) \ + if (vddk_debug_datapath) \ + nbdkit_debug ("VDDK call: %s (" fs ")", #fn, ##__VA_ARGS__); \ + do +#define VDDK_CALL_END(fn) while (0) /* Unload the plugin. */ static void vddk_unload (void) { if (init_called) { - DEBUG_CALL ("VixDiskLib_Exit", ""); - VixDiskLib_Exit (); + VDDK_CALL_START (VixDiskLib_Exit, "") { + VixDiskLib_Exit (); + } VDDK_CALL_END (VixDiskLib_Exit); } if (dl) dlclose (dl); @@ -449,15 +453,16 @@ vddk_after_fork (void) VixError err; /* Initialize VDDK library. */ - DEBUG_CALL ("VixDiskLib_InitEx", - "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", - VDDK_MAJOR, VDDK_MINOR, - 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_START (VixDiskLib_InitEx, + "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", + VDDK_MAJOR, VDDK_MINOR, + 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); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_InitEx"); exit (EXIT_FAILURE); @@ -519,8 +524,9 @@ allocate_connect_params (void) VixDiskLibConnectParams *ret; if (VixDiskLib_AllocateConnectParams != NULL) { - DEBUG_CALL ("VixDiskLib_AllocateConnectParams", ""); - ret = VixDiskLib_AllocateConnectParams (); + VDDK_CALL_START (VixDiskLib_AllocateConnectParams, "") { + ret = VixDiskLib_AllocateConnectParams (); + } VDDK_CALL_END (VixDiskLib_AllocateConnectParams); } else ret = calloc (1, sizeof (VixDiskLibConnectParams)); @@ -535,8 +541,9 @@ free_connect_params (VixDiskLibConnectParams *params) * originally called. Otherwise use free. */ if (VixDiskLib_AllocateConnectParams != NULL) { - DEBUG_CALL ("VixDiskLib_FreeConnectParams", "params"); - VixDiskLib_FreeConnectParams (params); + VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params") { + VixDiskLib_FreeConnectParams (params); + } VDDK_CALL_END (VixDiskLib_FreeConnectParams); } else free (params); @@ -589,16 +596,17 @@ vddk_open (int readonly) * either ESXi or vCenter servers. */ - DEBUG_CALL ("VixDiskLib_ConnectEx", - "h->params, %d, %s, %s, &connection", - readonly, - snapshot_moref ? : "NULL", - transport_modes ? : "NULL"); - err = VixDiskLib_ConnectEx (h->params, - readonly, - snapshot_moref, - transport_modes, - &h->connection); + VDDK_CALL_START (VixDiskLib_ConnectEx, + "h->params, %d, %s, %s, &connection", + readonly, + snapshot_moref ? : "NULL", + transport_modes ? : "NULL") { + err = VixDiskLib_ConnectEx (h->params, + readonly, + snapshot_moref, + transport_modes, + &h->connection); + } VDDK_CALL_END (VixDiskLib_ConnectEx); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_ConnectEx"); goto err1; @@ -618,9 +626,10 @@ vddk_open (int readonly) case NONE: break; } - DEBUG_CALL ("VixDiskLib_Open", - "connection, %s, %d, &handle", filename, flags); - err = VixDiskLib_Open (h->connection, filename, flags, &h->handle); + VDDK_CALL_START (VixDiskLib_Open, + "connection, %s, %d, &handle", filename, flags) { + err = VixDiskLib_Open (h->connection, filename, flags, &h->handle); + } VDDK_CALL_END (VixDiskLib_Open); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_Open: %s", filename); goto err2; @@ -632,8 +641,9 @@ vddk_open (int readonly) return h; err2: - DEBUG_CALL ("VixDiskLib_Disconnect", "connection"); - VixDiskLib_Disconnect (h->connection); + VDDK_CALL_START (VixDiskLib_Disconnect, "connection") { + VixDiskLib_Disconnect (h->connection); + } VDDK_CALL_END (VixDiskLib_Disconnect); err1: free_connect_params (h->params); err0: @@ -648,10 +658,13 @@ vddk_close (void *handle) ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); struct vddk_handle *h = handle; - DEBUG_CALL ("VixDiskLib_Close", "handle"); - VixDiskLib_Close (h->handle); - DEBUG_CALL ("VixDiskLib_Disconnect", "connection"); - VixDiskLib_Disconnect (h->connection); + VDDK_CALL_START (VixDiskLib_Close, "handle") { + VixDiskLib_Close (h->handle); + } VDDK_CALL_END (VixDiskLib_Close); + VDDK_CALL_START (VixDiskLib_Disconnect, "connection") { + VixDiskLib_Disconnect (h->connection); + } VDDK_CALL_END (VixDiskLib_Disconnect); + free_connect_params (h->params); free (h); } @@ -665,8 +678,9 @@ vddk_get_size (void *handle) VixError err; uint64_t size; - DEBUG_CALL ("VixDiskLib_GetInfo", "handle, &info"); - err = VixDiskLib_GetInfo (h->handle, &info); + VDDK_CALL_START (VixDiskLib_GetInfo, "handle, &info") { + err = VixDiskLib_GetInfo (h->handle, &info); + } VDDK_CALL_END (VixDiskLib_GetInfo); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_GetInfo"); return -1; @@ -694,8 +708,9 @@ vddk_get_size (void *handle) info->uuid ? : "NULL"); } - DEBUG_CALL ("VixDiskLib_FreeInfo", "info"); - VixDiskLib_FreeInfo (info); + VDDK_CALL_START (VixDiskLib_FreeInfo, "info") { + VixDiskLib_FreeInfo (info); + } VDDK_CALL_END (VixDiskLib_FreeInfo); return (int64_t) size; } @@ -723,11 +738,12 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, offset /= VIXDISKLIB_SECTOR_SIZE; count /= VIXDISKLIB_SECTOR_SIZE; - DEBUG_CALL_DATAPATH ("VixDiskLib_Read", - "handle, %" PRIu64 " sectors, " - "%" PRIu32 " sectors, buffer", - offset, count); - err = VixDiskLib_Read (h->handle, offset, count, buf); + VDDK_CALL_START_DATAPATH (VixDiskLib_Read, + "handle, %" PRIu64 " sectors, " + "%" PRIu32 " sectors, buffer", + offset, count) { + err = VixDiskLib_Read (h->handle, offset, count, buf); + } VDDK_CALL_END (VixDiskLib_Read); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_Read"); return -1; @@ -762,11 +778,12 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, offset /= VIXDISKLIB_SECTOR_SIZE; count /= VIXDISKLIB_SECTOR_SIZE; - DEBUG_CALL_DATAPATH ("VixDiskLib_Write", - "handle, %" PRIu64 " sectors, " - "%" PRIu32 " sectors, buffer", - offset, count); - err = VixDiskLib_Write (h->handle, offset, count, buf); + VDDK_CALL_START_DATAPATH (VixDiskLib_Write, + "handle, %" PRIu64 " sectors, " + "%" PRIu32 " sectors, buffer", + offset, count) { + err = VixDiskLib_Write (h->handle, offset, count, buf); + } VDDK_CALL_END (VixDiskLib_Write); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_Write"); return -1; @@ -801,8 +818,9 @@ vddk_flush (void *handle, uint32_t flags) struct vddk_handle *h = handle; VixError err; - DEBUG_CALL ("VixDiskLib_Flush", "handle"); - err = VixDiskLib_Flush (h->handle); + VDDK_CALL_START (VixDiskLib_Flush, "handle") { + err = VixDiskLib_Flush (h->handle); + } VDDK_CALL_END (VixDiskLib_Flush); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_Flush"); return -1; @@ -836,17 +854,19 @@ vddk_can_extents (void *handle) * the best thing we can do here is to try the call and if it's * non-functional return false. */ - DEBUG_CALL ("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_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); error_suppression = 0; if (err == VIX_OK) { - DEBUG_CALL ("VixDiskLib_FreeBlockList", "block_list"); - VixDiskLib_FreeBlockList (block_list); + VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { + VixDiskLib_FreeBlockList (block_list); + } VDDK_CALL_END (VixDiskLib_FreeBlockList); } if (err != VIX_OK) { char *errmsg = VixDiskLib_GetErrorText (err, NULL); @@ -923,14 +943,15 @@ vddk_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, nr_chunks = MIN (nr_chunks, VIXDISKLIB_MAX_CHUNK_NUMBER); nr_sectors = nr_chunks * VIXDISKLIB_MIN_CHUNK_SIZE; - DEBUG_CALL ("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_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); if (err != VIX_OK) { VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks"); return -1; @@ -949,13 +970,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)) { - DEBUG_CALL ("VixDiskLib_FreeBlockList", "block_list"); - VixDiskLib_FreeBlockList (block_list); + VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { + VixDiskLib_FreeBlockList (block_list); + } VDDK_CALL_END (VixDiskLib_FreeBlockList); return -1; } } - DEBUG_CALL ("VixDiskLib_FreeBlockList", "block_list"); - VixDiskLib_FreeBlockList (block_list); + VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list") { + VixDiskLib_FreeBlockList (block_list); + } VDDK_CALL_END (VixDiskLib_FreeBlockList); /* There's an implicit hole after the returned list of blocks, up * to the end of the QueryAllocatedBlocks request. -- 2.31.1