| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 17:24:55 | dansmith | mriedem: totally your call | |
| 17:25:24 | gibi | mriedem, dansmith: but I have to run now. thanks for the reviews! | |
| 17:25:25 | dansmith | gibi: I was going to suggest that first patch not adding/using the contextmanager | |
| 17:25:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce basic framework for security proxying https://review.openstack.org/345396 | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce framework for RFB authentication https://review.openstack.org/345397 | |
| 17:25:40 | dansmith | gibi: but if it's not going to cause conflicts, it's not as much of a problem I guess | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce the VeNCrypt RFB authentication scheme https://review.openstack.org/345398 | |
| 17:25:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: provide an RFB security proxy implementation https://review.openstack.org/345399 | |
| 17:26:11 | gibi | dansmith: it didn't caused conflict a couple of days ago when I cherry-picked the patch to stable/ocata | |
| 17:26:55 | dansmith | gibi: not now for sure, I just mean in terms of backporting less code. the context manager add is more code, but it's contained | |
| 17:26:56 | dansmith | gibi: if mriedem is cool with it then it's fine | |
| 17:26:56 | gibi | dansmith: here is the cherry-pick https://review.openstack.org/#/c/441171/ I will update that as well | |
| 17:27:08 | gibi | dansmith: ahh I see | |
| 17:27:23 | gibi | dansmith: I agree that the ctx manager is quite self contained solution | |
| 17:27:43 | dansmith | gibi: anyway, I'll look at this in a few, thanks! | |
| 17:27:52 | gibi | dansmith: thanks a lot! | |
| 17:28:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Fix formatting of network options https://review.openstack.org/443086 | |
| 17:28:58 | mriedem | catch him! | |
| 17:29:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Final cleanups in conf/network https://review.openstack.org/431023 | |
| 17:30:16 | openstackgerrit | John Garbutt proposed openstack/nova-specs master: Add spec to use cinder's new attachment API https://review.openstack.org/373203 | |
| 17:34:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 17:41:51 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova-specs master: Enable SR-IOV NIC offload feature discovery https://review.openstack.org/435954 | |
| 17:43:30 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs master: Spec to add per instance timeout for LM https://review.openstack.org/442809 | |
| 17:44:12 | openstackgerrit | Feodor Tersin proposed openstack/nova master: Give names to volumes during block device mapping https://review.openstack.org/213433 | |
| 17:47:37 | openstackgerrit | Rafael Folco proposed openstack/nova master: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:50:58 | dansmith | mriedem: I'm going to pull down gibi's patch and fix a test comment of mine | |
| 17:51:26 | dansmith | mriedem: are you cool with the heavier context manager approach for the backport or do you want to do the simplest thing, which is just call the normal methods in the places where we're not? | |
| 17:51:56 | dansmith | I think his approach here ends up getting us a begin/end notification that we wouldn't get otherwise | |
| 17:52:03 | dansmith | which is probably good, it's just .. more code | |
| 17:59:04 | mriedem | if it's not making the backport a mess i'm fine with it | |
| 17:59:09 | mriedem | i haven't dug in in detail | |
| 18:03:11 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix missing instance.delete notification https://review.openstack.org/437222 | |
| 18:03:51 | dansmith | I also sneakily capitalized the first letter of the commit message because .. I hate that. | |
| 18:05:13 | edleafe | rfolco: A could of nits on https://review.openstack.org/416669, but one serious issue | |
| 18:07:00 | rfolco | edleafe, ack | |
| 18:10:31 | mriedem | dansmith: but not period at the end? | |
| 18:10:34 | mriedem | *no | |
| 18:10:37 | dansmith | no I hate theperiod | |
| 18:10:45 | mriedem | I like pizza. I also like cookies. | |
| 18:10:55 | dansmith | heh | |
| 18:14:47 | openstackgerrit | Rafael Folco proposed openstack/nova master: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 18:36:53 | mdbooth | ... when you realise you created dicts called host_cell_mapping_mapping and cell_mapping_host_mapping | |
| 18:40:08 | cburgess | OMG | |
| 18:40:18 | cburgess | You should be taken out back and flogged for that. | |
| 18:40:27 | mdbooth | cburgess: Done ;) | |
| 18:40:44 | efried | mdbooth Clearly the former is mapping the mappings of hosts to cells, and the latter is mapping the .... sh!t, I got nothing. | |
| 18:41:02 | mdbooth | hosts to cell mappings, and cell mappings to hosts | |
| 18:41:22 | mdbooth | It made sense when I wrote it | |
| 18:41:52 | cburgess | Where you are a ballmer peak when you wrote that? | |
| 18:43:10 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Move detach logic from manager into driver BDM https://review.openstack.org/439520 | |
| 18:43:10 | openstackgerrit | Lee Yarwood proposed openstack/nova master: objects: Add attachment_id to BlockDeviceMapping https://review.openstack.org/437665 | |
| 18:43:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Only destroy BDMs after successful detach call https://review.openstack.org/440693 | |
| 18:43:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: db: Add attachment_id to block_device_mapping https://review.openstack.org/437597 | |
| 18:43:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Make BlockDeviceMapping object support uuid https://review.openstack.org/242603 | |
| 18:43:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add uuid column to BlockDeviceMapping https://review.openstack.org/242602 | |
| 18:43:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Use detach_volume during default rebuild https://review.openstack.org/443932 | |
| 18:57:09 | openstackgerrit | Dan Smith proposed openstack/nova master: Make discover_hosts only query for unmapped ComputeNode records https://review.openstack.org/427903 | |
| 18:57:10 | openstackgerrit | Dan Smith proposed openstack/nova master: Add ability to query for ComputeNodes by their mapped value https://review.openstack.org/427902 | |
| 18:57:10 | openstackgerrit | Dan Smith proposed openstack/nova master: Add ComputeNode.mapped field https://review.openstack.org/427901 | |
| 19:01:41 | oomichi | mriedem: sdague: is nova-network deprecated already? I guess so, but I cannot find clear doc on nova repo | |
| 19:02:47 | mriedem | oomichi: since newton | |
| 19:03:03 | mriedem | oomichi: the config options, release notes, starting the service, etc etc | |
| 19:03:14 | mriedem | the official manuals probably don't say anything about it | |
| 19:03:42 | oomichi | mriedem: thanks, I got it. reno is good for me | |
| 19:05:06 | mriedem | asettle: so https://docs.openstack.org/admin-guide/compute-networking-nova.html | |
| 19:05:16 | openstackgerrit | Jay Pipes proposed openstack/nova master: virt: implement get_inventory() for Ironic https://review.openstack.org/441544 | |
| 19:05:25 | jaypipes | cdent: fixed up per your review ^^ dansmith, you were already +2 on it :) | |
| 19:05:30 | mriedem | asettle: nova-network has been deprecated since newton, and in ocata we made it such that you can only run nova-network in a cells v1 configuration (which is not recommended for new deployments either) | |
| 19:05:39 | mriedem | asettle: i guess we need a big fat warning in that page now right? | |
| 19:06:04 | cdent | thank you jaypipes, I felt meh about dissing dansmith's +2, but seemed better to get it now instead of later | |
| 19:06:20 | jaypipes | no worries, those were good comments, cdent. | |
| 19:06:46 | cdent | i'm in review mode at the moment, so should get back round to the new version shortly | |
| 19:09:55 | cdent | that means you win mdbooth | |
| 19:10:28 | mdbooth | It's a mechanical change. I just added an additional assert and I couldn't see why the hell it didn't still work. | |
| 19:12:09 | mdbooth | dansmith: Is it possible to migrate an instance between cells? Excepting, of course that the answer's no because there's only 1 cell. | |
| 19:12:36 | dansmith | mdbooth: eventually, but not currently | |
| 19:16:10 | mdbooth | external_instance_event() associates a migrating instance with both its hosts. It then assumes that all instances it associated with a particular host have the same cell_mapping. However, if an instance were migrating to a new cell, it would be listed against the destination host, but have a different cell mapping to all other instances in that list. | |
| 19:18:08 | dansmith | mdbooth: https://review.openstack.org/#/c/438025/7 | |
| 19:19:27 | mdbooth | dansmith: Sure. The assumption in this code is kinda obtuse though (I stared at it for an hour without noticing it). It'll break when you eventually allow that, and it won't be at all obvious that it needs to be fixed. | |
| 19:20:07 | mdbooth | dansmith: I wonder if it's worth addressing while it's in mind. | |
| 19:20:59 | dansmith | mdbooth: the context in the instance doesn't have anything to do with where the rpc call will go | |
| 19:21:13 | dansmith | and on the other side of the call, the context is reset to what was used for the rpc call | |
| 19:22:24 | mdbooth | dansmith: In this case the cell_mapping comes from a weird mappings dict passed in | |
| 19:23:02 | mdbooth | The bug is that if the first instance in the host list is actually from the wrong cell, it'll use the wrong cell context | |
| 19:23:05 | cdent | jaypipes: are we decided that step_size of 1 is okay on the ironic inventory because the modulo math comes out right in that (special) case? | |
| 19:23:24 | jaypipes | cdent: I think so | |
| 19:23:34 | cdent | cool, just checking | |
| 19:24:39 | openstackgerrit | melanie witt proposed openstack/nova master: Count resources to check quota for cells https://review.openstack.org/416521 | |
| 19:25:06 | mdbooth | dansmith: NM. I have to shoot, but I'm going to leave this open and look at it again first thing in the morning. | |
| 19:25:12 | dansmith | mdbooth: okay I see what you mean, but I don't think that's right.. the instancemapping will point to the hostmapping where its record currently resides | |
| 19:25:23 | dansmith | mdbooth: then we add in the other destination, | |
| 19:25:40 | dansmith | but the instance gets added to a host list based on instance.host, which matches hostmapping, and thus the cell | |
| 19:25:54 | mdbooth | dansmith: That's the bug, btw | |
| 19:26:06 | mdbooth | instance is added to host list based on _get_relevant_hosts | |
| 19:26:24 | mdbooth | So a migrating instance will be in *2* host lists, one of which it's not actually on | |
| 19:26:50 | mdbooth | If the first entry in the list just happened to be a migrating instance which wasn't actually on this host | |
| 19:26:59 | mdbooth | We'd use its cell_mapping anyway | |
| 19:27:07 | dansmith | ah, this is actually what you mean: | |
| 19:27:07 | mdbooth | That's ok as long as we're not migrating between cells | |
| 19:27:13 | dansmith | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4082 | |