| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-10 | |||
| 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 | dansmith | jaypipes: _simply_ changing the master version shouldn't require a child bump, but there would be no reason | |
| 13:35:30 | jaypipes | dansmith: oh, am I confusing the composition rules with inheritance rules? | |
| 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 | |
| 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 | |