Patchwork [19/19] migration: add a parser to accept FT migration incoming mode.

login
register
mail settings
Submitter Yoshiaki Tamura
Date Jan. 28, 2011, 7:21 a.m.
Message ID <1296199312-26334-20-git-send-email-tamura.yoshiaki@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/80790/
State New
Headers show

Comments

Yoshiaki Tamura - Jan. 28, 2011, 7:21 a.m.
The option looks like, -incoming <protocol>:<address>:<port>,ft_mode

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 migration.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Paolo Bonzini - Jan. 28, 2011, 12:58 p.m.
On 01/28/2011 08:21 AM, Yoshiaki Tamura wrote:
>
> +    /* check ft_mode option  */
> +    p = strstr(uri, "ft_mode");
> +    if (p&&  !strcmp(p, "ft_mode")) {
> +        ft_mode = FT_INIT;
> +    }
> +

This works for TCP mode, but:

1) I am not sure what would happen with -incoming exec;

2) it is tricky! :)  It works only because anything after the port is 
truncated by parse_host_port.

Is there any reason why the code cannot be in 
tcp_start_incoming_migration, where we know the URI has the right 
scheme?  Alternatively you could put it _before_ the scheme, like 
"kemari:tcp:host:port".

Paolo
Yoshiaki Tamura - Jan. 28, 2011, 1:53 p.m.
2011/1/28 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/28/2011 08:21 AM, Yoshiaki Tamura wrote:
>>
>> +    /* check ft_mode option  */
>> +    p = strstr(uri, "ft_mode");
>> +    if (p&&  !strcmp(p, "ft_mode")) {
>> +        ft_mode = FT_INIT;
>> +    }
>> +
>
> This works for TCP mode, but:
>
> 1) I am not sure what would happen with -incoming exec;

Nothing happens if used with other protocols, but I assume you're
mentioning that it's not clear from the code, which makes sense.

> 2) it is tricky! :)  It works only because anything after the port is
> truncated by parse_host_port.
>
> Is there any reason why the code cannot be in tcp_start_incoming_migration,
> where we know the URI has the right scheme?  Alternatively you could put it
> _before_ the scheme, like "kemari:tcp:host:port".

I placed it here just because string parsing was mainly done in
this function except for parse_host_port.  I have no objection
placing the parsing at the beginning of
tcp_start_incoming_migration.

Thanks,

Yoshi

>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Paolo Bonzini - Jan. 28, 2011, 2:43 p.m.
On 01/28/2011 02:53 PM, Yoshiaki Tamura wrote:
>> >  1) I am not sure what would happen with -incoming exec;
>
> Nothing happens if used with other protocols, but I assume you're
> mentioning that it's not clear from the code, which makes sense.

I assume nothing just because the code for other protocols isn't using 
ft_mode.  However, for -incoming exec the parsing code as it is now 
would trigger if the executed file ended with "ft_mode".

So now I think it should be at the beginning of the scheme for forward 
compatibility with everything.  Is it possible to detect a migration 
scheme that does not support Kemari, and give an error in that case?

Paolo
Yoshiaki Tamura - Jan. 28, 2011, 3:05 p.m.
2011/1/28 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/28/2011 02:53 PM, Yoshiaki Tamura wrote:
>>>
>>> >  1) I am not sure what would happen with -incoming exec;
>>
>> Nothing happens if used with other protocols, but I assume you're
>> mentioning that it's not clear from the code, which makes sense.
>
> I assume nothing just because the code for other protocols isn't using
> ft_mode.  However, for -incoming exec the parsing code as it is now would
> trigger if the executed file ended with "ft_mode".

Hmm.  Haven't thought about it.

> So now I think it should be at the beginning of the scheme for forward
> compatibility with everything.  Is it possible to detect a migration scheme
> that does not support Kemari, and give an error in that case?

Having a scheme like "kemari:tcp:host:port" looks quite
challenging to me.  We can of course add some quick hacks for it,
but adding a nice layered architecture should be more
appropriate.  Similar to protocols and formats in block layer?
At the same time, I want to avoid anything over engineered.

Thanks,

Yoshi

>
> Paolo
>
>
Paolo Bonzini - Jan. 28, 2011, 3:10 p.m.
On 01/28/2011 04:05 PM, Yoshiaki Tamura wrote:
> Having a scheme like "kemari:tcp:host:port" looks quite
> challenging to me.  We can of course add some quick hacks for it,
> but adding a nice layered architecture should be more
> appropriate.  Similar to protocols and formats in block layer?
> At the same time, I want to avoid anything over engineered.

I was simply thinking of

    if (strstart (uri, "kemari:", &p)) {
        ft_mode = FT_INIT;
        uri = p;
    }

:)

I think the same could be done for outgoing migration instead of -k 
actually.

Paolo
Yoshiaki Tamura - Jan. 28, 2011, 3:31 p.m.
2011/1/29 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/28/2011 04:05 PM, Yoshiaki Tamura wrote:
>>
>> Having a scheme like "kemari:tcp:host:port" looks quite
>> challenging to me.  We can of course add some quick hacks for it,
>> but adding a nice layered architecture should be more
>> appropriate.  Similar to protocols and formats in block layer?
>> At the same time, I want to avoid anything over engineered.
>
> I was simply thinking of
>
>   if (strstart (uri, "kemari:", &p)) {
>       ft_mode = FT_INIT;
>       uri = p;
>   }
>
> :)

That's the hack I was imaging :)

Maybe this is just an issue of preference, but I'm not sure
adding "kemari:" to be intuitive.  If there were similar
extensions having the same problem, I would have agreed quickly.
I originally didn't have this idea, but simply adding -kemari
separate from -incoming isn't enough?

