| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-05 | |||
| 10:30:08 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: report VPMEM resources by provider tree https://review.opendev.org/678454 | |
| 10:30:08 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Support VM creation with vpmems and vpmems cleanup https://review.opendev.org/678455 | |
| 10:30:09 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Parse vpmem related flavor extra spec https://review.opendev.org/678456 | |
| 10:30:09 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver configuring PMEM namespaces https://review.opendev.org/679640 | |
| 10:30:10 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | |
| 10:30:11 | openstackgerrit | Luyao Zhong proposed openstack/nova master: doc: attaching virtual persistent memory to guests https://review.opendev.org/680300 | |
| 10:30:13 | aspiers | but I was struggling to figure out whether SEV is libvirt specific. At least machine types are QEMU-specific | |
| 10:30:40 | sean-k-mooney | sev is tecnically not | |
| 10:30:48 | sean-k-mooney | but the way you are checking for it is | |
| 10:30:55 | kashyap | aspiers: SEV is hardware-specific; machine types indeed are not "libvirt | |
| 10:31:04 | kashyap | ... -specific" | |
| 10:31:13 | kashyap | As you discovered (read the chat). | |
| 10:31:25 | kashyap | But they're typically handled by libvirt for Nova. | |
| 10:31:29 | aspiers | Right | |
| 10:32:41 | aspiers | The problem is that get_mem_encryption_constraint() and its helpers are tied to virt.hardware | |
| 10:33:11 | sean-k-mooney | aspiers: move sev_enabled out of utils into the driver | |
| 10:33:12 | aspiers | because they rely on _get_flavor_image_meta() in that file, and so do other functions there | |
| 10:34:06 | aspiers | sean-k-mooney: originally sev_enabled had to be outside driver.py | |
| 10:34:13 | aspiers | so it could be called from vif.py | |
| 10:34:25 | aspiers | but now I moved the iommu stuff into designer.py | |
| 10:34:31 | aspiers | so maybe I could move it back | |
| 10:34:36 | sean-k-mooney | all calls from vif.py are form the driver | |
| 10:34:46 | sean-k-mooney | so you can pass it in a flag if its needed | |
| 10:34:58 | aspiers | that wasn't easily doable before | |
| 10:35:04 | sean-k-mooney | but we dont pas the libvirt host object to any other funciton in utils | |
| 10:35:07 | aspiers | but now it's not needed anyway | |
| 10:35:30 | sean-k-mooney | the utils dont actully talk to libvirt as far as i can see | |
| 10:35:36 | aspiers | right | |
| 10:35:52 | sean-k-mooney | so the sev_enabled check does not fit with the rest of the module | |
| 10:36:41 | sean-k-mooney | so ya simple fix is just make it a member funciton of the driver then you can do self.host and just pass in the flavor and image | |
| 10:37:14 | aspiers | I don't think this is sufficient | |
| 10:37:27 | aspiers | how would https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/utils.py@38 change? | |
| 10:37:34 | aspiers | oh sorry | |
| 10:37:37 | sean-k-mooney | i will remove the only use of hardware.py form the utils | |
| 10:37:42 | aspiers | wrong link | |
| 10:38:06 | aspiers | how would https://review.opendev.org/#/c/680065/4/nova/virt/hardware.py change? | |
| 10:38:18 | sean-k-mooney | that would not | |
| 10:38:30 | aspiers | but that already causes a cycle | |
| 10:38:33 | sean-k-mooney | your only calling libvirt_utils.get_machine_type(image_meta) | |
| 10:38:49 | sean-k-mooney | how | |
| 10:38:56 | aspiers | look at the CI failures :) | |
| 10:39:07 | sean-k-mooney | hardware.py will import utils but utils wont import it | |
| 10:39:33 | sean-k-mooney | so if you remove the change in https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/utils.py@38 | |
| 10:39:38 | sean-k-mooney | its not an issue right | |
| 10:40:30 | aspiers | ohhh OK those CI failures on the first patch are something else | |
| 10:40:42 | aspiers | I thought I saw circular import errors in both patches but I was wrong | |
| 10:40:50 | aspiers | cool | |
| 10:42:11 | sean-k-mooney | there is another way to break circual dependcies in python but moveing htat function is the simplest | |
| 10:42:23 | aspiers | which one? | |
| 10:42:28 | aspiers | sev_enabled? | |
| 10:42:31 | sean-k-mooney | yes | |
| 10:42:34 | sean-k-mooney | moving it to the driver | |
| 10:42:42 | aspiers | you mentioned another function in utils you are gonna move? | |
| 10:42:45 | aspiers | which one? | |
| 10:42:59 | sean-k-mooney | no you dont need to move anything else | |
| 10:43:04 | aspiers | oh that's the only one OK cool | |
| 10:43:06 | sean-k-mooney | just the new funtion your intoducing | |
| 10:43:14 | aspiers | so move that to libvirt/driver.py | |
| 10:43:18 | aspiers | where it was originally :) | |
| 10:43:20 | sean-k-mooney | ya | |
| 10:43:22 | aspiers | cool | |
| 10:43:29 | aspiers | using designer.py made everything a lot cleaner | |
| 10:43:33 | aspiers | this is another example of that | |
| 10:43:42 | aspiers | thanks a lot! | |
| 10:43:45 | sean-k-mooney | we want to kill desigenr.py | |
| 10:43:48 | yonglihe | sean-k-mooney, stephenfin: good morning, guys. | |
| 10:43:51 | aspiers | my brain was hurting from thinking about this | |
| 10:43:53 | sean-k-mooney | or at least we did | |
| 10:44:10 | aspiers | sean-k-mooney: there should be a list of tech debt goals somewhere public | |
| 10:44:11 | sean-k-mooney | yonglihe: hi | |
| 10:44:15 | aspiers | sean-k-mooney: e.g. storyboard | |
| 10:44:28 | aspiers | maybe it's on an etherpad somewhere | |
| 10:44:29 | sean-k-mooney | aspiers: i was going to say etherpad would be good. | |
| 10:44:35 | aspiers | WFM too | |
| 10:44:49 | aspiers | OK, gonna take a break | |
| 10:45:23 | sean-k-mooney | aspiers: the designer.py was intoduced by danpb to breakout some fo the code out of the driver but it was started and never finish and became techdebt | |
| 10:45:39 | sean-k-mooney | really we shoudl merge it into the config.py | |
| 10:45:48 | aspiers | I disagree | |
| 10:45:56 | sean-k-mooney | its basically doing the same thing | |
| 10:46:00 | aspiers | I think the scoping text at the top of config.py is good | |
| 10:46:10 | sean-k-mooney | well i gues | |
| 10:46:12 | aspiers | it's good to keep logic out of config,py | |
| 10:46:22 | sean-k-mooney | config.py is giving you the datamodels | |
| 10:46:24 | aspiers | just keep it parsing/generating XML only | |
| 10:46:28 | sean-k-mooney | and serialistaion code | |
| 10:46:38 | sean-k-mooney | and the designer is wiring up the elements | |
| 10:46:51 | sean-k-mooney | but the problem is so is the driver.py | |
| 10:47:21 | sean-k-mooney | so without moveing more of the xml logic out of the dirver and into the desinger among other things its kind of messey to have it in two places | |
| 10:47:48 | sean-k-mooney | yonglihe: o/ | |
| 10:50:25 | sean-k-mooney | yonglihe: i see stephenfin has +2'd the toplogy api seires. it should merge soon | |
| 10:50:55 | sean-k-mooney | yonglihe: that just leaves your orpahn vm series right | |
| 10:51:22 | openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure q35 machine type is used when booting with SEV https://review.opendev.org/680065 | |
| 10:51:37 | openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure q35 machine type is used when booting with SEV https://review.opendev.org/680065 | |
| 10:52:10 | yonglihe | sure, i notice that. thank you guys. That orphan is real orphan now. -:) | |
| 10:52:44 | sean-k-mooney | ill try and check it out again this week. | |
| 10:53:20 | yonglihe | That's great ! | |
| 10:53:46 | yonglihe | Pile of patches recently. | |
| 10:54:12 | sean-k-mooney | well code freeze is in a week so it always gets hechtic at this time | |
| 10:55:29 | yonglihe | deja vu | |
| 11:12:09 | stephenfin | yaawang, alex_xu: I'm going to address my nits in the docs for https://review.opendev.org/#/c/670298/ and then fast approve, if that's okay. I'd like to see a follow-up for the non-doc fixes though | |