Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-02-28
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 mriedem ?
14:32:13 johnthetubaguy although maybe thats what we always do today anyways?
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 mriedem the placement service is the means to the end
14:35:14 mdbooth Anyway, have a look at how big glusterfs is using it :)
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 mdbooth It's all about error handling, and it can be reused without modification by all the other filesystem drivers
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: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 mdbooth I can do that to ever other filesystem driver
14:36:52 jdandrea :)
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

Earlier   Later