| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-23 | |||
| 10:33:20 | kashyap | specific feature policy | |
| 10:33:23 | kashyap | - LibvirtConfigGuestCPU - extension for setting the guest specific | |
| 10:33:25 | kashyap | match policy, and allowing use of host CPU model passthrough | |
| 10:33:58 | jianghuaw | bauzas, sure. I will look at the patches. | |
| 10:34:23 | kashyap | Context: It was in reference to https://review.openstack.org/#/c/534384/ | |
| 10:34:42 | mdbooth | kashyap: Yep, guessed. | |
| 10:35:30 | mdbooth | kashyap: Incidentally, it sounded from that ml discussion that we might want to propose adding PCID automatically in Nova if it's not present in the explicit cpu model? | |
| 10:36:21 | mdbooth | Or were we still erring towards triple-o? | |
| 10:36:31 | kashyap | mdbooth: It's the latter. | |
| 10:36:40 | mdbooth | I guess the advantage of doing it in Nova is that you can query libvirt to expand the cpu flags from the specified model | |
| 10:36:43 | kashyap | mdbooth: Because, adding it automatically indeed is a bit too magic | |
| 10:37:03 | mdbooth | i.e. You don't need to duplicate the list | |
| 10:37:30 | gibi | bauzas: I put back my +2 | |
| 10:37:32 | kashyap | Yes, it should be. | |
| 10:39:20 | kashyap | mdbooth: Do you have a few more minutes to confirm I'm reading code correctly? /me begins to type | |
| 10:39:34 | bauzas | gibi: <3 | |
| 10:40:07 | kashyap | mdbooth: So if you look at the class LibvirtConfigGuestCPUFeature() in libvirt/config.py. The infrastrucutre already _exists_ | |
| 10:40:26 | kashyap | <cpu match='exact'> | |
| 10:40:26 | kashyap | To generate something like: | |
| 10:40:27 | kashyap | <feature policy='require' name='pcid'/> | |
| 10:40:27 | kashyap | <vendor>Intel</vendor> | |
| 10:40:27 | kashyap | <model fallback='forbid'>IvyBridge</model> | |
| 10:40:29 | kashyap | </cpu> | |
| 10:41:16 | kashyap | mdbooth: Also see corresponding test: LibvirtConfigGuestCPUFeatureTest() in test_libvirt_config.py. | |
| 10:42:48 | kashyap | So since I'm introducing a config variable, I need to generate the objects for LibvirtConfigGuestCPUFeature() -- for every CPU flag configured in nova.conf | |
| 10:43:23 | mdbooth | kashyap: Yep | |
| 10:45:06 | ameeda | WARNING: when I pull the last version of nova "DevStack" , the system doesn't work anymore . | |
| 10:45:13 | kashyap | mdbooth: Okido. So my change here is wrong -- https://review.openstack.org/#/c/534384/2/nova/virt/libvirt/config.py. Because the root element 'feature' is already handled in the existing class LibvirtConfigCPUFeature() | |
| 10:45:49 | mdbooth | kashyap: Yeah, that whole bit of your patch looked confused, but as it was a WIP and you hadn't written any tests yet I figured you'd get there on your own. | |
| 10:46:36 | mdbooth | Not at all. I've learned loads. | |
| 10:47:26 | mdbooth | The motivation is much more interesting than some libvirt config wrangling code. | |
| 10:51:25 | kashyap | mdbooth: Hmm. I'm wondering if I should make a single class that will handle the complete root element: | |
| 10:51:28 | kashyap | <feature policy='require' name='pcid'/> | |
| 10:51:38 | kashyap | <feature policy='require' name='mtrr'/> | |
| 10:51:42 | kashyap | ... | |
| 10:52:10 | kashyap | Because, for now, the 'name' attribute is handled in the class LibvirtConfigCPUFeature() | |
| 10:52:26 | kashyap | And the 'policy' in LibvirtConfigGuestCPUFeature(). | |
| 10:52:56 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: QEMU native LUKS decryption for encrypted volumes https://review.openstack.org/523958 | |
| 10:52:57 | mdbooth | kashyap: LibvirtConfigGuestCPUFeature already does that, no? | |
| 10:53:06 | mdbooth | lyarwood: Looking :) | |
| 10:53:27 | kashyap | mdbooth: This is what it does currently: | |
| 10:53:28 | kashyap | 489 super(LibvirtConfigGuestCPUFeature, self).__init__(name, **kwargs) | |
| 10:53:28 | kashyap | 488 def __init__(self, name=None, **kwargs): | |
| 10:53:28 | kashyap | 487 | |
| 10:53:28 | kashyap | 486 class LibvirtConfigGuestCPUFeature(LibvirtConfigCPUFeature): | |
| 10:53:31 | kashyap | 490 | |
| 10:53:33 | kashyap | 491 self.policy = "require" | |
| 10:53:36 | kashyap | 492 | |
| 10:53:38 | kashyap | 493 def format_dom(self): | |
| 10:53:41 | kashyap | 494 ft = super(LibvirtConfigGuestCPUFeature, self).format_dom() | |
| 10:53:44 | kashyap | 495 | |
| 10:53:46 | kashyap | 496 ft.set("policy", self.policy) | |
| 10:53:49 | kashyap | 497 | |
| 10:53:51 | kashyap | 498 return ft | |
| 10:54:24 | kashyap | Okay, it inherits from the LibvirtConfigCPUFeature()... | |
| 11:01:23 | openstackgerrit | Zhenyu Zheng proposed openstack/nova-specs master: Add request_id field to migrations object https://review.openstack.org/536765 | |
| 11:02:14 | Spazmotic | I need to maek my first nova-spec.. need to read up on how to do that. | |
| 11:06:39 | Spazmotic | sounds like a job for tomorrowwwwwman | |
| 11:09:42 | openstackgerrit | Lee Yarwood proposed openstack/nova master: DNM: Test LM with encrypted volumes https://review.openstack.org/536350 | |
| 11:12:14 | cdent | gibi, bauzas: this would be a useful debugging improvement to have for the post-freeze cleanup: https://review.openstack.org/#/c/519462/ Kenichi has a concern about a private member, but I think that's just the way it goes for now. Already has +2 from stephenfin | |
| 11:14:09 | openstackgerrit | Zhenyu Zheng proposed openstack/nova-specs master: Allow abort live migrations in queued status https://review.openstack.org/536722 | |
| 11:17:06 | kashyap | stephenfin or anyone: Do we have a precendence in Nova where we allow comma-separated values for a config attribute? | |
| 11:17:26 | stephenfin | kashyap: Look at oslo_config.types | |
| 11:17:33 | stephenfin | I'm guessing ListOpt is what you want | |
| 11:18:13 | stephenfin | I _think_ ListOpt uses CSV, but it should be easy enough to verify (look for existing usages in the nova tree) | |
| 11:19:08 | kashyap | stephenfin: I see, will look once I make quick lunch | |
| 11:19:50 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('hw_machine_type', | |
| 11:19:50 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('disk_cachemodes', | |
| 11:19:50 | kashyap | $> git grep ListOpt nova/conf/libvirt.py | |
| 11:19:50 | kashyap | stephenfin: So there is some precedence: | |
| 11:19:51 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('uid_maps', | |
| 11:19:53 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('gid_maps', | |
| 11:19:55 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('enabled_perf_events', | |
| 11:19:58 | kashyap | nova/conf/libvirt.py: cfg.ListOpt('vzstorage_mount_opts', | |
| 11:20:55 | stephenfin | kashyap: Then you're golden. That's the approach I'd take | |
| 11:21:06 | stephenfin | Offloads all the hard work (parsing) to oslo.config | |
| 11:22:06 | kashyap | I see, noted. | |
| 11:22:27 | kashyap | stephenfin: So, what I'm looking to do is something like: | |
| 11:23:03 | kashyap | If you specify: | |
| 11:23:10 | kashyap | cpu_model_extra_flags="+pcid,-mtrr,pdpe1gb" | |
| 11:23:10 | kashyap | cpu_model=IvyBridge | |
| 11:23:10 | kashyap | [libvirt] | |
| 11:23:28 | kashyap | Which means: Add the PCID feature and 'pdpel1gb' feature (assume '+' is | |
| 11:23:29 | kashyap | given if it's not specified), and *remove* the 'mtrr' feature. | |
| 11:23:38 | kashyap | Then Nova should be able to generate an XML something like: | |
| 11:23:46 | kashyap | <feature policy='require' name='pdpe1gb'/> | |
| 11:23:46 | kashyap | <feature policy='require' name='pcid'/> | |
| 11:23:46 | kashyap | <vendor>Intel</vendor> | |
| 11:23:46 | kashyap | <model fallback='forbid'>IvyBridge</model> | |
| 11:23:46 | kashyap | <cpu match='exact'> | |
| 11:23:49 | kashyap | </cpu> | |
| 11:23:56 | kashyap | stephenfin: Makes sense? | |
| 11:24:37 | stephenfin | Sounds sane. Why would you remove something though? | |
| 11:24:41 | kashyap | The +/- in nova.conf will be optional. If nothing is specified, assume '+'. | |
| 11:24:57 | stephenfin | I assume 'mtrr' would be added by default if you didn't explicitly add '-mtrr'? | |
| 11:25:16 | kashyap | stephenfin: If an advanced operator wants to generate a baseline CPU config, and a certain CPU flag is causing performance issue or other nuisance | |
| 11:25:34 | kashyap | And if she isolated the problem to _that_ specific CPU flag | |
| 11:25:40 | kashyap | Thens she may want to remove it. | |
| 11:25:43 | kashyap | Reasonable? | |
| 11:26:01 | kashyap | stephenfin: Yep, your assumption about 'mtrr' is true. | |