| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-28 | |||
| 14:22:35 | mriedem | also, | |
| 14:22:39 | mriedem | this is jumping the gun a bit | |
| 14:22:49 | mdbooth | mriedem: Yeah, that one | |
| 14:23:14 | mdbooth | mriedem: Originally I thought so, which is why I removed it promptly when it turned out it was problematic. | |
| 14:23:27 | mriedem | i think the plan was, | |
| 14:23:43 | mdbooth | However, I keep hearing multi-attach noises, and this change allows it to work with multi-attach | |
| 14:23:50 | mdbooth | I don't want my code to be a barrier to that | |
| 14:23:56 | mriedem | we don't even call disconnect_volume on the volume driver if cinder is reporting more than one attachment for the same volume/host | |
| 14:24:20 | lyarwood | yeah that's the plan in the cinderv3 spec | |
| 14:24:27 | mriedem | that's in johnthetubaguy's spec somewhere | |
| 14:24:37 | mdbooth | Surely that's wrong, because it depends entirely on the volume driver implementation? | |
| 14:25:04 | mdbooth | If it's a direct attachment to the instance rather than eg a host mount, that's not going to work correctly | |
| 14:25:12 | lyarwood | https://review.openstack.org/#/c/373203/15/specs/pike/approved/cinder-new-attach-apis.rst@196 | |
| 14:26:01 | mdbooth | Yeah, I don't think you can do that at the api level. It's a driver thing. | |
| 14:26:02 | johnthetubaguy | I am curious, reading the scroll back | |
| 14:26:13 | lyarwood | mdbooth: IIRC cinderv3 should tell us if the connection is shared or not directly from the volume driver | |
| 14:26:16 | johnthetubaguy | so its the cinder driver that tells the cinder API what should happen | |
| 14:26:25 | johnthetubaguy | we pass the cinder driver details from os-brick in the connector | |
| 14:26:39 | mdbooth | lyarwood: But it can't tell you if the driver needs to perform a global action or not | |
| 14:26:43 | johnthetubaguy | cinder folks seemed to think that could be specific enough for our needs | |
| 14:26:59 | mdbooth | e.g. connecting a gluster volume directly to a domain via a monitor call | |
| 14:27:26 | mdbooth | vs mounting a gluster volume on the host and passing a file to the domain | |
| 14:27:49 | mdbooth | The gluster driver supports both of those things currently, and only it knows which of them it's doing | |
| 14:28:07 | johnthetubaguy | so the cinder backend needs to tell cinder API what happens, if that needs to know which one we are doing, it needs to go into the connector, I assume? | |
| 14:28:09 | mdbooth | So no higher level can decide whether or not to call disconnect | |
| 14:28:16 | lyarwood | ah it's not configured locally on the host is it? | |
| 14:28:17 | johnthetubaguy | there is no higher level deciding here | |
| 14:28:18 | johnthetubaguy | its the driver | |
| 14:28:25 | lyarwood | connection_info isn't telling it which one to do? | |
| 14:28:34 | mdbooth | lyarwood: No | |
| 14:28:40 | johnthetubaguy | so it needs to add that now | |
| 14:28:48 | lyarwood | yeah | |
| 14:28:51 | johnthetubaguy | and default to a global lock if that might be possibe | |
| 14:28:55 | mdbooth | It literally can't | |
| 14:28:57 | johnthetubaguy | and it doesn't know which | |
| 14:29:01 | mdbooth | And it's none of its business anyway | |
| 14:29:07 | johnthetubaguy | it is now | |
| 14:29:37 | mdbooth | In the gluster case it's based on the qemu and libvirt versions on the host | |
| 14:29:42 | mdbooth | cinder doesn't know that | |
| 14:29:51 | johnthetubaguy | so what we are saying, I think, is the cinder driver, on the cinder side, needs to tell us if we need the lock or not | |
| 14:29:53 | mriedem | we can tell cinder via the connector dict | |
| 14:29:55 | smcginnis | There is no gluster driver, but anyway... | |
| 14:29:59 | mdbooth | As it happens we can safely remove it, but it's still an example of how cinder cannot know this | |
| 14:30:05 | johnthetubaguy | to make that possible cinder always stores the connector I think | |
| 14:30:09 | mdbooth | It's a host-side driver implementation detail | |
| 14:30:23 | mdbooth | All cinder knows is that there will be a connection from the host | |
| 14:30:30 | smcginnis | I'm trying to read the scrollback, but not sure what the issue is here. | |
| 14:30:38 | johnthetubaguy | my preference is to always call os-brick, and tell it what we know | |
| 14:30:42 | mdbooth | It's not cinder's business where that comes from | |
| 14:30:48 | johnthetubaguy | would that make things happier? | |
| 14:31:11 | mdbooth | johnthetubaguy: Yup. That's also entirely consistent with my patch. | |
| 14:31:40 | johnthetubaguy | mdbooth: I am totally fine with that approach, but I guess we will have to version that somehow, but thats fine | |
| 14:31:49 | mdbooth | johnthetubaguy: version what? | |
| 14:32:00 | johnthetubaguy | deciding if we always call detach | |
| 14:32:12 | mriedem | mdbooth: the nfs volume driver in nova today doesn't call os-brick right | |
| 14:32:13 | johnthetubaguy | although maybe thats what we always do today anyways? | |
| 14:32:13 | mriedem | ? | |
| 14:32:30 | mriedem | johnthetubaguy: we always call disconnect_volume on the volume driver today | |
| 14:32:32 | mdbooth | mriedem: No, it doesn't. None of the fs backed drivers do, I think. | |
| 14:32:42 | lyarwood | yeah correct | |
| 14:32:52 | mriedem | johnthetubaguy: then the nfs vol driver just handles a "device is busy" error and ignores it on umount | |
| 14:33:04 | johnthetubaguy | mriedem: ah, fair enough | |
| 14:33:12 | mdbooth | mriedem: ... which is broken for a few reasons. | |
| 14:33:20 | mriedem | mdbooth's patch https://review.openstack.org/#/c/383859/ is adding ref counting | |
| 14:33:26 | mriedem | so we don't blindly do that | |
| 14:33:29 | johnthetubaguy | so all this shared connection stuff is the very last bit of the cinder API changes | |
| 14:33:38 | mdbooth | Yeah | |
| 14:33:47 | mdbooth | Well "ref counting" | |
| 14:33:56 | johnthetubaguy | if we don't need it, thats cool | |
| 14:34:03 | mdbooth | Because our connect and disconnect calls don't actually match up in practise | |
| 14:34:22 | jdandrea | Can someone please clarify? From Placement API doc: "The placement service tracks the inventory and usage of each provider." Does the Placement service *perform* placement, or is it a means to that end, or is it something else? | |
| 14:34:25 | mriedem | "ref counting" is how i'm summarizing with wavy high-level hands a 1200 LOC patch :) | |
| 14:34:42 | mdbooth | It's not that big, is it? | |
| 14:34:51 | mriedem | mdbooth: massive | |
| 14:34:56 | mdbooth | Eurgh, it bloody is | |
| 14:35:10 | mriedem | jdandrea: nova-scheduler performs the actual placement, | |
| 14:35:14 | mdbooth | Anyway, have a look at how big glusterfs is using it :) | |
| 14:35:14 | mriedem | the placement service is the means to the end | |
| 14:35:50 | jdandrea | mriedem Thank you! My brain was running in circles, confused as to why it isn't called an Inventory API, f'rinstance. | |
| 14:35:55 | mriedem | jdandrea: the placement service holds the global view on resources in the deployment, and nova-scheduler asks it which resource providers it knows about can fit a given instance build/move request | |
| 14:35:55 | mdbooth | It's all about error handling, and it can be reused without modification by all the other filesystem drivers | |
| 14:36:06 | jdandrea | mriedem *nodnod* got it | |
| 14:36:44 | mdbooth | mriedem: Look at this: https://review.openstack.org/#/c/423402/ | |
| 14:36:46 | jdandrea | Appreciatd! | |
| 14:36:50 | jdandrea | (and also appreciated) | |
| 14:36:52 | jdandrea | :) | |
| 14:36:52 | mdbooth | I can do that to ever other filesystem driver | |
| 14:37:14 | mriedem | mdbooth: still https://review.openstack.org/#/c/383859/19/nova/virt/libvirt/volume/mount.py@192 | |
| 14:37:19 | mriedem | what am i missing there? | |
| 14:37:31 | mdbooth | mriedem: It's aset | |
| 14:37:34 | mdbooth | a set | |
| 14:37:59 | mdbooth | So you can add (vol_a, instance_a) and (vol_a, instance_b) | |
| 14:38:02 | mdbooth | and they're different things | |
| 14:38:16 | mdbooth | When you remove (vol_a, instance_b), the mountpoint is still in use | |
| 14:38:20 | mdbooth | There's a test for it | |
| 14:38:33 | mriedem | ah because self.attachments is still len == 1 | |
| 14:38:38 | mdbooth | Yeah | |
| 14:40:06 | mdbooth | mriedem: Anyway, don't look at this as a 1200 line nfs driver patch. It's a 1200 line driver for every filesystem volume driver with really good error handling and really good tests. | |
| 14:40:49 | mriedem | yeah i know - you also told me how really good it was right before rc1 and i found an issue in there, so the size and complexity does matter :) | |
| 14:40:53 | mriedem | not trying to knock it, but you know | |