Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-10
13:23:18 jangutter for the host_info objects, there's a mechanism to filter vif types by name and version, but not a corresponding mechanism for port_profiles.
13:23:23 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Remove the caching the resource on Request object https://review.openstack.org/609403
13:23:51 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Remove more code related to extensions and testing https://review.openstack.org/607088
13:23:59 sean-k-mooney stephenfin sorry taught that message was from jangutter am in what context
13:24:02 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Remove the extensions framework from wsgi.py https://review.openstack.org/607092
13:24:03 stephenfin sean-k-mooney: If we do and that hw_vif_model==virtio, we'll undo the hard work of https://github.com/openstack/nova/commit/622ebf2fab0a9bf75ee12437bef28f60e083f849 right?
13:24:21 openstackgerrit Yikun Jiang proposed openstack/nova-specs master: Support initial allocation ratios https://review.openstack.org/552105
13:24:31 sean-k-mooney jangutter: yes currently nothing uses the filter. it was there for future use but its still in the future
13:24:42 sean-k-mooney jangutter: so we can extend it
13:24:57 stephenfin sean-k-mooney: Heh, yeah, two people talking about related stuff is confusing :) I'll test this myself in a bit. Just sanity checking
13:25:29 gmann alex_xu: mriedem melwitt this complete the API extensions merge work - https://review.openstack.org/#/q/topic:bp/api-extensions-merge-stein+status:open
13:25:40 mriedem pooja_jadhav: gmann: i'm not sure why we need a new functional test for bfv?
13:25:45 sean-k-mooney well if you set hw_vif_model=Anything we shoudl respect that
13:25:50 mriedem we have existing functional tests that cover that flow
13:25:54 mriedem including the patch i just linked to you
13:26:00 gmann i will check the gate if any tests i need to fix otherwise it is good to go
13:26:09 sean-k-mooney stephenfin: but i need to check both patches to see how they interact
13:26:27 mriedem gmann: ack on the api extension merge series - if you haven't, you should queue that up in a runway
13:26:29 jangutter sean-k-mooney, jaypipes: (host_info context) right, so there's two ways of doing a check like this. the 'naive bayes' way, or the really explode the dependencies way.
13:26:41 gmann mriedem: right, those should work untill pooja_jadhav trying anything special or new feature (that is not up for review so not sure)
13:26:55 pooja_jadhav mriedem: I am writting in api-sample-test for (if instance is BFV, then local_gb_used should be 0 in in the simple tenant usage api)
13:27:20 sean-k-mooney stephenfin: if your asking should we remove https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L130-L133 the answer is no but we may need to adapt the queue size patch
13:27:34 openstack Launchpad bug 1715570 in OpenStack Compute (nova) "simple tenant usage api calculating disk usages incorrectly" [Medium,In progress] - Assigned to Bhagyashri Shewale (bhagyashri-shewale)
13:27:34 pooja_jadhav mriedem, gmann : https://bugs.launchpad.net/nova/+bug/1715570
13:27:40 jangutter sean-k-mooney, jaypipes: (host_info context) the difference comes in, are you going to ever worry about a case where VIFobject 1.0 supports PortProfile 1.0 and not 2.0?
13:27:42 jaypipes jangutter: the host_info should be able to communicate to Neutron's agents exactly the kinds of VIFs that it can plug. If that means we need to pass some additional information about port profiles supported by the host, so be it.
13:28:10 sean-k-mooney jaypipes: we should but we dont currently
13:28:15 gmann mriedem: added
13:28:19 jaypipes jangutter: if that makes a difference to what Neutron negotiates with the host, then yes...
13:29:17 jangutter jaypipes: (host_info context) it's similar to the question: does the base object need to bump its version if any of the members bump up a version?
13:29:46 mriedem pooja_jadhav: we don't need to use an api sample test for that bug
13:29:47 sean-k-mooney jangutter: unfortunetly no. i think it should be we do not bump for compostion
13:29:55 jaypipes jangutter: no
13:30:20 mriedem pooja_jadhav: a simple functional test which creates a volume-backed server and then queries the simple tenant usage API to assert it's showing disk usage when it shouldn't should suffice
13:30:25 jaypipes jangutter: but see my comment on the datapath offload port profile patch that you shouldn't be modifying the VIFPortProfileBase object like that.
13:30:26 pooja_jadhav mriedem: but existing tests for simple tenant usage api are in api sample tests.
13:30:38 mriedem pooja_jadhav: that doesn't really matter
13:30:39 sean-k-mooney jangutter: if you add a filed to base then you bump the version of all the derived types too
13:30:47 pooja_jadhav mriedem: okies
13:30:52 jangutter jaypipes: yep, that's revving because of inheritance, another story.
13:30:53 sean-k-mooney jangutter: so bump for inheritance changes but not for composition
13:30:54 jaypipes sean-k-mooney: that's not true any more AFAIK.
13:31:01 mriedem pooja_jadhav: something like the setup in https://review.openstack.org/#/c/608771/ should be most of the work,
13:31:08 mriedem then it's just querying the simple tenant usage API and asserting the results
13:31:17 sean-k-mooney jaypipes: it better be or we are screwed
13:31:38 pooja_jadhav mriedem: yeah.. thanks
13:31:48 mriedem pooja_jadhav: we could just put the test patch on top of ^ to re-use the same setup
13:31:56 mriedem i can try that quick
13:32:06 jaypipes sean-k-mooney: the version manifest tracks versions for sub-classes separately from the base classes, meaning you don't need to bump the sub-class versions when a base version increases.
13:32:42 sean-k-mooney jaypipes: correct but if we dont then adding a filed to the base mean the derived has a new filed also and no version bump
13:33:12 sean-k-mooney similary if we remove a filed form the base that filed goes away in the derived without a version bump and we are similarly screwed
13:33:13 jaypipes sean-k-mooney: the derived version is only indicating the version of the derived-specific fields.
13:33:54 jaypipes dansmith: you up yet? :)
13:34:00 sean-k-mooney jaypipes: no its indicating the version fo the whole object if not then its not safe to inherit OVOs
13:34:12 dansmith jaypipes: yes
13:34:42 jaypipes dansmith: we are discussing whether it is required to bump derived class object versions if a base versions is incremented.
13:34:44 jangutter jaypipes, sean-k-mooney: If you don't bump because of composition, then you can flatten out the list of objects, regardless of how which members they belong to.
13:35:08 dansmith jaypipes: if you change something in the base, you've changed it in the sub and the hash should change (thus need a version bump) IIRC
13:35:29 sean-k-mooney dansmith: that is my understanding too.
13:35:30 jaypipes dansmith: oh, am I confusing the composition rules with inheritance rules?
13:35:30 dansmith jaypipes: _simply_ changing the master version shouldn't require a child bump, but there would be no reason
13:35:35 stephenfin sean-k-mooney: Nope. Rather, we just should make that if conditional on whether we're requesting SR-IOV VIFs or not
13:35:49 jangutter jaypipes, sean-k-mooney: I mean then you don't need to specifically associate _which_ VIF and which port-profile object/version combination go together, you just need to check if you understand each object/version combination separately.
13:35:55 dansmith jaypipes: I think you're confusing how including an object in another object used to require lockstep versioning, but that isn't the case any more
13:36:13 sean-k-mooney stephenfin: nope for macvtap i think you can set the model
13:36:22 jaypipes dansmith: right. I was confusing composition rules with inheritance rules. sorry (again) :(
13:36:25 stephenfin *SR-IOV direct
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

Earlier   Later