| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-10 | |||
| 13:57:27 | sean-k-mooney | or more is that expected or should we have all the info in the build request? | |
| 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 | right | |
| 15:14:44 | dansmith | not in the command line tho | |
| 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 | but there were other issues that were specific to certain backports like the 'Conflicts' and 'NOTE' sections being the wrong way round | |
| 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: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/ | |