Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Jul 2019 10:16:16 +0200
From:   "michal.lowas-rzechonek@silvair.com" 
        <michal.lowas-rzechonek@silvair.com>
To:     "Gix, Brian" <brian.gix@intel.com>
Cc:     "Stotland, Inga" <inga.stotland@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>
Subject: Re: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
Message-ID: <20190718081616.ujsbh2ot5hbhmulz@mlowasrzechonek2133>
Mail-Followup-To: "Gix, Brian" <brian.gix@intel.com>,
        "Stotland, Inga" <inga.stotland@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>
References: <20190717083650.26346-1-michal.lowas-rzechonek@silvair.com>
 <20190717083650.26346-2-michal.lowas-rzechonek@silvair.com>
 <915ea1c10883aaf1e4d42c5a749bfda964b54b51.camel@intel.com>
 <20190717194712.i4dtiwhldq2is2z2@kynes>
 <DEBB0CAA2616974FAE35E4B560B9A4376CBD3539@ORSMSX103.amr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <DEBB0CAA2616974FAE35E4B560B9A4376CBD3539@ORSMSX103.amr.corp.intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Brian, Inga,

On 07/17, Gix, Brian wrote:
> The application has no need to even exist at this point, as long as it 
> can attach to the token at some point in the future.  But this *does* 
> enable the ability to have a *generic* application that can inject 
> nodes (fully configured, or "New") into the daemon.

The same thing can be said about CreateNetwork() call - you don't really 
*need* to have an attached (of even "attachable") application in order 
to create a new node.

In fact, CreateNetwork() and ImportLocalNode() calls are very similar in 
this regard, it's just that CreateNetwork generates keys, addresses and 
starting sequence number on the daemon side, while ImportLocalNode() 
allows passing them from the application.

Since CreateNetwork() does query the app via D-Bus, I think it's more 
consistent to query the app during ImportLocalNode() as well.

That, or replace both calls with a more generic CreateNode() call that 
would accept create/import data as an agument, and not query the app at 
all.

> > If we say that it must also do the same via JSON, to call 
> > ImportLocalNode, it leads to code duplication on the application 
> > side.
> >
> > Moreover, the app still needs to be queried via D-Bus to check that 
> > the passed JSON matches the D-Bus structure - otherwise the app 
> > would then fail to Attach() and the user would be in deep trouble.
>
> The composition as reflected by the GetManagedObjects() call is 
> "sanity checked" against the internal storage *every* time the App 
> attaches...  I think Inga is concerned with code complexity and bloat 
> to repeat this during ImportLocalNode(),

This is covered by the same code path as other calls. I think it's 
cleaner to always perform GetManagedObjects() dance for all 4 calls.

And, as I mentioned before, this code path becomes even simpler when we 
refactor "sanity checking" to compare serialized composition data.

> This is different from the Join() case, in my opinion, where the JSON 
> (or other storage) is being created *totally* from scratch, via the 
> provisioning interaction with the remote Provisioner. In that case, 
> yes... the "owning application" needs to be present on the D-Bus.

Not necessarily. The application needs to provide ProvisionAgent 
interface only, and only in cases where provisioner selects 
authentication method that requires the app to perform some OOB action.

It is entirely possible for a joining node to say it doesn't support any 
OOB actions, and the mesh daemon can accept provisioning wihout any 
interaction with the app.

> > I'm not convinced that the "full" configuration is even needed. We 
> > certaintly don't use it in our use case, but it might be required in 
> > the future.
>
> We *definitely* need an option for importing/migrating a fully 
> configured node.  Phones are retired and replaced... Workstations are 
> retired and replaced...  Some nodes publications will inevitably need 
> to pick up the "current conversation" with the migrated node, where 
> the old conversation left off. And this will almost certainly be a 
> rare (but important) operation, and a "Utility" application to perform 
> the operation (that does not itself need to have the model/element 
> arrays implemented) will be easier to write and maintain.
(...)
> Again, I cannot think of any situations where Join/Attach/Create would 
> ever exist in the absence of the Application.
>
> This is an easy and obvious use case with Import.

I don't think it makes sense to have a utility application that would 
migrate the node without having anything that would Attach() to it.

IMO the migration operation should be a part of application logic, 
because this operation requires talking to external Provisioner, and 
Provisioner API is vendor specific. So I don't think you can't really 
create a 'generic migration tool'.

In case of replaced device, the use case I see is re-installing the same 
application on a new device, and having it re-create the node either 
from data received from the Provisioner, or exported from the previous 
instance of the application. I don't see a need to instantiate the node 
on the replaces device without reinstalling the application - such a 
node would not be operational anyway.

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND