----- Original Message -----
> From: "Igor Lvovsky" <ilvov...@redhat.com>
> To: "Antoni Segura Puimedon" <asegu...@redhat.com>
> Cc: "Alona Kaplan" <alkap...@redhat.com>, vdsm-devel@lists.fedorahosted.org
> Sent: Wednesday, December 5, 2012 10:17:50 AM
> Subject: Re: [vdsm] link state semantics
> 
> 
> 
> ----- Original Message -----
> > From: "Antoni Segura Puimedon" <asegu...@redhat.com>
> > To: vdsm-devel@lists.fedorahosted.org
> > Cc: "Alona Kaplan" <alkap...@redhat.com>
> > Sent: Tuesday, December 4, 2012 7:32:34 PM
> > Subject: [vdsm] link state semantics
> > 
> > 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']}
> > 
> 
> If you are going for this use 'linkState'
> 
> > 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'.
> 
> In general +1 for new one, with a little doubt.
> It looks a bit inconsistent that we leave the network as is if it
> omitted from input,
> but if linkState is 'disconnected' we will move it to dummy bridge.
> But I can live with it.

Yes, the 'disconnected' overrules the network and that, as you point
out, can be a source of confusion. I propose to add a warning to the
return dictionary that tells the user that setting disconnected overrules
any network setting.

> 
> > 
> > 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.
> > 
> 
> +1 for Adam's approach, just don't forget to unset portMirroring from
> all nets setted before
> if they not in new portMirroring = [a,b,z]

So you're saying:

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 AND unset any other mirroring.

I'm fine with it, I think it is even more complete and correct.
> 
> > I would really welcome comments on this to have finally an
> > agreement
> > to the api for this
> > feature.
> > 
> > Best,
> > 
> > Toni
> > _______________________________________________
> > 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