| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-26 | |||
| 17:40:44 | sean-k-mooney | dansmith: at that point have we "bound" the new attachmet propelry and got all the connector info so we can update the xml and proceed | |
| 17:41:24 | sean-k-mooney | presuamable deleteing the attcoemnt woudl only fail if cidner was unaviable or something like that | |
| 17:41:28 | dansmith | sean-k-mooney: well this is before the reimage, but yeah, I'm saying if we've recorded the new attachment and can't delete the old one, I'm not sure it gets us much to revert our own db record to the old attachment and then try to delete the new one | |
| 17:41:49 | sean-k-mooney | ah | |
| 17:41:55 | gibi | dansmith: so the root_bdm.attachment_id points to the new attachement_id and we just deleted that in cinder | |
| 17:42:06 | dansmith | reverting to the old one and deleting the new one is a more complicated cleanup, which I can do, but I just want to make sure it's worth it | |
| 17:42:07 | gibi | by L3441 | |
| 17:42:46 | dansmith | gibi: that's what I'm saying.. I think if we hit a cinder error in L3437, we should NOT do L3441 and abort | |
| 17:43:05 | gibi | dansmith: OK, that make sense | |
| 17:43:10 | dansmith | the clientexception will catch errors on L3432 *and* 3437 | |
| 17:43:14 | gibi | keep the bdm to use the new attachment id | |
| 17:43:34 | dansmith | ++ okay, it's much cleaner (on our side) to do that, but just wanted to make sure that was reasonable | |
| 17:44:10 | gibi | but we still need to handle the fact that we might detached the volume from the guest already at 3434 | |
| 17:44:54 | sean-k-mooney | i kind of feel like it migh make more sesen to break up that try | |
| 17:45:08 | dansmith | sean-k-mooney: s'what I'm doing (and said in the review) | |
| 17:45:25 | sean-k-mooney | ack | |
| 17:45:25 | dansmith | gibi: yeah, so I'm going to delete the new attachment if we fail to save, but otherwise, I'm going to log both and the situation and plow on | |
| 17:46:33 | gibi | and we say that the instance can be recovered with a hard reboot from this case? | |
| 17:46:49 | gibi | as at that point when the save fail we removed the volume from the guest | |
| 17:47:00 | dansmith | I don't know that we need even that | |
| 17:47:06 | sean-k-mooney | so the exception.InstanceNotFound is coming form the detac potieintally or can it come form the create too | |
| 17:47:58 | sean-k-mooney | gibi: if the bdms are correct and the atachment we created on line 3432 | |
| 17:48:04 | dansmith | sean-k-mooney: only the bdm save, AFAIK | |
| 17:48:07 | sean-k-mooney | is correct then a hard reboot might work | |
| 17:48:17 | dansmith | why do we need a hard reboot? | |
| 17:48:28 | dansmith | we've fixed the instance to point to the new attachment, we can just move on with the reimage | |
| 17:48:29 | dansmith | we' | |
| 17:48:46 | dansmith | we will have leaked an attachment that cinder didn't let us delete but... that doesn't impact the instance anymore right? | |
| 17:48:51 | gibi | ahh OK | |
| 17:48:52 | gibi | I see now | |
| 17:48:59 | gibi | I thought we would abort | |
| 17:49:07 | dansmith | I wish I could push this up, but I'll rebase the userdata one if I do :/ | |
| 17:49:20 | sean-k-mooney | ya that was what i was workign my way thoguht we abort and go to error | |
| 17:49:21 | dansmith | we abort all cases, except the last delete of the old attachment | |
| 17:49:28 | sean-k-mooney | but if we move on | |
| 17:49:32 | gibi | OK, that is fine | |
| 17:49:32 | dansmith | because it doesn't matter at that point | |
| 17:49:34 | sean-k-mooney | as you suggest no reboot needed | |
| 17:49:45 | sean-k-mooney | actully | |
| 17:49:51 | sean-k-mooney | if we get instance not found form our own db | |
| 17:50:02 | sean-k-mooney | well no form libvirt | |
| 17:50:11 | sean-k-mooney | prefumably that means we raced with a delete? | |
| 17:50:17 | dansmith | on bdm save, yeah | |
| 17:50:27 | sean-k-mooney | ya so nothing to try and recover | |
| 17:51:50 | sean-k-mooney | dansmith: you can do git review -R by the way to avoid a rebase | |
| 17:52:13 | dansmith | sean-k-mooney: no, I've already rebased user data locally so I could get this rebased and ready | |
| 17:52:51 | sean-k-mooney | ah ok well it would not be the end fo the world if it was rebased. it needs to be updated for the api sample issue anyway | |
| 17:53:30 | dansmith | I just don't want to step on the other author if they already have things in the middle | |
| 17:54:15 | sean-k-mooney | ack, but yes it sounds like you can just proceed and leak the attacment and may just log it so an op could clean it up later | |
| 17:56:39 | sean-k-mooney | so whats left is gettin gth image form galnce. using that to calualte the size, setting up the wait for the external event and then calling cinder to do the reimage | |
| 17:56:51 | opendevreview | Balazs Gibizer proposed openstack/nova master: Handle PCI dev reconf with allocations https://review.opendev.org/c/openstack/nova/+/852397 | |
| 17:56:52 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | |
| 17:56:52 | opendevreview | Balazs Gibizer proposed openstack/nova master: Generate request_id for Flavor based InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/853835 | |
| 17:56:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Create RequestGroups from InstancePCIRequests https://review.opendev.org/c/openstack/nova/+/852771 | |
| 17:56:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support resource_class and traits in PCI alias https://review.opendev.org/c/openstack/nova/+/853316 | |
| 17:56:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Split PCI pools per PF https://review.opendev.org/c/openstack/nova/+/854440 | |
| 17:56:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Map PCI pools to RP UUIDs https://review.opendev.org/c/openstack/nova/+/854118 | |
| 17:56:55 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make allocation candidates available for scheduler filters https://review.opendev.org/c/openstack/nova/+/854119 | |
| 17:56:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Filter PCI pools based on Placement allocation https://review.opendev.org/c/openstack/nova/+/854120 | |
| 17:56:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Store allocated RP in InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/854121 | |
| 17:56:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Func test for PCI in placement scheduling https://review.opendev.org/c/openstack/nova/+/854122 | |
| 17:56:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support cold migrate and resize with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | |
| 17:57:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support evacuate with PCI in placement https://review.opendev.org/c/openstack/nova/+/854615 | |
| 17:57:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support unshelve with PCI in placement https://review.opendev.org/c/openstack/nova/+/854616 | |
| 17:57:02 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | |
| 17:57:02 | opendevreview | Balazs Gibizer proposed openstack/nova master: Test reschedule with PCI in placement https://review.opendev.org/c/openstack/nova/+/854626 | |
| 17:57:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support multi create with PCI in placement https://review.opendev.org/c/openstack/nova/+/854663 | |
| 17:57:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Heal allocation for same host resize https://review.opendev.org/c/openstack/nova/+/854822 | |
| 17:58:06 | dansmith | ugh, that causes another failure later, which may be a quirk of our cinder fixture | |
| 17:58:19 | dansmith | the attachment isn't deleted, so much later it complains that the instance is double-attached | |
| 17:58:46 | sean-k-mooney | well form the cinder side i guess it is | |
| 17:59:04 | sean-k-mooney | we could try and recover at that point and delete the old one again | |
| 17:59:16 | sean-k-mooney | which would possisble stop the leak | |
| 17:59:18 | dansmith | that's what I'm trying to avoid because it ends up a pretty nested mess | |
| 17:59:24 | sean-k-mooney | ah ok | |
| 17:59:38 | dansmith | and it's likely to fail in reality if we just failed to delete | |
| 18:00:27 | sean-k-mooney | we are not doing the attchment delete before the save to prevent the other case right | |
| 18:00:27 | dansmith | I'm also not sure I know why we're detaching and re-attaching here, just to do the reimage | |
| 18:00:38 | sean-k-mooney | where our db is now out of sync if we fail to save | |
| 18:00:41 | dansmith | must be some cinder reason why that happens, to reset the state while the instance is powered off or something | |
| 18:00:58 | dansmith | yeah I think we have to update our db before the delete for races | |
| 18:01:11 | sean-k-mooney | ya i think so too. | |
| 18:01:58 | opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (db) https://review.opendev.org/c/openstack/nova/+/831193 | |
| 18:01:59 | opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (objects) https://review.opendev.org/c/openstack/nova/+/839401 | |
| 18:01:59 | opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (manila abstraction) https://review.opendev.org/c/openstack/nova/+/831194 | |
| 18:02:00 | opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (drivers and compute manager part) https://review.opendev.org/c/openstack/nova/+/833090 | |
| 18:02:00 | opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (api) https://review.opendev.org/c/openstack/nova/+/836830 | |
| 18:02:01 | opendevreview | ribaudr proposed openstack/nova master: Bump compute version and check shares support https://review.opendev.org/c/openstack/nova/+/850499 | |
| 18:02:02 | opendevreview | ribaudr proposed openstack/nova master: Add metadata for shares https://review.opendev.org/c/openstack/nova/+/850500 | |
| 18:02:02 | opendevreview | ribaudr proposed openstack/nova master: Add instance.share_attach notification https://review.opendev.org/c/openstack/nova/+/850501 | |
| 18:02:03 | opendevreview | ribaudr proposed openstack/nova master: Add instance.share_detach notification https://review.opendev.org/c/openstack/nova/+/851028 | |
| 18:02:04 | opendevreview | ribaudr proposed openstack/nova master: Add shares to InstancePayload https://review.opendev.org/c/openstack/nova/+/851029 | |
| 18:02:04 | opendevreview | ribaudr proposed openstack/nova master: Add instance.power_on_error notification https://review.opendev.org/c/openstack/nova/+/852084 | |
| 18:02:06 | opendevreview | ribaudr proposed openstack/nova master: Add instance.power_off_error notification https://review.opendev.org/c/openstack/nova/+/852278 | |
| 18:02:06 | opendevreview | ribaudr proposed openstack/nova master: Add helper methods to attach/detach shares https://review.opendev.org/c/openstack/nova/+/852085 | |
| 18:02:08 | opendevreview | ribaudr proposed openstack/nova master: Add libvirt test to ensure metadata are working. https://review.opendev.org/c/openstack/nova/+/852086 | |
| 18:02:08 | opendevreview | ribaudr proposed openstack/nova master: Add virt/libvirt error test cases https://review.opendev.org/c/openstack/nova/+/852087 | |
| 18:02:10 | opendevreview | ribaudr proposed openstack/nova master: Change microversion to 2.93 https://review.opendev.org/c/openstack/nova/+/852088 | |
| 18:02:10 | opendevreview | ribaudr proposed openstack/nova master: Add share_info parameter to reboot method for each driver (driver part) https://review.opendev.org/c/openstack/nova/+/854823 | |
| 18:02:12 | opendevreview | ribaudr proposed openstack/nova master: Support rebooting an instance with shares (compute and API part) https://review.opendev.org/c/openstack/nova/+/854824 | |