| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-02-21 | |||
| 14:01:21 | gibi | bauzas: during the attach REST API call | |
| 14:01:44 | bauzas | ah, now I remember, that's the async RPC call | |
| 14:02:04 | gibi | if I understood Sean correctly they want to do the host mount during spawn instead | |
| 14:02:13 | sean-k-mooney | yep | |
| 14:02:28 | bauzas | I'm fine whenever | |
| 14:02:52 | sean-k-mooney | i would also expect the share to be unmoutned when we stop the vm for what its worth | |
| 14:02:58 | bauzas | my point is that the share mount can be done whenever we want, at least *before* we start | |
| 14:03:14 | sean-k-mooney | because i would expect hard reboot to unmount it and remount it | |
| 14:03:24 | sean-k-mooney | since we normally use hard reboot to fix things like that | |
| 14:03:24 | bauzas | sean-k-mooney: you can attach the same mount multiple times | |
| 14:03:30 | sean-k-mooney | thats fine | |
| 14:03:53 | sean-k-mooney | the mount and unmount could be based on the first/last active vm | |
| 14:04:16 | sean-k-mooney | or better each instance could have its onw mount and we could avoid reusign it. that what i orgianly wanted in the spec | |
| 14:04:25 | bauzas | sean-k-mooney: fwiw, in the spec it says that an instance stop does make the share umounted | |
| 14:04:30 | sean-k-mooney | to have the same share mounted multipel times in seperat locations | |
| 14:04:35 | sean-k-mooney | but i know that is not what is in the code | |
| 14:04:37 | bauzas | so it's already fine | |
| 14:04:51 | bauzas | (if the implementation is the same) | |
| 14:04:51 | sean-k-mooney | so a few weeks ago i was sruprised | |
| 14:05:01 | gibi | I guess the RPC during the attach REST API call is created to prepare for the future when a share can be mounted live without a stop/start cycle. | |
| 14:05:02 | sean-k-mooney | wehn the share was not mounted under the instance state dir | |
| 14:05:28 | sean-k-mooney | because i was orgianlly expecting the share not to be shared betwen instnace on the same host | |
| 14:05:28 | bauzas | I think the word "inactive" is correct | |
| 14:05:39 | bauzas | nothing is using the share | |
| 14:05:52 | bauzas | but we still have the reservation | |
| 14:06:13 | sean-k-mooney | bauzas: the active/inactive state of a share mapping cant be mapped ot if its mounted on the hsot | |
| 14:06:27 | bauzas | I hope so | |
| 14:06:32 | sean-k-mooney | since if we are sharing the mount point betwen vms it woudl depend on if other vms on the host are using it | |
| 14:06:59 | bauzas | yeah, inactive just means that the instance doesn't use the share but it's still 'mapped' to it | |
| 14:07:13 | bauzas | whether the share is umounted or not, that's something internal | |
| 14:07:39 | bauzas | as you say, the share could be continued to be mounted, because used by other instances | |
| 14:07:55 | bauzas | now, back to the original question from gibi, | |
| 14:08:08 | bauzas | there are two possibilities | |
| 14:08:13 | sean-k-mooney | for me active means we have done what is required in nova and maniall so that the vm can use it if you start the vm | |
| 14:08:52 | Uggla | sean-k-mooney, no active means VM is up and uses the share. | |
| 14:09:07 | sean-k-mooney | then it has no extra info | |
| 14:09:15 | bauzas | IMHO, I think that a detach action that eventually goes into unmounting and which is impossible to do so should make the instance action on error | |
| 14:09:17 | sean-k-mooney | that is what instnace status=active means | |
| 14:09:32 | bauzas | or we need a periodic | |
| 14:09:49 | bauzas | but honestly, I think we should say the truth | |
| 14:10:22 | sean-k-mooney | the active status of a share mappign shoudl not depend on the vm status (active vs powered off) | |
| 14:10:41 | bauzas | that's written in the spec | |
| 14:10:42 | sean-k-mooney | these are two diffenet logical resouces in the api | |
| 14:11:36 | gibi | bauzas: it is not about the instance action but about the state of the sharemapping. We can keep it inactive or we can put it in error. | |
| 14:12:37 | bauzas | gibi: I got that, lemme summarize the exception handling case | |
| 14:12:59 | bauzas | user stops a VM, share mapping goes inactive | |
| 14:13:12 | sean-k-mooney | looking at the Share mapping status diagram in the spec | |
| 14:13:21 | bauzas | when it stops the VM, some internal call is doing two actions : | |
| 14:13:34 | bauzas | 1/ detach the share from the VM | |
| 14:13:43 | sean-k-mooney | i can see where you were going with it but it does not follow the same conventiosn as cinder or neutron ports | |
| 14:13:47 | bauzas | 2/ umount the share if not used elsewhere | |
| 14:14:04 | gibi | bauzas: OK so you suggest to do the unmount during stop, not during the detach REST API call | |
| 14:14:20 | sean-k-mooney | well the spec says we do the unmount during stop | |
| 14:14:21 | bauzas | nah, sorry | |
| 14:14:28 | bauzas | I'm waaaay confused | |
| 14:14:39 | bauzas | user has to do two actions then | |
| 14:14:44 | bauzas | stop then detach | |
| 14:14:50 | bauzas | right? | |
| 14:14:56 | Uggla | yep | |
| 14:15:00 | bauzas | and detach does two things | |
| 14:15:04 | bauzas | (internally) | |
| 14:15:16 | bauzas | one is updating the guest XML | |
| 14:15:23 | gibi | nope | |
| 14:15:25 | bauzas | the other is to umount the share | |
| 14:15:32 | gibi | the vm is stopped no need to update the XML | |
| 14:15:41 | Uggla | gibi ++ | |
| 14:15:57 | bauzas | my head is full of vGPU crap for now | |
| 14:16:03 | gibi | the XML is generated during start based on the share_mappings | |
| 14:16:05 | bauzas | I have to reload that context | |
| 14:16:12 | sean-k-mooney | the vm is stopped meaning the share is also unmoutned already | |
| 14:16:17 | gibi | not | |
| 14:16:21 | gibi | today | |
| 14:16:31 | sean-k-mooney | in the spec that what the diagram states | |
| 14:16:39 | gibi | today stop is not unmounting the share from the host | |
| 14:16:50 | gibi | OK, then that is a gap between the spec and the code | |
| 14:16:51 | sean-k-mooney | well that is not in line with the spec then | |
| 14:16:54 | sean-k-mooney | yep | |
| 14:17:09 | sean-k-mooney | what in the spec i think aligns with what i would like to see | |
| 14:17:09 | gibi | do we want to move the host unmount to stop? | |
| 14:17:25 | sean-k-mooney | if we do that then detach cant fail in this edge case | |
| 14:17:35 | gibi | and do we want to fail the stop action if unmount fails? | |
| 14:18:02 | bauzas | do we all agree on the fact that unmount is not strictly required ? | |
| 14:18:15 | sean-k-mooney | and i belive unmounting the share (if no other vm is using it) in stop is also more secure | |
| 14:18:18 | gibi | bauzas: do you mean, is it OK to leak mounts? | |
| 14:18:25 | sean-k-mooney | bauzas: no | |
| 14:18:57 | gibi | I'm not sure what happens with a mount if the export on the manila side is removed | |
| 14:18:59 | bauzas | umount is not strictly required if we still have a share mapping, right ? | |
| 14:19:21 | gibi | bauzas: if you mean we have a record of the mount so we can do the unmount later, then yes | |
| 14:19:21 | sean-k-mooney | as far as im concerned the attach and detach rps shoudl not try to mount/unmount the share | |
| 14:19:55 | bauzas | gibi: I'm going back to my original statement : I don't want to have share leaks | |
| 14:20:26 | bauzas | gibi: if we're not able to guarantee that some share mapping state will continue to handle shares that are sitll residues on the compute, then we need to error the detach call | |
| 14:20:27 | sean-k-mooney | form the spec | |
| 14:20:30 | sean-k-mooney | """Share attachment/detachment can only be done if the VM state is STOPPED or ERROR. These are operations only on the database, and no RPC calls will be required to the compute API. This is an intentional design for this spec. As a result, this could lead to situation where the VM start operation fails as an underlying share attach fails.""" | |
| 14:20:41 | sean-k-mooney | that ^ is what the code should be doing | |
| 14:20:41 | gibi | bauzas: I agree | |
| 14:21:21 | bauzas | the phi-state in the diagram actually needs to be explained more | |
| 14:21:27 | gibi | OK. so If we follow that statement from the spec then both the mount and unmount RPC is unnecessary | |
| 14:21:45 | sean-k-mooney | correct | |
| 14:21:51 | sean-k-mooney | its just a cell db update | |
| 14:21:58 | bauzas | I have a vgpu meeting in 8 mins so I'll need to bail out shortly | |