----- Original Message -----
> From: "Dan Kenigsberg" <dan...@redhat.com>
> To: "Livnat Peer" <lp...@redhat.com>
> Cc: "Adam Litke" <a...@us.ibm.com>, "Alona Kaplan" <alkap...@redhat.com>, 
> vdsm-devel@lists.fedorahosted.org, "Antoni
> Segura Puimedon" <asegu...@redhat.com>
> Sent: Wednesday, December 5, 2012 2:06:18 PM
> Subject: Re: [vdsm] link state semantics
> 
> On Wed, Dec 05, 2012 at 02:40:44PM +0200, Livnat Peer wrote:
> > On 05/12/12 14:21, Dan Kenigsberg wrote:
> > > On Tue, Dec 04, 2012 at 03:10:00PM -0600, Adam Litke wrote:
> > >> On Tue, Dec 04, 2012 at 12:32:34PM -0500, Antoni Segura Puimedon
> > >> wrote:
> > >>> Hi list!
> > >>>
> > >>> We are working on the new 3.2 feature for adding support for
> > >>> updating VM
> > >>> devices, more specifically at the moment network devices.
> > >>>
> > >>> There is one point of the design which is not yet consensual
> > >>> and we'd
> > >>> need to agree on a proper and clean design that would satisfy
> > >>> us all:
> > >>>
> > >>> My current proposal, as reflected by patch:
> > >>>    http://gerrit.ovirt.org/#/c/9560/5/vdsm_api/vdsmapi-schema.json
> > >>> and its parent is to have a linkActive boolean that is true for
> > >>> link
> > >>> status 'up' and false for link status 'down'.
> > >>>
> > >>> We want to support a none (dummy) network that is used to
> > >>> dissociate vnics
> > >>> from any real network. The semantics, as you can see in the
> > >>> patch are that
> > >>> unless you specify a network, updateDevice will place the
> > >>> interface on that
> > >>> network. However, Adam Litke argues that not specifying a
> > >>> network should
> > >>> keep the vnic on the network it currently is, as network is an
> > >>> optional
> > >>> parameter and 'linkActive' is also optional and has this
> > >>> "preserve current
> > >>> state" semantics.
> > >>>
> > >>> I can certainly see the merit of what Adam proposes, and the
> > >>> implementation
> > >>> would be that linkActive becomes an enum like so:
> > >>>
> > >>> {'enum': 'linkState'/* or linkActive */ , 'data': ['up',
> > >>> 'down', 'disconnected']}
> > >>>
> > >>> With this change, network would only be changed if one
> > >>> different than the current
> > >>> one is specified and the vnic would be taken to the dummy
> > >>> bridge when the linkState
> > >>> would be set to 'disconnected'.
> > >>>
> > >>> There is also an objection, raised by Adam about the semantics
> > >>> of portMirroring.
> > >>> The current behavior from my patch is:
> > >>>
> > >>> portMirroring is None or is not set -> No action taken.
> > >>> portMirroring = [] -> No action taken.
> > >>> portMirroring = [a,b,z] -> Set port mirroring for nets a,b and
> > >>> z to the specified vnic.
> > >>>
> > >>> His proposal is:
> > >>> portMirroring is None or is not set -> No action taken.
> > >>> portMirroring = [] -> Unset port mirroring to the vnic that is
> > >>> currently set.
> > >>> portMirroring = [a,b,z] -> Set port mirroring for nets a,b and
> > >>> z to the specified vnic.
> > >>>
> > >>> I would really welcome comments on this to have finally an
> > >>> agreement to the api for this
> > >>> feature.
> > > 
> > > ack for portMirroring semantics. [] must mean "no networks are
> > > mirrored
> > > to this nic".
> > > 
> > 
> > +1, It makes sense.
> > 
> > > I am less sure about allowing a "No action taken" semantics, as
> > > it
> > > add the current state to the API and complicates it. I'd also
> > > like to
> > > keep the updateDevice API as similar as possible to vmCreate one.
> > > 
> > > However, we already have defaults in vmCreate (for pci address
> > > allocation, portmirroring off) so I suppose I can join Adam here.
> > > 
> > > But this does not mean we should allow a default everywhere. I
> > > find it
> > > unintuitive to have a linkState=disconnected, as setting this
> > > attribute
> > > should clear the network attribute - they become non-orthogonal.
> > > So in
> > > my opinion, a missing network should mean "nic is connected to no
> > > network" rather than "keep current connection".
> > 
> > I find the 'disconnect' state a bit problematic, a 'test' I do to
> > see if
> > things make sense is a comparison to the 'physical' world.
> > In the physical world a link can have up or down, any other state
> > that
> > we add is less intuitive and IMO should be avoided.
> > The 'problematic' status we are trying to express in the API is a
> > network card that has no wire connected to it, how about using an
> > empty
> > network name to express that no network is connected to this card?
> > 
> > Then we can keep the semantics of not sending a network-name means
> > no
> > change in the network.
> 
> Usually I dislike attaching special semantics to the empty string,
> but
> it sounds like a reasonable way out this time.
> 
> +1 to network='' meaning "connect this nic to nothing".

I agree as well. AFAICT, I think you bought us a way out.
> 
> > 
> > And I also agree with danken that we should keep the API
> > consistent,
> > createVM hotplug and updateDevice should behave similarly.

Indeed! I'm going over them to try to make it so.

> > 
> > Livnat
> > 
> > > 
> > > Dan.
> > > _______________________________________________
> > > vdsm-devel mailing list
> > > vdsm-devel@lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
> > > 
> > 
> 
_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to