Re: [PATCH v5 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.

[Het Gala]

On 30/05/23 1:26 pm, Markus Armbruster wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
> > On 30/05/23 12:28 pm, Markus Armbruster wrote:
> > > Het Gala<het.gala@nutanix.com>  writes:
> > >
> > > > On 25/05/23 11:04 pm, Markus Armbruster wrote:
> [...]
>
> > > > > Aside: a more powerful type system would let us extend SocketAddress
> > > > > with additional variants instead of wrapping it in a union.
> > > > Markus, what do you mean by additional variants here in context of socket? Can 
> > > > you give a small example.
> > > As is, we have a nest of two unions:
> > >
> > > * The outer union has branches @socket, @exec, @rdma.
> > >
> > > * Branch @socket is the inner union, it has branches @inet, @unix, ...
> > >
> > > A more powerful type system would let us extend SocketAddress instead,
> > > so MigrateAddress has everything SocketAddress has, plus additional
> > > branches @exec, @rdma.  Naturally, the type of the discriminator also
> > > needs to be extended, so that it has everything SocketAddress's
> > > discriminator type has, plus additional members @exec, @rdma.
> > Okay, so you mean something like :
> >
> > +# Since 8.1
> > +##
> > +{ 'union': 'MigrateAddress',
> > +  'base': { 'transport' : 'MigrateTransport'},
> > +  'discriminator': 'transport',
> > +  'data': {
> > +    'inet': 'InetSocketAddress',
> > +    'unix': 'UnixSocketAddress',
> > +    'vsock': 'VsockSocketAddress',
> > +    'fd': 'str',
> > +    'exec': 'MigrateExecCommand',
> > +    'rdma': 'InetSocketAddress' } }
> >
> > Even I agree that directly leveraging this is the best option, but then 
> > wouldn't it introduce redundancy ? we would not be able to leverage socket 
> > union right in that case or am I missing something.
> Yes, there's redundancy, due to QAPI's insufficient expressive power.
>
> Is the cleaner external interface worth the (internal) redundancy, and
> possibly coding complications that go with it?

Honestly, I would like to have it this way, where the user is aware of 
all the transport mechanisms available. But I guess for external 
interface problem statement, the migration code flow has similar path 
for SocketAddress and non-socketAddress (exec and rdma). So even if on 
the QAPI side we express explicitly all the transport mechanisms, while 
implementing it we either would need to brinf them under a single 
umbrella. For now, I would keep the implementation as it is (union 
inside a union) but would want to have a more powerful and better 
appraoch out there if possible.

I would like to have Migration maintainers - Juan, Peter Xu and others 
to comment on what approach from the above two is a better one.

Regards,
Het Gala