Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-08
15:55:42 mriedem edleafe: addressed my nits in the regression test patch, and fixed my -1 in the regression bug fix patch in the middle, rebased the series to master also ^
15:56:38 matrohon mriedem: hi
15:56:47 openstackgerrit Stephen Finucane proposed openstack/nova master: console: Send bytes to sockets https://review.openstack.org/531834
15:56:47 openstackgerrit Stephen Finucane proposed openstack/nova master: fixup! console: introduce the VeNCrypt RFB authentication scheme https://review.openstack.org/531833
15:57:14 stephenfin Oops
15:58:04 mdbooth Hehe
15:58:25 openstackgerrit Stephen Finucane proposed openstack/nova master: console: introduce framework for RFB authentication https://review.openstack.org/345397
15:58:26 openstackgerrit Stephen Finucane proposed openstack/nova master: console: Provide an RFB security proxy implementation https://review.openstack.org/345399
15:58:26 openstackgerrit Stephen Finucane proposed openstack/nova master: console: introduce the VeNCrypt RFB authentication scheme https://review.openstack.org/345398
15:58:27 openstackgerrit Stephen Finucane proposed openstack/nova master: console: Send bytes to sockets https://review.openstack.org/531834
15:58:27 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Document TLS security setup for noVNC proxy https://review.openstack.org/500544
15:58:31 mriedem matrohon: hi
15:58:51 mriedem dansmith: i'm +2 on edleafe's fix for the cold migration + reschedule regression (and the test patch below it) https://review.openstack.org/#/c/531022/
15:59:42 matrohon mriedem: I was trying to boot a VM without an IP, but I can't find a way to do so
16:00:01 mriedem matrohon: use at least microversion 2.37 to create the server and pass networks='none'
16:00:32 mriedem see the 'networks' parameter here https://developer.openstack.org/api-ref/compute/#create-server
16:01:05 matrohon mriedem: great! I reported a related bug, but I'll try it the way you mention
16:01:41 matrohon mriedem: However, I'm not sur my bug is pointless : https://bugs.launchpad.net/nova/+bug/1741575
16:01:42 openstack Launchpad bug 1741575 in OpenStack Compute (nova) "creating a VM without IP (ip_allocation=None)" [Undecided,New]
16:02:05 mriedem matrohon: yeah https://bugs.launchpad.net/nova/+bug/1741575 is something else
16:02:52 mdbooth lyarwood: Ah, yes
16:03:18 mdbooth lyarwood: So, we call _connect_volume in _get_guest_xml, which is pretty unambigously a bug
16:03:32 mdbooth But it's a bug we rely on in a couple of places
16:03:54 mdbooth I have some notes I made before the break about how to unwind that
16:05:06 mdbooth Apart from that, we'd need context in swap_volume
16:05:07 matrohon mriedem: I briefly discussed with carl_baldwin a long time ago, how told me he didn't finished the job on the nova side. At least, he didn't submit anything related to the case where "ip_allocation=none".
16:05:22 lyarwood mdbooth: for the bugfix I'm now providing that
16:05:23 mdbooth lyarwood: Which... we do anyway. Doesn't look like we're handling encryptors there.
16:05:47 mdbooth _create_domain_setup_lxc()
16:06:45 mdbooth lyarwood: pre_live_migration() ?
16:07:01 mdbooth I don't immediately see where we're attaching encryptors there
16:07:37 mdbooth So my vote would be:
16:08:00 mdbooth 1. unpick _connect_volume() in _get_guest_xml()
16:08:11 mdbooth 2. pass context to swap_volume()
16:08:31 mdbooth 3. Implement ajttach/detach encryptors in LibvirtDriver._connect/disconnect_volume
16:09:03 lyarwood urgh, 1 would make this a risk to backport
16:09:04 mdbooth Then we can be reasonably confident that we caught all the cases, and nobody will introduce any more in the future.
16:09:25 mdbooth lyarwood: I don't think it's as bad as you think
16:09:26 lyarwood also another reason not to do this for the bugfix would be the change in error handling
16:09:56 lyarwood we often don't connect/disconnect in the same try block as attaching/detaching encryptors
16:10:17 mdbooth Well we should...
16:10:28 lyarwood yeah agreed, but that's not part of this bugfix
16:10:32 mdbooth That's only going to simplify error handling, no?
16:10:37 dansmith mriedem: okay I'm going through bauzas' gpu set and then will circle back to that
16:11:34 lyarwood mdbooth: yeah but the _get_guest_xml() cleanup and merging attach/detach encryptors into connect/disconnect volume just seems way over the top for a simple, easily backportable fix for swap_volume tbh
16:11:38 mdbooth lyarwood: I don't think a 'proper' fix is all that messy in this case. My concern is that if we apply 4 more bits of band-aid we won't ever fix it.
16:12:02 mdbooth What about pre_live_migration()?
16:12:02 cdent efried: i'm back, for a while
16:12:06 lyarwood mdbooth: it's hardly band-aid
16:12:08 mdbooth Are we handling that somewhere else?
16:12:21 efried cdent I'm writing up a summary and adding it to the review (https://review.openstack.org/#/c/526539/)
16:12:35 cdent efried++
16:15:12 lyarwood mdbooth: https://bugs.launchpad.net/nova/+bug/1633033 - sorry that took a little digging
16:15:15 openstack Launchpad bug 1633033 in OpenStack Compute (nova) "live migration with encrypted volume fails" [Undecided,In progress] - Assigned to Lee Yarwood (lyarwood)
16:16:02 mdbooth lyarwood: Ok, so looks like that's broken too, same root cause
16:16:07 lyarwood yup
16:16:21 mriedem lyarwood: mdbooth: why don't you do the short-term backportable fix and then do the refactor on master which isn't backported?
16:16:32 mriedem don't refactor a bunch of code that you're going to backport
16:16:33 mdbooth Looking back over my notes, actually I think the _get_guest_xml fix is really easy
16:17:14 lyarwood mriedem: I'm trying to suggest that, I really don't want to play around with _get_guest_xml in backports
16:17:42 mdbooth lyarwood: The reason is that everywhere that calls _get_guest_xml either subsequently calls _create_domain_and_network, or doesn't pass block_device_info, so doesn't call _connect_volume
16:18:08 mdbooth Which means that all you need to do is move _connect_volume from _get_guest_xml to _create_domain_and_network
16:18:15 mdbooth Which is also totally intuitive
16:18:28 mdbooth I've audited ^^^, but feel free to do the same
16:18:51 mdbooth _create_domain_and_network already has context, which also makes sense
16:19:03 mdbooth So that change isn't going to be invasive at all
16:19:04 matrohon mriedem: I'm not sure networks=none would help me. My VM needs a port, but a port without IP.
16:19:29 mriedem matrohon: yeah it won't
16:19:34 mriedem networks=none means no ports
16:19:48 mriedem i didn't know about ip_allocation='none'
16:20:01 mdbooth lyarwood: Whatever way you look at it, we need context in swap_volume anyway
16:20:09 mdbooth So that part needs to be done anyway
16:20:14 lyarwood mdbooth: and is in the bugfix
16:20:31 mdbooth lyarwood: I don't see it in that review stack
16:20:47 lyarwood mdbooth: https://review.openstack.org/#/c/531233/3/nova/virt/libvirt/driver.py
16:20:50 mdbooth NP, now I do
16:21:03 matrohon mriedem: ok, I can work on fixing 1741575, with the attached patch as a first step
16:21:11 mdbooth lyarwood: Ok, that bit's already done.
16:21:37 mdbooth With those bits in place, moving the encryptor call into _connect/_disconnect is trivial
16:21:46 mdbooth And no harder to backport
16:21:52 mdbooth And also fixes pre_live_migration
16:22:04 mdbooth And anything else we missed
16:22:33 mdbooth Does lxc have encryptors?
16:23:01 matrohon mriedem: do you think I need something else than the bug report to start submitting code review?
16:23:14 lyarwood mdbooth: well, the frontend encryptors are host level so yeah, I can't see why not.
16:23:23 lyarwood mdbooth: not that they are tested anywhere.
16:23:30 mdbooth lyarwood: Ok, that's probably a bug too, then
16:23:52 mdbooth So it's a 3 for 1 fix
16:24:02 openstackgerrit Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766
16:24:22 lyarwood mdbooth: right, I'll respin without moving _connect_volume out of _get_guest_xml
16:24:35 mdbooth I don't think you can do that
16:24:54 lyarwood right, context
16:25:02 mdbooth Actually it's already there
16:25:19 mdbooth But you'd have to remove the encryptor stuff from _create_domain_and_network
16:25:28 mdbooth Which is just backwards
16:25:32 mriedem matrohon: no
16:25:35 mdbooth Might as well go the other way
16:25:37 mriedem bug fix is fine
16:25:50 matrohon mriedem: ack, thanks
16:25:58 mriedem matrohon: but i looked at the patch and it doesn't really make sense to me, unless a port with ip_allocation='none' results in port.get('ip_allocation') == None?

Earlier   Later