| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-23 | |||
| 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. | |
| 11:26:58 | stephenfin | Yum, that sounds reasonable to me | |
| 11:27:04 | stephenfin | *Yup | |
| 11:27:06 | stephenfin | :) | |
| 11:27:14 | kashyap | I.e. if you specified: 'mtrr', then it will be '+mtrr'. (Additional note: some named libvirt CPU models _already_ included 'mtrr') | |
| 11:27:25 | kashyap | Heh, noted. | |
| 11:27:29 | kashyap | Thanks | |
| 11:27:59 | kashyap | Now to figure out the XML spaghetti | |
| 12:00:21 | openstackgerrit | Chris Dent proposed openstack/nova master: Fix nits in allocation candidate limit handling https://review.openstack.org/536784 | |
| 12:00:38 | cdent | stephenfin: that ^ gets some cleanups you asked for | |
| 12:02:42 | ameeda | alex_xu : can you please check this for me ? https://review.openstack.org/#/c/526900/ | |
| 12:39:10 | openstackgerrit | int32bit proposed openstack/nova master: Set server status to ERROR if rebuild failed https://review.openstack.org/536268 | |
| 12:43:04 | openstackgerrit | int32bit proposed openstack/nova master: Refix disk size during live migration with disk over-commit https://review.openstack.org/536351 | |
| 12:49:37 | ilyashakhat | hi! may I have cores attention to https://review.openstack.org/#/c/519664/ ? the patch is approved, but Zuul did not merge it | |
| 12:49:55 | ilyashakhat | most probably needs another +W to trigger the gate | |
| 12:51:20 | alex_xu | ilyashakhat: done | |
| 12:51:22 | openstackgerrit | Lee Yarwood proposed openstack/nova master: DNM: Test LM with encrypted volumes https://review.openstack.org/536350 | |
| 12:51:43 | ilyashakhat | alex_xu: thanks! | |
| 12:52:46 | alex_xu | ameeda: sorry, really busy on freeze, will get chance after freeze | |
| 12:52:49 | alex_xu | ilyashakhat: np | |
| 12:54:25 | ameeda | alex_xu: np :) | |
| 13:17:44 | openstackgerrit | sahid proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997 | |