On Wed, Dec 05, 2012 at 02:21:16PM +0200, 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".
> 
> 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".

Hmm, a split second after sending this, I've realized that my suggestion
has a similar non-orthogonality problem: when you remove the network
attribute, you implictly change the linkState to "down".

So both options are bad :-(
Still I feel that linkState is a "lesser" attribute, relative to
network, and it would be better to have network imply linkState and not
vice versa.

Dan.

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

Reply via email to