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

Re: [PATCH 1/2] vchan-socket-proxy: add reconnect marker support


  • To: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Wed, 7 Dec 2022 15:22:46 +0000
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jason Andryuk <jandryuk@xxxxxxxxx>, "Wei Liu" <wl@xxxxxxx>
  • Delivery-date: Wed, 07 Dec 2022 15:23:19 +0000
  • Ironport-data: A9a23:7GaMKatvu/ZAas89YwfCBBWB5+fnVGBeMUV32f8akzHdYApBsoF/q tZmKW+CPvfeYGXze4twaojg80hXu8TTz9UwQFNkrH0wFipD+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg0HVU/IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj5lv0gnRkPaoR5QWGyCFPZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMweQ8Vc0qfiOaK2OyyUdQ0xcYIC9LAFdZK0p1g5Wmx4fcORJnCR+PB5MNC3Sd2jcdLdRrcT 5NHM3w1Nk2GOkARfAdMYH49tL7Aan3XejtEqFWTtOwv7nLa1gBZ27nxKtvFPNeNQK25m27I+ TqWpzWnWnn2MvSl6jqm3C6LudTrgGDKcrlNBZyx1t1T1Qj7Kms7V0RNCArTTeOCoky3Xd5FO lEX0iUrpKk2skesS7HVQBmQsHOC+BkGVLJ4FuQg7QiXx6n84gCHB3MFRDpMdNwnssAtQTUgk FSOmrvBFTFp9bGYV3+Z3rOVti+pfzgYK3cYYi0JRhdD5MPsyLzflTqWEIwlSvTsyISoR3egm FhmsRTSmZ1CstYWjbm0+Wzf3Tu3m8CKTCUS/S7+CzfNAhxCWKapYImh6F7+5PlGLZqEQlTpg EXoi/Ry/8hVU8jTyXXlrPElWejwuq3baGG0bUtHRcFJyti7x5K0kWm8ChlaLVwhDMsLcCSBj KT76VIIv8870JdHgMZKj2ON5yYCl/CI+TfNDKq8gj9yjn9ZKme6ENlGPxL44owUuBFEfVsDE Zmaa92wKn0RFL5qyjG7L89Ej+BznX1nmzuKG8imp/hC7VZ5TCfEIYrpzXPUNrxphE96iFq9H ylj2zuilEwEDbyWjtj/+o8PN1EaRUXX9rivw/G7gtWre1I8cEl4Uq+5/F/UU9A990ijvruSr y7Vt44x4AaXuEAr3i3QNSA6Oe2wAcsnxZ/5VAR1VWuVN7EYSd7HxM8im1EfJOlPGDBLpRKsc 8Q4Rg==
  • Ironport-hdrordr: A9a23:8urx3KOKRJhM7MBcTnqjsMiBIKoaSvp037Dk7SFMoHtuA6qlfq GV7ZMmPHrP4gr5N0tMpTntAsW9qDbnhP1ICWd4B8bfYOCkghrUEGlahbGSvAEIYheOiNK1t5 0BT0EOMqyVMbEgt7eC3ODQKb9Jq+VvsprY59s2qU0DcegAUdAE0+4WMGim+2RNNXh7LKt8Op qAx9ZN4wGtcW4Qaa2AdwM4dtmGid3XtY7sJSULDR4/6AWIkFqTmcXHOind8BcCci9FhYwv+2 jdkwD/++GKvvyhxgXHvlWjn6h+qZ/OysZjGMfJsMQTJzn24zzYHLhcZw==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Sep 05, 2022 at 03:50:18PM +0200, Marek Marczykowski-Górecki wrote:
> +                reconnect_marker_value = atoi(optarg);

atoi() isn't great, if there's garbage it just return 0, which is
validated below.

Would there be value here in using strtol() which can detect input
error? To at least help with maybe hard to debug issues?

> +                if (reconnect_marker_value < 0 || reconnect_marker_value > 
> 255) {
> +                    fprintf(stderr, "invalid argument for 
> --reconnect-marker, "
> +                                    "must be a number between 0 and 255\n");
> +                    usage(argv);
> +                }
> +                break;
>              case '?':
>                  usage(argv);
>          }
> @@ -509,6 +549,15 @@ int main(int argc, char **argv)
>                  ret = 1;
>                  break;
>              }
> +            if (reconnect_marker_value != -1) {
> +                const char marker_buf[] = { reconnect_marker_value };
> +
> +                if (libxenvchan_write(state.ctrl, marker_buf, 
> sizeof(marker_buf))
> +                        != sizeof(marker_buf)) {
> +                    fprintf(stderr, "failed to send reconnect marker\n");

Is this an expected failure? If so, maybe adding "(ignored)" might be
valuable to someone reading the logs?

> +                    break;
> +                }
> +            }
>              if (data_loop(&state) != 0)
>                  break;
>              /* don't reconnect if output was stdout */


Otherwise, the patch looks fine. Even if kept as is:
Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD



 


Rackspace

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