| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-11 | |||
| 13:34:49 | efried | yes, I'm drawing a parallel | |
| 13:35:49 | efried | I'm saying: {the way placement is used as the persistence mechanism for provider inventories etc} is equivalent in principle to {the way the nova db would need to be used for claim/assignment information we're discussing} | |
| 13:36:59 | alex_xu | actually we needn't persistent the claim and assigment in nova db, since libvirt persistent the info as dansmith said | |
| 13:37:49 | efried | You mean via the domain XML? | |
| 13:38:11 | alex_xu | yes, we read all assigned devices from the domain xml in the startup of compute service | |
| 13:38:12 | efried | But we rebuild the domain XML when we hard reboot the instance, right? | |
| 13:38:49 | efried | So if the instance is hard stopped, and then we restart the compute service, we have to rebuild the domain XML from somewhere. | |
| 13:39:13 | efried | or am I misunderstanding that lifecycle flow completely? | |
| 13:39:15 | alex_xu | efried: if you just stop the instance, the xml still in libvirt. do you mean remove the domain from the libvirt? | |
| 13:39:31 | dansmith | but the xml doesn't go away until you rebuild it, so you can read it, then do your regeneration | |
| 13:39:50 | efried | okay, cool. Then I guess we're okay. | |
| 13:39:52 | efried | but | |
| 13:40:00 | dansmith | we depend on libvirt keeping the domain xml for us in lots of cases | |
| 13:40:09 | efried | in this flow, the claim needs to happen before spawn is invoked | |
| 13:40:16 | efried | i.e. before we even start to build the domain xml | |
| 13:40:46 | efried | so the claim info will need to be stored temporarily somewhere else | |
| 13:40:53 | alex_xu | yes | |
| 13:40:56 | alex_xu | in memory | |
| 13:40:58 | efried | and then we can flush it once the instance is alive | |
| 13:41:01 | efried | okay, that wfm. | |
| 13:41:07 | stephenfin | bauzas: (Because I don't have a deployment available to test this on) Can standard, non-admin users specify the availability zone an instance lands in? | |
| 13:41:28 | bauzas | stephenfin: that's the whole purpose of AZs :) | |
| 13:41:40 | efried | alex_xu: Are you planning to work on this? | |
| 13:41:54 | alex_xu | yes, I can take a look that | |
| 13:42:01 | stephenfin | bauzas: okay, phew. So they can specify an AZ but not a destination host/node within that AZ, yeah? | |
| 13:42:05 | bauzas | stephenfin: to tell a way how to land somewhere you don't know but different from some other way you don't want | |
| 13:42:05 | stephenfin | By default, that is | |
| 13:42:25 | bauzas | stephenfin: yeah it's borked, because the AZ hack is a terrible way | |
| 13:42:50 | bauzas | so, yeah, we're accepting users to set an AZ name but they're trampled if they use ':' | |
| 13:43:00 | stephenfin | bauzas: yeah, that's why https://review.opendev.org/#/c/666767/ and its predecessor exists, presumably | |
| 13:43:36 | bauzas | stephenfin: I need to take a look on those things | |
| 13:43:45 | bauzas | efried: alex_xu: my meeting is done, wazzup ? | |
| 13:43:50 | stephenfin | bauzas: I'm rewriting the AZ/host aggregate docs atm. They're not done but when they are, you'll be the first person I ping for reviews ;) https://review.opendev.org/#/c/667133/ | |
| 13:44:05 | bauzas | (well I have other meetings in 20 mins, so my time is counted :( ) | |
| 13:44:12 | efried | bauzas: We were just discussing the race condition whereby two instances could try to assign the same vgpu | |
| 13:44:31 | bauzas | efried: k, basically alex_xu made a good point | |
| 13:44:31 | efried | talking about moving the vgpu "assignment" (though actually not the assignment, see below) into instance_claim | |
| 13:44:46 | bauzas | efried: that's overthinking I feel | |
| 13:44:59 | bauzas | stephenfin: ack, bump me with review requests tomorrow :) | |
| 13:45:27 | efried | tldr we're talking about adding claim_for_instance/unclaim_for_instance to the ComputeDriver interface, since only the specific virt driver can know how to identify the specific device, and what needs to be done to "reserve" it. | |
| 13:45:48 | efried | In the case of the libvirt driver for vgpus, it would be sufficient to mark it in an in-memory dict as belonging to the instance | |
| 13:46:05 | efried | then purge that dict entry during spawn, since that information can be gleaned from the domain XML thereafter. | |
| 13:46:45 | efried | this takes hypervisor-specific code out of the RT, which is ++ | |
| 13:46:49 | efried | and fixes the race | |
| 13:47:01 | efried | and is reusable for other things like vpmem | |
| 13:47:21 | bauzas | efried: FWIW, this is somehow related to the spec you love :) | |
| 13:47:36 | efried | which, vgpu affinity? | |
| 13:48:21 | bauzas | efried: correct, as we check things in instance_claim() already | |
| 13:48:35 | bauzas | efried: and that's what I really want to amend for vGPUs | |
| 13:48:48 | efried | yeah, basically "figure out which specific thingy on the system is going to be assigned to the instance for a particular unit of resource class in an allocation" should be the purview of the virt driver. | |
| 13:49:00 | bauzas | as I said, the vGPU weigher and the object update aren't necessary | |
| 13:49:01 | efried | because only the virt driver has a view to "specific thingy on the system" | |
| 13:49:13 | bauzas | sec, giving you a link | |
| 13:50:54 | sean-k-mooney | bauzas: its even more related to the draft i wrote for that | |
| 13:52:39 | sean-k-mooney | efried: i think bauzas means https://review.opendev.org/#/c/650963/9/specs/train/approved/libvirt-vgpu-numa-affinity.rst | |
| 13:53:20 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2223 eventualls calls https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L220 | |
| 13:53:27 | bauzas | efried: ^ | |
| 13:55:37 | bauzas | efried: so what you'd like is that the device assignment (ie. the mdev choice of https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6416) would be called by something close to https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L220 ? | |
| 13:55:47 | bauzas | if so, I'm not opposed to | |
| 13:56:18 | bauzas | but xen needs substantial changes too | |
| 13:57:12 | coreycb | kashyap: cpaelzer added it to his 20.04 virt stack work. thanks for the heads up. | |
| 13:57:24 | efried | bauzas: I would like it if the hypervisor-specific aspects of all of that were in the virt driver -- but yes, to fix the race, the "mdev choice" needs to happen somewhere inside instance_claim. | |
| 13:57:35 | kashyap | coreycb: Cool. I'll send a note to the list as well once I get around | |
| 13:57:44 | coreycb | ok | |
| 13:57:45 | kashyap | Also need to remind the SUSE folks. And Debian | |
| 13:58:00 | efried | bauzas: I'll take this opportunity to point out that _test_numa_topology is far from hypervisor-agnostic. | |
| 13:58:04 | bauzas | efried: that's what I said yes | |
| 13:58:11 | sean-k-mooney | personally i think we should be calling calim_for_instacnce form the conductor by they way way before we get to the compute node after we select the host but before we call spawn | |
| 13:58:20 | bauzas | oh | |
| 13:58:22 | bauzas | surze | |
| 13:58:29 | bauzas | but that's a waaaay more complicated | |
| 13:58:34 | efried | baby steps | |
| 13:58:51 | sean-k-mooney | efried: babysteps that predate placement | |
| 13:58:59 | sean-k-mooney | we keep putting that off | |
| 13:59:36 | sean-k-mooney | it prevents all the races that placemetn will eventurally solve | |
| 14:00:20 | sean-k-mooney | if we do claim for instance in teh conductor via an rpc to the compute we nolonger race on any pci or numa resouce | |
| 14:00:31 | mriedem | meeting? | |
| 14:00:48 | artom | meating. | |
| 14:00:57 | efried | oh, craaap | |
| 14:01:25 | alex_xu | ok, that is my fault :) | |
| 14:11:38 | mnaser | i'm really curious if there's a possible case where nova-compute gets a request an complains abut InstanceActionNotFound_Remote | |
| 14:13:10 | mnaser | it means that the instane action was *not* created (or committed to db) when the rpc request was recieved ? | |
| 14:27:58 | mriedem | check the request id - the actions are keyed by request id so if the request id isn't the same for some reason it wouldn't be found | |
| 14:28:27 | mriedem | e.g. a periodic task in compute using an admin context with a generated request id hitting some method that expects an action to exist | |
| 14:46:52 | bauzas | efried: FWIW https://review.opendev.org/#/c/552924/ | |
| 14:47:13 | bauzas | it needs a bit of caring ^ | |
| 14:47:18 | efried | ah, nice bauzas, thanks. | |
| 14:52:04 | bauzas | mriedem: https://review.opendev.org/#/c/645520/35/doc/api_samples/servers/v2.74/server-create-req-with-only-host.json I thought we were no longer accepting to only provide a host field without also providing the node name ? | |
| 14:52:12 | bauzas | lemme look at the spec | |
| 14:52:39 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Convert nova-next to a zuul v3 job https://review.opendev.org/670196 | |
| 14:53:10 | bauzas | mriedem: hell, yeah, I already had that concern https://review.opendev.org/#/c/645458/18/specs/train/approved/add-host-and-hypervisor-hostname-flag-to-create-server.rst@125 | |
| 14:54:16 | mriedem | bauzas: not sure what you're talking about | |
| 14:54:31 | bauzas | mriedem: asking a destination by only the 'host' field | |
| 14:55:07 | mriedem | so what? for non-ironic the host == hypervisor_hostname | |
| 14:55:12 | mriedem | so why require the user to specify both? | |
| 14:55:15 | mriedem | that's just shitty ux imo | |
| 14:55:37 | bauzas | mriedem: I'm all good with only providing the node name | |
| 14:55:54 | bauzas | what I'm concerned is by only asking for a service name | |
| 14:56:14 | bauzas | and depending on the virt driver, the service name can be different from the node name | |
| 14:56:26 | mriedem | only for ironic, right? | |