| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-28 | |||
| 19:42:02 | mriedem | efried: the point is the comment in the else block below it | |
| 19:43:08 | jgriffith | Oh, specifically that call right there :) | |
| 19:44:23 | mriedem | i'm having a hard time remembering too | |
| 19:44:28 | efried | Lemme make sure I have the logic right: | |
| 19:44:28 | efried | If multiattach not supported by cinder, proceed. | |
| 19:44:28 | efried | Otherwise, fail if the volume is attached. | |
| 19:44:41 | mriedem | 3.44 isn't about multiattach | |
| 19:44:45 | mriedem | it starts with https://review.openstack.org/#/c/525622/ | |
| 19:44:56 | mriedem | with the new attachments api stuff, cinder will let us create multiple attachments between the same volume and instance | |
| 19:45:09 | mriedem | to enable things like live migration where the instance is attached to the volume on different hosts | |
| 19:45:24 | mriedem | but because of that, we found we had a bug where you could attach the same volume to the instance via the api multiple times | |
| 19:45:30 | mriedem | so we had to add a specific check for that | |
| 19:46:07 | jgriffith | mriedem: yeah, the comment actually is pretty good :) | |
| 19:46:07 | mriedem | before using new style attachments, cinder would not allow this | |
| 19:46:18 | mriedem | that's why in https://review.openstack.org/#/c/525622/ we have to rely on the bdm table | |
| 19:47:54 | efried | Ultimately I was trying to figure out if I could get rid of that microversion check :) I think I now understand why I can't. | |
| 19:47:55 | mriedem | so going back to https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3964 | |
| 19:48:20 | efried | Which kinda sucks, because it means I'm gonna have to build up a whole cinder client just to do that microversion check. | |
| 19:48:24 | mriedem | if 3.44 is supported, we know we're doing the new style attachment_create, and we have to make sure the instance isn't already attached to that volume, so we call _check_volume_already_attached_to_instance | |
| 19:48:34 | mriedem | if 3.44 isn't available, we'll do the old style reserve flow | |
| 19:48:49 | mriedem | which cinder enforces you can only have the instance attached to the volume once | |
| 19:49:01 | mriedem | efried: yeah but that is only temporary for the backport, | |
| 19:49:12 | mriedem | and we can replace that stuff with ksa in master right? | |
| 19:49:34 | efried | mriedem: Except I still haven't figured out how to get ksa stuff working for cinder, period. | |
| 19:49:41 | mriedem | heh | |
| 19:49:42 | mriedem | well, | |
| 19:49:47 | mriedem | it's just code right?! | |
| 19:49:56 | efried | Sigh. Right. | |
| 19:50:02 | mriedem | sylvain's daughter will have it done by tomorrow | |
| 19:50:19 | efried | I know, I can't let her show me up like that. Not again. | |
| 19:54:57 | dansmith | cripes | |
| 19:56:11 | openstackgerrit | Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002 | |
| 20:12:46 | mriedem | dansmith: not sure if you care about any of the comments in https://review.openstack.org/#/c/544730/15 | |
| 20:14:45 | dansmith | mriedem: sorry hadn't seen those | |
| 20:14:46 | dansmith | replied | |
| 20:18:25 | mriedem | i've got this nova-status check ready to go basically for the cells api service version check thing, i think i'll just munge the api log warning into the same patch | |
| 20:20:19 | dansmith | seems reasonable | |
| 20:38:30 | openstackgerrit | Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730 | |
| 20:38:31 | openstackgerrit | Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729 | |
| 20:38:31 | openstackgerrit | Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990 | |
| 20:38:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Add an index on aggregate_metadata.value https://review.openstack.org/555851 | |
| 20:38:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Add AggregateList.get_by_metadata() query method https://review.openstack.org/544728 | |
| 20:38:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002 | |
| 20:38:33 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Documentation for tenant isolation with placement https://review.openstack.org/557490 | |
| 20:38:34 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282 | |
| 20:40:36 | mikal | mriedem: yeah, I can be around to interate on privsep for the next couple of weeks. Noting that both of the next two weeks are short because of easter. | |
| 20:41:26 | openstackgerrit | Merged openstack/nova stable/queens: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/551948 | |
| 20:41:27 | mikal | mriedem: what's blocking removing nova-net? The last couple of privsep patches aren't mergable until nova-net is gone, but I can WIP those or something. It would be nice to get them landed this cycle though. | |
| 20:43:34 | mriedem | mikal: nothing is blocking it | |
| 20:43:44 | mriedem | mikal: but, | |
| 20:43:57 | mriedem | i imagine it's not trivial since a ton of the test code in-tree assumes nova-net | |
| 20:44:05 | mriedem | and that's including non-nova-net things | |
| 20:44:20 | mikal | Is anyone working on it? | |
| 20:44:25 | mriedem | so all of that has to be unwound | |
| 20:44:28 | mriedem | mikal: not that i know of, | |
| 20:44:36 | mriedem | i got the cells v1 job running with neutron and that was step 1 | |
| 20:46:53 | mriedem | which reminds me, | |
| 20:47:01 | mriedem | melwitt: dansmith: efried: i'm out on monday and tuesday | |
| 20:47:22 | dansmith | orly | |
| 20:47:22 | melwitt | k, thanks for the heads up | |
| 20:47:23 | dansmith | I'm out friday for "GENERIC SPRING HOLIDAY" | |
| 20:47:34 | melwitt | me too | |
| 20:47:40 | efried | roger that, mriedem. Means you have to review this bug fix tonight/tomorrow I guess. | |
| 20:49:18 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Log a warning and add nova-status check for old API service versions https://review.openstack.org/557506 | |
| 20:49:19 | mriedem | SamYaple: ^ for your bug from yesterday, i hope i get a bag of jelly beans for this | |
| 20:50:05 | SamYaple | oh snap! | |
| 20:50:33 | SamYaple | that was quick. i promise you regular flavored jelly beans | |
| 20:52:18 | mriedem | is there such a thing as unflavored jelly beans? | |
| 20:52:52 | SamYaple | i said "regular". as opposed to.... other flavors | |
| 20:53:00 | mriedem | oh right | |
| 20:53:01 | mriedem | yeah brachs | |
| 20:53:07 | mriedem | not that new fangled starburts stuff | |
| 20:53:12 | mriedem | *starbursts | |
| 20:53:19 | SamYaple | haha | |
| 20:53:42 | dansmith | starburts? | |
| 20:53:58 | openstackgerrit | Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990 | |
| 20:53:59 | openstackgerrit | Dan Smith proposed openstack/nova master: Add an index on aggregate_metadata.value https://review.openstack.org/555851 | |
| 20:53:59 | openstackgerrit | Dan Smith proposed openstack/nova master: Add AggregateList.get_by_metadata() query method https://review.openstack.org/544728 | |
| 20:54:00 | openstackgerrit | Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002 | |
| 20:54:00 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Documentation for tenant isolation with placement https://review.openstack.org/557490 | |
| 20:54:01 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282 | |
| 20:54:01 | openstackgerrit | Eric Fried proposed openstack/nova master: Use ksa session for cinder microversion check https://review.openstack.org/557508 | |
| 20:54:02 | dansmith | (man I suck) | |
| 20:54:05 | efried | mriedem: ^ | |
| 20:54:08 | efried | Hahaha | |
| 20:54:26 | mriedem | ? | |
| 20:54:38 | mriedem | starburts are burt reynolds flavored candies | |
| 20:54:46 | mriedem | mostly taste like oil and stache | |
| 20:55:02 | mikal | Does anyone else see pep8 failures with master by the way? | |
| 20:55:05 | efried | mriedem: I was pointing out the bug fix patch I just posted. But crossed streams with dansmith, so it looked like I was bringing your attention to the fact that dansmith, by his own admission, sucks. | |
| 20:55:13 | mikal | Running flake8 on all files | |
| 20:55:13 | mikal | ./nova/tests/unit/virt/libvirt/volume/test_quobyte.py:230:18: E211 whitespace before '(' | |
| 20:55:16 | mikal | ./nova/virt/libvirt/guest.py:664:68: F821 undefined name 'unicode' | |
| 20:55:34 | mriedem | mikal: nope | |
| 20:55:41 | mikal | Herm, how odd. | |
| 20:55:41 | dansmith | efried: I ain't swordfightin' with you dude.. ever. | |
| 20:55:58 | openstackgerrit | Merged openstack/nova stable/pike: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/551950 | |
| 20:55:58 | efried | Promises promises. | |
| 20:56:07 | openstackgerrit | Merged openstack/nova stable/queens: Add functional regression test for bug 1746509 https://review.openstack.org/555914 | |
| 20:56:07 | openstack | bug 1746509 in OpenStack Compute (nova) pike "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" [High,In progress] https://launchpad.net/bugs/1746509 - Assigned to melanie witt (melwitt) | |