[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/6] In addition to preceding changes to ring disconnects, and associated logging, we also add some logging to check whether state change notifications are being sent in a timely manner between frontend and backend. Also a great assistance to customer debugging.


  • To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Paul Durrant <xadimgnik@xxxxxxxxx>
  • Date: Wed, 21 Jul 2021 18:17:33 +0100
  • Delivery-date: Wed, 21 Jul 2021 17:17:38 +0000
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

On 20/07/2021 14:29, Martin Harvey wrote:
Signed-off-by: Martin Harvey <martin.harvey@xxxxxxxxxx>
---
  src/xenvif/frontend.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
index 5940e16..c27b2cd 100644
--- a/src/xenvif/frontend.c
+++ b/src/xenvif/frontend.c
@@ -1346,6 +1346,7 @@ FrontendWaitForBackendXenbusStateChange(
      LARGE_INTEGER               Timeout;
      XenbusState                 Old = *State;
      NTSTATUS                    status;
+    const ULONGLONG             TimeoutDelta = 120000;

NIT: I don't think you really need 'Delta' in the name, just 'Timeout' would suffice (and I think it would be more consistent with other code).

Trace("%s: ====> %s\n",
            __FrontendGetBackendPath(Frontend),
@@ -1369,7 +1370,7 @@ FrontendWaitForBackendXenbusStateChange(
Timeout.QuadPart = 0; - while (*State == Old && TimeDelta < 120000) {
+    while (*State == Old && TimeDelta < TimeoutDelta) {
          PCHAR           Buffer;
          LARGE_INTEGER   Now;
@@ -1417,6 +1418,10 @@ FrontendWaitForBackendXenbusStateChange(
          TimeDelta = (Now.QuadPart - Start.QuadPart) / 10000ull;
      }
+ if (TimeDelta >= TimeoutDelta) {
+        Error("%s timed out.\n", __FrontendGetBackendPath(Frontend));
+    }

No need for braces on single-line ifs and no full stop needed in the log message. Since it's not in context and I can't remember... does the path include the actual 'state' key name? It's a lack of change in that particular key we're triggering on here.

  Paul

+
      if (Watch != NULL)
          (VOID) XENBUS_STORE(WatchRemove,
                              &Frontend->StoreInterface,





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.