| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-29 | |||
| 18:27:56 | dansmith | assertTrue(foo.called) is okay with me for LOG.warning just to make sure it was called, | |
| 18:28:08 | dansmith | but mriedem tends to do a looooot of string compare assertions that I do not | |
| 18:28:21 | dansmith | so he would do assert_called_with | |
| 18:30:15 | mriedem | assertTrue(foo.called) is so 2015 | |
| 18:30:22 | mriedem | foo.assert_called() | |
| 18:30:28 | mriedem | foo.assert_not_called() | |
| 18:31:15 | mriedem | just because i commented on the release note, does that mean you expect me to go back and re-review this all again?! | |
| 18:31:40 | kashyap | mriedem: No. I'll actually nicely summarize in the main comment as to what all I addressed. | |
| 18:31:59 | mriedem | that can't be trusted | |
| 18:32:03 | kashyap | LOL | |
| 18:32:05 | kashyap | E.g. | |
| 18:32:06 | kashyap | PS21: | |
| 18:32:06 | kashyap | - Undid the whitespace change in driver.py | |
| 18:32:07 | kashyap | - Changed the 'exception' to a LOG.warning in driver.py | |
| 18:32:07 | kashyap | - Added an additional test in test_driver.py | |
| 18:32:08 | kashyap | - Clarify the release note that we're talking about libvirt where appropriate | |
| 18:32:12 | kashyap | - Remove the confusing comment about a future change & instead update the 'conf' file | |
| 18:32:14 | kashyap | [...] | |
| 18:32:17 | mriedem | omg | |
| 18:32:20 | kashyap | Of course you should review, and not trust me | |
| 18:32:23 | mriedem | you know you can diff patch sets in gerrit right? | |
| 18:32:27 | kashyap | I know; just kidding | |
| 18:32:34 | mriedem | i'm not sure you are, | |
| 18:32:39 | mriedem | you're used to the ML style reviews in qemu | |
| 18:32:44 | kashyap | mriedem: Damn | |
| 18:32:45 | kashyap | You caught me | |
| 18:34:04 | kashyap | As you caught me, that's indeed a thing in ML land; that goes under the three dashes below a patch (---): which *won't* go into Git commit, obviously | |
| 18:34:17 | kashyap | s/below a patch/below the main commit message/ | |
| 18:35:04 | openstackgerrit | Arvind Nadendla proposed openstack/nova master: Update ImageMetaProp object to expose traits https://review.openstack.org/557795 | |
| 18:35:55 | melwitt | stephenfin: I've been meaning to ask you if you could take a look at this https://bugs.launchpad.net/nova/+bug/1750672 it's a failure to be able to generate nova docs under py3. something to do with sphinx and we couldn't tell if it's a blockdiag bug or if it's something we need to fix on our side | |
| 18:35:55 | openstack | Launchpad bug 1750672 in OpenStack Compute (nova) "failure to generate Nova's doc in Python 3.6" [Medium,Confirmed] | |
| 18:36:14 | mriedem | dansmith: now that looks pretty http://logs.openstack.org/90/557490/8/check/build-openstack-sphinx-docs/413d6d9/html/admin/configuration/schedulers.html#aggregates-in-placement | |
| 18:36:27 | openstackgerrit | melanie witt proposed openstack/nova-specs master: Document Rocky review priorities https://review.openstack.org/557796 | |
| 18:37:52 | dansmith | mriedem: hell to the yeah | |
| 18:39:59 | dansmith | fwiw, | |
| 18:40:06 | dansmith | runways are doing a good job of keeping me jumpin' | |
| 18:40:25 | melwitt | like, jumpin' for joy? | |
| 18:40:42 | dansmith | like jumpin' busy responding to all the feedback firehose | |
| 18:40:55 | melwitt | ah yeah | |
| 18:41:29 | jmlowe_ | mriedem: looks like _update_ports_for_instance does update device_owner while _update_port_binding_for_instance doesn't | |
| 18:41:49 | jmlowe_ | no chance of confusion there! | |
| 18:43:22 | mriedem | jmlowe_: get familiar enough with the nova.network.neutronv2.api.API code, and you'll find there are at least 3 similarly named methods for about every operation that all do slightly different things and are all called in tightly-coupled ways from different places in nova | |
| 18:43:25 | mriedem | it's, great | |
| 18:44:21 | mriedem | see the amount of talking-to-myself i've done in https://review.openstack.org/#/c/551371/ | |
| 18:46:12 | jmlowe_ | bug 1759924 is away | |
| 18:46:12 | openstack | bug 1759924 in OpenStack Compute (nova) "Port device owner isn't updated with new host availability zone during unshelve" [Undecided,New] https://launchpad.net/bugs/1759924 | |
| 18:46:51 | mriedem | thanks | |
| 18:48:18 | jmlowe_ | I need about 6 more cups of coffee before I can muster the strength to propose a half assed copy and paste patch | |
| 18:49:18 | jmlowe_ | very tempted to just do the one line search opts fix because why do it the right way when you can do it with sed | |
| 18:50:04 | kashyap | mriedem: That's nice, "consciousness stream" writing on the review. (If only there's an easier for people to "discover" easily all the sluething you've done.) | |
| 18:50:15 | kashyap | (Barring you having to mention here on IRC, live.) | |
| 18:50:23 | openstackgerrit | Merged openstack/nova stable/pike: Save admin password to sysmeta in libvirt driver https://review.openstack.org/550498 | |
| 18:50:30 | openstackgerrit | Merged openstack/nova stable/pike: Add functional regression test for bug 1746509 https://review.openstack.org/555058 | |
| 18:50:30 | openstack | bug 1746509 in OpenStack Compute (nova) pike "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" [High,In progress] https://launchpad.net/bugs/1746509 - Assigned to melanie witt (melwitt) | |
| 18:50:35 | openstackgerrit | Merged openstack/nova stable/pike: Move _make_instance_list call outside of DB transaction context https://review.openstack.org/555923 | |
| 18:50:49 | melwitt | yaaaay | |
| 18:53:06 | mriedem | jmlowe_: i left notes in the bug, i think it's a one line fix | |
| 18:55:26 | openstackgerrit | Jay Pipes proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.openstack.org/555081 | |
| 18:56:27 | mriedem | cfriesen: you might enjoy https://bugs.launchpad.net/nova/+bug/1759924/comments/2 | |
| 18:56:27 | openstack | Launchpad bug 1759924 in OpenStack Compute (nova) "Port device owner isn't updated with new host availability zone during unshelve" [Medium,Triaged] | |
| 18:56:33 | mriedem | when we shelve, we don't cleanup any pci stuff | |
| 18:58:12 | cfriesen | mriedem: for certain values of "enjoy" | |
| 18:58:20 | cfriesen | can't we just drop shelve yet? | |
| 18:59:41 | melwitt | cfriesen: I dunno if you saw I invoked your name earlier when fghaas was asking about nested virt and cpu policy 'require' stuff | |
| 19:00:12 | cfriesen | melwitt: whoops, missed it. | |
| 19:00:15 | mriedem | cfriesen requires at least one goat sacrifice if you invoke his name | |
| 19:00:59 | fghaas | you're being awesome melwitt :) But I figured everyone including cfriesen was rightfully busy so I posted this to the mailing list (http://lists.openstack.org/pipermail/openstack/2018-March/046313.html) — but I'll be happy to follow up with a writeup from whatever we discuss here! | |
| 19:01:54 | melwitt | a-ha, cool | |
| 19:03:30 | cfriesen | fghaas: the Intel CPU models in libvirt/qemu generally do not include the "vmx" flag, as you have discovered. The functionality that you're looking for is exposed in kashyap's feature ("https://review.openstack.org/#/c/534384/ ") which is currently under review. | |
| 19:03:44 | kashyap | fghaas: Hi :-) | |
| 19:04:06 | melwitt | I wondered if kashyap's patch would be the answer. noyce | |
| 19:04:26 | cfriesen | fghaas: note that I've experienced instability with nested kvm, but kashyap says it's good with more recent kernel/qemu/libvirt | |
| 19:05:14 | kashyap | Yes; cfriesen -- let's not repeat "unstability" without data :-). I recall participating in an email thread w/ fghaas on KVM list about :-) | |
| 19:05:41 | kashyap | cfriesen: BTW -- Paolo (one of the KVM maintainers) said he's going the toggle the 'nested' flag to default for Intel in the upstream kernel "soon" | |
| 19:06:33 | kashyap | (It's on for AMD in the upstream kernel; but amusingly enough ... AMD is more instable w/ 'nested' than Intel!) | |
| 19:06:56 | cfriesen | kashyap: isn't it on for the AMD cpu models too? | |
| 19:07:17 | kashyap | cfriesen: See my previous comment. It *is* on for AMD. But not for Intel. At this moment. | |
| 19:07:33 | cfriesen | kashyap: I meant in libvirt/qemu, not kernel | |
| 19:09:17 | kashyap | cfriesen: What do you mean? There's no such thing as "enabling nested in libvirt / QEMU". As you know, you can configure Kernel, then tell libvirt, which inturn tells QEMU to use it. | |
| 19:10:36 | cfriesen | kashyap: I meant that the "svm" flag is included in the AMD cpu models | |
| 19:11:49 | kashyap | cfriesen: Ah, that's true. | |
| 19:12:04 | kashyap | It's late here, /me is losing ability to parse text messages | |
| 19:12:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: tests for alloc candidates with nested and traits https://review.openstack.org/531899 | |
| 19:12:33 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: resource requests for nested providers https://review.openstack.org/554529 | |
| 19:13:05 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Fix rebuild of baremetal instance when vm_state is ERROR https://review.openstack.org/523559 | |
| 19:14:52 | openstackgerrit | Jay Pipes proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.openstack.org/555081 | |
| 19:21:18 | fghaas | cfriesen, yes kashyap and I discussed the pros and cons at length recently, but this is really a generic question about cpu feature flags | |
| 19:21:35 | fghaas | not so much passing in the "vmx" flag specifically | |
| 19:22:26 | fghaas | but yeah that patch looks excellent :) | |
| 19:23:50 | melwitt | yeah, feature is being added this cycle https://blueprints.launchpad.net/nova/+spec/libvirt-cpu-model-extra-flags but the backports are going to be restricted to only the 'pcid' flag to address the meltdown/spectre perf mitigation. we weren't intending to backport a brand new feature in a free-form way | |
| 19:24:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Migrate old style volume attachments on nova-compute startup https://review.openstack.org/549130 | |
| 19:24:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Move get_stashed_volume_connector to compute.utils https://review.openstack.org/557810 | |
| 19:25:14 | melwitt | (to not add unknown instability to branches that are supposed to be considered quite stable) | |
| 19:43:42 | mriedem | wolfenstein | |
| 19:43:45 | mriedem | comin' for ya | |
| 19:57:40 | openstackgerrit | Merged openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730 | |
| 20:07:10 | openstackgerrit | Merged openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729 | |
| 20:07:18 | openstackgerrit | Merged openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990 | |
| 20:07:29 | openstackgerrit | Merged openstack/nova master: Add an index on aggregate_metadata.value https://review.openstack.org/555851 | |
| 20:20:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status check for ironic flavor migration https://review.openstack.org/527541 | |