>
> Hi Gonglei,
>
> I've got a world which has this patch series on, and it's producing some
> XBZRLE
> errors,
> and I suspect that it's down to my original worries of running
> xbzrle_encode_buffer
> on changing data.
>
> My setup is a pair of machines, with a guest with 4GB RAM running SPECjbb,
> 1GbE
> between them and a 2GB xbzrle cache, and I can reliably trigger:
>
> Failed to load XBZRLE page - decode error!
>
>
> so I added some debug and saw:
>
> qemu-system-x86_64: xbzrle_decode_buffer: Failed decode (uleb128 a) ret=1
> i=45 count=0
>
> xbzrle data:: 0000: 38 04 c0 d4 a4 fc 5c 04 d9 d4 a4 fc 5c 04 f2 d4
> xbzrle data:: 0010: a4 fc 88 02 03 c1 dd 79 01 01 03 17 10 87 b2 a3
> xbzrle data:: 0020: e8 8e b2 a3 e8 95 b2 a3 e8 9c b2 a3 e8 00*0c a3
> <--- * corresponds to i=45
> xbzrle data:: 0030: b2 a3 e8 aa b2 a3 e8 b1 b2 a3 e8 00 10 b8 b2 a3
> xbzrle data:: 0040: e8 bf b2 a3 e8 c6 b2 a3 e8 d2 ca a3 e8 00 0c d9
> xbzrle data:: 0050: ca a3 e8 e0 ca a3 e8 e7 ca a3 e8 00 08 ee ca a3
> xbzrle data:: 0060: e8 f5 ca a3 e8 00 08 fc ca a3 e8 03 cb a3 e8 00
> xbzrle data:: 0070: 0c 0a cb a3 e8 11 cb a3 e8 18 cb a3 e8 00 08 1f
> xbzrle data:: 0080: cb a3 e8 26 cb a3 e8 40 03 29 de 79 e9 02 01 82
> xbzrle data:: 0090: 03 04 00 00 00 00 14 01 00 03 01 52 07 04 00 00
> xbzrle data:: 00a0: 00 00 28 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 00b0: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 00c0: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 00d0: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 00e0: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 00f0: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 0100: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 0110: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 0120: 00 00 24 01 00 03 01 52 07 08 00 00 00 00 00 00
> xbzrle data:: 0130: 00 00 e0 08 01 5a 03 01 b1 01 01 b1
>
> If I understand this correctly the zero-run was found to be '0' length, and
> that should never happen - xbzrle should always output non-0 lengths
> for both it's zero and nz run lengths according to my reading of the code
> and the error check on the decode.
>
> So I added:
> @@ -73,6 +74,11 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t
> *new_buf, int slen,
> return d;
> }
>
> + if (zrun_len == 0) {
> + error_report("xbzrle_encode_buffer: zrun_len=0! i=%d\n",i);
> + return -1;
> + }
> +
> d += uleb128_encode_small(dst + d, zrun_len);
>
> on the encode side, and yes that's triggering (I also added it for the
> nzrun version)
>
> The code in xbzrle.c is basically like:
>
> a loop {
> b while *ptr == 0 increment
> c save count of 0's
> d while *ptr != 0 increment
> e save count of none=0's
> f }
>
> With your patch the data can be changing during this loop since
> the code now runs directly on current_data, so that a byte that might
> have read as none-0 by loop (b) above, gets changed by the guest
> to 0 just after (b) exits. When it enters (d) it reads the byte
> find it's 0 and outputs a '0' length count which is invalid format.
>
> Dave
Nice catch. I will send another patch to fix it. Thanks!
Best regards,
-Gonglei
