Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-28
16:23:46 openstackgerrit Matt Riedemann proposed openstack/nova-specs master: Server move operations with ports having resource request https://review.opendev.org/652608
16:46:24 cdent cfriesen: you might have thoughts on the discussion that starts near http://eavesdrop.openstack.org/irclogs/%23openstack-placement/%23openstack-placement.2019-05-28.log.html#t2019-05-28T16:20:51
16:46:40 cfriesen looking
16:47:35 cdent cfriesen: the spec in question is https://review.opendev.org/658510
16:48:15 cfriesen cdent: there is a flaw in the current handling of 4K pages which means that you can't mix numa and non-numa instances using 4k pages on the same host.
16:55:11 cdent cfriesen: if you've got a chance to dig at that spec that would be great. i've got to take off for today
16:55:12 openstackgerrit Adam Spiers proposed openstack/nova master: Provide HW_CPU_X86_AMD_SEV trait when SEV is supported https://review.opendev.org/638680
16:56:38 cfriesen cdent: will try. I left a comment elaborating a bit on the 4k page thing
16:56:47 cdent thanks
16:57:08 aspiers efried, mriedem: https://review.opendev.org/#/c/638680/16 temporarily adds os-traits to .zuul.yaml. I've put W-1 since IIUC we don't want to merge it like that.
16:57:22 aspiers kashyap: I guess you should be aware too ^^^
16:59:16 aspiers mriedem: you want the comment inline, or in the commit message?
16:59:59 aspiers would be nice if there was a magic string in commit messages which prevented zuul from merging
17:00:31 mriedem aspiers: i have a magic -2
17:00:35 mriedem aspiers: inline
17:00:47 mriedem and prefix your commit title with WIP:
17:00:55 mriedem so -W isn't lost between rebases
17:00:59 aspiers OK
17:01:20 aspiers IIRC, -2 is sticky across patchsets and has to be explicitly removed?
17:01:34 aspiers would be good if mortals could achieve something equivalent
17:01:46 aspiers I'm suggesting it in #zuul
17:04:15 aspiers probably more of a Gerrit thing actually
17:06:05 openstackgerrit Adam Spiers proposed openstack/nova master: WIP: Provide HW_CPU_X86_AMD_SEV trait when SEV is supported https://review.opendev.org/638680
17:19:28 openstackgerrit Adam Spiers proposed openstack/nova master: WIP: Provide HW_CPU_X86_AMD_SEV trait when SEV is supported https://review.opendev.org/638680
17:25:20 efried aspiers: I'm pretty confused about the rebasing state of all your patches.
17:25:32 aspiers efried: how can I help with that?
17:25:45 efried are you working on rebasing all of them to be in one chain?
17:25:57 aspiers only where necessary
17:26:38 aspiers my goals, in decreasing order, are: 1) correctness, 2) ease of review, 3) minimise churn on CI resources
17:27:33 aspiers it did recently occur to me that I could (theoretically, in a parallel universe where time is unconstrained) teach git-review to automatically rebase as necessary by using my git-deps module
17:29:12 efried what I mean is, for example, https://review.opendev.org/#/c/661782/2 is based on https://review.opendev.org/#/c/661266/3 when PS4 is available (and merging). And then https://review.opendev.org/#/c/638680/18 is based on https://review.opendev.org/#/c/661782/2, but https://review.opendev.org/#/c/636334/5 is based on https://review.opendev.org/#/c/638680/14
17:30:32 aspiers efried: that's because I forgot that you submitted PS4
17:30:50 aspiers efried: I just based it on the latest PS I had locally, without thinking
17:30:52 efried okay, my fault on that one
17:30:59 aspiers haha, hardly :)
17:31:18 aspiers but I'll generously share some of the blame with you if you insist ;)
17:31:37 efried anyway, as a reviewer I'm basically too confused to jump in atm
17:31:49 aspiers looking at the other ones now
17:31:50 efried which is fine, I have lots to keep me busy, not like I'm sitting on my hands waiting for this to be ready.
17:32:15 efried just want you to be aware that I'm likely to wait until there's no orange dots or green tildes on the right before I dig in.
17:32:25 spatel sean-k-mooney: hey
17:32:38 efried aspiers: (are you aware of the meaning of the orange dots and green tildes?)
17:32:41 aspiers OK, so essentially you want me to ditch my third goal? which is totally fine by me
17:32:46 aspiers yep, I had figured that out
17:33:41 aspiers I can understand the desire to eliminate orange dots. Why do you want to avoid green tildes though?
17:34:03 efried aspiers: well, I may be telling you that you may be reversing goals 2 and 3 :) at least from my pov
17:34:27 aspiers ACK, just trying to understand the cause of the confusion
17:34:29 efried because it means that those patches are based on older versions of these
17:34:43 efried it's usually a signal that you're still in the process of rebasing and cleaning things up.
17:34:56 aspiers ... which in the third example you listed is true
17:35:10 efried which is reinforced by having just WIP'd one near the bottom
17:35:20 efried anyway, again, I'm not trying to hurry you along.
17:35:29 aspiers I don't mind being hurried :)
17:35:56 efried just wanted to confirm that you're still shuffling the paperwork, as opposed to thinking you're ready for reviews and then getting frustrated when they don't happen
17:36:21 aspiers Well, this is a very useful discussion
17:37:18 aspiers It highlights that the unavoidable impact of deciding not to review anything within a given topic until all tildes and dots are gone, is that every time I need to change something minor lower in the dependency tree, I have to rebase every single thing above it before the new patchset gets reviewed
17:37:40 aspiers To me this highlights a failure in Gerrit's UI
17:38:17 efried or a failure in my method of working.
17:38:20 aspiers Ideally reviews would only be blocked if stale stuff is *below* them, not above
17:38:28 efried "blocked" is a bit strong
17:39:11 efried It's just the typical workflow I see (and use) is, when something needs to be fixed on a lower patch, download the whole series, git restack, fix the issues below, which automatically rebases the rest of the way up, and resubmit the whole series.
17:39:17 efried Agree that consumes a lot of CI resources
17:39:37 efried but it also ensures that the top changes will legitimately work with however the lower changes have been altered.
17:39:48 aspiers Sure
17:39:57 aspiers I'm not saying I want to avoid rebasing things
17:40:25 aspiers Just wondering if the presence of WIP higher up the tree should have an impact on stuff lower in the tree
17:41:16 aspiers If I understand the Gerrit UI correctly, we're talking about whether a patch in the Related Changes list has dots or tildes *below* it, right?
17:42:07 efried I'm not sure it's possible to have tildes below or dots above.
17:42:29 aspiers Oh, good point
17:42:29 efried I mean, technically possible, but you would really have to be trying.
17:42:29 efried ...unless you're looking at a PS that itself has a dot
17:42:30 efried anyway
17:42:57 efried the presence of WIP higher in the tree (which is not the same thing as a tilde, btw) should not impact reviews lower.
17:43:21 efried so this is probably just me being used to one workflow and you using a different one to (rightly) try to conserve CI resources.
17:44:18 aspiers Well, it's also about me needing to pay more attention to orange dots in Related Changes, so I can spot when rebases are needed
17:44:20 efried moot right now because I've reviewed everything up to the first one that's actually titled WIP, so regardless of colored decorations, I'm stopping here.
17:44:52 aspiers the WIP one is only marked WIP because of the required-projects hack. Other than that I think it's ready to go
17:44:56 efried but yeah, usually a patch that's riding an orange dot should be rebased if it's to get serious attention.
17:45:20 aspiers At least, other than rebasing (which I'll do now) I've run out of improvements to make to it
17:46:21 aspiers I put WIP on it because mriedem asked me to, but the only element of WIP there is that kashyap's os-traits patch hasn't landed and been released yet.
17:48:07 aspiers efried: also, since we're putting the workflow under the microscope, should I be putting the SEV blueprint: in the commit message and topic for changes like refactorings, which aren't per se SEV-related, but still form part of the SEV series?
17:48:39 efried nah
17:48:43 aspiers ok
17:50:37 openstackgerrit Adam Spiers proposed openstack/nova master: extract baselineCPU API call from _get_cpu_traits() https://review.opendev.org/661782
17:53:59 openstackgerrit Adam Spiers proposed openstack/nova master: WIP: Provide HW_CPU_X86_AMD_SEV trait when SEV is supported https://review.opendev.org/638680
17:58:34 openstackgerrit Merged openstack/nova-specs master: Update SEV work item to new approach based on MEM_ENCRYPTION_CONTEXT https://review.opendev.org/655717
17:59:08 openstackgerrit Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334
18:01:18 aspiers efried: got rid of some orange dots
18:05:09 aspiers (and that's from someone who's worked with them)
18:16:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Workaround missing RequestSpec.instance_group.uuid https://review.opendev.org/661786
18:17:00 openstackgerrit Matt Riedemann proposed openstack/nova master: Add regression recreate test for bug 1830747 https://review.opendev.org/661822
18:17:01 openstack bug 1830747 in OpenStack Compute (nova) "Error 500 trying to migrate an instance after wrong request_spec" [High,In progress] https://launchpad.net/bugs/1830747 - Assigned to Matt Riedemann (mriedem)
18:17:01 mriedem zigo: with a functional recreate test of the issue ^
18:24:42 openstackgerrit Adam Spiers proposed openstack/nova master: Reduce logging of host hypervisor capabilities to DEBUG level https://review.opendev.org/661379
18:34:09 ganso lyarwood, melwitt, dansmith, tonyb: Hello! When you have a minute, could you please take a look at those backports? https://review.opendev.org/#/c/652153 , https://review.opendev.org/#/c/658136 and https://review.opendev.org/#/c/661349 ? Thanks in advance! =)
18:44:14 aspiers efried: if CONF.libvirt.num_memory_encrypted_guests is not set and therefore defaults to None (meaning as we agreed in the spec, "auto-detect the inventory, if if this isn't possible, don't impose any limit), what should the MEM_ENCRYPTION_CONTEXT inventory get set to?
18:44:30 aspiers I'm not sure we came to a consensus with jaypipes on this http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-23.log.html#t2019-04-23T17:55:57
18:48:58 efried aspiers: MAXINT
18:49:10 jaypipes aspiers: not sure you need any consensus. :)
18:51:18 aspiers jaypipes: OK :)

Earlier   Later