opendevreview | Callum Dickinson proposed openstack/nova master: Fix image ID in libvirt metadata when unshelving https://review.opendev.org/c/openstack/nova/+/942973 | 00:09 |
---|---|---|
opendevreview | Callum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata https://review.opendev.org/c/openstack/nova/+/942766 | 03:00 |
opendevreview | Callum Dickinson proposed openstack/nova master: Add more flavor metadata to libvirt guest XML https://review.opendev.org/c/openstack/nova/+/942974 | 05:03 |
opendevreview | Takashi Kajinami proposed openstack/nova master: Drop quota options from the DEFAULT section https://review.opendev.org/c/openstack/nova/+/943131 | 08:36 |
opendevreview | Tobias Urdin proposed openstack/nova master: wip: Reserve volumes when doing instance resize https://review.opendev.org/c/openstack/nova/+/942985 | 09:04 |
tobias-urdin | when testing above^ patch i just realized that you cannot reserve a cinder volume when status is in-use, based on the issue described there i guess the only two alternatives to solve the problem in that patch is either 1) add a simple check for volume status and live with the potential race condition between check of volume status and actual _terminate_volume_connections call | 09:08 |
tobias-urdin | or 2) catch InvalidVolume exceptions directly in _terminate_volume_connections and try to rollback volume attachments and then raise a InstanceFaultRollback exception, anybody else has any better ideas? | 09:08 |
opendevreview | ribaudr proposed openstack/nova master: Update driver to map the targeted address for SR-IOV PCI devices https://review.opendev.org/c/openstack/nova/+/942147 | 09:37 |
opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | 09:37 |
sean-k-mooney | tobias-urdin: i have not looked into your patches yet but waht was the problem you were tryign to resolve? | 09:44 |
sean-k-mooney | tobias-urdin: i vagely saw them coming in last week but didnt have time to read them | 09:44 |
sean-k-mooney | tobias-urdin: oh ok... so thisi is kind of a cinder bug | 09:46 |
sean-k-mooney | tobias-urdin: so the problem is that cinder does not knwo that nova is in the middle of doing a resize and nova does not know that cidner wants to do a backup | 09:46 |
tobias-urdin | sean-k-mooney: if i try to resize and instance it goes to ERROR and needs operator to restore it (shutdown has already been called) because attachment_create() volume API raises InvalidVolume when an attached volume has backing-up status | 09:46 |
sean-k-mooney | so we can get in a broken state | 09:46 |
sean-k-mooney | tobias-urdin: well you can resize instnace with cinder volumes | 09:47 |
sean-k-mooney | that not the problem its because you also happen to have a cinder backup at the same time | 09:47 |
sean-k-mooney | tobias-urdin: both nova and cidner backup exepct to be the only thing modifying the volume state when doign a resize or backup | 09:48 |
tobias-urdin | yea, i could just loop volumes and check status in the context and raise InstanceFaultRollback i guess for resize to not hit it, but there would still be a race between me checking the status and attachment code being called in resize | 09:49 |
sean-k-mooney | that not really the fix | 09:49 |
tobias-urdin | i was hoping i could reserve the volumes before starting the operation, but my research failed because it didn't accommodate for in-use not being possible on in-use volume :( | 09:49 |
sean-k-mooney | we either need nova to annotate the volume with somethjing to indicate that we are migrating | 09:50 |
sean-k-mooney | or we need to have cidner check the status of attached instnace before doing a backup | 09:50 |
sean-k-mooney | tobias-urdin: is cidner backup changing the state of the volume to something other then inuse | 09:51 |
tobias-urdin | yes "backing-up" is set | 09:51 |
sean-k-mooney | right os that also a bug | 09:51 |
sean-k-mooney | once a volume is attached to a nova instnace we do not exect the voluem status to change unless its via a call to the nova api | 09:52 |
sean-k-mooney | so form a nova perspecitve it should remain in use | 09:52 |
sean-k-mooney | it would be fine to have a seperate task_state that backing-up | 09:53 |
sean-k-mooney | but if cidner want to modify it they need to make the attchment api treat backing-up the same as in-use | 09:54 |
tobias-urdin | that actually makes more sense | 09:54 |
sean-k-mooney | we have a bunch of task_state that we translate to "active" because semanticly external service should treat them the same | 09:55 |
sean-k-mooney | i dont know how to express that in cidner api but i think if they have internal states liek backing-up they may need a similar mechanisum | 09:56 |
sean-k-mooney | like this https://github.com/openstack/nova/blob/master/nova/api/openstack/common.py#L46-L63 | 09:59 |
tobias-urdin | sean-k-mooney: thanks for the directions! i will investigate further and see if i can find a better path forward | 10:03 |
sean-k-mooney | ill try an comment on the bug and maybe a good dicusion for the ptg in a nova/cinder cross project if we dont have a path forward by then | 10:04 |
tobias-urdin | sean-k-mooney: ack, looks like they account for some migration logic in _attachment_reserve() https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L2409 | 10:08 |
tobias-urdin | so imo handling backing-up (if "previous status", that it sets back when backup is complete, is in-use) as if it were in-use makes sense | 10:09 |
sean-k-mooney | tobias-urdin: https://bugs.launchpad.net/nova/+bug/2077512/comments/5 i tried to summeriese my toughts and i added cinder to the bug | 10:44 |
tobias-urdin | sean-k-mooney: this is the real path forward https://review.opendev.org/c/openstack/cinder-specs/+/915973 instead of my new hack approach https://review.opendev.org/c/openstack/cinder/+/943176 | 13:10 |
opendevreview | Merged openstack/nova master: Update driver to deal with managed flag https://review.opendev.org/c/openstack/nova/+/938405 | 13:54 |
sean-k-mooney | tobias-urdin: ah yes that does seam to be adressign this edgecase | 14:04 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!