| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-28 | |||
| 13:43:02 | zioproto | lyarwood, ping me when you are here :) | |
| 13:43:39 | lyarwood | zioproto: https://review.openstack.org/#/c/438570/ but I'd wait until the tag lands | |
| 13:44:32 | sfinucan | macsz: I wonder if it's time to look at switching back to nose, now that it supports parallel testing? :P https://review.openstack.org/#/c/435111 | |
| 13:44:46 | zioproto | great that is the correct commit | |
| 13:45:20 | lyarwood | zioproto: right but I'd wait until the tag lands in the repo to build, we've already respun that to include an additional commit in the last 24 hours | |
| 13:45:29 | lyarwood | zioproto: it shouldn't be long until that lands tbh | |
| 13:48:12 | zioproto | thanks | |
| 13:55:09 | bauzas | johnthetubaguy: I do wonder, do we need to change some modules if we're just changing where a conf opt is set ? | |
| 13:55:24 | bauzas | johnthetubaguy: like the one you made about moving default_schedule_az | |
| 13:55:35 | johnthetubaguy | bauzas: its still looked up by doing CONF.xxx_xxx | |
| 13:55:42 | bauzas | yeah I know | |
| 13:55:46 | johnthetubaguy | bauzas: if the tests pass we should be fine | |
| 13:56:02 | bauzas | but I do wonder if we weren't explicitely saying "this or that"' | |
| 13:56:08 | bauzas | oh wait | |
| 13:56:12 | bauzas | that's a global opt | |
| 13:56:15 | johnthetubaguy | its not moving group, right | |
| 13:56:20 | johnthetubaguy | its just in the default group | |
| 13:56:21 | bauzas | we're only do that for groups | |
| 13:56:23 | bauzas | yeah | |
| 13:56:27 | bauzas | nevermind then | |
| 13:57:02 | bauzas | johnthetubaguy: FWIW, saw your spec about local scheduler, will review it soon | |
| 13:57:37 | lyarwood | hmmm is gerrit still messed up for anyone? | |
| 13:58:10 | johnthetubaguy | mine seems OK now, but its worth a shift-F5 maybe? | |
| 13:58:25 | johnthetubaguy | bauzas: that the one we discussed at the PTG | |
| 13:58:39 | bauzas | johnthetubaguy: yeah I remember | |
| 13:58:40 | johnthetubaguy | although I don't remember quite when we covered it | |
| 13:58:57 | lyarwood | ah there we go | |
| 13:59:00 | bauzas | tbh, sometimes my French tricks me during summits | |
| 13:59:08 | bauzas | but I was pretty following that convo | |
| 13:59:26 | bauzas | s/summits/f2f | |
| 14:08:34 | openstackgerrit | Matthew Booth proposed openstack/nova master: libvirt: Pass instance to connect_volume and disconnect_volume https://review.openstack.org/438970 | |
| 14:08:34 | openstackgerrit | Matthew Booth proposed openstack/nova master: libvirt: Fix races with nfs volume mount/umount https://review.openstack.org/383859 | |
| 14:20:51 | mriedem | mdbooth: so you plumbed an instance through for mount/unmount but i don't see where you are using the instance counting to tell when it's OK to unmount the volume in the case of multi-attach? | |
| 14:21:10 | mdbooth | mriedem: In the next patch. I explain in the commit message. | |
| 14:21:35 | mdbooth | mriedem: I could squish them together if you like, but as I explained in the commit message I think it's clearer this way. | |
| 14:22:06 | mriedem | next patch being https://review.openstack.org/#/c/383859/ ? | |
| 14:22:18 | mdbooth | Just makes it a bit easier to read imho, because this is a mechanical change which touches a bunch of files. | |
| 14:22:23 | mriedem | because as i commented in that change, i don't see where you're looking at the set of attachments for the same volume to see when it's fully gone | |
| 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 | |