Re: [Xen-devel] [PATCH 5 of 5 V3] tools/xl: Remus - Network buffering cmdline switch

[Shriram Rajagopalan <rshriram@xxxxxxxxx>]

On Thu, Oct 31, 2013 at 1:38 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

On Sun, 2013-10-20 at 22:58 -0700, Shriram Rajagopalan wrote:
> diff -r a8deb9499e9d -r f4eea1e0ac3e docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1       Sun Oct 20 11:59:06 2013 -0700
> +++ b/docs/man/xl.pod.1       Sun Oct 20 11:59:16 2013 -0700
> @@ -398,8 +398,7 @@ Print huge (!) amount of debug during th
>  Enable Remus HA for domain. By default B<xl> relies on ssh as a transport
>  mechanism between the two hosts.
>
> -N.B: Remus support in xl is still in experimental (proof-of-concept) phase.
> -     There is no support for network or disk buffering at the moment.
> +N.B: There is no support for disk buffering at the moment.
>
>  B<OPTIONS>
>
> @@ -418,6 +417,13 @@ Generally useful for debugging.
>
>  Disable memory checkpoint compression.
>
> +=item B<-n>
> +
> +Enable network output buffering.  The default script used to configure
> +network buffering is /etc/xen/scripts/remus-netbuf-setup. If you wish to
> +use a custom script, set the global variable "remus.default.netbufscript"
> +in /etc/xen/xl.conf to point to your script.

No docs for -N ?

> @@ -7133,6 +7140,11 @@ int main_remus(int argc, char **argv)
>      domid = find_domain(argv[optind]);
>      host = argv[optind + 1];
>
> +    if (r_info.netbuf && !r_info.netbufscript) {

Will the code do anything if netbuf==False but netbufscript!=NULL?

> +        if (default_remus_netbufscript)

This if is redundant isn't it, it just avoids assigning NULL to a
variable which is already NULL.

Given the above two comments this whole block could be
        if (!r_info.netbufscript)
                r_info.netbufscript = default...
[...]

> -    fprintf(stderr, "remus sender: libxl_domain_suspend failed"
> -            " (rc=%d)\n", rc);

Was this removed on purpose? It seemed useful looking and wasn't
mentioned in the commit message.

Yes, because the error message was not representative. That code block

currently looks like this (after removing the fprintf), which is bit more informative.

 /* If we are here, it means remus setup/domain suspend/backup has                                                                                                                                                       

     * failed. Try to resume the domain and exit gracefully.                                                                                                                                                                

     * TODO: Split-Brain check.                                                                                                                                                                                             

     */

    if (rc == ERROR_GUEST_TIMEDOUT)

        fprintf(stderr, "Failed to suspend domain at primary.\n");

    else {

        fprintf(stderr, "Remus: Backup failed? resuming domain at primary.\n");

        libxl_domain_resume(ctx, domid, 1, 0);

    }

 

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel