| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-17 | |||
| 13:15:02 | lyarwood | I *think* instance_update_and_get_original is trying to update machine_type in the instance db for some reason | |
| 13:15:24 | brinzhang_ | gibi, sean-k-mooney: we found this issue from you comments in vGPU support spec, depends on the "resume_guests_state_on_host_boot=True" config | |
| 13:15:37 | brinzhang_ | https://review.opendev.org/c/openstack/nova-specs/+/750116/9/specs/wallaby/approved/support-vGPU-nova-cyborg-interaction.rst#183 | |
| 13:16:03 | sean-k-mooney | lyarwood: ill take a look but we shoudl not have a machine_type field. it is ment to be img_machine_type in the instance_system_metadata table | |
| 13:16:57 | Yumeng | sean-k-mooney: "the agent automaticaly create the devices on start up" Does this mean cyborg will need another periodic task to sync arq in db with mdevs in the sys path? | |
| 13:16:58 | sean-k-mooney | so "mapper.column_attrs[key], value" looks wrong to me | |
| 13:16:59 | lyarwood | sean-k-mooney: the above series adds machine_type as a field to the instance_extras table etc | |
| 13:17:22 | lyarwood | sean-k-mooney: I've just missed something somewhere leading to this error when I set it in the instance object and try to save | |
| 13:17:48 | sean-k-mooney | lyarwood right but you shoudl not be doing that | |
| 13:18:03 | sean-k-mooney | lyarwood: wasnt the plan to have no db migraiton requireed for this | |
| 13:18:15 | sean-k-mooney | that is what we dicussed at the ptg | |
| 13:19:05 | lyarwood | I think you suggested adding this to system metadata but I then said it may as well go into instance extras | |
| 13:19:17 | lyarwood | it's in the spec as an instance extra | |
| 13:19:22 | openstackgerrit | Elod Illes proposed openstack/nova stable/stein: [stable-only] Cap bandit to 1.6.2 https://review.opendev.org/c/openstack/nova/+/766487 | |
| 13:19:36 | sean-k-mooney | ok i think that is not the right way to do this | |
| 13:19:52 | sean-k-mooney | at least not as a general pattern if we are adding more fileds | |
| 13:20:01 | sean-k-mooney | which is why it wanted it in system metadata to begin with | |
| 13:20:19 | sean-k-mooney | since we will likely want to do this again going forward when we change defaults | |
| 13:20:39 | lyarwood | what's the issue with this being an instance extra? | |
| 13:21:00 | sean-k-mooney | it will require a new column and db migration for every filed for not real value | |
| 13:21:41 | sean-k-mooney | if we use system metadata we dont require either | |
| 13:22:02 | sean-k-mooney | its also where the image metadata is currently sotred | |
| 13:22:28 | sean-k-mooney | so if we recored itn in img_machine_type we dont need to update any code that currently uses it | |
| 13:22:50 | sean-k-mooney | that way we dont miss anythign and it minimsies the code changes | |
| 13:22:51 | lyarwood | oh hell no, I'm not overloading img_machine_type | |
| 13:23:00 | sean-k-mooney | that was the whole point | |
| 13:23:53 | lyarwood | I didn't get that memo tbh, I've always wanted to track the machine type of the instance seperate to anything else | |
| 13:24:27 | lyarwood | so we can move it forward in the future without overwriting the image meta etc | |
| 13:24:30 | sean-k-mooney | we could use a different prefix in the system metadata | |
| 13:25:01 | sean-k-mooney | but im pretty stongly -1 on a new db column | |
| 13:25:44 | lyarwood | kk let me look into using it instead | |
| 13:26:00 | lyarwood | I honestly didn't think a new extra col for this would be a big deal | |
| 13:26:10 | lyarwood | given what's already in there | |
| 13:26:53 | sean-k-mooney | well given we want to evenually recored the value of all image properties and the machine_type propsoal was just a scoped down verion of that i think its a problematic design | |
| 13:27:30 | lyarwood | it wasn't a scoped down version of that at all | |
| 13:27:46 | lyarwood | I think you're crossing wires here tbh | |
| 13:27:49 | sean-k-mooney | it was ment to be | |
| 13:28:16 | lyarwood | it was always just about recording the machine type of existing instances | |
| 13:28:24 | sean-k-mooney | thats what i discussed with stephen when we were deciding if i or you would write the spec | |
| 13:28:55 | lyarwood | the image properties are part of it but not all instances will have the machine type set that way | |
| 13:29:15 | sean-k-mooney | yes i know | |
| 13:29:36 | lyarwood | this was always about capturing the currently used machine type and stashing it somewhere | |
| 13:29:43 | sean-k-mooney | yep | |
| 13:29:45 | lyarwood | allowing the config to change | |
| 13:29:58 | lyarwood | I didn't take from that the need to use the image properties | |
| 13:30:14 | lyarwood | I wasn't even planning to look at the image properties tbh | |
| 13:30:20 | lyarwood | Just the defined domains | |
| 13:30:41 | sean-k-mooney | well i had a very differnet plan for implementing it then | |
| 13:31:53 | sean-k-mooney | i will also point out form a downstream perspecitve we may need to backport this to 16.2 to support 17 upgrades | |
| 13:32:11 | sean-k-mooney | that would depend on how we handel the defualt machine type in 17/wallaby | |
| 13:32:34 | sean-k-mooney | its intended to be q35 for all new installs | |
| 13:32:47 | lyarwood | I wasn't planning on backporting this | |
| 13:32:49 | sean-k-mooney | for upgrade we need to ensure that the machine type does not change | |
| 13:32:55 | lyarwood | new installs would be q35 | |
| 13:33:01 | lyarwood | upgrades would be the old default | |
| 13:33:11 | lyarwood | until the env was fully upgraded | |
| 13:33:20 | lyarwood | and the original machine types stashed | |
| 13:33:24 | sean-k-mooney | ok but that means the ooo work is more involed but that is fine | |
| 13:33:40 | lyarwood | yeah it's a wrinkle we would need to document | |
| 13:33:48 | lyarwood | if we go with system metadata then I could backport it | |
| 13:34:01 | sean-k-mooney | yep | |
| 13:34:02 | lyarwood | so I'll take another look at that | |
| 13:41:12 | gibi | lyarwood, sean-k-mooney: I barely followed the machine type dicussion above, if your decide on a change in direction then please propose a spec update accordingly | |
| 13:42:02 | sean-k-mooney | ack im not sure it sa change in direction at elast form the ptg discussion | |
| 13:42:23 | sean-k-mooney | https://etherpad.opendev.org/p/nova-wallaby-ptg line 196 is the aggreed statement | |
| 13:42:30 | sean-k-mooney | *796 | |
| 13:43:02 | gibi | sean-k-mooney: anyhow, please keep the spec in sync with the impl : | |
| 13:43:03 | gibi | :) | |
| 13:43:15 | sean-k-mooney | but the main delta is lyarwood is going to look at using system metatdata table instead of a new column which would be a change in spec | |
| 13:43:34 | gibi | ack | |
| 13:44:24 | sean-k-mooney | i guess its tbd how easy that is but i think it shoudl be a smaller code change in general | |
| 14:08:41 | lyarwood | sean-k-mooney: sorry I can't recall why I dropped the generic record all the metadata props part of this | |
| 14:09:09 | sean-k-mooney | it was likely not in the downstream bz/dod | |
| 14:09:14 | lyarwood | sean-k-mooney: I think it might be due to that approach not capturing the config side | |
| 14:09:22 | lyarwood | sean-k-mooney: yeah or that | |
| 14:09:28 | sean-k-mooney | it was going to capture the config too | |
| 14:09:46 | sean-k-mooney | the idea was to sotre the config value in the imgage prerty filed if it was not set | |
| 14:09:57 | sean-k-mooney | but its fine to strat with just machien_type | |
| 14:10:14 | sean-k-mooney | if its easy to repate the same pattern for the other value when we need them | |
| 14:11:02 | lyarwood | yup, let me rewrite this and update the spec | |
| 14:11:33 | gibi | ^^ +1 | |
| 14:11:34 | gibi | :) | |
| 14:11:41 | gibi | on the spec side | |
| 14:11:51 | lyarwood | yup sorry about this | |
| 14:12:33 | gibi | no problem on my side | |
| 14:39:22 | openstackgerrit | YumengBao proposed openstack/nova-specs master: libvirt supports composing cyborg owned vGPU accelerator into domain XML https://review.opendev.org/c/openstack/nova-specs/+/750116 | |
| 14:40:30 | stephenfin | bauzas: any chance you could look at https://review.opendev.org/q/topic:%2522sriov-functional-tests%2522+status:open too | |
| 14:40:31 | stephenfin | ?? | |
| 14:45:16 | Yumeng | gibi: just updated vGPU spec according to today's discuss. Please help to mention in nova's meeting today so cores can know about the update! need their review and comment,Thanks! https://review.opendev.org/c/openstack/nova-specs/+/750116 | |
| 14:45:59 | gibi | Yumeng: thanks for the update. I will mention the fact that the spec needs core review on today's meeting | |
| 14:52:19 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Bump oslo.log version to 4.3.0 https://review.opendev.org/c/openstack/placement/+/760229 | |
| 14:53:54 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Pass context objects to oslo.policy directly https://review.opendev.org/c/openstack/placement/+/766785 | |
| 14:54:08 | openstackgerrit | Aleksey Ulyanov proposed openstack/nova master: Fix graceful shutdown timeout https://review.opendev.org/c/openstack/nova/+/764927 | |
| 15:02:33 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240 | |
| 15:02:56 | openstackgerrit | Aleksey Ulyanov proposed openstack/nova master: Fix graceful shutdown timeout https://review.opendev.org/c/openstack/nova/+/764927 | |
| 15:13:34 | bauzas | Yumeng: gibi: saw the update but honestly, I won't have time for reviewing this large spec before end of this year | |
| 15:13:57 | bauzas | which is for me in 24 hours (with Friday afternoon full of meetings) | |
| 15:17:53 | gibi | bauzas: no worries | |
| 15:18:12 | gibi | Yumeng knows that we are just about to go for PTO for two weeks | |
| 15:23:42 | bauzas | gibi: actually, I lied, I just commented out loud | |