| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 15:49:13 | zioproto | dansmith: so you mean that if I delete from the instance_extra table all the records where the uuid is not in instances, then I dont need any patch ? | |
| 15:50:04 | mdbooth | imacdonn: https://docs.oracle.com/cd/E51475_01/html/E52433/makehtml-id-181.html | |
| 15:50:24 | mdbooth | Except it's doing a PUT, not a POST | |
| 15:50:42 | dansmith | zioproto: yeah, that's my expectation.. let me know if it doesn't and we can see if something else is wrong too | |
| 15:50:42 | zioproto | dansmith: I have to rush away from the office... I will ping you tomorrow about this | |
| 15:50:46 | dansmith | zioproto: okay thanks | |
| 15:51:01 | imacdonn | mdbooth: Yeah, so does "successful response" mean "I've completed it"? In practice I believe it does .. so not sure why they're using 202 | |
| 15:51:58 | mdbooth | Meh, I wonder if it's reasonable to assert this is sync and make it the cinder driver's problem if it isn't | |
| 15:52:03 | mdbooth | lyarwood: ^^^ ? | |
| 15:52:21 | imacdonn | mdbooth: in any case, I think that nova-compute has to assume that the cinder API call won't return until the work as completed .. if that's not happening, it's either a bug in cinder, or a large other can of worms (need an async mechanism between nova and cinder | |
| 15:52:32 | lyarwood | mdbooth: yeah that's my view | |
| 15:52:49 | mdbooth | lyarwood: Ok | |
| 15:53:09 | lyarwood | mdbooth: but we should really make that assumption known to the cinder folks | |
| 15:53:17 | lyarwood | mdbooth: using this driver as an example | |
| 15:53:19 | mdbooth | Yep | |
| 15:53:36 | openstackgerrit | Dan Smith proposed openstack/nova master: Warn the user about orphaned extra records during keypair migration https://review.openstack.org/458564 | |
| 15:53:43 | dansmith | mriedem: this is what I was thinking ^ | |
| 15:54:04 | dansmith | mriedem: I'll mark the bug as invalid and also comment on the original bug warning people that they eff'd themselves | |
| 15:54:45 | zioproto | dansmith: with this patch https://review.openstack.org/#/c/458564 I should be able to go on without cleaning the db, right ? | |
| 15:54:54 | zioproto | it is the patch I applied manually, right? | |
| 15:55:10 | dansmith | zioproto: no | |
| 15:55:14 | dansmith | zioproto: it's just a warning | |
| 15:55:30 | dansmith | zioproto: it won't fix the problem, which will make the second keypair migration unable to proceed | |
| 15:55:34 | dansmith | zioproto: you have to clean the database | |
| 15:56:04 | sfinucan | vladikr, sahid: Little cleanup here, if you could take a gawk https://review.openstack.org/#/c/417378/ | |
| 15:57:13 | vladikr | sfinucan, sure | |
| 15:58:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Deprecate os-hosts API https://review.openstack.org/456504 | |
| 15:59:01 | mriedem | alex_xu: ^ fixed that unit test | |
| 16:00:38 | mdbooth | lyarwood: My patch won't fix this, btw, because it needs to be global: https://review.openstack.org/#/c/430213/2/nova/compute/manager.py | |
| 16:01:53 | mdbooth | lyarwood: When using iscsi, I think all connect and disconnect calls must be serialised, where disconnect also include terminate connection | |
| 16:02:34 | lyarwood | mdbooth: global in terms of the compute node right? | |
| 16:02:40 | mdbooth | lyarwood: Yes | |
| 16:02:57 | lyarwood | mdbooth: I assumed the manager was, maybe I'm missing something | |
| 16:03:10 | mdbooth | lyarwood: The lock has to be global | |
| 16:03:22 | mdbooth | In my patch it's not, it's keyed on instance uuid | |
| 16:03:26 | imacdonn | lyarwood: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2261 | |
| 16:04:11 | mdbooth | i.e. No 2 connect/disconnect operations affecting iscsi can occur concurrently, regardless of target or instance | |
| 16:04:50 | mdbooth | But that's not true of other volume drivers, which is evil | |
| 16:06:32 | mdbooth | So unless we want to push this restriction onto all volume backends, terminate connection would have to be managed by os-brick | |
| 16:07:10 | mdbooth | lyarwood johnthetubaguy: ^^^ | |
| 16:07:34 | lyarwood | mdbooth: yeah sorry the cinderv3 meeting just started, I see what you mean know re global | |
| 16:07:39 | lyarwood | now* | |
| 16:08:18 | sdague | so, given the issues with https://bugs.launchpad.net/nova/+bug/1511466, how about bump to critical, and provide some better guidance to get through it | |
| 16:08:18 | openstack | Launchpad bug 1511466 in OpenStack Compute (nova) "migrate flavor data impossible" [Critical,Confirmed] | |
| 16:08:36 | sdague | because it does seem like it's coming up on google hunts :) | |
| 16:09:47 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/destroy #2: functional https://review.openstack.org/438598 | |
| 16:10:30 | efried | sdague mriedem ^^ Thanks for the review, back atcha. | |
| 16:11:06 | openstackgerrit | Chris Dent proposed openstack/nova master: Update resource tracker to PUT custom resource classes https://review.openstack.org/456915 | |
| 16:11:07 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Idempotent PUT /resource_classes/{name} https://review.openstack.org/448791 | |
| 16:15:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Using max api version in notification sample test https://review.openstack.org/443686 | |
| 16:17:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Using max api version in notification sample test https://review.openstack.org/443686 | |
| 16:22:52 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: power_on/off and reboot https://review.openstack.org/427380 | |
| 16:24:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Using max api version in notification sample test https://review.openstack.org/443686 | |
| 16:24:21 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: add tags field to instance.update notification https://review.openstack.org/407228 | |
| 16:26:14 | gibi | mriedem: I update the patch that adds tags to the instance.update notification as we agreed https://review.openstack.org/#/c/407228/ | |
| 16:26:26 | openstackgerrit | Dan Smith proposed openstack/nova master: Warn the user about orphaned extra records during keypair migration https://review.openstack.org/458564 | |
| 16:27:26 | openstackgerrit | Dan Smith proposed openstack/nova master: Make server groups api aware of multiple cells for membership https://review.openstack.org/457338 | |
| 16:27:27 | openstackgerrit | Dan Smith proposed openstack/nova master: Sort CellMappingList.get_all() for safety https://review.openstack.org/443174 | |
| 16:27:28 | openstackgerrit | Dan Smith proposed openstack/nova master: Clean up ClientRouter debt https://review.openstack.org/444487 | |
| 16:27:29 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Canary test against multi-cell devstack https://review.openstack.org/458537 | |
| 16:27:31 | openstackgerrit | Dan Smith proposed openstack/nova master: Add workaround to disable group policy check upcall https://review.openstack.org/442736 | |
| 16:29:37 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/destroy #3: TaskFlow https://review.openstack.org/438729 | |
| 16:30:55 | sfinucan | johnthetubaguy: could you take a look at https://review.openstack.org/#/c/449288/ at some point? I'd like to know if there are backwards-compatibility implications for doing something like that | |
| 16:42:42 | sfinucan | mdbooth: Happy to push on with https://review.openstack.org/#/c/383859/ if you fancy sticking in that context comment | |
| 16:42:54 | sfinucan | (in case that wasn't obvious from earlier) | |
| 16:58:39 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/destroy #4: full flavor https://review.openstack.org/391288 | |
| 16:59:19 | clarkb | mriedem: just fyi I'm fixing the live migration job fails on ocata right now | |
| 16:59:26 | clarkb | mriedem: only a portion of the UCA stuff was backported... | |
| 17:00:04 | mriedem | clarkb: ok, thanks | |
| 17:01:00 | mriedem | didn't realize it was broken, but i see that now | |
| 17:02:44 | clarkb | mriedem: I've been checking e-r every morning and looking at the top gate bug :) just to make sure everything is happy | |
| 17:08:00 | johnthetubaguy | mriedem: so I just -1ed the thing I just +Wed https://review.openstack.org/#/c/438750 | |
| 17:08:15 | johnthetubaguy | mriedem: with a note about that begin_detaching stuff | |
| 17:08:53 | johnthetubaguy | mriedem: if what I said is total nonsense, I am happy for someone to over-ride that worry, the rest of it looks fine | |
| 17:19:59 | openstackgerrit | Lee Yarwood proposed openstack/nova master: encryptors: Switch to os-brick encryptor classes https://review.openstack.org/391597 | |
| 17:21:52 | ildikov | johnthetubaguy: isn't begin_detaching an API-level call? | |
| 17:22:15 | ildikov | johnthetubaguy: I mean it gets called before we even get deeper down to detach with the new flow | |
| 17:39:58 | mriedem | johnthetubaguy: i don't follow the concern in this patch | |
| 17:40:42 | mriedem | johnthetubaguy: as mentioned during the meeting, i thought begin_detaching could be used in both the old and new flows | |
| 17:45:08 | ildikov | mriedem: I think it's currently used before we get into the actual detach parts with both flows right now | |
| 17:45:18 | mriedem | yes it is | |
| 17:45:22 | mriedem | b/c it's used in the api | |
| 17:46:14 | ildikov | that's what I thought that it's in a different layer so it happens before we get to choose between the new/old flow | |
| 18:06:48 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: implement get_inventory() for libvirt https://review.openstack.org/457782 | |
| 18:18:48 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Add Cinder v3 detach call to _terminate_volume_connections https://review.openstack.org/456896 | |
| 18:18:48 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Add Cinder v3 detach to swap_volume https://review.openstack.org/456971 | |
| 18:18:49 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Add Cinder v3 detach call to post_live_migration https://review.openstack.org/456988 | |
| 18:29:21 | ildikov | mriedem: lyarwood: johnthetubaguy: fixed up the chain ^^ | |
| 18:30:19 | lyarwood | ildikov: cool thanks | |
| 18:30:21 | ildikov | mriedem: lyarwood: johnthetubaguy: these two are not relying on _terminate_volume_connections: https://review.openstack.org/#/c/456851/ https://review.openstack.org/#/c/456877/ | |
| 18:30:50 | mriedem | ok | |
| 18:31:20 | ildikov | the last two need the almost approved base change as that's where attachment_delete is added | |
| 18:31:38 | ildikov | I can move those on top of that patch if we think it's going to be blocked for longer | |
| 18:31:45 | ildikov | I hope not though | |
| 18:31:53 | mriedem | you could just use the Depends-On tag | |
| 18:32:06 | ildikov | oh, ok good point | |
| 18:32:19 | ildikov | I always thought that's more for cross-repo dependencies... | |
| 18:33:23 | ildikov | lyarwood: all the patches will need some additional tests, so that can be picked up by anyone who's available | |
| 18:33:36 | ildikov | lyarwood: stvnoyes helped out with the base patch | |
| 18:34:06 | ildikov | lyarwood: stvnoyes: the two independent patches could be good candidates if either of you have bandwidth | |