| opendevreview | Kiran Pawar proposed openstack/nova-specs master: SR-IOV NIC device tracking in Placement https://review.opendev.org/c/openstack/nova-specs/+/884569 | 05:28 |
|---|---|---|
| plibeau1 | hello guys, do you have sometime to review: https://review.opendev.org/c/openstack/nova/+/861172 | 08:54 |
| sean-k-mooney | plibeau1: i dont see any of the previous feedback adressed | 12:52 |
| sean-k-mooney | port creation should not be using the admin client | 12:52 |
| sean-k-mooney | port binding should be using the admin client or service user | 12:52 |
| opendevreview | ribaudr proposed openstack/nova master: Deletion of associated share mappings on instance deletion https://review.opendev.org/c/openstack/nova/+/881472 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add metadata for shares https://review.opendev.org/c/openstack/nova/+/850500 | 12:53 |
| 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 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support rebooting an instance with shares (compute manager part) https://review.opendev.org/c/openstack/nova/+/854824 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add share_info parameter to resume method for each driver (driver part) https://review.opendev.org/c/openstack/nova/+/860284 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support resuming an instance with shares (compute manager part) https://review.opendev.org/c/openstack/nova/+/860285 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add helper methods to rescue/unrescue shares https://review.opendev.org/c/openstack/nova/+/860286 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support rescuing an instance with shares (driver part) https://review.opendev.org/c/openstack/nova/+/860287 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support rescuing an instance with shares (compute manager part) https://review.opendev.org/c/openstack/nova/+/860288 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Allow to mount manila share using Cephfs protocol https://review.opendev.org/c/openstack/nova/+/883862 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add configuration option share_local_fs https://review.opendev.org/c/openstack/nova/+/884994 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add hw_share_local_fs extra specs and image property https://review.opendev.org/c/openstack/nova/+/884995 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add LOCAL storage type https://review.opendev.org/c/openstack/nova/+/884996 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add share_local_fs scheduler filter https://review.opendev.org/c/openstack/nova/+/884997 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add new api to retrieve local share (object and db) https://review.opendev.org/c/openstack/nova/+/884998 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Manage local share (driver part) https://review.opendev.org/c/openstack/nova/+/884999 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Manage local share (compute manager part) https://review.opendev.org/c/openstack/nova/+/885000 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Do not mount local share at startup (init_instance) https://review.opendev.org/c/openstack/nova/+/885001 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Do not umount local share if instance id deleted https://review.opendev.org/c/openstack/nova/+/885002 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support spawn an instance with a local share (driver part) https://review.opendev.org/c/openstack/nova/+/885003 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Support spawn an instance with a local share (compute manager part) https://review.opendev.org/c/openstack/nova/+/885004 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Attach Manila shares via virtiofs (API) https://review.opendev.org/c/openstack/nova/+/836830 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Check shares support (API) https://review.opendev.org/c/openstack/nova/+/850499 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_attach notification https://review.opendev.org/c/openstack/nova/+/850501 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_detach notification https://review.opendev.org/c/openstack/nova/+/851028 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add shares to InstancePayload https://review.opendev.org/c/openstack/nova/+/851029 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_attach_error notification https://review.opendev.org/c/openstack/nova/+/860282 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add instance.share_detach_error notification https://review.opendev.org/c/openstack/nova/+/860283 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add libvirt test to ensure metadata are working. https://review.opendev.org/c/openstack/nova/+/852086 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add virt/libvirt error test cases https://review.opendev.org/c/openstack/nova/+/852087 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Docs about Manila shares API usage https://review.opendev.org/c/openstack/nova/+/871642 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Check shares support (compute manager) https://review.opendev.org/c/openstack/nova/+/885751 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Check shares support (only API exception) https://review.opendev.org/c/openstack/nova/+/885752 | 12:53 |
| opendevreview | ribaudr proposed openstack/nova master: Add helper methods to attach/detach shares https://review.opendev.org/c/openstack/nova/+/885753 | 12:53 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/881457 | 12:59 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 12:59 |
| opendevreview | Amit Uniyal proposed openstack/nova master: WIP: Refactor CinderFixture https://review.opendev.org/c/openstack/nova/+/885756 | 12:59 |
| Uggla_ | sean-k-mooney, bauzas ^ I have reorganized the patch in the way that API is merged after introducing manilla_shares and scaphandre. I have also flaged the patches in 2 differents topics. Is it ok for ypu ? | 13:01 |
| sean-k-mooney | Uggla_: the scaphandre changes are still after the db and object changes | 13:05 |
| Uggla_ | sean-k-mooney, yep because I need them | 13:05 |
| sean-k-mooney | you shoudl not need themn | 13:05 |
| sean-k-mooney | because the scaphandre code shoudl not be using the db or ojbect changes form the manial feature | 13:06 |
| Uggla_ | sean-k-mooney, I define a LOCAL share type --> and this to use metadata and block the ops due to virtiofs. | 13:06 |
| sean-k-mooney | the scaphandre usage fo virtio fs si not a share | 13:07 |
| sean-k-mooney | we shoudl not have a LOCAL share type | 13:07 |
| sean-k-mooney | i assumne you did it that way to mirror the block device mappings... | 13:08 |
| Uggla_ | sean-k-mooney, also to share some common code parts. | 13:09 |
| sean-k-mooney | ya so i disgree with that approch | 13:09 |
| sean-k-mooney | if its not a manial share it should not be modeled using share objects | 13:09 |
| Uggla_ | sean-k-mooney, hum, but it so "close" so I though to reuse a maximum of stuff. | 13:13 |
| sean-k-mooney | well im -2 on that and it is not what we approved in the spec | 13:15 |
| sean-k-mooney | i need ot go to the pharmacy quickly but there are many sideffect to the approch you have taken that i dont think are correct form a design or mantaince point of view | 13:16 |
| sean-k-mooney | we do not intent to allwo the share attapchment api to attach/detach "local shares" if we did that would eb a sperate feature and spec form either of the two you are working on | 13:16 |
| sean-k-mooney | ill hopefully be back in 30-45 mins | 13:17 |
| Uggla_ | ok | 13:17 |
| bauzas | Uggla_: sean-k-mooney: I need to review the scaphandre patches tbh, but I don't have any problem to use a specific local share type | 13:17 |
| bauzas | so we could persist it | 13:17 |
| Uggla_ | sean-k-mooney, bauzas to be honest I thought that was a not so bad approach as 90% of the code is the same except that the mountpoint on the compute does not come from manila. | 13:23 |
| Uggla_ | but let's wait sean-k-mooney to be back. | 13:23 |
| dansmith | I haven't reviewed them, but if the idea is to create share objects for the local shares as if they were manila shares, I agree that seems wrong | 13:41 |
| sean-k-mooney | that is the current approch | 13:42 |
| sean-k-mooney | if we were to have a local share type and model it tha tway i would expect use to provde a way for end users to attach/detach them and have nova create a folder dynmicaly to hold the content and copy the content form one host to anohter on cold migration | 13:43 |
| sean-k-mooney | the share attchment api as propsoed is not desginse to hanel that | 13:43 |
| sean-k-mooney | for scaphandre there is nor requiremtn to copy data on cold migration it doesnt even make sense in that specific case | 13:44 |
| dansmith | presumably it means the user sees that local share in the shares api, and (hopefully) is barred from deleting it | 13:44 |
| sean-k-mooney | the share api has no way to speicf a capasity | 13:44 |
| sean-k-mooney | dansmith: ya so we woudl not want it to be listed or deletable as that provides a second way to configure scaphandre mounts | 13:45 |
| dansmith | right | 13:45 |
| sean-k-mooney | the only thingn the manilla shares feature and scaphandre have in common is they both use virtio fs | 13:46 |
| dansmith | right | 13:46 |
| dansmith | so I would treat these local shares as "do whatever the conf says" that we handle when we're building the xml on reboot or whatever | 13:46 |
| sean-k-mooney | the flavor extra spec/image porpety is the only metadta reiqured ot know if the feature is used | 13:46 |
| dansmith | and not represent them in the database, api, etc | 13:46 |
| dansmith | more like configdrive than anything else | 13:46 |
| sean-k-mooney | yes | 13:47 |
| sean-k-mooney | configdrive is a good example | 13:47 |
| sean-k-mooney | by the way i can see usecasue for being able to have local storage on a host exposed to the guest as virtio-fs passthough. i just dont htinke the share attachemt feature is what we should use to build that | 13:49 |
| dansmith | you mean more use cases than just scaphandre right? I definitely can | 13:49 |
| sean-k-mooney | that is its own seperate feature | 13:50 |
| dansmith | but not user-controllable and not visible like an attachment | 13:50 |
| sean-k-mooney | well i coudl see it being user contoleable but there is a quota aspect that woudl have to be adress | 13:50 |
| dansmith | when would we ever want to allow the user to request "show me /etc on the host" ? | 13:51 |
| sean-k-mooney | that not what i mean | 13:51 |
| sean-k-mooney | i mean we coudl allow the user to have addtional storage form a folder we create on the host like ephemeal storage | 13:51 |
| dansmith | oh sure, right | 13:52 |
| dansmith | not sure we should expose that as a share in the same way as manila though | 13:52 |
| sean-k-mooney | i.e. a way to add addtional sotrage without having to do it via a resize | 13:52 |
| sean-k-mooney | ya its a seperate thing | 13:52 |
| dansmith | agree | 13:52 |
| dansmith | I was just typing out "we might want to expose ephemeral disks as filesystems instead" before you said it | 13:53 |
| dansmith | so yeah, I can definitely see that, but I think that's a sort of third use case for this | 13:53 |
| sean-k-mooney | its more "As a user i would like to be ablt to attch addtion local storage to an instance without needign to resize" | 13:53 |
| dansmith | first being manila, second being "the admin wants this configdrive-like thing inserted into me", and third epehemeral fs space | 13:53 |
| dansmith | well, resize or mkfs but yeah | 13:54 |
| sean-k-mooney | honestly the motivation for ti is pretty low until you can do it while the vm is running | 13:54 |
| dansmith | I can also imagine people wanting two instances with hard affinity to share a fast local disk as a filesystem | 13:55 |
| dansmith | i.e. db-and-http workers and things | 13:56 |
| sean-k-mooney | anyway Uggla_ i have concerns with the current apptoch. i have not done a full review of the patchs but i thing directionally we shoudl try and only share the virtiofs xml generation btween these two features | 13:56 |
| dansmith | but the local share thing should really be a tack-on at the end of this right? | 13:56 |
| dansmith | the set is extremely long right now so just breaking that stuff off the end for now would make sense to me and focus on getting the manila bit working as the primary goal | 13:56 |
| sean-k-mooney | well i atcully dont think we shoudl do the local share as part of this work this cyle | 13:57 |
| sean-k-mooney | i was ortgnallly hoping that the virtiofs xml change woudl be first then scaphandre abd mainila could progess in parallel | 13:58 |
| sean-k-mooney | but i entrily expect us to be able to merge scaphandre support without any of the manilla supprot being merge outside the virtiofs part in the libvirt driver | 13:58 |
| dansmith | yes I would think so too, but you're also saying do that after manila (now) right? | 13:59 |
| sean-k-mooney | not really | 13:59 |
| dansmith | [06:57:02] <sean-k-mooney> well i atcully dont think we shoudl do the local share as part of this work this cyle | 13:59 |
| dansmith | what does that mean then? | 13:59 |
| sean-k-mooney | the intoduction of a local share type | 14:00 |
| sean-k-mooney | if we were to have that i would prefer to defer that to the other uscase of epmperal fs or simialr | 14:00 |
| dansmith | the scaphandre case is just a special configuration of a local share no? | 14:00 |
| sean-k-mooney | im conlifcted on how to respond to that | 14:00 |
| sean-k-mooney | i want to say no because i dont think scaphandre's use of virtio-fs shoudl me modeled as a share in the db | 14:01 |
| dansmith | by "local share" I'm meaning the "operator has configured nova to always insert this share into all guests on this host" | 14:01 |
| sean-k-mooney | so its a special case fo virtio-fs | 14:01 |
| dansmith | maybe we need to define some terms | 14:01 |
| sean-k-mooney | i would cinsider it a special case foinsterting this "filesystem path" into all guest that request it | 14:02 |
| dansmith | okay I'm a bit confused then | 14:02 |
| sean-k-mooney | i would reall like to keep "share" to refer only to manialla shares | 14:02 |
| dansmith | I agree with that | 14:02 |
| sean-k-mooney | if we want to allow arbairy addtional filesystem to be provided to a guest via an api that woudl be a new filesystem attach api | 14:03 |
| sean-k-mooney | for scaphandre as you said i think config drive is a better mental model | 14:03 |
| sean-k-mooney | we have an image property/extra spec that act as a request for it and a trait on host that supprot it | 14:04 |
| dansmith | how about a 15 minute meet on the topic real quick? | 14:04 |
| sean-k-mooney | there is no requirement for modileign it in the db or as object or in the api beyond the image property/extra spec | 14:04 |
| dansmith | Uggla_: ? | 14:04 |
| sean-k-mooney | sure i can do that | 14:06 |
| dansmith | meet.google.com/duc-bfai-mrv | 14:06 |
| sean-k-mooney | anyone who is interested is welcome to join by the way ^ | 14:07 |
| dansmith | yup | 14:07 |
| Uggla_ | dansmith, sean-k-mooney sorry was out to get Rose, reading | 14:14 |
| bauzas | dansmith: sean-k-mooney: Uggla_ : doh, I need to go to get my daughter in 5 mins | 14:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!