Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-20
13:30:03 mdbooth I'm worried we might sell this as a problem to a rogue administrator.
13:30:16 mdbooth That's a seriously hard problem, and I can't believe we're planning to address it.
13:32:32 mdbooth Specifically, our implementation of LVM disk encryption provides no data security whatsoever.
13:32:56 mdbooth If it's a model we plan to copy, I'd be against it on the basis of complexity with no advantages.
13:35:24 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Use source_bdms to reset attachment_ids during LM rollback https://review.opendev.org/652800
13:35:24 openstackgerrit Lee Yarwood proposed openstack/nova master: Restore connection_info after live migration rollback https://review.opendev.org/551349
13:35:25 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: refactor volume bdm rollback error handling https://review.opendev.org/656500
13:40:06 dansmith sean-k-mooney: I'm really opposed to that
13:40:18 sean-k-mooney mdbooth: the goal of that work is to ensure that the image is never decrypted in a persitent way on the compute node
13:40:27 dansmith sean-k-mooney: what I want is for us to wait for events before they are triggered, or not wait for them at all
13:40:30 sean-k-mooney dansmith: because its just hiding the root issue
13:40:42 dansmith sean-k-mooney: I think that attach-detach-attach would fool that buffering approach
13:40:43 dansmith sean-k-mooney: yes
13:41:14 sean-k-mooney dansmith: right i get that i was just wondering if we wanted to be a bit mor defensive untill we figure out how to do that
13:41:45 dansmith I'd rather not wait at all
13:42:07 sean-k-mooney dansmith: it may or may not the idea was to store event that were recived but not expected and when you wait for a new event check if you have already recived it
13:42:17 sean-k-mooney dansmith: well not waiting is also an option
13:42:53 sean-k-mooney dansmith: its what artom has already done here https://review.opendev.org/#/c/639396/16
13:44:48 sean-k-mooney dansmith: anyway im going to spend some time trancing through this today to see if i can find a way to determin if we can wait or not but presently im not sure we can do that without a new rpc call from the dest to the compute to start it waiting
13:45:04 sean-k-mooney and im not sure that is worth it
13:45:32 dansmith I'd rather that than buffering, but...ack
13:46:32 sean-k-mooney well its certenly an option i just did not want to jump to that if there was something less invasive.
13:46:50 sean-k-mooney anyway thank for the input ill pause the buffering investigation so for now
13:48:55 openstackgerrit Lee Yarwood proposed openstack/nova master: Block swap volume on volumes with >1 rw attachment https://review.opendev.org/572790
13:48:56 openstackgerrit Lee Yarwood proposed openstack/nova master: Keep attach_mode as top-level field in _translate_attachment_ref https://review.opendev.org/574413
13:50:22 cdent placement meeting in #openstack-meeting-alt in 10 minutes
13:52:24 mriedem sean-k-mooney: i'm not in favor of https://review.opendev.org/#/c/639396/ since it breaks a thing in the gate
13:52:38 mriedem hence the patch on top
13:53:03 mriedem https://review.opendev.org/#/c/639396/5//COMMIT_MSG@25
13:54:20 sean-k-mooney mriedem: ya i am aware of why you made the original change
13:54:34 sean-k-mooney which is why im trying to figure out how to wait reliably
13:54:56 sean-k-mooney the patch on top still wait to late in some cases
13:56:14 sean-k-mooney if we take bufferign off the table my next idea is a pre_revert_at_source rpc function what will start the source node waithing for the vif-plugged event before we start the revert
13:56:50 sean-k-mooney that feel heavy wait but it shoudl resolve the issue of missing the event
13:57:05 sean-k-mooney *heavyweight
13:58:22 mriedem umm, isn't the issue just that we're not waiting for the unplugged even on the dest when we unplug vifs before casting to the source?
13:58:58 mriedem i'm kind of in the middle of changing all of my internal domain and email shit b/c of this embargo so can't really dig into this right now, but pretty sure i've gone over this (with dansmith and artom) already in detail once or twice
13:59:32 cdent blargh, that sounds painful
13:59:43 mriedem things are not fun right now no
14:02:02 sean-k-mooney mriedem: ack.
14:05:10 sean-k-mooney as far as i can tell for the logs we are still reciving the event before we start waiting
14:05:57 sean-k-mooney recive the vif-plugged event here http://logs.openstack.org/81/644881/15/check/nova-multinode/628c731/logs/subnode-2/screen-n-cpu.txt.gz#_Apr_29_15_18_02_574298 but starte to wait for it 2 seconds later here http://logs.openstack.org/81/644881/15/check/nova-multinode/628c731/logs/subnode-2/screen-n-cpu.txt.gz#_Apr_29_15_18_04_002370
14:06:11 sean-k-mooney if we fix that i think that will resolve the issue
14:17:14 mriedem sean-k-mooney: i believe i pointed out the issue here https://review.opendev.org/#/c/639396/3/nova/compute/manager.py@4184
14:17:58 mriedem which is what artom's final patch in the series is doing https://review.opendev.org/#/c/644881/15/nova/compute/manager.py
14:18:05 mriedem waiting for the event before doing migrate_instance_finish
14:18:58 sean-k-mooney mriedem thanks im looking at the code now but ill read your comment to make sure im on the same page
14:19:21 mriedem did the bug get punted to you at this point or are you just trying to get caught up and review the series?
14:19:46 sean-k-mooney mriedem: artom is on PTO for a few days so both
14:20:10 sean-k-mooney im trying to get caught up to review and determin if the current fix is correct and if not fix it
14:22:49 sean-k-mooney we think that this issue happens when you are using hybrid_plug e.g. the iptables firewall driver which we use downstream but not upstream so one of the things that i need to test is modifying the gate job temporally to repoduce this upstream without code change to confrim
15:12:32 aarents Hi, If need some more review on change that fix please (only +1 for now) https://review.opendev.org/#/c/659054/
15:12:40 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove 'etc/nova/cells.json' https://review.opendev.org/660146
15:12:40 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove conductor_api and _last_host_check from manager.py https://review.opendev.org/651059
15:13:21 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Stop starting consoleauth in functional tests https://review.opendev.org/652966
15:13:21 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Remove references to nova-consoleauth https://review.opendev.org/652965
15:13:22 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-status: Remove consoleauth workaround check https://review.opendev.org/652968
15:13:22 openstackgerrit Stephen Finucane proposed openstack/nova master: xvp: Start using consoleauth tokens https://review.opendev.org/652967
15:13:23 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Remove ConsoleAuthToken.to_dict https://review.opendev.org/652970
15:13:23 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove nova-consoleauth https://review.opendev.org/652969
15:13:24 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Rework nova console diagram https://review.opendev.org/660147
15:13:48 aarents with less typo: I need some more review on that fix please (only +1 for now) https://review.opendev.org/#/c/659054/ thks !
15:14:52 efried aarents: Looks like you have a merge conflict to resolve.
15:16:19 efried adrianc: are you around to make minor tweaks to https://review.opendev.org/#/c/643024/ if needed?
15:16:27 sean-k-mooney oh passing size when the image is deleted. the logic still seam right but i dont know that code well enough to review it so ill contiue to abstane form that review
15:17:37 adrianc efried: yes
15:18:10 aarents sean-k-mooney: ok no prob, efried: exact, will check that
15:21:13 efried adrianc: commented.
15:21:48 sean-k-mooney aarents: for what its worth if mdbooth thinks its ok then that is a +1.5 when it comes to the image cache so once you rebase against master it shoudl be easier to get review form the cores
15:21:52 adrianc efried: thx, will address shortly (meeting ...)
15:22:05 efried adrianc: A little background: First off, if you're reraising the original exception, you want to just say
15:22:06 efried because the former preserves the original stack trace and exception context.
15:22:06 efried raise e
15:22:06 efried rather than
15:22:06 efried raise
15:22:34 efried But it's also theoretically possible for intervening calls (like LOG.*) to mess with the exception context
15:22:54 sean-k-mooney efried: i like your deffintion of spelling :)
15:23:07 efried adrianc: so oslo provides a save_and_reraise_exception context manager that saves off the exception context while you do whatever, then makes sure to restore it properly before reraising.
15:23:32 efried sean-k-mooney: I have no words for you.
15:24:33 sean-k-mooney efried: i was refering to to your comment on adrianc patch
15:24:36 efried and misspelled
15:24:41 efried yeah, I know :)
15:24:48 tssurya mriedem, dansmith: could you both circle back to https://review.opendev.org/#/c/636132/ whenever you get some time ? thanks in advance!
15:24:55 sean-k-mooney ya i avoid that word for irony reasons
15:25:06 dansmith tssurya: yeah, sorry
15:25:13 efried I was trying to come up with some way to respond to sean-k-mooney commenting on anything to do with spelling, and I just came up blank. Totally speechless.
15:25:43 tssurya dansmith: no need to be sorry :) I understand you must be busy with lots of other stuff
15:26:19 tssurya this spec is a little too specific after all
15:33:42 aspiers efried: I've switched the SEV capability patch we discussed on Friday to instead set an instance variable and then provide the trait in u_p_t(). I'm now wondering where I should put the functional tests: in nova.tests.functional.test_servers.TraitsTrackingTests, or nova.tests.functional.test_report_cpu_traits.LibvirtReportTraitsTests
15:34:06 aspiers The latter seems more appropriate, I guess
15:34:24 efried aspiers: looking...
15:34:41 aspiers but the former already has tests closer to what I need (I wrote them, in fact)
15:38:14 efried aspiers: Is FakeLibvirtDriver in the way here?
15:38:56 efried I still don't fully understand how that thing works.
15:39:11 efried Obviously we want your tests to run through the real libvirt driver's upt
15:39:25 efried so as long as that's happening, it doesn't matter from a code point of view.
15:39:26 efried but
15:39:33 efried test_servers is way too effin big already.
15:39:34 aspiers efried: Ideally it should test not just that a) it provides the SEV trait on detection, but also b) it replaces it if the admin removes it, and that it removes it if c) the host isn't SEV-capable but an admin provides it or d) if the host *becomes* SEV-incapable
15:39:56 aspiers i.e. the same scenarios I tested when doing the cap-to-trait thing before

Earlier   Later