Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-06-10
14:17:26 dansmith sean-k-mooney: it means something tried to read that field on the instance.. if we're doing that based on something in the request, then it's probably good to lazy-load it, so we're not always passing it regardless of whether or not pci is used
14:17:43 dansmith if we're always looking at that regardless, then the lazy load is just a perf hit and we should pre-join it
14:18:36 sean-k-mooney this was just before the xml was generated for an instance with no pci requetss or numa policy set
14:19:11 sean-k-mooney i might take a look at why its doing that in a bit
14:19:18 sean-k-mooney im currently testing file backed memroy
14:19:47 dansmith in that case, we could either be smarter about when we look at that field to avoid the lazy-load, or we should just pre-join it
14:25:56 sean-k-mooney alex_xu: https://review.opendev.org/#/c/734658/4/doc/source/admin/file-backed-memory.rst@70 i added info about the numa and hugepage behavior
14:26:48 sean-k-mooney numa more or less works although the numa toplogy filter will be looking at the host ram when considering numa affinity not the file backed memory
14:26:59 sean-k-mooney i think i a werid way that is actully the correct thing to do
14:27:18 sean-k-mooney as it will not allow a guests virtual numa node to be large then a hosts numa node
14:28:47 sean-k-mooney libvirt is also passing the correct numa affinity hints to qemu for the files too
14:29:40 sean-k-mooney so the things we could fix are hugepages, and how the local_GB is reported. both of which could be done via existing config options
14:31:39 sean-k-mooney so we can just have one more pathc to document it. in the long run we might want to adress this diffrently. we dont need a code change to do the write thing but i think we should warn when we detect you have not set the config options correctly as the current patch does
14:40:10 openstackgerrit Alexandre Arents proposed openstack/nova master: Snapshot: offload glance upload in a native thread https://review.opendev.org/734776
15:02:18 stephenfin Can anyone say why we include "cherry-picked from" lines in stable? They seem like a waste of time, particularly when you've to make changes to the backport
15:02:40 stephenfin and Gerrit already does the linking based on change ID
15:04:35 sean-k-mooney convention
15:04:43 sean-k-mooney but other then that its not really neeed
15:04:54 sean-k-mooney also if you do it via gerrit it does it differrently
15:05:03 sean-k-mooney e.g. it only add the first one
15:05:12 sean-k-mooney unless you do the backport on by one i think
15:05:14 stephenfin Looks like it's to work around a Gerrit bug that has since been fixed https://docs.openstack.org/project-team-guide/stable-branches.html
15:05:27 stephenfin cherry-pick -x option includes ‘cherry-picked from …’ line in the commit message which is required to avoid Gerrit bug
15:05:27 stephenfin cherry-pick -x option includes ‘cherry-picked from …’ line in the commit message which is required to avoid Gerrit bug
15:05:35 stephenfin which points to https://code.google.com/p/gerrit/issues/detail?id=1107
15:05:52 sean-k-mooney its not required by any of our tooling
15:05:58 stephenfin nope
15:06:00 sean-k-mooney upstream or downstrema
15:06:08 stephenfin I propose we stop doing it. It's annoying and often wrong
15:06:14 stephenfin Will bring up in the next team meeting
15:06:19 sean-k-mooney so use -X
15:06:29 sean-k-mooney instead of -x
15:08:38 openstackgerrit Merged openstack/nova-specs master: Encrypted Emulated Virtual TPM https://review.opendev.org/728505
15:09:06 stephenfin sean-k-mooney: What would -X do? the man page says something about merge strategy
15:09:33 sean-k-mooney git-review -x add the cherry picked line -X does not
15:10:13 sean-k-mooney oh sorry its the other way around
15:10:20 sean-k-mooney -X adds it and -x does not
15:10:33 stephenfin I don't think the -X is related, actually
15:10:38 stephenfin Reading the docs
15:10:50 sean-k-mooney so if you backporting using git review
15:10:51 stephenfin you won't get the line unless you specify -x
15:10:57 sean-k-mooney the workflow is checkout stable/whatever
15:11:04 sean-k-mooney then do git review -X
15:11:09 stephenfin ahhhh
15:11:12 sean-k-mooney to add the cherry picked lins
15:11:14 stephenfin I just use git-cherrypick
15:11:26 sean-k-mooney but if you do -x it does not add them
15:11:34 sean-k-mooney ya i was talkingbaout the arge for git review
15:11:39 dansmith I guess I'm confused.. why is it not helpful? When I'm browsing history at the command line, it's helpful.. otherwise I have to go into gerrit by change-id to find the siblings
15:11:40 sean-k-mooney not git cherry-pick
15:12:08 sean-k-mooney dansmith: im assuming stephne it ac case where it was wrong
15:12:24 dansmith when is it ever wrong? I can't think of a case.
15:12:31 melwitt most of the time, I like the cherry picked lines because it lets me easily see if the changes were picked from N branch to N - 1 branch. but agreed it's a pain when you have to update one
15:12:37 sean-k-mooney when people do it manually and get it wrong
15:12:55 sean-k-mooney i honestly dont mind either way to be honest
15:13:08 stephenfin dansmith: in general they're correct, but things break if you've to make any modification to an earlier patch
15:13:11 sean-k-mooney the only thing i find annoying is that gerrit does not do it the way we want
15:13:35 dansmith it also helps me see how far something was backported.. like if I'm looking at something on stein and it has N backport lines, I know that came from stein+N
15:14:29 sean-k-mooney stephenfin: maybe try using git review for backport for a while
15:14:34 stephenfin True, but you have that information in the cherry picks box on the right
15:14:39 stephenfin in the UI, that is
15:14:44 dansmith not in the command line tho
15:14:44 dansmith right
15:15:19 stephenfin fair point
15:15:32 stephenfin sean-k-mooney: in general, there's no issue. I just use 'git cherry-pick -x'
15:16:03 stephenfin but I've been bitten a few times now where I had a stack of stable changes and had to make a tweak to a single patch higher up
15:16:50 stephenfin so you've add the missing mock or the typo in your commit message or whatever else you missed
15:17:22 stephenfin and then you've to go update the cherry-picked lines in every backported patch plus any patch on top in each branch. not fun :)
15:17:32 stephenfin it'd be nice if it was optional
15:18:10 stephenfin as something that's nice-to-have but not really necessary, particularly if it incurs pain
15:18:34 stephenfin *when it incurs pain
15:21:08 dansmith well, my vote is that it's worthwhile bookkeeping
15:22:19 stephenfin Definitely worthwhile but not essential. I'd like to be able to loosen the cuffs on the occasion they start hurting
15:24:49 elod if i'm not mistaken, then gerrit adds the cherry-picked lines correctly, iff the original patch has merged. if the patch is not merged yet, it does not add the cherry-picked line, for logical reasons, as it might not be the one that gets merged
15:25:14 stephenfin elod: Ah, so that's how it makes the decision :) Can't confirm but makes sense
15:26:09 stephenfin Waiting for a patch series to land on stable/X before backporting to the stable/X-1 etc. is less than optional though. It causes unnecessary context switching for both submitter and reviewer, assuming the submitter even remembers to keep backporting things
15:27:00 elod that's true, we had the same discussion in our team :]
15:27:34 stephenfin Ha! I'm about to discuss it downstream too 😇
15:27:34 stephenfin Ha! I'm about to discuss it downstream too 😇
15:27:37 dansmith I'd much prefer to have the bookkeeping right and I think it's very unusual that backports to N releases are that urgent
15:28:25 elod otherwise, if a patch needs to be fixed, then all its backports need to be fixed, so that's another extra burden. so there are two sides
15:29:35 elod if we stick to policy and do the backporting branch-by-branch, when a patch has merged, then this is not an issue, though
15:29:39 stephenfin dansmith: so not including the cherry-picked from line would be very unusual too
15:29:48 stephenfin it'd be exceptional, not the norm
15:31:03 stephenfin elod: yes and no. It really depends on the nature of the change to the higher up patch
15:32:57 stephenfin Consider that series I reviewed from lyarwood yesterday. The extra 'cherry-picked from' line was something that affected all commit messages
15:32:57 stephenfin but there were other issues that were specific to certain backports like the 'Conflicts' and 'NOTE' sections being the wrong way round
15:33:56 stephenfin it's not black and white
15:34:19 stephenfin anyway, I've made my point :) now to forget about it until next time I hit this
15:35:56 elod if the editing of a 'middle' patch is allowed, then that could be a source of an inproper backport. i mean most of the time it's not a problem, but sometimes we forget to backport some new changes
15:36:34 elod the cherry-picked line is good to identify where the backport is coming from
15:37:18 elod and as dansmith said, it's good to see how many previous backports there are
15:38:44 elod but yes, if we want to backport to multiple branches at the same time, it could be painful :)
15:39:01 stephenfin yup, all fair points
15:39:09 stephenfin it'd just be nice have the optional, is all
15:39:15 stephenfin *the option
15:53:30 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184
16:13:21 openstackgerrit Jiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301
16:16:14 stephenfin sean-k-mooney: your cherry-picked from lines are wrong in this https://review.opendev.org/#/c/728057/
16:19:18 sean-k-mooney they may be yes

Earlier   Later