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

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 <msivak(a)redhat.com&gt; wrote: >> The proposed solution is focused on making sure a command does one thing and >> not two: > >>>> 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 <ehaas(a)redhat.com&gt; wrote: >> >> >> On Mon, Aug 7, 2017 at 11:06 PM, Nir Soffer <nsoffer(a)redhat.com&gt; wrote: >>> >>> On Mon, Aug 7, 2017 at 5:28 PM Roy Golan <rgolan(a)redhat.com&gt; 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 <phoracek(a)redhat.com&gt; 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(a)ovirt.org >>>>> http://lists.ovirt.org/mailman/listinfo/devel >>>> >>>> _______________________________________________ >>>> Devel mailing list >>>> Devel(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/devel >>> >>> >>> _______________________________________________ >>> Devel mailing list >>> Devel(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/devel >> >> >> >> _______________________________________________ >> Devel mailing list >> Devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/devel