| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-12 | |||
| 14:40:11 | mriedem | gibi: https://review.openstack.org/#/c/448779/ | |
| 14:40:28 | mriedem | questions in there about why we aren't using the existing in scope BDMs when building the payload, like during instance.create | |
| 14:40:50 | mriedem | seems that could be optimized to use the bdms we already have when we notify, rather than always look them up from the db - but maybe that's a later optimization | |
| 14:42:37 | gibi | mriedem: looking... | |
| 14:44:56 | gibi | mriedem: I think we can do some optimization on the loading of BDM. But I feel that it would be better to do that separately | |
| 14:45:31 | mriedem | heh, of course you would :) | |
| 14:45:34 | mriedem | also https://review.openstack.org/#/c/448779/38/nova/tests/fixtures.py | |
| 14:45:49 | mriedem | gibi: it's not even 5pm yet! :) | |
| 14:45:58 | stephenfin | sean-k-mooney, moshele, jaypipes: What on earth is IVS, in the context of network interfaces? | |
| 14:46:05 | gibi | mriedem: true :) | |
| 14:46:17 | jaypipes | stephenfin: it's Indigo (plumbgrid) stuff | |
| 14:46:28 | jaypipes | plumgrid | |
| 14:46:29 | mriedem | gibi: fwiw i'm ok with deferring the optimization part since loading bdms is disabled by default | |
| 14:46:38 | mriedem | but i wanted to make sure we discussed it first | |
| 14:47:03 | jaypipes | stephenfin: NIH OVS for NFV? :) | |
| 14:47:13 | gibi | mriedem: let's see if I can make the optimization this week then we can squash the two patches but if I run out of time then it would be nice to merge at least what we have already | |
| 14:47:19 | stephenfin | jaypipes: So not an "initialization vector"? :) It's rather un-Googleable | |
| 14:47:56 | jaypipes | stephenfin: no. I mean, there's the ivshmem thing but that's DPDK-related. | |
| 14:47:56 | mriedem | gibi: how about you put a TODO in the code in the InstanceAction init where it calls BlockDevicePayload.from_instance to investigate optimizing that by passing in any existing in-scope BDMs, rather than pull from the db | |
| 14:48:01 | mriedem | gibi: i'll +2 with the todo | |
| 14:48:06 | mriedem | check out my question in the cinder fixture also | |
| 14:48:10 | gibi | mriedem: sure, I can do that til 5 pm :) | |
| 14:48:10 | jaypipes | stephenfin: depends on the context I guess. | |
| 14:48:15 | mriedem | \o/ | |
| 14:48:42 | stephenfin | jaypipes: Cool cool. TA | |
| 14:48:45 | stephenfin | *Ta | |
| 14:50:21 | jaypipes | mriedem, dansmith, edleafe, bauzas: so do you think we should be returning alternate *destination hosts* or do you think we should be returning alternate *allocation requests*? I'm questioning my thoughts here. :( | |
| 14:50:45 | dansmith | jaypipes: I thought the whole point was to store alternate allocation requests | |
| 14:51:05 | dansmith | jaypipes: otherwise cell conductor has to formulate such a thing, without the details the scheduler has, and I don't think that's going to work | |
| 14:51:15 | dansmith | s/store/return/ | |
| 14:51:37 | dansmith | so conductor can almost blindly throw the allocation request at placement, and if it sticks, send it along | |
| 14:51:49 | cdent | I agree with dansmith. I seem to recall that's why we ended up down the allocation_candidates path in the first place? | |
| 14:51:57 | dansmith | one of yeah | |
| 14:52:03 | jaypipes | dansmith: yeah, that was my initial thought. I guess I've gotten bent out of shape on this latest patch series. :( | |
| 14:52:12 | jaypipes | I think I need a braincation. | |
| 14:53:14 | cdent | dansmith: since you're here, I was reading some cells docs and code: Is the main reason that discover_hosts doesn't do map_instances so that you can have granularity of timing and volume of change? Are there other reasons? | |
| 14:53:56 | dansmith | cdent: we only needed map_instances to bridge between like mitaka and newton | |
| 14:54:08 | dansmith | cdent: now instance maps are created when we create instances | |
| 14:54:47 | dansmith | cdent: and we would need it if we were grafting a cell from one deployment onto another or something like that, so I don't know that we should remove it necessarily | |
| 14:54:53 | cdent | ah, right, I see. thanks. | |
| 14:55:03 | cdent | yeah, keeping it around seems a handy thing | |
| 14:56:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add BDM to InstancePayload https://review.openstack.org/448779 | |
| 14:58:19 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix api-ref for instance action events's attribute as optional https://review.openstack.org/482994 | |
| 14:58:45 | moshele | jaypipes: hi can you send an invite to my gmaiil moshe010@gmail.com ? | |
| 14:58:58 | ralonsoh | jaypipes: and rodolfo.ah@gmail.com | |
| 14:59:28 | bauzas | jaypipes: allocation requests looks good to me | |
| 14:59:45 | bauzas | jaypipes: but I just think we need to make sure those allocation requests would be for RPs in the same cell, nope ? | |
| 14:59:46 | jaypipes | moshele, ralonsoh: done | |
| 15:00:03 | dansmith | bauzas: yes, and he noted that | |
| 15:00:25 | jaypipes | bauzas: yes, I agree with that. but I need to rework these patches. I got off track returning alternate host states instead of a dict of allocation requests. | |
| 15:00:34 | bauzas | jaypipes: btw. I just left a question in https://review.openstack.org/#/c/482382/3 | |
| 15:00:52 | bauzas | jaypipes: about why we would not use the current existing method | |
| 15:01:11 | jangutter | jaypipes: If my work hangouts doesn't work, please yell. I can also do personal. | |
| 15:01:16 | bauzas | jaypipes: dansmith: btw. sorry about me not reviewing the patches previously, I was (again) in PTO | |
| 15:01:36 | dansmith | bauzas: I just assume you're on PTO unless you say otherwise | |
| 15:01:54 | bauzas | .... and Friday is an holiday day :( | |
| 15:02:02 | bauzas | so terrible | |
| 15:02:18 | bauzas | I should be done with all my PTOs for last year FWIW | |
| 15:02:58 | bauzas | I sincerely apologize for that | |
| 15:05:02 | gibi | mriedem: added the TODO and fixed / answered the fixture comments in https://review.openstack.org/#/c/448779 | |
| 15:06:36 | ftersin | mdbooth: hi, thank you for reviewing. If you have the opportunity, pls, look at my comments there, i just asked some details for the reworking | |
| 15:07:35 | mriedem | gibi: is the indent intentional here? https://review.openstack.org/#/c/448779/39/nova/tests/fixtures.py@1270 | |
| 15:08:03 | gibi | mriedem: yes, I tried to explain the reason of that in my response to your comment | |
| 15:08:57 | gibi | mriedem: either need the explicity detach in the test or I break the swap volume tests. | |
| 15:09:14 | mriedem | ok just checking | |
| 15:09:43 | gibi | mriedem: will propose a clean up on this fixture at some point | |
| 15:10:41 | mriedem | ok, +2 | |
| 15:10:48 | mriedem | i left a comment for other reviewers too | |
| 15:10:56 | mriedem | a todo or note in there would also be good, | |
| 15:10:56 | gibi | mriedem: thanks | |
| 15:10:59 | mriedem | but only if you respin | |
| 15:11:18 | gibi | mriedem: OK | |
| 15:11:59 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Enable cold migration with target host(1/2) https://review.openstack.org/408955 | |
| 15:12:30 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: List/show all server migration types (1/2) https://review.openstack.org/430608 | |
| 15:12:55 | bauzas | gibi: https://review.openstack.org/#/c/482497/1 is needed for what ? | |
| 15:13:16 | bauzas | just a clean-up right? | |
| 15:13:28 | gibi | bauzas: splited out from the BDM patch on top of it | |
| 15:13:48 | gibi | bauzas: it basically optimize out some data generation if the notification will not be sent out at the end | |
| 15:14:04 | bauzas | gibi: oh nevermind, it was just something missed from https://review.openstack.org/#/c/428260/ ? | |
| 15:14:32 | gibi | bauzas: yes, from that and from other patches later on | |
| 15:15:06 | bauzas | gibi: all good | |
| 15:15:16 | gibi | bauzas: thanks for the review | |
| 15:20:14 | ildikov | gibi: that fixture is a mess :( | |
| 15:22:47 | gibi | ildikov: I have my sticker to clean up it, but first I will optimize out some db query related to the instance.create | |
| 15:23:15 | gibi | ildikov: or do you have some time to work on it? ;] | |
| 15:23:23 | ildikov | gibi: I have a patch touching it with a note to figure something out there :) | |
| 15:23:42 | ildikov | gibi: well, let's see who gets there first ;) | |
| 15:24:31 | gibi | ildikov: well, that will be a real fight for that juicy commit | |
| 15:24:44 | ildikov | gibi: lol :) | |
| 15:26:37 | mriedem | gmann: i'm not sure i agree with this https://review.openstack.org/#/c/482994/ | |
| 15:26:45 | mriedem | it gets really confusing which is why i left it out of my patch | |
| 15:26:50 | mriedem | to mark 'events' as optional | |
| 15:30:46 | bauzas | gibi: may I ask a silly question about https://review.openstack.org/#/c/448779/39 ? | |
| 15:31:18 | bauzas | gibi: why some notifications samples provide an empty list for bdm notifications objects while others provide it? | |
| 15:31:20 | gibi | bauzas: sure | |
| 15:32:27 | gibi | bauzas: in some cases the bdm is overridden during the test | |
| 15:32:46 | gibi | bauzas: like test_volume_swap_server_with_error | |
| 15:33:04 | gibi | bauzas: or test_create_delete_server_with_instance_update | |
| 15:33:52 | gibi | bauzas: in test_create_delete_server_with_instance_update the bdm appears only after the instance goes through the block device mapping step during the create | |
| 15:34:20 | gibi | bauzas: so the early instance-updates has no bdm but at that point it starts appearing | |