Re: [Xen-devel] [PATCH v12 26/26] cmdline switches and config vars to control colo-proxy

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Changlong Xie writes ("[PATCH v12 26/26] cmdline switches and config vars to 
control colo-proxy"):
> From: Wen Congyang <wency@xxxxxxxxxxxxxx>
> 
> Add cmdline switches to 'xl migrate-receive' command to specify
> a domain-specific hotplug script to setup COLO proxy.

> +    if (nic->forwarddev) {
> +        flexarray_append(back, "forwarddev");
> +        flexarray_append(back, nic->forwarddev);
> +    }

I'd prefer a name with `coloft' in it, throughout, even for the
xenstore node.  But again this is not critical if we are not trying to
make a stable API.

> +    static struct option opts[] = {
> +        {"script", 1, 0, 0x100},

This should surely be --coloft-script, or --colo-script, something.

I think this script is (actually, in your setup) created by your
management software, and contains some actual functionality.  Am I
right ?  In which case my comments about including that functionality
in xen.git apply here too.  But again this is not a blocker for 4.7.

> -            xasprintf(&rune, "exec %s %s xl migrate-receive %s %s",
> -                      ssh_command, host,
> -                      libxl_defbool_val(r_info.colo) ? "-c" : "-r",
> -                      daemonize ? "" : " -e");
> +            if (!libxl_defbool_val(r_info.colo)) {
> +                xasprintf(&rune, "exec %s %s xl migrate-receive %s %s",
> +                          ssh_command, host,
> +                          "-r",
> +                          daemonize ? "" : " -e");
> +            } else {
> +                xasprintf(&rune, "exec %s %s xl migrate-receive %s %s %s %s",
> +                          ssh_command, host,
> +                          "-c",
> +                          r_info.netbufscript ? "--script" : "",
> +                          r_info.netbufscript ? r_info.netbufscript : "",
> +                          daemonize ? "" : " -e");

I have just noticed here that you have introduced `-c' (in an earlier
patch) to mean to receive a COLO checkpointed stream.

Can you please change this to `--colo' ?

Sorry,
Ian.

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