| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-16 | |||
| 10:09:45 | sean-k-mooney | what im concened about it how much memoy this will use and can it impact other guests | |
| 10:09:51 | kashyap | sean-k-mooney: That is not relevant. Because you only launch the process *once* with 256MB of RAM | |
| 10:09:57 | kashyap | And generate the VARS file, and that's it. | |
| 10:10:06 | sean-k-mooney | once per vm | |
| 10:10:09 | kashyap | sean-k-mooney: No need for concern. It's a one-time thing for a few seconds | |
| 10:10:12 | kashyap | No. | |
| 10:10:13 | kashyap | Just once. | |
| 10:10:17 | kashyap | We will use a template file. | |
| 10:10:34 | sean-k-mooney | ok so why is this not a step triplo should be doing | |
| 10:10:44 | sean-k-mooney | or why are we not just shipping a template file | |
| 10:10:58 | kashyap | That is a different topic. I don't want to derail the core thing at hand | |
| 10:11:02 | sean-k-mooney | or requiring the operator to do it manually as part of install. | |
| 10:11:05 | kashyap | (Two steps at a time.) | |
| 10:11:13 | kashyap | sean-k-mooney: Yes, that's what I discussed with Lee. | |
| 10:11:26 | kashyap | The first step is to document that it is an external step an operator must take | |
| 10:11:32 | kashyap | Please take a few minutes to read the conversation before. | |
| 10:11:37 | sean-k-mooney | well i dont think nova should be doing it. | |
| 10:11:42 | kashyap | (I'm repeating myself here) | |
| 10:11:43 | sean-k-mooney | ok ill reread | |
| 10:12:13 | kashyap | sean-k-mooney: Nova won't do it. | |
| 10:12:58 | sean-k-mooney | ok then it should not be in the spec. we should have domumention for it or at most mentionit in the other deployer impact if it in the spec | |
| 10:13:43 | kashyap | Yes, will mention it as such | |
| 10:13:45 | sean-k-mooney | the current way the spec is layed out implied that it was a required change to nova. the actuly require change will be knowing where the template file is | |
| 10:13:59 | sean-k-mooney | ok | |
| 10:15:15 | kashyap | No, it is _not_ required | |
| 10:15:19 | kashyap | I didn't say it as such | |
| 10:15:29 | sean-k-mooney | rather then copying over all my comments ill leve them on patchset 8 | |
| 10:15:31 | kashyap | I said "consider" if we should. That is different than "required". | |
| 10:18:38 | sean-k-mooney | actully you said "Work out a way to integrate the external Python tool, | |
| 10:18:40 | sean-k-mooney | `ovmf-vars-generator` into Nova. | |
| 10:18:42 | sean-k-mooney | " | |
| 10:19:13 | sean-k-mooney | i was reviewing pathset 8 remember | |
| 10:19:29 | aspiers | hi folks | |
| 10:19:31 | sean-k-mooney | in patchset 10 you have change it to document that the operator ... | |
| 10:19:49 | sean-k-mooney | aspiers: hi | |
| 10:19:51 | kashyap | Yes. PS 10 was there when you started reviewing :-) | |
| 10:20:08 | aspiers | Yesterday I updated the AMD SEV spec in case anyone has time to review today | |
| 10:20:19 | aspiers | Not sure how many other specs are in your queue ;-) | |
| 10:20:25 | sean-k-mooney | kashyap: yep but i click on it from a gerrit comment form lee and never checked it was the latest :) | |
| 10:20:31 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Use source_bdms to reset attachment_ids during LM rollback https://review.openstack.org/652800 | |
| 10:20:32 | lyarwood | mdbooth: ^ you might be interested in this if you have time today | |
| 10:20:32 | aspiers | I'll be around in case there are things to discuss | |
| 10:20:36 | sean-k-mooney | aspiers: link? | |
| 10:20:52 | aspiers | sean-k-mooney: https://review.openstack.org/#/c/641994/ | |
| 10:20:59 | mdbooth | lyarwood: Shiny | |
| 10:21:00 | sean-k-mooney | yep just found it | |
| 10:21:01 | aspiers | I also put it in https://etherpad.openstack.org/p/nova-spec-review-day | |
| 10:21:57 | aspiers | sean-k-mooney: In particular I am wondering if hugepages can help with memory accounting. It might be a problem if it can only account for guest RAM and not the other QEMU memory chunks | |
| 10:22:37 | kashyap | sean-k-mooney: Can't parse this sentence: "-1 while i see if any of the comments i just left on patchset 8 have been adressed" | |
| 10:22:38 | sean-k-mooney | kashyap: ill clear my head a bit and take it form the top later today. by the way haveing matial content in the work items section makes it much harder to review IMO | |
| 10:23:12 | sean-k-mooney | kashyap: well i was going to -1 pathset 8 and i wanted to see if my coment were adressed in 10 | |
| 10:23:22 | sean-k-mooney | most werent so my -1 woudl still stand | |
| 10:24:29 | kashyap | sean-k-mooney: After I read you review in full. I'll post a new iteration, and write a top-level summary addressing your comments (as needed). (Instead of more inline replies there.) | |
| 10:24:35 | kashyap | Thanks for looking. | |
| 10:25:58 | aspiers | sean-k-mooney: but TBH I am a bit confused because the QEMU memory usage breakdown in the table https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/amd-sev-libvirt-support.html#proposed-change seems to contradict what Daniel Berrange has said more recently https://review.openstack.org/#/c/641994/2/specs/train/approved/amd-sev-libvirt-support.rst@167 where he suggests a fudge factor | |
| 10:25:59 | aspiers | of 1.5 | |
| 10:27:28 | sean-k-mooney | kashyap: cool sounds good. the main reason for the -1 is the implication that the nova libvirt driver ever supporte secure boot before ant that it was "actully" insecure. | |
| 10:27:43 | kashyap | sean-k-mooney: Just responding to that point. Will fix it. | |
| 10:28:00 | kashyap | sean-k-mooney: Why I wrote is that, some customers and users were _confused_ that Nova supports "SB" | |
| 10:28:20 | sean-k-mooney | yes they were wrong | |
| 10:28:38 | sean-k-mooney | but the release notes were pretty clear | |
| 10:28:54 | kashyap | Correct. I'll adjust the phrasing in spec | |
| 10:28:55 | sean-k-mooney | hyperv supprot SB and libvir only supports uefi boot | |
| 10:32:24 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Remove references to nova-consoleauth https://review.openstack.org/652965 | |
| 10:32:24 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Stop starting consoleauth in functional tests https://review.openstack.org/652966 | |
| 10:32:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: xvp: Start using consoleauth tokens https://review.openstack.org/652967 | |
| 10:32:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-status: Remove consoleauth workaround check https://review.openstack.org/652968 | |
| 10:32:26 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove nova-consoleauth https://review.openstack.org/652969 | |
| 10:32:26 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove ConsoleAuthToken.to_dict https://review.openstack.org/652970 | |
| 10:33:04 | kashyap | sean-k-mooney: While removing "implications", I will definitely mention the point that some people can get confused. | |
| 10:33:09 | kashyap | (Wording in the new iteration coming soon.) | |
| 10:34:55 | sean-k-mooney | sure that is resonable i guess | |
| 10:35:32 | sean-k-mooney | it should part of the problem statement section right? | |
| 10:37:11 | kashyap | (Yeah) | |
| 10:37:45 | kashyap | sean-k-mooney: I see that you were commenitng as you read the spec; as I mentioned the 'os_secure_boot' & 'os:secure_boot' in the Work Items | |
| 10:37:58 | kashyap | sean-k-mooney: How about I add a pointer to Work Items in the "Proposed change". | |
| 10:37:59 | sean-k-mooney | ya | |
| 10:38:04 | kashyap | (It is clearer than paragraphs of text.) | |
| 10:38:25 | sean-k-mooney | no the work items section as i said really should be sort like 10 lines max | |
| 10:38:45 | sean-k-mooney | all that content in my opipion shoudl be in teh proposed chages section | |
| 10:39:56 | sean-k-mooney | just grabbed a random spec but this is about the extent of what should be in work items https://github.com/openstack/nova-specs/blob/master/specs/rocky/approved/list-show-all-server-migration-types.rst#work-items | |
| 10:43:29 | kashyap | sean-k-mooney: Note: on your remark: about extending Hyper-V also to report traits -- I don't want to "boil the lake" (much less an ocean) with one spec | |
| 10:43:38 | kashyap | Few things at a time, with reasonable scope. | |
| 10:43:50 | kashyap | (That Hyper-V thing should be a separate item) | |
| 10:44:10 | kashyap | sean-k-mooney: Well, "10 lines max" for Work Items is a personal preference. | |
| 10:44:25 | kashyap | I'd like to follow this structure (which I think is far more clearer): | |
| 10:44:35 | kashyap | - Describe the change at high-level in the "Proposed change" section | |
| 10:44:56 | kashyap | - Describe the "how" in _some_ detail in the Work Items section | |
| 10:45:34 | aspiers | In the SEV spec I've probably gone somewhere in between those two | |
| 10:45:59 | cdent | kashyap: I agree that that format would probably make more sense, but it isn't the pattern that's been followed in the past | |
| 10:46:04 | aspiers | "Work Items" is not short, but "Proposed change" has more detail. | |
| 10:46:23 | kashyap | cdent: Nod | |
| 10:46:28 | aspiers | However that's because for SEV, the design part is much more difficult than the actual implementation | |
| 10:47:00 | aspiers | We've had to debate the design for ages and consider all kinds of complex stuff, but once the design is decided, the actual coding reflected in Work Items is reasonably simple | |
| 10:47:35 | aspiers | I expect each spec will have a different ratio between design complexity and implementation complexity, therefore the two sections will differ in size accordingly | |
| 10:47:44 | lpetrut | hi guys, I work on maintaining the Hyper-v driver and I'd also be interested in reporting traits / provider inventory | |
| 10:48:15 | lpetrut | in fact, I was catching up on the specs / libvirt patches to see what needs to be done | |
| 10:55:32 | aspiers | lpetrut: I'm far from an expert so this might be wrong, but IIUC the driver needs to implement update_provider_tree() | |
| 10:55:54 | lpetrut | thanks for the hint | |
| 10:56:11 | aspiers | lpetrut: see the libvirt driver for an example | |