Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-30
19:12:20 mriedem i'll push the backport to queens once i get done with reviewing sahid's trusted vf series again
19:19:57 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: configure trust mode for vfs https://review.openstack.org/458514
19:19:58 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: add vf_trusted field for network metadata https://review.openstack.org/566343
19:19:59 openstackgerrit Matt Riedemann proposed openstack/nova master: metadata: add vf_trusted field to device metadata https://review.openstack.org/566344
19:20:00 openstackgerrit Matt Riedemann proposed openstack/nova master: network: update pci request spec to handle trusted tags https://review.openstack.org/458820
19:44:29 efried jaypipes: Done. mriedem: I'm +2 on https://review.openstack.org/#/c/553597/ now.
19:46:40 mriedem artom: you might want to take a quick run through this https://review.openstack.org/#/c/566344/
19:46:47 mriedem efried: ok
19:47:38 mriedem "Snarklepuss is actually my middle name, not my surname." blows my mind
19:47:52 mriedem that's professional snark at a PhD level
19:48:25 artom mriedem, indeed, thanks for motivating me
19:48:38 artom That series has been sitting on my review list since forever
19:49:15 mriedem artom: besides that one patch i'm +2 on the rest
19:49:30 mriedem artom: if you're so inclined, you could even address my comments in that one and we could get the entire thing +2ed today
19:49:50 artom mriedem, not a bad idea, sahid's done for the day anyways
19:50:04 artom I don't think he'll mind
19:50:28 artom Will we lose all your +2s if I push/rebaes tho?
19:50:29 mriedem i've been addressing my own nits to keep it going
19:50:31 mriedem so yeah he won't mind
19:50:34 mriedem no
19:50:41 artom "rebaes"
19:50:52 mriedem if you rebase like you spell, we might have problems
19:50:53 artom When you come back to your bae
19:51:03 tssurya mriedem, dansmith: thanks for the reviews, I just got back from a (2 weeks long) vacation and also don't have anything for the meeting. Will catch up on the summit stuff from tomorrow.
19:51:16 dansmith tssurya: cool
19:51:41 mriedem tssurya: rest assured we talked you up big time to belmiro and tim in vancouver
19:51:56 tssurya mriedem: haha thanks a ton!!
19:52:42 mriedem artom: just: git review -d 458820 && git rebase -i HEAD~2
19:52:52 mriedem fix the one, then git rebase --continue && git review -R -y
19:53:29 artom Ah, yeah, rebasing on the one below it instead of master isn't a bad idea
19:53:35 artom Tiny bit patronizing tho ;)
19:54:08 mriedem right don't rebase on master
19:54:15 mriedem that makes the diffs between patches hard to follow
19:54:38 mriedem i only rebase on master if i have a merge conflict
19:54:59 artom At some point, someone somewhere will make a diff of diffs tool
19:55:08 artom I remember Diana talking about it before she left Red hat
19:55:17 dansmith git review can do that for ou
19:55:21 mriedem tonyb probably already has one
19:55:39 artom dansmith, yeah? Will need to look into that
19:55:44 dansmith git review -m XXX,4,5
19:56:03 mriedem i'm currently explaining the difference, over text message, between bratwurst, kielbasa and summer sausage
19:56:10 mriedem and can't tell if it's some cruel joke
19:56:29 dansmith git review can't do that
19:56:48 dansmith git review will do anything for love, but it won't ... doo... thaaat.
19:56:51 mriedem git review --sausage-index
19:56:54 dansmith hah
19:57:24 dansmith I think you just did
19:57:38 artom So meta
20:01:03 artom mriedem, so is 'vf_trusted' always going to show in device metadata from now on? And set to True/False?
20:01:22 artom Or it'll only appear if the VF is trusted?
20:01:31 artom (Yeah, I could read, but... :/ )
20:01:44 mriedem he's always putting it in,
20:01:50 mriedem default to False for old devices, which is correct
20:02:04 mriedem the new field shows up if the requested version on the metadata api is new enough
20:02:07 artom I'm cool with whatever, just wanted to be sure
20:02:32 mriedem i tend to like adding new fields like we do in the compute rest api, which is we add the new field always, we don't expose it conditionally based on it's value
20:02:50 dansmith definitely
20:03:10 dansmith not all languages have a __contains__ type operator, which makes checking for the presence before the value annoying
20:09:40 mriedem efried: jaypipes: +W on the mirror patch
20:09:53 efried süsse
20:18:31 openstackgerrit Matt Riedemann proposed openstack/nova master: Skip ServerShowV254Test.test_rebuild_server in cells v1 job https://review.openstack.org/569454
20:21:56 artom mriedem, https://review.openstack.org/#/c/566344/7/nova/tests/unit/test_metadata.py@244
20:22:52 artom we need to include it though, no? We're asserting that the metadata will be equal to those dicts we define, so we need to add vf_trusted=False because we expect the code to generate that
20:23:36 mriedem oh is that the thing we're asserting against in the response?
20:23:39 mriedem if so, then sure
20:23:42 mriedem it wasn't obvious
20:24:04 artom Yeah, it's giving me a headache too
20:24:26 mriedem without reading it yet, is it fair to say the ML thread on review nit picking has itself succumbed to nit picking?
20:25:11 artom Duh
20:25:28 artom Really accessible topic, easy visibility points
20:25:32 artom Even *I* contributed
20:45:05 artom mriedem, need to relocate home (via daycare) will try to finish the metadata thing tonight
20:45:14 artom You east or west coast?
20:45:21 artom (Locality-wise, not rap)
20:45:44 artom Though I guess we already have your +2, it's jaypipes or whoever else we need to care about
20:46:28 jaypipes artom: hmm?
20:47:19 artom jaypipes, oh, apologies, for some reason I was sure mriedem roped you into reviewing https://review.openstack.org/#/q/topic:bp/sriov-trusted-vfs+(status:open+OR+status:merged)
20:47:28 artom He's +2 except for 1 patch with nits
20:47:31 artom Which I'm trying to fix
20:47:45 mriedem artom: MN
20:47:47 mriedem no coast
20:47:58 jaypipes artom: ok. can try tomorrow morning (there's a bunch of stuff ahead of that right now)
20:48:10 artom jaypipes, heh, it wasn't a request, but thank you!
20:48:16 mriedem we can throw it in the runways queue
20:48:23 mriedem sahid should have already done that anyway
20:48:28 mriedem i'll add it
20:48:45 mriedem artom: east coast rap-wise
20:49:00 jaypipes mriedem: definitely west coast.
20:49:04 artom mriedem, ok, you have an hour or so on me in terms of time left in day, you might still be around to +2 it if I succeed in making those damn tests pass
21:12:54 mriedem dansmith: before Kevin_Zheng goes off and adds some backoff looping call logic here, you might want to make sure we're not missing something on the problem for which he's trying to account https://review.openstack.org/#/c/568542/1/nova/compute/manager.py@6222
21:13:02 mriedem basically a race window during abort of a live migration
21:26:30 mriedem hmm, also, on surya's patch, she already has a release note https://review.openstack.org/#/c/567878/5/releasenotes/notes/migration-tool-to-populate-inst.avz-29fed2fe57a9764d.yaml
21:26:37 mriedem so i'm not sure what to add to that in the backport
21:27:54 dansmith mriedem: right which is why I suggested a new one
21:28:01 dansmith mriedem: commented on that live migration abort thing
21:28:29 dansmith I think proper ordering will make that mostly unracy for our purposes without a polling loop
21:35:09 mriedem dansmith: alright this is what i had added, but it overlaps pretty heavily http://paste.openstack.org/show/722381/
21:37:11 dansmith mriedem: I just wanted something that indicates that they have to run them again, if you think what is there now is enough, then that's fine
21:37:19 dansmith I agree your paste adds nothing to what is there,
21:37:28 dansmith I was just thinking about calling it out specifically

Earlier   Later