Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-16 Thread Piotr Kliczewski 
On Tue, Aug 8, 2017 at 2:50 PM, Roy Golan  wrote:
> This is for 4.1 https://gerrit.ovirt.org/#/c/78916/ , track that, it should
> be in.
>

Irit's work is about providing reconnect logic which was not
implemented till now.

I think that introducing ping2 won't solve the issue for all our
cases. As Martin
mentioned we already have HE and mom calling 'ping' (both local).

When introducing fix for this issue we need to consider backward compatibility
for setupNetworks, HE and mom.


> On Tue, 8 Aug 2017 at 15:10 Martin Sivak  wrote:
>>
>> What stomp heartbeat? There is no such thing in the Python json rpc
>> client.
>>
>> Martin
>>
>> On Tue, Aug 8, 2017 at 1:04 PM, Roy Golan  wrote:
>> > Why isn't the stomp heartbeat enough?
>> >
>> > On Tue, 8 Aug 2017 at 13:45 Martin Sivak  wrote:
>> >>
>> >> MOM and hosted engine use the ping verb to verify the connection is
>> >> still OK. The RPC client did not have any reconnect logic in the past
>> >> (and I think it still does not..).
>> >>
>> >> Martin
>> >>
>> >> On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
>> >> > Petr, why do you need the ping verb? what are you after?
>> >> >
>> >> > On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
>> >> >>
>> >> >> > The proposed solution is focused on making sure a command does one
>> >> >> > thing
>> >> >> > and
>> >> >> > not two:
>> >> >> > A ping that has no side effects and a "watchdog" mechanism to
>> >> >> > confirm
>> >> >> > connectivity.
>> >> >>
>> >> >> This sounds as exactly the right solution right now.
>> >> >>
>> >> >> >>> Still someone could call conirmConnectivity, no?
>> >> >>
>> >> >> We do not protect any other endpoints that can cause the host to go
>> >> >> wild (storage or even setupNetworks). I agree with Edward it is the
>> >> >> responsibility of the caller to do the right thing. You need to be
>> >> >> root or have the certificate to talk to VDSM anyway.
>> >> >>
>> >> >> Martin
>> >> >>
>> >> >> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas 
>> >> >> wrote:
>> >> >> >
>> >> >> >
>> >> >> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer 
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan 
>> >> >> >> wrote:
>> >> >> >>>
>> >> >> >>> Still someone could call conirmConnectivity, no? so the state
>> >> >> >>> isn't
>> >> >> >>> guarded from localhost tinkering anyhow. If you really need a
>> >> >> >>> solution
>> >> >> >>> you
>> >> >> >>> can acuire a token for this operation by setupNetworks, and
>> >> >> >>> confirm
>> >> >> >>> connectivity with this token passed back.
>> >> >> >>>
>> >> >> >>> I'm not sure about the severity of the problem here, I'll let
>> >> >> >>> other
>> >> >> >>> reply, but I'm against this kind of solution.
>> >> >> >>>
>> >> >> >>>
>> >> >> >>>
>> >> >> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek 
>> >> >> >>> wrote:
>> >> >> 
>> >> >>  Hello,
>> >> >> 
>> >> >>  current VDSM ping verb has a problem - it confirms network
>> >> >>  connectivity as a side-effect. After Engine calls setupNetwork
>> >> >>  it
>> >> >>  pings VDSM host to confirm that external network connectivity
>> >> >>  is
>> >> >>  not
>> >> >>  broken. This prohibits other users to call ping from localhost
>> >> >>  since
>> >> >>  it would confirm connectivity even though networking could be
>> >> >>  broken.
>> >> >> >>
>> >> >> >>
>> >> >> >> Vdsm can save the client ip setting up the network. Getting a
>> >> >> >> ping
>> >> >> >> from
>> >> >> >> this
>> >> >> >> client can confirm that the connectivity was restored. pings from
>> >> >> >> other
>> >> >> >> hosts
>> >> >> >> can be ignored.
>> >> >> >>
>> >> >> >> The client address is available in a thread local variable
>> >> >> >> (context.client_host)
>> >> >> >> during all api calls. see vdsm.common.api.context_string() for
>> >> >> >> example
>> >> >> >> usage.
>> >> >> >>
>> >> >> >> This infrastructure is available in 4.1.
>> >> >> >
>> >> >> >
>> >> >> > The proposed solution is focused on making sure a command does one
>> >> >> > thing
>> >> >> > and
>> >> >> > not two:
>> >> >> > A ping that has no side effects and a "watchdog" mechanism to
>> >> >> > confirm
>> >> >> > connectivity.
>> >> >> >
>> >> >> > Does it make sense to confirm connectivity from localhost? In many
>> >> >> > cases
>> >> >> > it
>> >> >> > probably does not,
>> >> >> > but there may be cases where it does make sense... it is not the
>> >> >> > functionality to determine what
>> >> >> > makes sense or not, it is the usage of it who has the
>> >> >> > responsibility
>> >> >> > to
>> >> >> > use
>> >> >> > it correctly.
>> >> >> >
>> >> >> >>
>> >> >> >> Nir
>> >> >> >>
>> >> >> 
>> >> >> 
>> >> >>  In order to fix this problem ping should be split to ping2
>> >> >>  (which
>> >>

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-09 Thread Petr Horacek 
Method with saved caller's IP address is an option, but it would
require rewriting of network's connectivity check mechanism. We would
need to introduce some extra IPC between supervdsm and vdsm for this
IIUIC.

The reason for ping2 is backward compatibility. It is undocumented,
but Engine still depends on it and older versions will probably stay
like that. In case we remove the side-effect and old Engine calls new
ping1 it would never confirm network connectivity.

2017-08-08 15:19 GMT+02:00 Martin Sivak :
>>> A ping that has no side effects and a "watchdog" mechanism to confirm
>>> connectivity.
>>
>> This sounds as exactly the right solution right now.
>
> Just to clarify: Removing the undocumented side effects from ping. Not
> introducing ping2.
>
> Martin
>
> On Tue, Aug 8, 2017 at 10:47 AM, Martin Sivak  wrote:
>>> The proposed solution is focused on making sure a command does one thing and
>>> not two:
>>> A ping that has no side effects and a "watchdog" mechanism to confirm
>>> connectivity.
>>
>> This sounds as exactly the right solution right now.
>>
> Still someone could call conirmConnectivity, no?
>>
>> We do not protect any other endpoints that can cause the host to go
>> wild (storage or even setupNetworks). I agree with Edward it is the
>> responsibility of the caller to do the right thing. You need to be
>> root or have the certificate to talk to VDSM anyway.
>>
>> Martin
>>
>> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
>>>
>>>
>>> On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:

 On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
>
> Still someone could call conirmConnectivity, no? so the state isn't
> guarded from localhost tinkering anyhow. If you really need a solution you
> can acuire a token for this operation by setupNetworks, and confirm
> connectivity with this token passed back.
>
> I'm not sure about the severity of the problem here, I'll let other
> reply, but I'm against this kind of solution.
>
>
>
> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>>
>> Hello,
>>
>> current VDSM ping verb has a problem - it confirms network
>> connectivity as a side-effect. After Engine calls setupNetwork it
>> pings VDSM host to confirm that external network connectivity is not
>> broken. This prohibits other users to call ping from localhost since
>> it would confirm connectivity even though networking could be broken.


 Vdsm can save the client ip setting up the network. Getting a ping from
 this
 client can confirm that the connectivity was restored. pings from other
 hosts
 can be ignored.

 The client address is available in a thread local variable
 (context.client_host)
 during all api calls. see vdsm.common.api.context_string() for example
 usage.

 This infrastructure is available in 4.1.
>>>
>>>
>>> The proposed solution is focused on making sure a command does one thing and
>>> not two:
>>> A ping that has no side effects and a "watchdog" mechanism to confirm
>>> connectivity.
>>>
>>> Does it make sense to confirm connectivity from localhost? In many cases it
>>> probably does not,
>>> but there may be cases where it does make sense... it is not the
>>> functionality to determine what
>>> makes sense or not, it is the usage of it who has the responsibility to use
>>> it correctly.
>>>

 Nir

>>
>>
>> In order to fix this problem ping should be split to ping2 (which just
>> returns Success with no side-effect) and confirmConnectivity. Change
>> on VDSM side was introduced in [1], we still need to expose new verbs
>> in Engine.
>>
>> Regards,
>> Petr
>>
>> [1] https://gerrit.ovirt.org/#/c/80119/
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel


 ___
 Devel mailing list
 Devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/devel
>>>
>>>
>>>
>>> ___
>>> Devel mailing list
>>> Devel@ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/devel
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Martin Sivak 
>> A ping that has no side effects and a "watchdog" mechanism to confirm
>> connectivity.
>
> This sounds as exactly the right solution right now.

Just to clarify: Removing the undocumented side effects from ping. Not
introducing ping2.

Martin

On Tue, Aug 8, 2017 at 10:47 AM, Martin Sivak  wrote:
>> The proposed solution is focused on making sure a command does one thing and
>> not two:
>> A ping that has no side effects and a "watchdog" mechanism to confirm
>> connectivity.
>
> This sounds as exactly the right solution right now.
>
 Still someone could call conirmConnectivity, no?
>
> We do not protect any other endpoints that can cause the host to go
> wild (storage or even setupNetworks). I agree with Edward it is the
> responsibility of the caller to do the right thing. You need to be
> root or have the certificate to talk to VDSM anyway.
>
> Martin
>
> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
>>
>>
>> On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:
>>>
>>> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:

 Still someone could call conirmConnectivity, no? so the state isn't
 guarded from localhost tinkering anyhow. If you really need a solution you
 can acuire a token for this operation by setupNetworks, and confirm
 connectivity with this token passed back.

 I'm not sure about the severity of the problem here, I'll let other
 reply, but I'm against this kind of solution.



 On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>
> Hello,
>
> current VDSM ping verb has a problem - it confirms network
> connectivity as a side-effect. After Engine calls setupNetwork it
> pings VDSM host to confirm that external network connectivity is not
> broken. This prohibits other users to call ping from localhost since
> it would confirm connectivity even though networking could be broken.
>>>
>>>
>>> Vdsm can save the client ip setting up the network. Getting a ping from
>>> this
>>> client can confirm that the connectivity was restored. pings from other
>>> hosts
>>> can be ignored.
>>>
>>> The client address is available in a thread local variable
>>> (context.client_host)
>>> during all api calls. see vdsm.common.api.context_string() for example
>>> usage.
>>>
>>> This infrastructure is available in 4.1.
>>
>>
>> The proposed solution is focused on making sure a command does one thing and
>> not two:
>> A ping that has no side effects and a "watchdog" mechanism to confirm
>> connectivity.
>>
>> Does it make sense to confirm connectivity from localhost? In many cases it
>> probably does not,
>> but there may be cases where it does make sense... it is not the
>> functionality to determine what
>> makes sense or not, it is the usage of it who has the responsibility to use
>> it correctly.
>>
>>>
>>> Nir
>>>
>
>
> In order to fix this problem ping should be split to ping2 (which just
> returns Success with no side-effect) and confirmConnectivity. Change
> on VDSM side was introduced in [1], we still need to expose new verbs
> in Engine.
>
> Regards,
> Petr
>
> [1] https://gerrit.ovirt.org/#/c/80119/
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel

 ___
 Devel mailing list
 Devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/devel
>>>
>>>
>>> ___
>>> Devel mailing list
>>> Devel@ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>>
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Martin Sivak 
But it isn't as I said already. And so we are keeping the ping code
and that is why vdsm needs to solve the setupNetworks/ping bug.

Martin

On Tue, Aug 8, 2017 at 2:50 PM, Roy Golan  wrote:
> This is for 4.1 https://gerrit.ovirt.org/#/c/78916/ , track that, it should
> be in.
>
> On Tue, 8 Aug 2017 at 15:10 Martin Sivak  wrote:
>>
>> What stomp heartbeat? There is no such thing in the Python json rpc
>> client.
>>
>> Martin
>>
>> On Tue, Aug 8, 2017 at 1:04 PM, Roy Golan  wrote:
>> > Why isn't the stomp heartbeat enough?
>> >
>> > On Tue, 8 Aug 2017 at 13:45 Martin Sivak  wrote:
>> >>
>> >> MOM and hosted engine use the ping verb to verify the connection is
>> >> still OK. The RPC client did not have any reconnect logic in the past
>> >> (and I think it still does not..).
>> >>
>> >> Martin
>> >>
>> >> On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
>> >> > Petr, why do you need the ping verb? what are you after?
>> >> >
>> >> > On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
>> >> >>
>> >> >> > The proposed solution is focused on making sure a command does one
>> >> >> > thing
>> >> >> > and
>> >> >> > not two:
>> >> >> > A ping that has no side effects and a "watchdog" mechanism to
>> >> >> > confirm
>> >> >> > connectivity.
>> >> >>
>> >> >> This sounds as exactly the right solution right now.
>> >> >>
>> >> >> >>> Still someone could call conirmConnectivity, no?
>> >> >>
>> >> >> We do not protect any other endpoints that can cause the host to go
>> >> >> wild (storage or even setupNetworks). I agree with Edward it is the
>> >> >> responsibility of the caller to do the right thing. You need to be
>> >> >> root or have the certificate to talk to VDSM anyway.
>> >> >>
>> >> >> Martin
>> >> >>
>> >> >> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas 
>> >> >> wrote:
>> >> >> >
>> >> >> >
>> >> >> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer 
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan 
>> >> >> >> wrote:
>> >> >> >>>
>> >> >> >>> Still someone could call conirmConnectivity, no? so the state
>> >> >> >>> isn't
>> >> >> >>> guarded from localhost tinkering anyhow. If you really need a
>> >> >> >>> solution
>> >> >> >>> you
>> >> >> >>> can acuire a token for this operation by setupNetworks, and
>> >> >> >>> confirm
>> >> >> >>> connectivity with this token passed back.
>> >> >> >>>
>> >> >> >>> I'm not sure about the severity of the problem here, I'll let
>> >> >> >>> other
>> >> >> >>> reply, but I'm against this kind of solution.
>> >> >> >>>
>> >> >> >>>
>> >> >> >>>
>> >> >> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek 
>> >> >> >>> wrote:
>> >> >> 
>> >> >>  Hello,
>> >> >> 
>> >> >>  current VDSM ping verb has a problem - it confirms network
>> >> >>  connectivity as a side-effect. After Engine calls setupNetwork
>> >> >>  it
>> >> >>  pings VDSM host to confirm that external network connectivity
>> >> >>  is
>> >> >>  not
>> >> >>  broken. This prohibits other users to call ping from localhost
>> >> >>  since
>> >> >>  it would confirm connectivity even though networking could be
>> >> >>  broken.
>> >> >> >>
>> >> >> >>
>> >> >> >> Vdsm can save the client ip setting up the network. Getting a
>> >> >> >> ping
>> >> >> >> from
>> >> >> >> this
>> >> >> >> client can confirm that the connectivity was restored. pings from
>> >> >> >> other
>> >> >> >> hosts
>> >> >> >> can be ignored.
>> >> >> >>
>> >> >> >> The client address is available in a thread local variable
>> >> >> >> (context.client_host)
>> >> >> >> during all api calls. see vdsm.common.api.context_string() for
>> >> >> >> example
>> >> >> >> usage.
>> >> >> >>
>> >> >> >> This infrastructure is available in 4.1.
>> >> >> >
>> >> >> >
>> >> >> > The proposed solution is focused on making sure a command does one
>> >> >> > thing
>> >> >> > and
>> >> >> > not two:
>> >> >> > A ping that has no side effects and a "watchdog" mechanism to
>> >> >> > confirm
>> >> >> > connectivity.
>> >> >> >
>> >> >> > Does it make sense to confirm connectivity from localhost? In many
>> >> >> > cases
>> >> >> > it
>> >> >> > probably does not,
>> >> >> > but there may be cases where it does make sense... it is not the
>> >> >> > functionality to determine what
>> >> >> > makes sense or not, it is the usage of it who has the
>> >> >> > responsibility
>> >> >> > to
>> >> >> > use
>> >> >> > it correctly.
>> >> >> >
>> >> >> >>
>> >> >> >> Nir
>> >> >> >>
>> >> >> 
>> >> >> 
>> >> >>  In order to fix this problem ping should be split to ping2
>> >> >>  (which
>> >> >>  just
>> >> >>  returns Success with no side-effect) and confirmConnectivity.
>> >> >>  Change
>> >> >>  on VDSM side was introduced in [1], we still need to expose new
>> >> >>

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Roy Golan 
This is for 4.1 https://gerrit.ovirt.org/#/c/78916/ , track that, it should
be in.

On Tue, 8 Aug 2017 at 15:10 Martin Sivak  wrote:

> What stomp heartbeat? There is no such thing in the Python json rpc client.
>
> Martin
>
> On Tue, Aug 8, 2017 at 1:04 PM, Roy Golan  wrote:
> > Why isn't the stomp heartbeat enough?
> >
> > On Tue, 8 Aug 2017 at 13:45 Martin Sivak  wrote:
> >>
> >> MOM and hosted engine use the ping verb to verify the connection is
> >> still OK. The RPC client did not have any reconnect logic in the past
> >> (and I think it still does not..).
> >>
> >> Martin
> >>
> >> On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
> >> > Petr, why do you need the ping verb? what are you after?
> >> >
> >> > On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
> >> >>
> >> >> > The proposed solution is focused on making sure a command does one
> >> >> > thing
> >> >> > and
> >> >> > not two:
> >> >> > A ping that has no side effects and a "watchdog" mechanism to
> confirm
> >> >> > connectivity.
> >> >>
> >> >> This sounds as exactly the right solution right now.
> >> >>
> >> >> >>> Still someone could call conirmConnectivity, no?
> >> >>
> >> >> We do not protect any other endpoints that can cause the host to go
> >> >> wild (storage or even setupNetworks). I agree with Edward it is the
> >> >> responsibility of the caller to do the right thing. You need to be
> >> >> root or have the certificate to talk to VDSM anyway.
> >> >>
> >> >> Martin
> >> >>
> >> >> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas 
> wrote:
> >> >> >
> >> >> >
> >> >> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer 
> >> >> > wrote:
> >> >> >>
> >> >> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan 
> wrote:
> >> >> >>>
> >> >> >>> Still someone could call conirmConnectivity, no? so the state
> isn't
> >> >> >>> guarded from localhost tinkering anyhow. If you really need a
> >> >> >>> solution
> >> >> >>> you
> >> >> >>> can acuire a token for this operation by setupNetworks, and
> confirm
> >> >> >>> connectivity with this token passed back.
> >> >> >>>
> >> >> >>> I'm not sure about the severity of the problem here, I'll let
> other
> >> >> >>> reply, but I'm against this kind of solution.
> >> >> >>>
> >> >> >>>
> >> >> >>>
> >> >> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek 
> >> >> >>> wrote:
> >> >> 
> >> >>  Hello,
> >> >> 
> >> >>  current VDSM ping verb has a problem - it confirms network
> >> >>  connectivity as a side-effect. After Engine calls setupNetwork
> it
> >> >>  pings VDSM host to confirm that external network connectivity is
> >> >>  not
> >> >>  broken. This prohibits other users to call ping from localhost
> >> >>  since
> >> >>  it would confirm connectivity even though networking could be
> >> >>  broken.
> >> >> >>
> >> >> >>
> >> >> >> Vdsm can save the client ip setting up the network. Getting a ping
> >> >> >> from
> >> >> >> this
> >> >> >> client can confirm that the connectivity was restored. pings from
> >> >> >> other
> >> >> >> hosts
> >> >> >> can be ignored.
> >> >> >>
> >> >> >> The client address is available in a thread local variable
> >> >> >> (context.client_host)
> >> >> >> during all api calls. see vdsm.common.api.context_string() for
> >> >> >> example
> >> >> >> usage.
> >> >> >>
> >> >> >> This infrastructure is available in 4.1.
> >> >> >
> >> >> >
> >> >> > The proposed solution is focused on making sure a command does one
> >> >> > thing
> >> >> > and
> >> >> > not two:
> >> >> > A ping that has no side effects and a "watchdog" mechanism to
> confirm
> >> >> > connectivity.
> >> >> >
> >> >> > Does it make sense to confirm connectivity from localhost? In many
> >> >> > cases
> >> >> > it
> >> >> > probably does not,
> >> >> > but there may be cases where it does make sense... it is not the
> >> >> > functionality to determine what
> >> >> > makes sense or not, it is the usage of it who has the
> responsibility
> >> >> > to
> >> >> > use
> >> >> > it correctly.
> >> >> >
> >> >> >>
> >> >> >> Nir
> >> >> >>
> >> >> 
> >> >> 
> >> >>  In order to fix this problem ping should be split to ping2
> (which
> >> >>  just
> >> >>  returns Success with no side-effect) and confirmConnectivity.
> >> >>  Change
> >> >>  on VDSM side was introduced in [1], we still need to expose new
> >> >>  verbs
> >> >>  in Engine.
> >> >> 
> >> >>  Regards,
> >> >>  Petr
> >> >> 
> >> >>  [1] https://gerrit.ovirt.org/#/c/80119/
> >> >>  ___
> >> >>  Devel mailing list
> >> >>  Devel@ovirt.org
> >> >>  http://lists.ovirt.org/mailman/listinfo/devel
> >> >> >>>
> >> >> >>> ___
> >> >> >>> Devel mailing list
> >> >> >>> Devel@ovirt.org
> >> >>

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Martin Sivak 
What stomp heartbeat? There is no such thing in the Python json rpc client.

Martin

On Tue, Aug 8, 2017 at 1:04 PM, Roy Golan  wrote:
> Why isn't the stomp heartbeat enough?
>
> On Tue, 8 Aug 2017 at 13:45 Martin Sivak  wrote:
>>
>> MOM and hosted engine use the ping verb to verify the connection is
>> still OK. The RPC client did not have any reconnect logic in the past
>> (and I think it still does not..).
>>
>> Martin
>>
>> On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
>> > Petr, why do you need the ping verb? what are you after?
>> >
>> > On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
>> >>
>> >> > The proposed solution is focused on making sure a command does one
>> >> > thing
>> >> > and
>> >> > not two:
>> >> > A ping that has no side effects and a "watchdog" mechanism to confirm
>> >> > connectivity.
>> >>
>> >> This sounds as exactly the right solution right now.
>> >>
>> >> >>> Still someone could call conirmConnectivity, no?
>> >>
>> >> We do not protect any other endpoints that can cause the host to go
>> >> wild (storage or even setupNetworks). I agree with Edward it is the
>> >> responsibility of the caller to do the right thing. You need to be
>> >> root or have the certificate to talk to VDSM anyway.
>> >>
>> >> Martin
>> >>
>> >> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
>> >> >
>> >> >
>> >> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer 
>> >> > wrote:
>> >> >>
>> >> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
>> >> >>>
>> >> >>> Still someone could call conirmConnectivity, no? so the state isn't
>> >> >>> guarded from localhost tinkering anyhow. If you really need a
>> >> >>> solution
>> >> >>> you
>> >> >>> can acuire a token for this operation by setupNetworks, and confirm
>> >> >>> connectivity with this token passed back.
>> >> >>>
>> >> >>> I'm not sure about the severity of the problem here, I'll let other
>> >> >>> reply, but I'm against this kind of solution.
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek 
>> >> >>> wrote:
>> >> 
>> >>  Hello,
>> >> 
>> >>  current VDSM ping verb has a problem - it confirms network
>> >>  connectivity as a side-effect. After Engine calls setupNetwork it
>> >>  pings VDSM host to confirm that external network connectivity is
>> >>  not
>> >>  broken. This prohibits other users to call ping from localhost
>> >>  since
>> >>  it would confirm connectivity even though networking could be
>> >>  broken.
>> >> >>
>> >> >>
>> >> >> Vdsm can save the client ip setting up the network. Getting a ping
>> >> >> from
>> >> >> this
>> >> >> client can confirm that the connectivity was restored. pings from
>> >> >> other
>> >> >> hosts
>> >> >> can be ignored.
>> >> >>
>> >> >> The client address is available in a thread local variable
>> >> >> (context.client_host)
>> >> >> during all api calls. see vdsm.common.api.context_string() for
>> >> >> example
>> >> >> usage.
>> >> >>
>> >> >> This infrastructure is available in 4.1.
>> >> >
>> >> >
>> >> > The proposed solution is focused on making sure a command does one
>> >> > thing
>> >> > and
>> >> > not two:
>> >> > A ping that has no side effects and a "watchdog" mechanism to confirm
>> >> > connectivity.
>> >> >
>> >> > Does it make sense to confirm connectivity from localhost? In many
>> >> > cases
>> >> > it
>> >> > probably does not,
>> >> > but there may be cases where it does make sense... it is not the
>> >> > functionality to determine what
>> >> > makes sense or not, it is the usage of it who has the responsibility
>> >> > to
>> >> > use
>> >> > it correctly.
>> >> >
>> >> >>
>> >> >> Nir
>> >> >>
>> >> 
>> >> 
>> >>  In order to fix this problem ping should be split to ping2 (which
>> >>  just
>> >>  returns Success with no side-effect) and confirmConnectivity.
>> >>  Change
>> >>  on VDSM side was introduced in [1], we still need to expose new
>> >>  verbs
>> >>  in Engine.
>> >> 
>> >>  Regards,
>> >>  Petr
>> >> 
>> >>  [1] https://gerrit.ovirt.org/#/c/80119/
>> >>  ___
>> >>  Devel mailing list
>> >>  Devel@ovirt.org
>> >>  http://lists.ovirt.org/mailman/listinfo/devel
>> >> >>>
>> >> >>> ___
>> >> >>> Devel mailing list
>> >> >>> Devel@ovirt.org
>> >> >>> http://lists.ovirt.org/mailman/listinfo/devel
>> >> >>
>> >> >>
>> >> >> ___
>> >> >> Devel mailing list
>> >> >> Devel@ovirt.org
>> >> >> http://lists.ovirt.org/mailman/listinfo/devel
>> >> >
>> >> >
>> >> >
>> >> > ___
>> >> > Devel mailing list
>> >> > Devel@ovirt.org
>> >> > http://lists.ovirt.org/mailman/listinfo/devel
>> >>

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Roy Golan 
Why isn't the stomp heartbeat enough?

On Tue, 8 Aug 2017 at 13:45 Martin Sivak  wrote:

> MOM and hosted engine use the ping verb to verify the connection is
> still OK. The RPC client did not have any reconnect logic in the past
> (and I think it still does not..).
>
> Martin
>
> On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
> > Petr, why do you need the ping verb? what are you after?
> >
> > On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
> >>
> >> > The proposed solution is focused on making sure a command does one
> thing
> >> > and
> >> > not two:
> >> > A ping that has no side effects and a "watchdog" mechanism to confirm
> >> > connectivity.
> >>
> >> This sounds as exactly the right solution right now.
> >>
> >> >>> Still someone could call conirmConnectivity, no?
> >>
> >> We do not protect any other endpoints that can cause the host to go
> >> wild (storage or even setupNetworks). I agree with Edward it is the
> >> responsibility of the caller to do the right thing. You need to be
> >> root or have the certificate to talk to VDSM anyway.
> >>
> >> Martin
> >>
> >> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
> >> >
> >> >
> >> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer 
> wrote:
> >> >>
> >> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
> >> >>>
> >> >>> Still someone could call conirmConnectivity, no? so the state isn't
> >> >>> guarded from localhost tinkering anyhow. If you really need a
> solution
> >> >>> you
> >> >>> can acuire a token for this operation by setupNetworks, and confirm
> >> >>> connectivity with this token passed back.
> >> >>>
> >> >>> I'm not sure about the severity of the problem here, I'll let other
> >> >>> reply, but I'm against this kind of solution.
> >> >>>
> >> >>>
> >> >>>
> >> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek 
> wrote:
> >> 
> >>  Hello,
> >> 
> >>  current VDSM ping verb has a problem - it confirms network
> >>  connectivity as a side-effect. After Engine calls setupNetwork it
> >>  pings VDSM host to confirm that external network connectivity is
> not
> >>  broken. This prohibits other users to call ping from localhost
> since
> >>  it would confirm connectivity even though networking could be
> broken.
> >> >>
> >> >>
> >> >> Vdsm can save the client ip setting up the network. Getting a ping
> from
> >> >> this
> >> >> client can confirm that the connectivity was restored. pings from
> other
> >> >> hosts
> >> >> can be ignored.
> >> >>
> >> >> The client address is available in a thread local variable
> >> >> (context.client_host)
> >> >> during all api calls. see vdsm.common.api.context_string() for
> example
> >> >> usage.
> >> >>
> >> >> This infrastructure is available in 4.1.
> >> >
> >> >
> >> > The proposed solution is focused on making sure a command does one
> thing
> >> > and
> >> > not two:
> >> > A ping that has no side effects and a "watchdog" mechanism to confirm
> >> > connectivity.
> >> >
> >> > Does it make sense to confirm connectivity from localhost? In many
> cases
> >> > it
> >> > probably does not,
> >> > but there may be cases where it does make sense... it is not the
> >> > functionality to determine what
> >> > makes sense or not, it is the usage of it who has the responsibility
> to
> >> > use
> >> > it correctly.
> >> >
> >> >>
> >> >> Nir
> >> >>
> >> 
> >> 
> >>  In order to fix this problem ping should be split to ping2 (which
> >>  just
> >>  returns Success with no side-effect) and confirmConnectivity.
> Change
> >>  on VDSM side was introduced in [1], we still need to expose new
> verbs
> >>  in Engine.
> >> 
> >>  Regards,
> >>  Petr
> >> 
> >>  [1] https://gerrit.ovirt.org/#/c/80119/
> >>  ___
> >>  Devel mailing list
> >>  Devel@ovirt.org
> >>  http://lists.ovirt.org/mailman/listinfo/devel
> >> >>>
> >> >>> ___
> >> >>> Devel mailing list
> >> >>> Devel@ovirt.org
> >> >>> http://lists.ovirt.org/mailman/listinfo/devel
> >> >>
> >> >>
> >> >> ___
> >> >> Devel mailing list
> >> >> Devel@ovirt.org
> >> >> http://lists.ovirt.org/mailman/listinfo/devel
> >> >
> >> >
> >> >
> >> > ___
> >> > Devel mailing list
> >> > Devel@ovirt.org
> >> > http://lists.ovirt.org/mailman/listinfo/devel
> >> ___
> >> Devel mailing list
> >> Devel@ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Martin Sivak 
MOM and hosted engine use the ping verb to verify the connection is
still OK. The RPC client did not have any reconnect logic in the past
(and I think it still does not..).

Martin

On Tue, Aug 8, 2017 at 12:01 PM, Roy Golan  wrote:
> Petr, why do you need the ping verb? what are you after?
>
> On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:
>>
>> > The proposed solution is focused on making sure a command does one thing
>> > and
>> > not two:
>> > A ping that has no side effects and a "watchdog" mechanism to confirm
>> > connectivity.
>>
>> This sounds as exactly the right solution right now.
>>
>> >>> Still someone could call conirmConnectivity, no?
>>
>> We do not protect any other endpoints that can cause the host to go
>> wild (storage or even setupNetworks). I agree with Edward it is the
>> responsibility of the caller to do the right thing. You need to be
>> root or have the certificate to talk to VDSM anyway.
>>
>> Martin
>>
>> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
>> >
>> >
>> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:
>> >>
>> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
>> >>>
>> >>> Still someone could call conirmConnectivity, no? so the state isn't
>> >>> guarded from localhost tinkering anyhow. If you really need a solution
>> >>> you
>> >>> can acuire a token for this operation by setupNetworks, and confirm
>> >>> connectivity with this token passed back.
>> >>>
>> >>> I'm not sure about the severity of the problem here, I'll let other
>> >>> reply, but I'm against this kind of solution.
>> >>>
>> >>>
>> >>>
>> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>> 
>>  Hello,
>> 
>>  current VDSM ping verb has a problem - it confirms network
>>  connectivity as a side-effect. After Engine calls setupNetwork it
>>  pings VDSM host to confirm that external network connectivity is not
>>  broken. This prohibits other users to call ping from localhost since
>>  it would confirm connectivity even though networking could be broken.
>> >>
>> >>
>> >> Vdsm can save the client ip setting up the network. Getting a ping from
>> >> this
>> >> client can confirm that the connectivity was restored. pings from other
>> >> hosts
>> >> can be ignored.
>> >>
>> >> The client address is available in a thread local variable
>> >> (context.client_host)
>> >> during all api calls. see vdsm.common.api.context_string() for example
>> >> usage.
>> >>
>> >> This infrastructure is available in 4.1.
>> >
>> >
>> > The proposed solution is focused on making sure a command does one thing
>> > and
>> > not two:
>> > A ping that has no side effects and a "watchdog" mechanism to confirm
>> > connectivity.
>> >
>> > Does it make sense to confirm connectivity from localhost? In many cases
>> > it
>> > probably does not,
>> > but there may be cases where it does make sense... it is not the
>> > functionality to determine what
>> > makes sense or not, it is the usage of it who has the responsibility to
>> > use
>> > it correctly.
>> >
>> >>
>> >> Nir
>> >>
>> 
>> 
>>  In order to fix this problem ping should be split to ping2 (which
>>  just
>>  returns Success with no side-effect) and confirmConnectivity. Change
>>  on VDSM side was introduced in [1], we still need to expose new verbs
>>  in Engine.
>> 
>>  Regards,
>>  Petr
>> 
>>  [1] https://gerrit.ovirt.org/#/c/80119/
>>  ___
>>  Devel mailing list
>>  Devel@ovirt.org
>>  http://lists.ovirt.org/mailman/listinfo/devel
>> >>>
>> >>> ___
>> >>> Devel mailing list
>> >>> Devel@ovirt.org
>> >>> http://lists.ovirt.org/mailman/listinfo/devel
>> >>
>> >>
>> >> ___
>> >> Devel mailing list
>> >> Devel@ovirt.org
>> >> http://lists.ovirt.org/mailman/listinfo/devel
>> >
>> >
>> >
>> > ___
>> > Devel mailing list
>> > Devel@ovirt.org
>> > http://lists.ovirt.org/mailman/listinfo/devel
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Roy Golan 
Petr, why do you need the ping verb? what are you after?

On Tue, 8 Aug 2017 at 11:54 Martin Sivak  wrote:

> > The proposed solution is focused on making sure a command does one thing
> and
> > not two:
> > A ping that has no side effects and a "watchdog" mechanism to confirm
> > connectivity.
>
> This sounds as exactly the right solution right now.
>
> >>> Still someone could call conirmConnectivity, no?
>
> We do not protect any other endpoints that can cause the host to go
> wild (storage or even setupNetworks). I agree with Edward it is the
> responsibility of the caller to do the right thing. You need to be
> root or have the certificate to talk to VDSM anyway.
>
> Martin
>
> On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
> >
> >
> > On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:
> >>
> >> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
> >>>
> >>> Still someone could call conirmConnectivity, no? so the state isn't
> >>> guarded from localhost tinkering anyhow. If you really need a solution
> you
> >>> can acuire a token for this operation by setupNetworks, and confirm
> >>> connectivity with this token passed back.
> >>>
> >>> I'm not sure about the severity of the problem here, I'll let other
> >>> reply, but I'm against this kind of solution.
> >>>
> >>>
> >>>
> >>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
> 
>  Hello,
> 
>  current VDSM ping verb has a problem - it confirms network
>  connectivity as a side-effect. After Engine calls setupNetwork it
>  pings VDSM host to confirm that external network connectivity is not
>  broken. This prohibits other users to call ping from localhost since
>  it would confirm connectivity even though networking could be broken.
> >>
> >>
> >> Vdsm can save the client ip setting up the network. Getting a ping from
> >> this
> >> client can confirm that the connectivity was restored. pings from other
> >> hosts
> >> can be ignored.
> >>
> >> The client address is available in a thread local variable
> >> (context.client_host)
> >> during all api calls. see vdsm.common.api.context_string() for example
> >> usage.
> >>
> >> This infrastructure is available in 4.1.
> >
> >
> > The proposed solution is focused on making sure a command does one thing
> and
> > not two:
> > A ping that has no side effects and a "watchdog" mechanism to confirm
> > connectivity.
> >
> > Does it make sense to confirm connectivity from localhost? In many cases
> it
> > probably does not,
> > but there may be cases where it does make sense... it is not the
> > functionality to determine what
> > makes sense or not, it is the usage of it who has the responsibility to
> use
> > it correctly.
> >
> >>
> >> Nir
> >>
> 
> 
>  In order to fix this problem ping should be split to ping2 (which just
>  returns Success with no side-effect) and confirmConnectivity. Change
>  on VDSM side was introduced in [1], we still need to expose new verbs
>  in Engine.
> 
>  Regards,
>  Petr
> 
>  [1] https://gerrit.ovirt.org/#/c/80119/
>  ___
>  Devel mailing list
>  Devel@ovirt.org
>  http://lists.ovirt.org/mailman/listinfo/devel
> >>>
> >>> ___
> >>> Devel mailing list
> >>> Devel@ovirt.org
> >>> http://lists.ovirt.org/mailman/listinfo/devel
> >>
> >>
> >> ___
> >> Devel mailing list
> >> Devel@ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/devel
> >
> >
> >
> > ___
> > Devel mailing list
> > Devel@ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Martin Sivak 
> The proposed solution is focused on making sure a command does one thing and
> not two:
> A ping that has no side effects and a "watchdog" mechanism to confirm
> connectivity.

This sounds as exactly the right solution right now.

>>> Still someone could call conirmConnectivity, no?

We do not protect any other endpoints that can cause the host to go
wild (storage or even setupNetworks). I agree with Edward it is the
responsibility of the caller to do the right thing. You need to be
root or have the certificate to talk to VDSM anyway.

Martin

On Tue, Aug 8, 2017 at 8:24 AM, Edward Haas  wrote:
>
>
> On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:
>>
>> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
>>>
>>> Still someone could call conirmConnectivity, no? so the state isn't
>>> guarded from localhost tinkering anyhow. If you really need a solution you
>>> can acuire a token for this operation by setupNetworks, and confirm
>>> connectivity with this token passed back.
>>>
>>> I'm not sure about the severity of the problem here, I'll let other
>>> reply, but I'm against this kind of solution.
>>>
>>>
>>>
>>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:

 Hello,

 current VDSM ping verb has a problem - it confirms network
 connectivity as a side-effect. After Engine calls setupNetwork it
 pings VDSM host to confirm that external network connectivity is not
 broken. This prohibits other users to call ping from localhost since
 it would confirm connectivity even though networking could be broken.
>>
>>
>> Vdsm can save the client ip setting up the network. Getting a ping from
>> this
>> client can confirm that the connectivity was restored. pings from other
>> hosts
>> can be ignored.
>>
>> The client address is available in a thread local variable
>> (context.client_host)
>> during all api calls. see vdsm.common.api.context_string() for example
>> usage.
>>
>> This infrastructure is available in 4.1.
>
>
> The proposed solution is focused on making sure a command does one thing and
> not two:
> A ping that has no side effects and a "watchdog" mechanism to confirm
> connectivity.
>
> Does it make sense to confirm connectivity from localhost? In many cases it
> probably does not,
> but there may be cases where it does make sense... it is not the
> functionality to determine what
> makes sense or not, it is the usage of it who has the responsibility to use
> it correctly.
>
>>
>> Nir
>>


 In order to fix this problem ping should be split to ping2 (which just
 returns Success with no side-effect) and confirmConnectivity. Change
 on VDSM side was introduced in [1], we still need to expose new verbs
 in Engine.

 Regards,
 Petr

 [1] https://gerrit.ovirt.org/#/c/80119/
 ___
 Devel mailing list
 Devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/devel
>>>
>>> ___
>>> Devel mailing list
>>> Devel@ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Edward Haas 
On Mon, Aug 7, 2017 at 5:26 PM, Roy Golan  wrote:

> Still someone could call conirmConnectivity, no? so the state isn't
> guarded from localhost tinkering anyhow. If you really need a solution you
> can acuire a token for this operation by setupNetworks, and confirm
> connectivity with this token passed back.
>

At this stage, the problem is not focus on security. If the usage is wrong
it will indeed break things, attacking that will require some more advance
means (but I am not sure we need it in a close system).


> I'm not sure about the severity of the problem here, I'll let other reply,
> but I'm against this kind of solution.
>
>
>
> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>
>> Hello,
>>
>> current VDSM ping verb has a problem - it confirms network
>> connectivity as a side-effect. After Engine calls setupNetwork it
>> pings VDSM host to confirm that external network connectivity is not
>> broken. This prohibits other users to call ping from localhost since
>> it would confirm connectivity even though networking could be broken.
>>
>> In order to fix this problem ping should be split to ping2 (which just
>> returns Success with no side-effect) and confirmConnectivity. Change
>> on VDSM side was introduced in [1], we still need to expose new verbs
>> in Engine.
>>
>> Regards,
>> Petr
>>
>> [1] https://gerrit.ovirt.org/#/c/80119/
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-08 Thread Edward Haas 
On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer  wrote:

> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:
>
>> Still someone could call conirmConnectivity, no? so the state isn't
>> guarded from localhost tinkering anyhow. If you really need a solution you
>> can acuire a token for this operation by setupNetworks, and confirm
>> connectivity with this token passed back.
>>
>> I'm not sure about the severity of the problem here, I'll let other
>> reply, but I'm against this kind of solution.
>>
>>
>>
>> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>>
>>> Hello,
>>>
>>> current VDSM ping verb has a problem - it confirms network
>>> connectivity as a side-effect. After Engine calls setupNetwork it
>>> pings VDSM host to confirm that external network connectivity is not
>>> broken. This prohibits other users to call ping from localhost since
>>> it would confirm connectivity even though networking could be broken.
>>>
>>
> Vdsm can save the client ip setting up the network. Getting a ping from
> this
> client can confirm that the connectivity was restored. pings from other
> hosts
> can be ignored.
>
> The client address is available in a thread local variable
> (context.client_host)
> during all api calls. see vdsm.common.api.context_string() for example
> usage.
>
> This infrastructure is available in 4.1.
>

The proposed solution is focused on making sure a command does one thing
and not two:
A ping that has no side effects and a "watchdog" mechanism to confirm
connectivity.

Does it make sense to confirm connectivity from localhost? In many cases it
probably does not,
but there may be cases where it does make sense... it is not the
functionality to determine what
makes sense or not, it is the usage of it who has the responsibility to use
it correctly.


> Nir
>
>
>>
>>> In order to fix this problem ping should be split to ping2 (which just
>>> returns Success with no side-effect) and confirmConnectivity. Change
>>> on VDSM side was introduced in [1], we still need to expose new verbs
>>> in Engine.
>>>
>>> Regards,
>>> Petr
>>>
>>> [1] https://gerrit.ovirt.org/#/c/80119/
>>> ___
>>> Devel mailing list
>>> Devel@ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/devel
>>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-07 Thread Nir Soffer 
On Mon, Aug 7, 2017 at 5:28 PM Roy Golan  wrote:

> Still someone could call conirmConnectivity, no? so the state isn't
> guarded from localhost tinkering anyhow. If you really need a solution you
> can acuire a token for this operation by setupNetworks, and confirm
> connectivity with this token passed back.
>
> I'm not sure about the severity of the problem here, I'll let other reply,
> but I'm against this kind of solution.
>
>
>
> On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:
>
>> Hello,
>>
>> current VDSM ping verb has a problem - it confirms network
>> connectivity as a side-effect. After Engine calls setupNetwork it
>> pings VDSM host to confirm that external network connectivity is not
>> broken. This prohibits other users to call ping from localhost since
>> it would confirm connectivity even though networking could be broken.
>>
>
Vdsm can save the client ip setting up the network. Getting a ping from this
client can confirm that the connectivity was restored. pings from other
hosts
can be ignored.

The client address is available in a thread local variable
(context.client_host)
during all api calls. see vdsm.common.api.context_string() for example
usage.

This infrastructure is available in 4.1.

Nir


>
>> In order to fix this problem ping should be split to ping2 (which just
>> returns Success with no side-effect) and confirmConnectivity. Change
>> on VDSM side was introduced in [1], we still need to expose new verbs
>> in Engine.
>>
>> Regards,
>> Petr
>>
>> [1] https://gerrit.ovirt.org/#/c/80119/
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-07 Thread Roy Golan 
Still someone could call conirmConnectivity, no? so the state isn't guarded
from localhost tinkering anyhow. If you really need a solution you can
acuire a token for this operation by setupNetworks, and confirm
connectivity with this token passed back.

I'm not sure about the severity of the problem here, I'll let other reply,
but I'm against this kind of solution.



On Mon, 7 Aug 2017 at 15:32 Petr Horacek  wrote:

> Hello,
>
> current VDSM ping verb has a problem - it confirms network
> connectivity as a side-effect. After Engine calls setupNetwork it
> pings VDSM host to confirm that external network connectivity is not
> broken. This prohibits other users to call ping from localhost since
> it would confirm connectivity even though networking could be broken.
>
> In order to fix this problem ping should be split to ping2 (which just
> returns Success with no side-effect) and confirmConnectivity. Change
> on VDSM side was introduced in [1], we still need to expose new verbs
> in Engine.
>
> Regards,
> Petr
>
> [1] https://gerrit.ovirt.org/#/c/80119/
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

[ovirt-devel] [proposal] deprecate VDSM ping in favor of ping2 and confirmConnectivity

2017-08-07 Thread Petr Horacek 
Hello,

current VDSM ping verb has a problem - it confirms network
connectivity as a side-effect. After Engine calls setupNetwork it
pings VDSM host to confirm that external network connectivity is not
broken. This prohibits other users to call ping from localhost since
it would confirm connectivity even though networking could be broken.

In order to fix this problem ping should be split to ping2 (which just
returns Success with no side-effect) and confirmConnectivity. Change
on VDSM side was introduced in [1], we still need to expose new verbs
in Engine.

Regards,
Petr

[1] https://gerrit.ovirt.org/#/c/80119/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel