From cbcf2a2f158a9889bd597b31159ab357dea05cd6 Mon Sep 17 00:00:00 2001

From: "Richard W.M. Jones" <rjones@redhat.com>

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 5f1d223b..993f2d76 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);

@@ -716,13 +704,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;

@@ -743,25 +731,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:

@@ -776,12 +764,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);

@@ -796,9 +784,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;

@@ -827,9 +815,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;

 }

@@ -857,12 +845,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;

@@ -897,12 +885,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;

@@ -945,9 +933,9 @@ vddk_flush (void *handle, uint32_t flags)

    * file so it appears to be the correct call to use here.

    */

-  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;

@@ -983,17 +971,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);

@@ -1073,12 +1061,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;

@@ -1097,15 +1085,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