Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-10
13:36:52 sean-k-mooney stephenfin: im also not sure if that assumtion is safe to make but it might be
13:37:10 pooja_jadhav mriedem: Let me try if u dont mind?
13:37:13 stephenfin moshele: Any thoughts on the above?
13:37:32 stephenfin moshele: tl;dr: I wonder if we should be setting 'model' here for SR-IOV VIFs? https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L130-L133
13:37:42 sean-k-mooney dansmith: right but if the composed object version change how we make object compatible today is actully inccorect
13:37:46 stephenfin moshele: If we do and that hw_vif_model==virtio, we'll undo the hard work of https://github.com/openstack/nova/commit/622ebf2fab0a9bf75ee12437bef28f60e083f849 I think
13:38:02 stephenfin moshele: SR-IOV direct VIFs, that is
13:38:27 jangutter dansmith: if you use composition, would it make sense to check a list of classes and versions for compatibility, or would you need to check the combinations?
13:38:53 sean-k-mooney stephenfin: you can have VF that are virtio just an FYI
13:38:58 dansmith sean-k-mooney: I don't understand what you're saying
13:39:31 dansmith jangutter: heh, I'm also not sure what you mean
13:39:42 dansmith point me at code?
13:39:46 jangutter dansmith: let me set up a quick etherpad?
13:39:49 moshele stephenfin: sorry I don't follow
13:40:00 dansmith yah
13:40:05 sean-k-mooney dansmith: if you look at https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/objects/migrate_data.py#L243-L270 we call support with the target version fo the derived but that may not corralate to a target version of the base
13:40:12 jaypipes dansmith: they are wondering how the versioning system works when you have an ObjectField field...
13:40:30 jaypipes dansmith: and if the version of the ObjectField changes, why the base doesn't need to change.
13:40:53 stephenfin moshele: In https://github.com/openstack/nova/commit/622ebf2fab0a9bf75ee12437bef28f60e083f849 a check was added to ensure we don't see RX/TX queue sizes for non-virtio interfaces
13:41:08 jangutter jaypipes, sean-k-mooney, dansmith: https://etherpad.openstack.org/p/ovo-versioning
13:41:14 dansmith jaypipes: oh, because we send a manifest.. a list of object names and versions.. for rpc
13:41:23 sean-k-mooney jaypipes: yes. and in the past we used to have a verion map thing that track the version fo the object fileds and what version the correspondeed to
13:41:29 dansmith well, when we backport, we do that
13:41:59 stephenfin I'm thinking it's possible we could break the check if a user sets 'hw_vif_type=virtio' in image metadata. If they do that, then this line will be true
13:42:36 stephenfin https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L156
13:43:01 sean-k-mooney dansmith: that is part of my argument as to why we should version for compoltion and inheritenc but since the has does not change for compostion versiosn its not enforece by our tests
13:43:05 dansmith sean-k-mooney: yeah that was a total disaster
13:43:14 stephenfin which wouldn't be the case normally because this line would be false for direct https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L137
13:43:26 dansmith sean-k-mooney: it doesn't need to change
13:43:29 sean-k-mooney dansmith: the lockstep stuff. it was a pain but it worked
13:43:39 dansmith sean-k-mooney: it was unnecessary for us
13:43:49 stephenfin moshele: If that makes sense? I'm just thinking it's a latent bug. I'll test myself but doing so requires me setting up an environment :)
13:43:56 dansmith maybe for you because you don't have the same communication we have between nodes
13:44:48 sean-k-mooney dansmith: it maye if version 1 of my containing clase used version 1 of a composed calss and version 2 of the conatine uses version 5 then downgrading the containing class to 1 results in it having an object filed of 5
13:44:52 moshele stephenfin: not sure, but you maybe right here
13:45:26 dansmith sean-k-mooney: yep, which is fine, because the objects are forward compatible
13:45:45 mriedem gibi: i'm late to this party but a few comments inline https://review.openstack.org/#/c/609330/
13:45:47 dansmith sean-k-mooney: and if there's some reason, the containing class can downlevel the version it has
13:45:49 mriedem i haven't read the ML thread yet
13:46:00 mriedem pooja_jadhav: go ahead
13:46:53 sean-k-mooney dansmith: today we dont track that it should down level the object filed to 2 when it downlevels to 1 which is the gap
13:46:53 moshele stephenfin: please update me with your result
13:47:08 stephenfin moshele: Yup, will open a bug if it's an issue
13:47:42 dansmith sean-k-mooney: we don't backlevel it if and only if the requesting party says that they support version 5 of the sub object
13:47:43 sean-k-mooney dansmith: its totally solveable in the make_compatible function just we dont enforce it today and we should when we start passing os-vif objects over the api
13:47:50 dansmith sean-k-mooney: which wouldn't happen, and thus we'd backport both
13:47:53 gibi mriedem: you are not late at all
13:48:07 dansmith sean-k-mooney: sure we don't enforce it
13:48:10 moshele stephenfin: is 'hw_vif_type=virtio' in image metadata per network interface?
13:48:20 dansmith sean-k-mooney: that's why I'm saying if you are making objects where that matters, then you can backlevel it
13:48:25 stephenfin moshele: I don't think so. I don't see how it could be
13:48:40 sean-k-mooney dansmith: this isnt in relation to RPC by they way. this is relation to passing os-vif object betwen nova an neutron via the rest api
13:48:51 stephenfin moshele: Assuming you mean can it be configured per interface. I assume it affects all network interfaces
13:49:01 moshele stephenfin: I see, so we need to ignore it or reject it
13:49:06 dansmith sean-k-mooney: I know that, which is why I said you may have less flexibility because you don't control the communication in the same way
13:49:12 stephenfin For direct SR-IOV, I would imagine
13:49:32 sean-k-mooney dansmith: sure. we have added code in the past to do this and taken it out as part of code review because it did not go over rpc
13:49:34 dansmith sean-k-mooney: fwiw, the relation mapping stuff is all still in o.vo because it's a library, so you can use it if you really think it's necessary
13:49:46 moshele stephenfin: for VNIC_TYPES_DIRECT_PASSTHROUGH
13:49:52 stephenfin moshele: yeah
13:50:14 sean-k-mooney dansmith: ya we can but its also easy to do this in the make compatible fuction without that
13:50:40 sean-k-mooney jangutter: does any of this help?
13:51:10 openstackgerrit sean mooney proposed openstack/os-vif master: add support for generic tap device plug https://review.openstack.org/602384
13:51:11 openstackgerrit sean mooney proposed openstack/os-vif master: clean up ip_command interface https://review.openstack.org/609414
13:51:36 jangutter sean-k-mooney: I'm still downleveling the conversation to the simple version I can understand :-p
13:52:03 gibi mriedem: good point about the pre 2.29 and pre 2.30 microversion. I have to think that through.
13:52:41 mriedem gibi: i don't see operators on this thread either in the ML...has anyone reached out to the ops community to see if they are cool with dropping support for the force parameter? and if not, why not.
13:53:06 gibi mriedem: bauzas forwarded it to the ops I think
13:53:07 mriedem although as noted you can still use the force flag with the older microversion, or use 2.1
13:53:11 dansmith jangutter: so you might be assuming there's more magic going on here than there is
13:53:19 mriedem this spec is really just about signaling
13:53:25 mriedem like when we deprecated personality files
13:53:25 dansmith jangutter: you can't define that a field is an object at (or below or above) a particular version
13:53:26 bauzas gibi: mriedem: yup but no answers yet
13:53:42 bauzas gibi: and I asked you about two separate microversions in the change
13:53:45 jangutter dansmith: right, this comes into play when serializing
13:53:55 dansmith jangutter: within a scope, it's assumed that you support a set of object schemas (by name) at a particular version, with compatibility for older versions
13:53:57 mriedem bauzas: no one but you wants 2 microversions for this
13:54:07 gibi bauzas: it seems to me that others more like having a single microversion instead
13:54:08 mriedem 2 microversions for the same thing is excessive
13:54:36 mriedem as efried noted in the spec review, we don't do multiple microversions for something just because it touches multiple APIs
13:54:38 dansmith jaypipes: so when we serialize today, we have the object and a list of versions that the other side supports, and we backlevel everything in the tree to the versions they support, as needed
13:54:41 dansmith er jangutter ^
13:54:47 mriedem otherwise i would have had multiple microversions for volume multiattach and removing personality files
13:55:07 dansmith jangutter: if you don't have that manifest of supported versions, then you'd have to make inferences based on the version of the parent you're pinned to, which is what we used to do
13:55:40 dansmith jangutter: like this: https://github.com/openstack/nova/blob/kilo-eol/nova/objects/instance.py#L257
13:55:51 dansmith jangutter: if you set those up on your object then o.vo will still respect them
13:55:58 jangutter dansmith: yep. the trick comes when downlevelling an object that has members containing other objects which are versioned.
13:56:15 dansmith jangutter: right, but that's what this obj_relationships is for
13:56:17 sean-k-mooney dansmith: that is assume that the latest version of each object both side knows about is the desired version correct
13:56:38 dansmith jangutter: note that it specifies which versions the contains-another-object fields refer to
13:56:44 bauzas mriedem: gibi: okay, just the fact that we had 2 different versions previously (2.29 and 2.30)
13:56:47 jangutter dansmith: I've seen that mapping, the question is, do we need/want it in os-vif?
13:57:11 dansmith sean-k-mooney: it means within a scope you don't randomly upgrade one object and not another yeah
13:57:34 dansmith sean-k-mooney: which is what this was designed to do, not for "I loaded a new plugin which loaded one new object into a sea of older ones"
13:58:15 sean-k-mooney dansmith: right... so we might have to deal with the lather in os-vif but perhaps we dont
13:58:39 dansmith yeah I dunno
13:58:44 mriedem bauzas: we had 2 different versions previously because of all the related plumbing those required
13:59:04 mriedem this is a simple schema change
13:59:09 sean-k-mooney dansmith: did any of this impact out of tree virt drivers on the nova side? or are all the OVOs managed above the virt dirver level

Earlier   Later