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

[Changlong Xie <xiecl.fnst@xxxxxxxxxxxxxx>]

On 03/25/2016 08:29 PM, Wei Liu wrote:
> On Fri, Mar 25, 2016 at 02:10:23PM +0800, Changlong Xie wrote:
> > On 03/25/2016 12:12 AM, Ian Jackson wrote:
> > > 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.
> > Also, I can't find your "comments about xxx".
> Ian's comment on 20/26:
>
>      It is a shame that this management code is not also here.
>
>      We would like to have enough management code in xen.git that we can
>      introduce a COLO test in osstest.  That will ensure that your feature
>      does not regress.
>
> The above comment applies to the script in this patch, too. Ian is
> asking you to upstream all relevant management code so that there is a
> chance upstream tests COLO -- but that's not a blocker for 4.7.
>
> As a side note, we should devise a plan to test COLO -- either do it in
> OSStest or rely on third party testing. But that's a topic for another
Yes, we can talk about it later

Thanks
        -Xie

> day.
>
> Wei.
>
> > Thanks
> >         -Xie
> > > > -            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