Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-22
19:02:01 sdague which is a stack trace
19:02:07 cburgess Yeah... cute.
19:02:13 mriedem so uh, no group rates in sydney huh?
19:03:00 sdague cburgess: qemu actually just gives a clean exit, but it's definitely not expected by nova code, so nova code stack traces
19:03:02 superdan mriedem: ack on all thanks
19:03:09 mriedem \o/
19:03:29 cburgess sdague Also depends on which version of RHEL 7 you are on. I believe later versions of RHEL, or the OSP repos include newer versions of QEMU. On 7.4 for instance we have 2.6
19:03:46 superdan mriedem: I wonder if we don't have a thing that actually tests server['fault'] from the api?
19:03:58 sdague cburgess: ah, yeh with OSP it might be different. I just have a centos 7 vanilla install
19:04:01 mriedem superdan: i'd expect a functional test somewhere,
19:04:10 mriedem superdan: fault testing is weird since the instance has to be deleted or in error state
19:04:10 superdan I would have thought
19:04:15 mriedem always trips me up
19:04:17 cburgess sdague yeah there are various additional repos you get access to with OSP.
19:04:22 superdan mriedem: um, what?
19:04:38 superdan oh for the api to show it?
19:04:41 superdan I see
19:04:42 mriedem yeah,
19:04:47 mriedem like these failure recreate tests we do,
19:04:50 superdan like your cell0 test, you could just boot things what will never schedule
19:04:57 mriedem i'm always wanting to just poll until the fault shows up, but most of the time that won't work
19:05:04 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506754
19:05:08 superdan mriedem: gotcha
19:05:18 mriedem so i've become astute and the instance actions api
19:08:11 superdan mriedem: the args to that function aren't kwargs
19:08:23 superdan I can call them that way for reference, but it's wrong
19:08:36 mriedem i know they aren't kwargs
19:08:44 superdan can I comment?
19:08:45 mriedem but you could have some variables in the test instead or something, like you did elsewhere
19:09:02 mriedem i just hate having to remember what None, None, {}, None, [], [], None means
19:09:10 superdan yep, I can do that
19:10:01 superdan mriedem: I'm not seeing a functional test that actually checks fault
19:10:05 superdan you know we have one somewhere?
19:10:14 mriedem superdan: i don't
19:10:17 mriedem w/o searching
19:10:35 superdan okay
19:10:44 mriedem gd these sydney hotel rates
19:15:42 stvnoyes mriedem: going thru your review ccomments on live migrate/v3 - https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py line 7077.
19:16:28 stvnoyes init connection should be done for the new flow so that is a problem that needs to be fixed
19:17:11 stvnoyes 2 ways to do it, leave libvirt driver as it was and it will get done for both new and old flows or
19:18:10 stvnoyes init_connection is already being done during pre_live_migrate but the updated bdm is not saved. If we make a change to save it during pre, it will be updated.
19:18:33 stvnoyes then it will be available on the source node
19:19:27 stvnoyes i like that better as then we can remove code from libvirt driver and I noticed that the xen driver doesn't do an init connection so this would fix that too.
19:22:39 mriedem so post_live_migration is on the source node, and init_connection is called to get the connection_info to disconnect from the source host
19:22:59 stvnoyes yes
19:23:16 mriedem pre_live_migration runs on the dest node, let me see what you're doing there
19:24:34 stvnoyes the bdm update is buried - _get_instance_block_device_info > driver_block_device.refresh_conn_infos > device.refresh_connection_info
19:24:38 mriedem hmm, the driver.pre_live_migration is what connects the volume on the dest host, but where do we call attachment_update?
19:24:56 mriedem oh right i commented on that too
19:25:17 stvnoyes very not crazy about all this stuff happening down under a get method
19:25:25 mriedem yeah
19:25:28 mriedem me neither
19:25:38 mriedem ok so https://review.openstack.org/#/c/463987/18/nova/compute/manager.py@5431 is what would eventually update the attachment with the host connector on the dest host
19:25:53 mriedem and then we call self.driver.pre_live_migration to connect the volume on the dest host
19:26:04 mriedem and then volume_api.attachment_complete to finalize on the dest
19:27:14 mriedem so this is where we have the updated bdm on the dest during pre_live_migration right? https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py@6908
19:27:21 stvnoyes I think the issue is that init_connection on the dest does not persist the new info to the db. so the source still has the stale info
19:27:31 mriedem stvnoyes: not in the nova db, but i think that's the idea,
19:27:49 mriedem attachment_update for the new attachment on the dest host passes the dest host connector,
19:27:56 mriedem and cinder stores the connection info in the cinder db
19:28:13 stvnoyes right. I was thinking that if we did a bdm.save() on the destination after getting the update, when the source later pulls the bdm from the db, the connector info would be up to date
19:28:18 mriedem that's why i said in https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py@7077 can't we just get the attachment record from cinder?
19:28:59 mriedem if we update the bdm connection_info during pre_live_migration, it's pointing at the dest host, so anything that relies on that on the source host could get mixed up
19:29:17 mriedem although i'm not sure we use the bdm connection_info on the source host after pre_live_migration do we?
19:29:24 mriedem maybe on rollback
19:29:41 openstackgerrit Elod Illes proposed openstack/nova master: Add error notification for instance.interface_attach https://review.openstack.org/506643
19:30:43 mriedem stvnoyes: i think the best thing for us to do is let cinder store the connection_info per attachment
19:30:52 mriedem and for the new flow, we just pull that from cinder when we need it
19:30:56 mriedem using the attachment_id
19:31:03 mriedem let's keep the nova bdm out of it
19:31:11 stvnoyes ok, I like that.
19:31:20 mriedem that was one of the main things jgriffith was shooting for anyway
19:31:25 stvnoyes thanks
19:31:28 mriedem when we started this like a year ago :)
19:31:53 melwitt superdan, mriedem: there's some instance fault testing stuff in nova/tests/functional/test_server_group.py
19:31:56 mriedem stvnoyes: so for your patch, i think that post_live_migration thing in the driver was my main hangup, otherwise there were just a couple of other small things
19:32:25 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506760
19:32:27 stvnoyes I've fixed most of them. After this part goes in, then just lots of testing before I update
19:34:18 superdan melwitt: okay that might not have found this because it was a show not a list
19:34:50 stvnoyes mriedem: btw, I don't see an init connection in xen driver's post_live_migrate. Is that a problem? (not for this rv)
19:35:15 mriedem stvnoyes: not sure, i'm not even sure how this is not failing..
19:35:34 jgriffith mriedem you just made my week!!! "mriedem:that was one of the main things jgriffith was shooting for anyway"
19:36:04 mriedem <3
19:40:45 mriedem stvnoyes: re the xen driver, it doesn't do the volume connect/disconnect like the libvirt driver, so i can't really say
19:40:58 mriedem we would need the xen driver team to take a look at it again and test things out
19:41:03 stvnoyes ok
19:41:38 mriedem that would be jianghua wang
19:42:12 mriedem heh, actually, they said in july:
19:42:13 mriedem "The connection_info from bdm works well for XenAPI both in v2 and v3.27. It'd be good if someone can help us to understand why libvirt needs initialize_connection."
19:42:20 mriedem :)
19:42:51 mriedem ah that's why this works
19:42:53 mriedem "Per the commit message from the following commit, the connection_info from DBM is for destination. So if the connection info for source side is different from destination, it will have problem. That's why libvirt changed to use initialize_connection. https://github.com/openstack/nova/commit/8b649aa86fb26e998d66e75e5cebfd19c396942d"
19:43:12 mriedem so apparently the bdm.connection_info is already getting updated with the dest host connection_info
19:44:04 mriedem nvm, that doesn't explain why your change isn't failing when we disconnect on the source host
19:44:50 mriedem let's just be safe and pull that thing out of cinder if we're new flow, we know it's there, and we have the attachment_id to get it via the migrate_data object
19:47:28 stvnoyes kk
20:00:09 openstackgerrit Jackie Truong proposed openstack/nova master: Add trusted_certs to Instance object https://review.openstack.org/489408
20:00:59 openstackgerrit Jackie Truong proposed openstack/nova master: Add trusted_certificates to REST API https://review.openstack.org/486204
20:09:52 openstackgerrit Merged openstack/nova master: Add tests to validate instance_list handles faults correctly https://review.openstack.org/505392
20:30:55 mriedem sdague: one sort of weird thing in the live snapshot patch https://review.openstack.org/#/c/454323/

Earlier   Later