| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-04 | |||
| 12:48:24 | sean-k-mooney | so this became much more involed of a change involving object changes | |
| 12:48:42 | lyarwood | kk | |
| 12:48:45 | sean-k-mooney | but it was quite late/early this moring when i figured that out and didnt want to start agin | |
| 12:49:40 | sean-k-mooney | it will include hw_machine_type because you are using the same key as the image proerty - a sufix | |
| 12:49:50 | sean-k-mooney | lyarwood: if you add a prefix it would not be included | |
| 12:50:22 | lyarwood | kk, well I think I might leave it, the actual behaviour means I don't need to make any other changes to the util method fetching the machine type | |
| 12:50:26 | sean-k-mooney | what im going to do in the simpler version is remove key = key[len(SM_IMAGE_PROP_PREFIX):] | |
| 12:50:50 | lyarwood | kk | |
| 12:50:54 | sean-k-mooney | and build the proerties differnetly so it only contianes the one with image_ | |
| 12:51:22 | sean-k-mooney | lyarwood:hehe | |
| 12:51:46 | lyarwood | so that would cause me to also add a system_metadata lookup in that util method | |
| 12:51:56 | lyarwood | that isn't the end of the world, it's what I had originally | |
| 12:52:05 | lyarwood | in a seperate change | |
| 12:52:30 | sean-k-mooney | well i wanted you to use image_ orginally as a prefix to avoid the extra lookup | |
| 12:52:35 | sean-k-mooney | we can do it either way | |
| 12:52:49 | sean-k-mooney | i was going to look at wriing the other patch to have someting backportable | |
| 12:52:54 | sean-k-mooney | assumeing we wanted too | |
| 12:53:07 | sean-k-mooney | if we dont then i can just finish fixing this patch | |
| 12:53:31 | lyarwood | that's also something I could do, always storing it as SM_IMAGE_PROP_PREFIX_hw_machine_type | |
| 12:53:41 | lyarwood | and that wouldn't bork me | |
| 12:53:45 | lyarwood | with this fix in place | |
| 12:53:57 | sean-k-mooney | ya | |
| 12:54:40 | sean-k-mooney | do you have an opipion on what way i should go with the fix. should i also do the version without the ovo change | |
| 12:59:18 | lyarwood | I don't at the moment tbh | |
| 12:59:33 | lyarwood | Let me post this and grab something to eat and then I'll think it through once I'm back | |
| 12:59:56 | sean-k-mooney | cool its more dod we need a backportable fix or not | |
| 13:00:21 | sean-k-mooney | my current fix uses the ovo field but we forgot to add at least 2 when we created the object | |
| 13:00:40 | sean-k-mooney | so it cant be backported but i can fix it without relying on the fields too | |
| 13:00:43 | sean-k-mooney | which would be | |
| 13:01:28 | sean-k-mooney | i can do both too so its not either or | |
| 13:02:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Record the machine_type of instances in system_metadata https://review.opendev.org/c/openstack/nova/+/767533 | |
| 13:02:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-manage: Add commands for managing instance machine type https://review.opendev.org/c/openstack/nova/+/769548 | |
| 13:02:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-status: Add hw_machine_type check for libvirt instances https://review.opendev.org/c/openstack/nova/+/770643 | |
| 13:04:24 | lyarwood | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/767533/5/nova/tests/functional/libvirt/test_machine_type.py - comments on the tests here would be appreciated btw | |
| 13:05:25 | sean-k-mooney | sure. i think i owe bauzas a review of his routed stuff first but i now have your open in front of me :) | |
| 13:08:51 | lyarwood | thanks | |
| 13:09:25 | bauzas | sean-k-mooney: I'll upload a new revision https://review.opendev.org/c/openstack/nova/+/773976 today later | |
| 13:10:37 | sean-k-mooney | bauzas: ok ill review lyarwood patch then now and ill look at yours when you push it | |
| 13:11:20 | bauzas | thanks | |
| 13:26:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove __unicode__() from nova unit test Exception https://review.opendev.org/c/openstack/nova/+/769894 | |
| 14:05:51 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add regression test for bug #1908075 https://review.opendev.org/c/openstack/nova/+/766976 | |
| 14:05:54 | openstack | bug 1908075 in OpenStack Compute (nova) "Nova allows a non-multiattach volume to be attached to multiple instances *if* its volume state is reset by an admin" [Undecided,In progress] https://launchpad.net/bugs/1908075 - Assigned to Lee Yarwood (lyarwood) | |
| 14:05:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: api: Reject volume attach requests when an active bdm exists https://review.opendev.org/c/openstack/nova/+/768472 | |
| 14:05:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: fup: Merge duplicate volume attachment checks https://review.opendev.org/c/openstack/nova/+/773380 | |
| 14:05:56 | lyarwood | stephenfin: ^ updated, would you mind hitting the changes below that fup as well? | |
| 14:37:03 | openstackgerrit | Ghanshyam proposed openstack/placement master: DNM: testing direct l-c https://review.opendev.org/c/openstack/placement/+/773813 | |
| 14:44:40 | lyarwood | sean-k-mooney: ^ sorry forgot to update the unit tests in that fist change | |
| 14:44:54 | lyarwood | argh git-review is still slow | |
| 14:45:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Record the machine_type of instances in system_metadata https://review.opendev.org/c/openstack/nova/+/767533 | |
| 14:45:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-manage: Add commands for managing instance machine type https://review.opendev.org/c/openstack/nova/+/769548 | |
| 14:45:16 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-status: Add hw_machine_type check for libvirt instances https://review.opendev.org/c/openstack/nova/+/770643 | |
| 14:55:32 | gibi | lyarwood, stephenfin: do we need both? https://review.opendev.org/c/openstack/nova/+/773727 https://review.opendev.org/c/openstack/nova/+/769920 | |
| 14:56:17 | stephenfin | Yes, I think so. The fixtures proves the stubbing isn't complete and would be useful even when it is to prevent regressions | |
| 14:56:40 | lyarwood | yeah what stephenfin said, already has shown a few things we missed AFAICT | |
| 15:04:35 | gibi | thanks | |
| 15:23:55 | gibi | I'm +2 on the fairly simple libvirt metadata feature https://review.opendev.org/c/openstack/nova/+/750552 | |
| 15:24:35 | gibi | so if some core has time then it is an easy win | |
| 15:26:35 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Add network and utils methods for getting routed networks and segments https://review.opendev.org/c/openstack/nova/+/773976 | |
| 15:26:36 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: Add a routed networks scheduler pre-filter https://review.opendev.org/c/openstack/nova/+/749068 | |
| 15:27:36 | bauzas | gibi: lemme look | |
| 15:28:03 | bauzas | gibi: btw. thanks for continuing to review the routed networks series | |
| 15:28:16 | gibi | bauzas: thanks | |
| 15:28:27 | bauzas | fwiw, I'm pretty done, just the last change needs to be having UTs and docs | |
| 15:33:09 | gibi | bauzas: ack, I will continue looking at it, actaully the self -1 made me stop so it is good that you stated now that it is basically ready | |
| 15:33:29 | bauzas | gibi: yeah I needed to add UTs | |
| 15:33:32 | bauzas | now it's done | |
| 15:33:44 | bauzas | those are easy peasy | |
| 15:37:00 | gibi | :) | |
| 15:44:31 | bauzas | gibi: concerns with reliability of the guest metadata information in https://review.opendev.org/c/openstack/nova/+/750552 | |
| 15:45:48 | sean-k-mooney | bauzas: reliablity? | |
| 15:46:03 | sean-k-mooney | this is an internal debug info | |
| 15:46:24 | sean-k-mooney | so if its a little out of sync i think its ok | |
| 15:47:24 | bauzas | sean-k-mooney: well, if so, we don't need it | |
| 15:47:45 | bauzas | operators could get their infos by other means, right? | |
| 15:47:45 | sean-k-mooney | we dont need it but it does make debuging from logs simpler | |
| 15:48:01 | sean-k-mooney | they could but this would be useful for us reading sosreports | |
| 15:48:06 | bauzas | sean-k-mooney: right, but then we need it to be reliable | |
| 15:48:07 | sean-k-mooney | where we cant | |
| 15:48:17 | sean-k-mooney | ya | |
| 15:48:22 | sean-k-mooney | well | |
| 15:48:29 | sean-k-mooney | it would be preferable | |
| 15:48:31 | bauzas | sean-k-mooney: I personnally voted on the spec because I do agree with the usecase | |
| 15:48:55 | bauzas | but if we go down the road, we need this information to be correct | |
| 15:48:59 | sean-k-mooney | i have not read your concern in context in the review | |
| 15:49:07 | sean-k-mooney | you belive there is a race in the code ? | |
| 15:49:15 | bauzas | right, when detaching | |
| 15:49:27 | sean-k-mooney | i see | |
| 15:49:37 | sean-k-mooney | if that can be fixed then i agree it shoudl be. | |
| 15:49:37 | bauzas | the proposer wrote to delete the info without waiting the neutron event | |
| 15:49:47 | bauzas | which could fail | |
| 15:50:17 | bauzas | and for most of the cases where operators would want to see the IPs, those would be for networking debugging | |
| 15:50:20 | sean-k-mooney | which neutron event? network-vif-unplugged? | |
| 15:50:29 | bauzas | yeah | |
| 15:50:36 | sean-k-mooney | we dont need to wait for that | |
| 15:50:37 | bauzas | sean-k-mooney: see the patch https://review.opendev.org/c/openstack/nova/+/750552 | |
| 15:50:45 | sean-k-mooney | we can but we dont need too. | |
| 15:51:12 | sean-k-mooney | once we detach it form libvirt its detacted form the vm | |
| 15:51:40 | sean-k-mooney | what could fail is removing the device owner(vm uuid) form the port | |
| 15:53:37 | bauzas | sean-k-mooney: sean-k-mooney: but then the IP would still be assigned to the instance, right? | |
| 15:53:44 | sean-k-mooney | this is the only place we use network-vif-unplugged i belvie https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L10079-L10083 | |