| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-02 | |||
| 15:22:51 | gibi | stephenfin: because you are fast :D | |
| 15:23:09 | sean-k-mooney | ya there is a tool you can run | |
| 15:23:17 | sean-k-mooney | that generate the patch for you | |
| 15:23:24 | gibi | stephenfin: thanks anyhow | |
| 15:23:27 | stephenfin | I know hberaud had one alright. Didn't know there was a generic one | |
| 15:23:39 | sean-k-mooney | its in the repo | |
| 15:23:43 | sean-k-mooney | you just use tox | |
| 15:23:47 | gibi | sean-k-mooney: hit me up with any of those patches if need a push on them before the freeze | |
| 15:24:09 | sean-k-mooney | i need to see if the previous comments have been adressed | |
| 15:24:27 | sean-k-mooney | it is a bug fix so we could backported anyway | |
| 15:25:55 | stephenfin | gibi: Could you carry your +2 from https://review.opendev.org/#/c/741995/10 to https://review.opendev.org/#/c/744958/8 ? They should be identical, commit message aside | |
| 15:25:58 | stephenfin | I had them separate to avoid intertwining two series, but that's not an issue now as the base patches for both have merged | |
| 15:26:16 | stephenfin | I'll abandon https://review.opendev.org/#/c/741995/10 now | |
| 15:27:20 | gibi | done | |
| 15:27:34 | stephenfin | thanks | |
| 15:27:35 | gibi | thanks for that ping, I forget about these duplications | |
| 15:29:03 | stephenfin | and while I have you (last one, promise) - this has two changes since your last +2: a change to some mocks to mock out the new "does this user/group exist" check, and a commit message nit from lyarwood https://review.opendev.org/#/c/639934/ | |
| 15:29:14 | stephenfin | if you could revisit and close out the vTPM series \o/ | |
| 15:29:17 | gibi | sure | |
| 15:29:19 | gibi | on it | |
| 15:29:28 | gibi | closing features before the FF week is nice :D | |
| 15:30:21 | gibi | and I'm planning for asking favors on the SRIOV series soon (as it is getting close to be ready)... | |
| 15:30:47 | stephenfin | fire 'em at me | |
| 15:31:17 | gibi | stephenfin: I will, working on functional tests now | |
| 15:33:12 | gibi | +2 | |
| 15:34:11 | sean-k-mooney | gibi: i havent reviewd the WIP patch fully although i tested it. are you ready to remove the WIP status from it | |
| 15:34:37 | sean-k-mooney | i do think its close too | |
| 15:35:07 | gibi | sean-k-mooney: from code perspective it is ready (and my next push will remove the WIP tag) from test perspective I will push the unit test coverage today, but the functional test needs another day | |
| 15:35:34 | sean-k-mooney | ok thanks ill start reviewing the code so. | |
| 15:35:56 | gibi | cool, thanks | |
| 15:47:43 | openstackgerrit | Merged openstack/nova-specs master: Correct some spell errors https://review.opendev.org/749449 | |
| 15:56:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Remove '[vnc] keymap', '[spice] keymap' options https://review.opendev.org/749056 | |
| 16:02:36 | openstackgerrit | Merged openstack/nova master: trivial: Use constants for libvirt version checks https://review.opendev.org/740921 | |
| 16:21:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support SRIOV interface attach and detach https://review.opendev.org/740995 | |
| 16:22:11 | gibi | sean-k-mooney, stephenfin: unit test coverage is ready, and there is the first passing functional test as well ^^ | |
| 16:22:22 | gibi | I will continue with the functional tests tomorrow | |
| 16:22:49 | stephenfin | great | |
| 16:23:00 | stephenfin | can review in the AM too | |
| 16:40:04 | sean-k-mooney | stephenfin: speaking fo funcitonal tests what is the state of your sriov functional tests | |
| 16:40:08 | sean-k-mooney | are those merged | |
| 16:40:29 | sean-k-mooney | gibi: did you base your functional tests on stephens work? | |
| 16:40:52 | sean-k-mooney | gibi: i guess you are not doing live migration tests | |
| 16:40:55 | sean-k-mooney | so dont need to | |
| 16:41:31 | sean-k-mooney | it is conflicting with stephens patches however | |
| 18:15:54 | ganso | Hi nova folks! I know everyone is busy at the moment, but I'd like to ask for some input in https://bugs.launchpad.net/nova/+bug/1893263 , it is a blocker bug that I have already pushed a fix, would like someone's blessing on it. | |
| 18:15:54 | openstack | Launchpad bug 1893263 in OpenStack Compute (nova) "Cannot create instance with multiqueue image and vif_type=tap (calico)" [Undecided,In progress] - Assigned to Rodrigo Barbieri (rodrigo-barbieri2010) | |
| 18:40:25 | sean-k-mooney | ganso: you should be checking the vif_model | |
| 18:41:11 | sean-k-mooney | you are also checking the image property via the system metadata table rather then form the image | |
| 18:41:42 | sean-k-mooney | but yes that is where you would fix this but just not how you should fix this | |
| 18:42:21 | ganso | sean-k-mooney: you mean check vif_model to confirm the vif_type before setting multiqueue? | |
| 18:42:40 | sean-k-mooney | no check the vif_model to confirm its virtio | |
| 18:43:19 | sean-k-mooney | if you set hw_vif_model=e1000 then you cant use multiqueue | |
| 18:43:27 | sean-k-mooney | since its a virtio feature | |
| 18:43:35 | ganso | sean-k-mooney: oh I see! | |
| 18:44:21 | ganso | regarding the system metadata. I don't have access to the image in the vif.plug() method (or I am missing something) | |
| 18:45:22 | sean-k-mooney | you have acess via the instance | |
| 18:45:32 | sean-k-mooney | instance.image_meta | |
| 18:45:40 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L250 | |
| 18:46:27 | ganso | oh I see! when I printed those fields I didn't get that, probably due to object property lazy loading | |
| 18:46:40 | ganso | sean-k-mooney: thanks, I will address those and push a new patch! | |
| 19:07:28 | sean-k-mooney | and yes the image_meta is a property on the instance its not stored in the instnace table directly so the proerty makes sure we correctly build an instance of the class meaning we get validation of the data | |
| 19:07:52 | sean-k-mooney | which is preferebale over parsing the raw data in the system metadta table | |
| 20:20:53 | sean-k-mooney | adrianc: https://review.opendev.org/#/c/745895/2/vif_plug_ovs/linux_net.py could you follow up with that offline | |
| 20:21:12 | sean-k-mooney | adrianc: the client lib freeze is tomorrow | |
| 20:21:31 | sean-k-mooney | so we need to resovle the outstaidn question or we wont have this in victoria | |
| 20:21:44 | sean-k-mooney | we could backport it technically but that will be after the upstream release | |
| 20:21:57 | sean-k-mooney | unless we ask for an exception. | |
| 20:23:26 | sean-k-mooney | i just +2d the precedding refactor patch. ill ping gibi and stephenfin to review it again in the morning but we need to confirm if that regex will work with other vendors too | |
| 20:24:20 | sean-k-mooney | the fact teh mellanox ci passed on it is somewhat reasuring unfortuntely the netronome one went away some time ago | |
| 20:38:25 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: post live migration: don't call Neutron needlessly https://review.opendev.org/747451 | |
| 20:38:45 | artom | sean-k-mooney, lyarwood ^^ if you still have the time/energy | |
| 20:56:50 | sean-k-mooney | so you just removed raise NotImplemted and moved the stub up | |
| 20:57:06 | sean-k-mooney | well and update the doc string | |
| 20:57:20 | artom | sean-k-mooney, I fixed the test logic as well | |
| 20:57:54 | sean-k-mooney | stubing _notify_about_instance_usage instead of post_live_migration_at_source | |
| 20:58:03 | sean-k-mooney | this is the delta https://review.opendev.org/#/c/747451/5..6/nova/tests/functional/compute/test_live_migration.py | |
| 20:58:05 | artom | sean-k-mooney, since your migrate_data.vifs patch hasn't merged yet, we currently call driver.post_live_migration_at_source with a different network_info | |
| 20:58:21 | artom | So we can't assert anything about it, so I went and mocked the notification helper instead | |
| 20:58:32 | artom | To assert that we're using the correct network_info | |
| 20:58:38 | sean-k-mooney | ah ok | |
| 20:58:51 | sean-k-mooney | its a litle non obvious but i guess that works | |
| 20:59:04 | artom | sean-k-mooney, I didn't have a better idea :/ | |
| 20:59:12 | artom | Maybe I should add a note? | |
| 20:59:32 | sean-k-mooney | neither do i so i +1d it | |
| 20:59:43 | sean-k-mooney | well other then merge my patches | |
| 20:59:44 | artom | Cheers! | |
| 20:59:55 | artom | I need to review that | |
| 21:00:04 | sean-k-mooney | sure https://review.opendev.org/#/q/topic:bug/1888395+(status:open+OR+status:merged) | |
| 21:01:21 | sean-k-mooney | also im so shocked a test fixture that does not behave liek reality :P | |
| 21:01:47 | sean-k-mooney | the sad thing is it proably did at one point and our assumetion and precondition have been broken in neturon | |
| 21:02:12 | sean-k-mooney | but its equally likely it was broken form the start just doiong the minium we needed | |
| 21:03:06 | sean-k-mooney | oh cool my devstack patch merged :) https://review.opendev.org/#/c/749295/ i can now stack and unstack without having to delete pyc file the whole time | |
| 21:04:56 | artom | sean-k-mooney, no the fixture's actually fine | |
| 21:05:06 | artom | We just clobber what it gives us with migrate_data.vifs | |
| 21:05:46 | artom | Which makes the test pointless, because that never contains `migrating_to` in the peofile | |
| 21:35:52 | openstackgerrit | Rodrigo Barbieri proposed openstack/nova master: Allow tap interface with multiqueue https://review.opendev.org/748533 | |
| 21:43:38 | openstackgerrit | Merged openstack/nova master: Don't unset Instance.old_flavor, new_flavor until necessary https://review.opendev.org/744958 | |
| 21:44:00 | openstackgerrit | Merged openstack/nova master: Describe '[glance]/enable_rbd_download' https://review.opendev.org/749437 | |
| 23:29:50 | gmann | stephenfin: replied on https://review.opendev.org/#/c/749505/. I remember we discussed this as one of the possible API clean during train cycle but dropped it as there are lot of API changes needed. But as this is API change we definitely need spec first. | |
| #openstack-nova - 2020-09-03 | |||
| 00:00:19 | brinzhang | alex_xu: ack. | |