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 [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
