On Wed, Dec 05, 2012 at 04:25:48AM -0500, Antoni Segura Puimedon wrote: > > > ----- 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.
Yes, +1. > > > > > 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 -- Adam Litke <a...@us.ibm.com> IBM Linux Technology Center _______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
