| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-10 | |||
| 18:28:33 | mriedem | yeah i'm fine with it | |
| 18:28:49 | mriedem | need to review Kevin_Zheng's abort queued live migration stuff today anyway | |
| 18:29:06 | mriedem | i'll drop my +2 on yikun's notification patch | |
| 18:30:27 | dansmith | aight | |
| 18:30:32 | dansmith | I'll comment | |
| 18:31:54 | openstackgerrit | Eric Fried proposed openstack/nova master: Tighten up ReportClient use of generation https://review.openstack.org/556669 | |
| 18:33:35 | mriedem | https://review.openstack.org/#/c/563401/28/nova/notifications/objects/server_group.py@41 | |
| 18:33:35 | mriedem | me too | |
| 18:33:42 | mriedem | gibi: fyi ^ | |
| 18:46:05 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Use ironic-tempest-dsvm-ipa-wholedisk-bios-agent_ipmitool-tinyipa in tree https://review.openstack.org/581444 | |
| 18:48:37 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Use ironic-tempest-dsvm-ipa-wholedisk-bios-agent_ipmitool-tinyipa in tree https://review.openstack.org/581445 | |
| 18:53:02 | melwitt | looking for a +W on this ironic driver change needed to solve a race during instance creates https://review.openstack.org/563722 | |
| 18:55:45 | efried | Looks like jaypipes, dansmith, and johnthetubaguy have reviewed ^ in the past, so I'll stay away for now. | |
| 18:56:11 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: unquiesce instance after quiesce failure https://review.openstack.org/581451 | |
| 19:04:14 | openstackgerrit | Eric Fried proposed openstack/nova master: Delete orphan nodes before updating resources https://review.openstack.org/579922 | |
| 19:04:20 | openstackgerrit | Ken'ichi Ohmichi proposed openstack/nova master: Avoid BadRequest error log on volume attachment https://review.openstack.org/581453 | |
| 19:09:34 | openstackgerrit | Eric Fried proposed openstack/nova master: Address nits from consumer generation https://review.openstack.org/577227 | |
| 19:09:38 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: unquiesce instance after quiesce failure https://review.openstack.org/581454 | |
| 19:11:41 | mriedem | 7 days left to submit talks for berlin.... | |
| 19:14:16 | melwitt | I just swapped the runways (a day late, apologies) https://etherpad.openstack.org/p/nova-runways-rocky if anyone would like to add log notes for the removed ones and if dansmith could please update the channel topic | |
| 19:18:06 | dansmith | the first one doesn't have the blueprint slug in it | |
| 19:18:18 | melwitt | argh, sorry | |
| 19:19:53 | melwitt | thanks | |
| 19:21:40 | melwitt | vdrok, jroll, TheJulia: is this patch needed for https://blueprints.launchpad.net/openstack/?searchtext=allow-reserved-equal-total-inventory ? it's in merge conflict https://review.openstack.org/565841 | |
| 19:22:28 | melwitt | link correction https://blueprints.launchpad.net/nova/+spec/allow-reserved-equal-total-inventory | |
| 19:26:57 | TheJulia | melwitt: I think jroll is the only person who can know for sure, it looks like it just ought to be abandoned based upon the discussion. I know jroll has been super busy as of recent. | |
| 19:27:24 | melwitt | TheJulia: ack, thanks | |
| 19:28:25 | jroll | melwitt: I'll look post-meeting, been meaning to get back to that | |
| 19:28:46 | melwitt | jroll: thx | |
| 19:28:54 | jroll | I'd say it's needed but not for that BP | |
| 19:31:19 | melwitt | jroll: thanks for confirming. I'll update https://etherpad.openstack.org/p/nova-rocky-blueprint-status to call out just the one patch as needing review | |
| 19:31:29 | jroll | ++ | |
| 19:31:34 | melwitt | (this one https://review.openstack.org/517921) | |
| 19:34:26 | jroll | right | |
| 19:34:33 | melwitt | ty | |
| 19:34:34 | jroll | thanks for checking on that :) | |
| 19:40:51 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Nova objects for Libvirt NUMA config https://review.openstack.org/581456 | |
| 19:46:02 | melwitt | does anyone know what's the deal with vgpu work this cycle? is it blocked on reshaper work or is some of it okay to merge at the moment? https://review.openstack.org/#/q/topic:bp/vgpu-rocky+(status:open+OR+status:merged) | |
| 19:47:25 | dansmith | the numa-ness is blocked on reshaper AFAIK | |
| 19:48:15 | dansmith | which would be the NRP ones up there I guess, and some libvirt ones that don't appear to be in the list (and maybe aren't written yet) | |
| 19:48:41 | melwitt | hm, ok | |
| 19:49:28 | melwitt | efried: should we -W this one if we need to hold it behind reshaper? https://review.openstack.org/520313 | |
| 19:50:50 | dansmith | man that's a lot of rechecks | |
| 19:51:11 | efried | melwitt: I had previously -2'd https://review.openstack.org/#/c/521041/ which is the patch that actually puts the code in the way of the compute manager. But perhaps we want to move that -2 to the bottom as you say, since the first two patches don't do anything without the top one. | |
| 19:52:25 | melwitt | efried: k. at a glance, it looked like things we could merge as progress so it would help to -W or -2 to show that it needs to wait | |
| 19:52:53 | efried | melwitt: Yes, we could merge the bottom two without hurting anything. | |
| 19:53:26 | efried | melwitt: I think some people might object to merging what's essentially "dead code". | |
| 19:53:51 | efried | melwitt: Me, I'd rather see it merged so we've got a shorter path when we're ready to pull the trigger. | |
| 19:54:09 | efried | But no strong feelings either way. | |
| 19:54:21 | dansmith | I think we should wait | |
| 19:54:46 | dansmith | merging refactors early in a set or something are useful, but if it's really dead code, we might as well wait, IMHO | |
| 19:55:48 | melwitt | I don't have a strong feeling about it. I think it'd be okay to merge some progress but waiting is also fine | |
| 19:56:59 | melwitt | mriedem: we have a proof-of-concept patch for the rbd erasure coding config option as of today, fyi https://review.openstack.org/581055 | |
| 19:57:23 | melwitt | jmlowe: is this something we can see working in the ceph job results? using the erasure coding? http://logs.openstack.org/55/581055/2/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/d6331b3/ | |
| 19:58:54 | jmlowe | you're thinking functional test? | |
| 19:59:44 | melwitt | jmlowe: no, I mean I was wondering if there's anything ceph would log (in the ceph job I linked) that shows passing the data_pool did something. just curious if it's something we could see | |
| 20:01:18 | jmlowe | I don't think so, let me poke around a little more, you can check the data pool of a rbd device with the ceph api but it's supposed to be relatively transparent aside from disk usage increasing in some other pool | |
| 20:02:01 | jmlowe | or is that what you are thinking, write a gig and check the disk usage of the pools? | |
| 20:02:57 | melwitt | okay, that's cool. no, I wasn't thinking ahead that complex, just thought it'd be handy if something got reflected in the ceph job run for free | |
| 20:04:55 | melwitt | for one thing, I doubt the job is running luminous so it wouldn't do anything anyway | |
| 20:07:26 | melwitt | oh, it actually is. cool. http://logs.openstack.org/55/581055/2/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/d6331b3/logs/ceph/ceph-mgr.x.txt.gz#_2018-07-10_01_16_55_913130 | |
| 20:08:12 | jmlowe | yeah, it is running luminous so it doesn't blow up | |
| 20:08:57 | melwitt | jmlowe: what do you mean, what would make it blow up if not luminous? | |
| 20:09:29 | dansmith | passing the flag that turns it on right? | |
| 20:09:39 | jmlowe | I'm pretty sure it's a generic abstraction, so you can do stupid ceph tricks like set the data pool to some other replicated pool, that wouldn't do anything useful except save you the extra work of setting up an erasure coded pool in a testing environment | |
| 20:09:39 | dansmith | because EC isn't supported pre-limunous? | |
| 20:09:44 | dansmith | luminous | |
| 20:10:21 | melwitt | well, I thought jmlowe had said that passing data_pool=None pre-luminous would make sure it doesn't hurt anything | |
| 20:10:26 | jmlowe | guessing if you used the EOL jewel python binding it wouldn't have the kwarg for data_pool | |
| 20:10:49 | melwitt | oh, I see. so we need to guard this behavior with a check for ceph version | |
| 20:11:09 | jmlowe | yeah, nobody should be using jewel by the time this lands | |
| 20:11:16 | melwitt | or are we safe assuming has to be >= luminous at this point? | |
| 20:11:21 | melwitt | okay | |
| 20:11:55 | jmlowe | you have to do stupid rpm tricks with centos if you wanted to use it with queens or later | |
| 20:12:37 | melwitt | okay, good to know | |
| 20:12:46 | jmlowe | I think the queens repo forces install of luminous repo for centos, was an annoyance when I was going to mimic | |
| 20:21:15 | mriedem | dansmith: i just went through Kevin_Zheng's 2nd in series live migration abort queued patch and only thing that kind of bothers me is the duplicate validation in the rpc api here https://review.openstack.org/#/c/568542/15/nova/compute/rpcapi.py - i've left an alternative and looking for a 2nd opinion on that | |
| 20:23:11 | dansmith | mriedem: your link in that comment doesn't seem to point to anything relevant | |
| 20:24:50 | dansmith | presumably you're looking at something that is checking service version | |
| 20:25:16 | dansmith | which is cool, but if the service_version says things are okay, but they still have their rpc api manually pinned, they could conflict, so failing somehow is probably appropriate | |
| 20:28:51 | dansmith | oh sorry, are you talking about the migration status? | |
| 20:29:01 | mriedem | yeah | |
| 20:29:15 | dansmith | sorry, I totally focused on the version checks | |
| 20:29:25 | mriedem | the api already checks if the migration status is running and if not it bombs out (today) | |
| 20:29:37 | mriedem | then calls the rpc api which if the compute is old, makes the same check | |
| 20:29:44 | dansmith | I don't think I understand why we need to check the status of the migration again | |
| 20:29:54 | mriedem | see my comment | |
| 20:30:28 | mriedem | i think the scenario is you are attempting to abort a queued live migration but the compute is old | |
| 20:30:49 | dansmith | yeah I see it, but.. this code runs in the api itself, and doesn't actually know the version of the compute it's going to talk to | |
| 20:30:52 | mriedem | his check for 'compute too old' is the can_send_version | |
| 20:31:06 | dansmith | that's not compute-aware | |
| 20:31:58 | mriedem | so his version check on the nova-compute service needs to happen in the api change if the user is trying to abort a !running migration and the host is old | |
| 20:32:02 | mriedem | s/host/service/ | |
| 20:32:09 | dansmith | can_send_version() is looking at the api service's rpc pin, which might be computed from service version, or manually set | |
| 20:32:23 | dansmith | mriedem: right | |
| 20:32:28 | mriedem | ok, he's just following similar checks in compute rpc api for things like tagged attachments and such | |
| 20:32:49 | dansmith | mriedem: and by the time we get to rpcapi, we don't have any new information we didn't have a couple frames up on the stack, so no need to do it again | |
| 20:32:57 | openstackgerrit | Matt Rabe proposed openstack/nova master: Add destination MSP IP address to PowerVM migrate data https://review.openstack.org/581463 | |
| 20:33:07 | dansmith | yeah, those are for things that haven't already checked service version for the actual compute and are relying on the pin I think | |
| 20:33:20 | dansmith | which is a way to do it, but you don't need to do it in both places | |