| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-30 | |||
| 13:31:20 | stephenfin | I'm happy with it. I'm happy without it. Just so long as I get my block indentation :) | |
| 13:31:25 | sean-k-mooney | aspiers: i read the xml all the time just not that xml its alway after its been parsed or pretty printed | |
| 13:32:25 | aspiers | sean-k-mooney: OK you meant "we don't read the XML as it is at *run-time*, we read it in the source"? I would mostly agree with that | |
| 13:32:26 | efried | aspiers: it's just entirely unnecessary. And in fact if you folded the result into the surrounding xml doc, it would look weird, because it's completely left-justifying | |
| 13:32:54 | sean-k-mooney | aspiers: yes | |
| 13:33:04 | aspiers | efried: what's entirely unnecessary, the dedent or the whole fixup? | |
| 13:33:08 | sean-k-mooney | aspiers: if im reading it at runtime its in a debugger or a traceback | |
| 13:33:27 | aspiers | another classic bikeshed topic :) | |
| 13:33:41 | sean-k-mooney | obviously | |
| 13:33:42 | aspiers | Is there a mandatory minimum of one per day? ;-p | |
| 13:34:05 | efried | well, both aspiers, and if it weren't for the fact that all these lines already belong to two very recent commits, I would -1 it for the sake of git history. I'm tempted to do that anyway. | |
| 13:34:16 | efried | but I have an actual problem with the dedent. | |
| 13:34:42 | efried | it's unnecessary processing (test only, but still) and the result is actually technically too dedented. | |
| 13:34:42 | aspiers | efried: I only submitted this because stephenfin asked for it - please negotiate with him not me :) | |
| 13:34:56 | efried | I hear ya. | |
| 13:34:56 | aspiers | I really don't mind what you guys agree on | |
| 13:35:41 | aspiers | thanks :) | |
| 13:36:53 | stephenfin | efried: straight up indentation fixes aren't an issue, tbh - git blame -w | |
| 13:37:01 | sean-k-mooney | mriedem: i may have ranted a bit but hopefully that helps. https://bugs.launchpad.net/nova/+bug/1842061 | |
| 13:37:03 | openstack | Launchpad bug 1842061 in OpenStack Compute (nova) "Compute schedulers in nova - AggregateInstanceExtraSpecsFilter docs are not clear" [Undecided,New] | |
| 13:37:16 | efried | stephenfin: where's the -w option in pycharm? | |
| 13:37:36 | sean-k-mooney | ill try and come up with a doc friendly way to say that | |
| 13:37:55 | stephenfin | efried: Google tells me it's on by default? https://stackoverflow.com/questions/37868225/ignore-whitespace-identation-in-annotate-mode-with-intelliji-and-git | |
| 13:38:08 | sean-k-mooney | efried: you can get the history of the lines you have slected and go back though each commit | |
| 13:38:38 | stephenfin | Also, am I the only sucker still working in Vim? | |
| 13:38:45 | sean-k-mooney | yes | |
| 13:38:50 | efried | sean-k-mooney: Yes, of course I can, but is that extra work worth it? IMO it is not. | |
| 13:38:54 | efried | especially for this | |
| 13:39:01 | efried | stephenfin: I believe cdent is with you. | |
| 13:39:17 | sean-k-mooney | efried: sooy i was not following the converstion | |
| 13:39:45 | sean-k-mooney | can you do git blame -w with pycharm | |
| 13:39:55 | sean-k-mooney | and yes if you want too | |
| 13:40:35 | efried | well, that's interesting, stephenfin you're right about -w being on by default. That takes some of the sting out. | |
| 13:40:50 | sean-k-mooney | how did i miss a full sentence... | |
| 13:40:53 | efried | I'll just abstain. | |
| 13:40:57 | stephenfin | \o/ | |
| 13:41:08 | stephenfin | that's a sane default | |
| 13:41:19 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Add useful error log when _determine_version_cap raises DBNotAllowed https://review.opendev.org/679449 | |
| 13:41:23 | sean-k-mooney | litally ment to type "all i saw was" before "can you do..." | |
| 13:41:38 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Add useful error log when _determine_version_cap raises DBNotAllowed https://review.opendev.org/679449 | |
| 13:41:51 | sean-k-mooney | stephenfin: proably because white space in lanagues like python matters | |
| 13:42:00 | sean-k-mooney | so changing indention could change behavior | |
| 13:42:09 | sean-k-mooney | or no one added it | |
| 13:42:11 | stephenfin | sean-k-mooney: Right, but make it toggleable | |
| 13:42:28 | stephenfin | Maybe 'git blame -W' to override whatever and re-enable it | |
| 13:42:35 | stephenfin | Hmm, that would be a decent patch | |
| 13:42:40 | stephenfin | Possible weekend project there | |
| 13:42:56 | sean-k-mooney | stephenfin: as long as you dont change the default behavior | |
| 13:43:05 | stephenfin | oh yeah, of cours | |
| 13:43:06 | stephenfin | *course | |
| 13:43:16 | sean-k-mooney | efried: speaking of which | |
| 13:43:44 | sean-k-mooney | efried: care to take a look at |https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/image-metadata-prefiltering | |
| 13:44:07 | sean-k-mooney | there are more more important reviews but im hoping those merge soon | |
| 13:44:21 | efried | sean-k-mooney: been on the list, but got some stuff in front of it. | |
| 13:44:32 | sean-k-mooney | cool | |
| 13:45:38 | sean-k-mooney | stephenfin: by they way i tested and fixed https://review.opendev.org/#/c/675776/ | |
| 13:45:50 | stephenfin | the revert? | |
| 13:45:52 | sean-k-mooney | stephenfin: we do need that fix to avoid out issue with libvirt | |
| 13:45:54 | stephenfin | yeah | |
| 13:45:54 | sean-k-mooney | yes | |
| 13:47:09 | sean-k-mooney | i guess we are still waiting for the melonox ci to run on it too | |
| 13:49:10 | mdbooth | mriedem dansmith: I don't think I'm going to get better confirmation of the fix in https://review.opendev.org/#/c/677736/ . It *has* run on a real system, though, and didn't break it. | |
| 13:49:50 | sean-k-mooney | artom: https://review.opendev.org/#/c/678887/ failed because its using qemu again | |
| 13:50:45 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add |
|
| 13:51:27 | aspiers | alex_xu, stephenfin, efried: need your take on this debate https://review.opendev.org/#/c/636318/49/nova/virt/libvirt/config.py@2701 | |
| 13:52:12 | mriedem | mdbooth: dan is out today, | |
| 13:52:16 | mriedem | but are you going to address https://review.opendev.org/#/c/677736/3/nova/virt/libvirt/host.py@a236 ? | |
| 13:52:22 | mriedem | otherwise i'll +2 it | |
| 13:53:09 | aspiers | stephenfin, efried: if the question isn't easy to parse from those comments, I can help explain here | |
| 13:53:33 | efried | aspiers: Give me 10 mins | |
| 13:53:38 | aspiers | efried: np | |
| 13:53:49 | sean-k-mooney | mdbooth: you know the py37 hang i was seeing before when we tryied to boot vms could also be fixed by https://review.opendev.org/#/c/677736/3 | |
| 13:54:41 | sean-k-mooney | i had assumed when i saw it it was due to eventlest and python37 not playing nicely but that looks like it is more likely the actual cause | |
| 13:57:25 | mdbooth | sean-k-mooney: Why would the behaviour be different on py37, though? | |
| 13:58:36 | mdbooth | Bug call time | |
| 14:02:10 | sean-k-mooney | well i was febuary, eventlet did not offically test/support py37 and we have see issue with it on python3 in the pasted that looked like it | |
| 14:02:44 | sean-k-mooney | but honestly it was just a guess and it happend on python 3 but python 2.7 | |
| 14:09:25 | efried | So this might be the first time we are cross-checking Thing A and Thing B for compatibility. | |
| 14:09:25 | efried | As of yet, I think all of these ^ checks are just checking whether Thing A is on in flavor and off in image meta, kind of thing. | |
| 14:09:25 | efried | We have a place where we validate flavor+image compatibility for things like this: nova.compute.api.API._validate_flavor_image | |
| 14:09:25 | efried | If it's possible for Thing A (via extra spec, image meta, config, etc) to ask for a switch to be flipped on, and Thing B (likewise) for it to be off, that's a conflict, and should be an error. | |
| 14:09:25 | efried | aspiers: I don't have the bandwidth to dig into this all the way right now, so the following is a bit from the hip: | |
| 14:09:26 | efried | Now, that's at the API layer. Once you get down to the compute, you can start vetting against things like config or whatever. Hopefully any conflicts there would have already been filtered out based on placement; but if not, late fail the spawn. | |
| 14:09:26 | efried | But that's fine - I believe that was one of the long-term goals of that infrastructure. stephenfin might be able to say more there. | |
| 14:09:27 | efried | So if all of that were in place, having a single function that checks sev *and anything else relevant* and either sets iommu on or raises an exception: sounds like the right plan. | |
| 14:13:30 | mriedem | artom: so https://review.opendev.org/#/c/678887/18 we're just waiting for another run after your rebase right? | |
| 14:21:49 | aspiers | efried: thanks, I think that's enough justification for me to start switching the driver iommu flags so they all get enabled from a single place | |
| 14:22:01 | aspiers | efried: right now the conflict is only a theoretical possibility | |
| 14:22:47 | aspiers | efried: so we may never need to worry about it, but yeah it makes sense what you say about there being enough places for us to proactively catch and handle any such conflicts if they did arise in future | |
| 14:25:23 | aspiers | efried: the only risk I can't see us easily avoiding is someone adding code which explicitly disables iommu but forgets to prevent the SEV code from overriding that. Moving the SEV iommu code away from each individual chunk responsible for building driver configuration means that it's "hidden" out of sight from that code, and therefore easily missed | |
| 14:25:44 | aspiers | efried: but that seems like a small risk, and in return we probably get much cleaner code enabling iommu for SEV | |
| 14:25:54 | aspiers | alex_xu: ^^^ | |
| 14:29:27 | mriedem | efried: dustinc: couple of questions in the ironic get_node sdk change https://review.opendev.org/#/c/642899/44/nova/virt/ironic/driver.py | |
| 14:33:29 | dansmith | artom: I think the version checking in here is wrong and that you'll fail spectacularly if you have an rpc version pin and have to roll back a migration: https://review.opendev.org/#/c/634606 | |
| 14:37:10 | efried | mriedem: responded | |
| 14:44:02 | mriedem | sean-k-mooney: this is all host agnostic right? https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L212-L228 | |
| 14:44:26 | mriedem | sean-k-mooney: meaning, if an instance has non-sriov pci requests, we're going to hit that same code for each alternate host during live migration and just fail on all of them since that validation doesn't involve the host | |
| 14:44:38 | mriedem | seems that should be pulled up higher in the conductor task before we even ask the scheduler for dest hosts | |
| 14:44:50 | mriedem | like https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L85 | |
| 14:45:20 | sean-k-mooney | we check for non neutron pci device requets in the conductor already | |