Thanks,

Yoshi

> I think the same could be done for outgoing migration instead of -k
> actually.
>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Paolo Bonzini - Jan. 28, 2011, 3:40 p.m.
On 01/28/2011 04:31 PM, Yoshiaki Tamura wrote:
> That's the hack I was imaging:)

So your original patch is also a hack? :)

> Maybe this is just an issue of preference, but I'm not sure
> adding "kemari:" to be intuitive.  If there were similar
> extensions having the same problem, I would have agreed quickly.
> I originally didn't have this idea, but simply adding -kemari
> separate from -incoming isn't enough?

No idea...  Only, having "migrate" on one side and something other than 
"-incoming" on the other side seems strange.

Paolo
Yoshiaki Tamura - Jan. 29, 2011, 9:31 a.m.
2011/1/29 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/28/2011 04:31 PM, Yoshiaki Tamura wrote:
>>
>> That's the hack I was imaging:)
>
> So your original patch is also a hack? :)

TBH, yeah :) I didn't came up better idea that is not over
engineered.

>> Maybe this is just an issue of preference, but I'm not sure
>> adding "kemari:" to be intuitive.  If there were similar
>> extensions having the same problem, I would have agreed quickly.
>> I originally didn't have this idea, but simply adding -kemari
>> separate from -incoming isn't enough?
>
> No idea...  Only, having "migrate" on one side and something other than
> "-incoming" on the other side seems strange.

OK, then while keeping "-incoming kemari:tcp:<host>:<port>" as a
strong solution, could you please explain why placing the original
parser under tcp handler wasn't a good idea?  With that,
-incoming exec .*,ft_mode shouldn't be a problem.  I
chose "-incoming tcp:<host>:<port>, ft_mode" because qemu usually
take "," for specifying variants for each option (e.g. -net
nic,macaddr=).  The problem was -incoming didn't have it yet.

Yoshi

>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Paolo Bonzini - Jan. 29, 2011, 10:43 a.m.
On 01/29/2011 10:31 AM, Yoshiaki Tamura wrote:
> OK, then while keeping "-incoming kemari:tcp:<host>:<port>" as a
> strong solution, could you please explain why placing the original
> parser under tcp handler wasn't a good idea?  With that,
> -incoming exec .*,ft_mode shouldn't be a problem.

But a hypothetical -incoming unix.*,ft_mode would have to be implemented 
twice.

Paolo
Yoshiaki Tamura - Jan. 29, 2011, 11:32 a.m.
2011/1/29 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/29/2011 10:31 AM, Yoshiaki Tamura wrote:
>>
>> OK, then while keeping "-incoming kemari:tcp:<host>:<port>" as a
>> strong solution, could you please explain why placing the original
>> parser under tcp handler wasn't a good idea?  With that,
>> -incoming exec .*,ft_mode shouldn't be a problem.
>
> But a hypothetical -incoming unix.*,ft_mode would have to be implemented
> twice.

You mean Kemari should be able to use with unix domain sockets,
or other local communication patch?  Since Kemari needs two
remote hosts, I don't see why need to use unix domain sockets
except for testing.  Maybe I'm missing the point :)

Yoshi

>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Paolo Bonzini - Jan. 29, 2011, 12:21 p.m.
On 01/29/2011 12:32 PM, Yoshiaki Tamura wrote:
>> >  But a hypothetical -incoming unix.*,ft_mode would have to be implemented
>> >  twice.
> You mean Kemari should be able to use with unix domain sockets,
> or other local communication patch?  Since Kemari needs two
> remote hosts, I don't see why need to use unix domain sockets
> except for testing.  Maybe I'm missing the point:)

Well, I mentioned unix because it is basically the only other migration 
protocol implemented in QEMU, but the file descriptor backend could also 
be used.  Kemari-over-SCTP could be an interesting application too for 
example.

I'm not saying you should adjust the other patches for the 
implementation, but the syntax to invoke Kemari should be future-proof.

Paolo
Yoshiaki Tamura - Jan. 29, 2011, 1:29 p.m.
2011/1/29 Paolo Bonzini <pbonzini@redhat.com>:
> On 01/29/2011 12:32 PM, Yoshiaki Tamura wrote:
>>>
>>> >  But a hypothetical -incoming unix.*,ft_mode would have to be
>>> > implemented
>>> >  twice.
>>
>> You mean Kemari should be able to use with unix domain sockets,
>> or other local communication patch?  Since Kemari needs two
>> remote hosts, I don't see why need to use unix domain sockets
>> except for testing.  Maybe I'm missing the point:)
>
> Well, I mentioned unix because it is basically the only other migration
> protocol implemented in QEMU, but the file descriptor backend could also be
> used.  Kemari-over-SCTP could be an interesting application too for example.
>
> I'm not saying you should adjust the other patches for the implementation,
> but the syntax to invoke Kemari should be future-proof.

Yes, I understand.  I'm not negative to your suggestion at all,
but trying to figure out what would be the best :) Let me take
your suggestion to use "-incoming kemari:tcp:<host>:<port>" in
the next spin, and see what others think.

Thanks,

Yoshi

>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Patch

diff --git a/migration.c b/migration.c
index 1752cf4..29d4fb1 100644
--- a/migration.c
+++ b/migration.c
@@ -45,6 +45,12 @@  int qemu_start_incoming_migration(const char *uri)
     const char *p;
     int ret;
 
+    /* check ft_mode option  */
+    p = strstr(uri, "ft_mode");
+    if (p && !strcmp(p, "ft_mode")) {
+        ft_mode = FT_INIT;
+    }
+
     if (strstart(uri, "tcp:", &p))
         ret = tcp_start_incoming_migration(p);
 #if !defined(WIN32)