Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-17
14:30:54 johnthetubaguy mriedem: so you think we just allow delete_on_termination for multi-attach volumes?
14:30:58 openstackgerrit Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
14:30:59 mriedem so if you boot from volume with a multi-attach volume and specify delete_on_termination=True, and that volume is attached to another instance when the first is deleted, it won't prevent the deletion on the first instance, it will just log a warning in the logs about being unable to delete the volume
14:31:32 johnthetubaguy I guess it would just be deleted by the last detach, probably
14:31:34 mriedem johnthetubaguy: i'm not sure there is a great reason to not allow it
14:31:44 mriedem johnthetubaguy: well, not necessarily
14:32:12 mriedem if instance A and B are attached to volume X, and A.delete_on_termination=True and B.delete_on_termination=False, then deleting A won't delete X and deleting B won't either
14:32:15 johnthetubaguy I think its a race for the delete, like it could delete it
14:32:17 johnthetubaguy yeah
14:32:30 johnthetubaguy I should have defined my "probably" bit better
14:32:51 mriedem if B is deleted before A, then yes X would be deleted
14:33:00 mriedem ultimately the lock is supposed to be on the cinder side
14:33:15 johnthetubaguy ++
14:33:43 mriedem and i think it's just something a user would have to understand if they are attaching a multi-attach volume to an instance with delete_on_termination, that nova can't actually delete that volume if it's attached to another instance
14:33:56 johnthetubaguy OK, my thinking was, seems odd to delete a multi-attach volume on delete, gonna be racey, lets just not allow that flag to be set.
14:33:59 ildikov so the conclusion is to allow it, right?
14:34:02 ildikov mriedem: +1
14:34:07 mriedem ildikov: i don't see a reason not to
14:34:18 ildikov mriedem: cool, my thinking as well
14:34:27 johnthetubaguy OK, so you are both thinking that way, willing to go with that
14:34:38 ildikov johnthetubaguy: thank you :)
14:35:25 openstackgerrit Eric Fried proposed openstack/nova master: Send Allocations to spawn https://review.openstack.org/511879
14:35:52 openstackgerrit Dan Smith proposed openstack/nova master: Make live migration hold resources with a migration allocation https://review.openstack.org/507638
14:35:52 openstackgerrit Dan Smith proposed openstack/nova master: Make migration uuid hold allocations for migrating instances https://review.openstack.org/506420
14:56:15 johnthetubaguy mriedem: if you get a moment, I attempted pseudo code for the shared connection locking, that feels like the last bit of detail we need in there: https://review.openstack.org/#/c/499777/5/specs/queens/approved/cinder-volume-multi-attach.rst@53
14:58:06 sean-k-mooney johnthetubaguy: cfriesen am i remebering correctly that ye were both interested in bandwidth based scheduling at the ptg
14:58:40 johnthetubaguy sean-k-mooney: I was probably incorrectly interested, snowed up with other things honestly
14:59:19 sean-k-mooney johnthetubaguy: ack ok i was just going to flag https://review.openstack.org/#/c/502306/12 to you if you cared to review
15:00:05 johnthetubaguy sean-k-mooney: cool, can't promise I will get to that I am afraid, chasing a few other things
15:00:26 sean-k-mooney johnthetubaguy: no worries
15:01:27 mriedem sean-k-mooney: i know huawei is interested in that
15:01:31 mriedem but i haven't had a chance to review the spec yet
15:02:56 sean-k-mooney mriedem: ralonsoh is getting a little concerned that it may get caught by the spec approval deadline on thrusday?
15:03:10 mriedem everyone is getting concerned about that for their specs
15:03:15 melwitt mriedem: the py3 packages are definitely in the pike uca. how does checking for the py3 env protect against packages missing < pike?
15:03:30 mriedem melwitt: because the jobs that this is breaking aren't running with py3
15:03:50 mriedem that check is based on a localrc flag set by devstack-gate which is set by the job config
15:04:08 mriedem our live migration job which uses ceph does not run under py3
15:04:12 sean-k-mooney efried: bauzas ye both previously reviewed https://review.openstack.org/#/c/502306/12 is it on your backlog to re review in the next few days?
15:04:25 melwitt mriedem: ack
15:04:38 bauzas sean-k-mooney: thanks for the ping, sure I'll do
15:04:48 mriedem johnthetubaguy: reading - not sure how the lock fixes things when the instances attached to the volume are on different compute hosts and we're not using a DLM
15:04:52 mriedem but i know we've been over that
15:05:30 openstackgerrit Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854
15:05:31 openstackgerrit Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357
15:05:31 openstackgerrit Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/510159
15:05:48 sean-k-mooney mriedem: ack, that is fair i just said i would flag it again incase anyone was interested.
15:06:00 johnthetubaguy mriedem: there is no conflict there, AFAIK, I thought it was just on the operations os-brick does on the hypervisor (and the operations cinder does on delete_attachment and update_attachment)
15:06:51 efried sean-k-mooney Can do; though I'm not sure I'm qualified to +1 it.
15:07:12 johnthetubaguy mriedem: although I think I see what you mean now
15:07:31 openstackgerrit Balazs Gibizer proposed openstack/nova master: Reproduce bug 1724172 in the functional test env https://review.openstack.org/512553
15:07:31 openstackgerrit Balazs Gibizer proposed openstack/nova master: Enhance service restart in functional env https://review.openstack.org/512552
15:07:32 openstackgerrit Balazs Gibizer proposed openstack/nova master: cleanup evacuated instances not on hypervisor https://review.openstack.org/512623
15:07:32 openstack bug 1724172 in OpenStack Compute (nova) "Allocation of an evacuated instance is not cleaned on the source host if instance is not defined on the hypervisor" [Undecided,In progress] https://launchpad.net/bugs/1724172 - Assigned to Balazs Gibizer (balazs-gibizer)
15:08:04 johnthetubaguy mriedem: I guess we must have assumed a separate connection from each hypervisor, which doesn't seem quite correct
15:08:34 sean-k-mooney efried: well if you have concers about the proposal or the wording others probably do too so adressing those in the spec makes it better in general
15:08:49 efried sean-k-mooney Agree. I'll give it a full pass later today.
15:08:57 ericyoung the scaleio imagebackend patch has been updated and gotten through CI if anyone wants to take a look
15:08:59 ericyoung https://review.openstack.org/#/c/407440/
15:11:32 cdent edleafe-: I don’t understand you caffeine comment. So I tried to clarify.
15:15:09 edleafe cdent: "Is this a candidate for on the Selection object?"
15:15:18 edleafe "for on" was the confusing bit
15:16:02 cdent “Is this method a candidate for on the selection object?”
15:16:38 cdent but yes, the caffeine is broken today
15:16:42 cdent bad vintage or something
15:16:51 edleafe To answer your clarification, there was pushback on adding too much stuff to the Selection object. It could easily be there instead of the scheduler manager. The only preference is that it would never be used outside of the scheduler manager.
15:21:53 openstackgerrit Eric Fried proposed openstack/nova-specs master: Granular Resource Request Syntax https://review.openstack.org/510244
15:22:21 cdent edleafe: if it’s only ever used the scheduler manager then yeah, there seems fine.
15:22:24 efried dansmith -10% ^
15:23:30 dansmith efried: standard negotiation tactic is expect being able to knock 10% off, so that seems substantial
15:23:48 dansmith efried: on a call now but I'll look in a bit
15:23:54 efried nod, thx
15:28:55 mriedem ildikov: johnthetubaguy: ok my comments inline https://review.openstack.org/#/c/499777/
15:29:04 bauzas mriedem: gibi: I have some concerns by seeing duplicate code in https://review.openstack.org/#/c/510938/5/nova/compute/manager.py
15:29:31 mriedem bauzas: duplicate with what?
15:29:33 mriedem the RT?
15:29:38 bauzas mriedem: yup
15:29:47 bauzas delete_alloc_for_moved()
15:29:47 mriedem that's intentional
15:29:49 bauzas I know
15:29:57 bauzas but that's still duplicate
15:30:04 bauzas even if intentional :p
15:30:05 mriedem bauzas: see PS2
15:30:44 bauzas mriedem: I agree with your point to not call the RT method
15:30:55 bauzas mriedem: but we have a compute.utils helper module AFAIK
15:31:26 bauzas my main concern is that a modification would require two places
15:32:25 mriedem the only duplication is L698-L706 https://review.openstack.org/#/c/510938/5/nova/compute/manager.py
15:32:35 bauzas I agree
15:32:51 mriedem :)
15:32:51 mriedem whch is also in _delete_allocation_after_move now
15:33:03 mriedem and _rollback_live_migration
15:33:17 bauzas mriedem: but what if someone wants to add another call ?
15:33:33 mriedem so how about we do the consolidation into a util in a follow up?
15:33:47 bauzas I'm fine with that
15:33:50 mriedem hell, we could just do a private method inthe compute manager itself
15:33:52 bauzas I haven't reviewed the change
15:33:53 mriedem if you don't want to touch the RT
15:34:03 bauzas I just wanted to open a discussion
15:34:26 mriedem i'm cool with refactoring out the duplication, but not in this change since it has to be backported
15:34:27 bauzas but yeah, if we were doing the consolidation, would be worth doing all of it in a single patch

Earlier   Later