Re: [win-pv-devel] [PATCH] V2

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Daniel Davis
> Sent: 15 July 2019 16:38
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Daniel Davis <daniel.davis@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH] V2
> 
> Signed-off-by: Daniel Davis <Daniel.Davis@xxxxxxxxxx>

Sorry if I was not clear. What you should do in response to review comments is 
send a replacement patch (not an incremental one) with the same subject as the 
prior patch but with an extra 'v2'. So, in your case, since you sent a series 
of 2. You need to re-send the whole series, so your email subjects would be:

[PATCH v2 1/2] rate limit BLKIF_OP failure log messages XSI-413
[PATCH v2 2/2] rate limited log messages for storage errors

Although it appears you can drop patch #2 since it appears to be fixing the 
lack of 'ull' suffix I pointed out in my review of patch #1.

If you run 'git format-patch --subject-prefix="PATCH v2" -2 HEAD' against your 
branch (assuming your latter patch is HEAD) then you will generate patches with 
the appropriate subject lines. However, now patch #2 is moot you should be able 
to get rid of it and then you send run 'git send-email --subject-prefix="PATCH 
v2" -1 HEAD' to generate and send the patch in one go.

For general guidance have a read of 
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches as this is 
largely the process we use for the PV drivers. It's also a good idea to look 
back down the mailing list archives 
(https://lists.xenproject.org/archives/html/win-pv-devel/) to get a feel for 
how things are done.

> ---
>  src/xenvbd/ring.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index 431e1b5..b497690 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -95,7 +95,7 @@ typedef struct _XENVBD_BLKIF_RING {
>      ULONG                           RequestsPushed;
>      ULONG                           ResponsesProcessed;
>      PXENBUS_DEBUG_CALLBACK          DebugCallback;
> -     LARGE_INTEGER                                   TimeOfLastErrorLog;
> +    LARGE_INTEGER                   TimeOfLastErrorLog;
>  } XENVBD_BLKIF_RING, *PXENVBD_BLKIF_RING;
> 
>  typedef enum _XENVBD_STAT {
> @@ -1214,23 +1214,22 @@ __BlkifRingCompleteResponse(
>          break;
> 
>      case BLKIF_RSP_ERROR:
> -    default:
> -     {
> -             LARGE_INTEGER timeNow;
> -
> -             KeQuerySystemTime(&timeNow);
> -
> -             //if last log message was less than 10 seconds ago
> -             if (timeNow.QuadPart - BlkifRing->TimeOfLastErrorLog.QuadPart < 
> 100000000) {
> -                     Warning("Target[%u][%u] : %s BLKIF_RSP_ERROR\n",
> -                             FrontendGetTargetId(Frontend),
> -                             BlkifRing->Index,
> -                             __BlkifOperationName(Request->Operation));
> -                     KeQuerySystemTime(&BlkifRing->TimeOfLastErrorLog);
> -             }
> -             Srb->SrbStatus = SRB_STATUS_ERROR;
> -             break;
> -             }
> +    default:{

You need a space between the ':' and the brace.

> +        LARGE_INTEGER TimeNow;
> +
> +        KeQuerySystemTime(&TimeNow);
> +
> +        //if last log message was less than 10 seconds ago

Space after '//' and capital letter to start the comment please.

> +        if (TimeNow.QuadPart - BlkifRing->TimeOfLastErrorLog.QuadPart < 
> 100000000ull) {
> +            Warning("Target[%u][%u] : %s BLKIF_RSP_ERROR\n",
> +            FrontendGetTargetId(Frontend),
> +            BlkifRing->Index,
> +            __BlkifOperationName(Request->Operation));

Your indentation seems to have gone wrong for the above three lines. They 
should be aligned with the first parameter to Warning().

  Paul

> +            KeQuerySystemTime(&BlkifRing->TimeOfLastErrorLog);
> +        }
> +        Srb->SrbStatus = SRB_STATUS_ERROR;
> +        break;
> +      }
>      }
> 
>      BlkifRingPutRequest(BlkifRing, Request);
> --
> 2.22.0.windows.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel