Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-03
21:59:10 aspiers Oh wait, you're suggesting that hw_machine_type=q35 should be added *on the fly*, not to the image via glance API?
21:59:44 aspiers I think that would be really bad, because it would override any hw_machine_type set on the real image, or any default in nova.conf
22:00:03 aspiers That overriding is what kashyap and sean-k-mooney were objecting to this morning, and I'm inclined to agree with them
22:00:28 efried okay, I'm happy to accept that that's a bad idea.
22:00:35 efried so back to this other thing
22:00:41 aspiers although this all feels a bit bikesheddy, because I have a hard time imagining that anyone would want to insist on 'pc', given that 'q35' has already been around for 11 years
22:00:46 efried you pointed out that there's already a requirement for the uefi thing in the image.
22:00:50 aspiers Yep
22:01:04 aspiers So we could just document an extra requirement for hw_machine_type=q35
22:01:06 efried is that a property they have to set (to some value) regardless?
22:01:16 efried or does it also default if you don't set it?
22:01:25 aspiers It defaults via the code in libvirt/utils.py
22:01:37 aspiers which looks in nova.conf first, and then falls back to the hardcoded mapping
22:01:51 aspiers https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/utils.py#L537
22:02:14 aspiers CONF.libvirt.hw_machine_type I mean
22:02:21 efried o-queso, I don't see the problem with requiring them to set two image props instead of one.
22:02:30 aspiers Yeah, that sounds reasonable to me too
22:02:44 efried whee
22:02:45 aspiers It's kind of in the same category as the UEFI requirement anyway
22:02:51 efried cool
22:02:55 aspiers "virtual hardware requirements"
22:03:23 aspiers OK that was very helpful, thanks a lot!
22:04:09 efried neat
22:04:10 aspiers BTW your "I don't know enough about libvirt to be helpful" stance just lost all credibility ;-)
22:04:27 aspiers not that I ever believed it anyway X-D
22:04:37 efried hah, read the transcript again. It should be even more clear that I know jack about libvirt.
22:04:40 sean-k-mooney efried: you can set the default machine_type in the nova.conf too
22:05:00 aspiers sean-k-mooney: that's what I just said ;-)
22:05:02 sean-k-mooney efried:so you dont have to set it in the image always
22:05:03 efried yeah, that ^ sounds like it would be a good option for deployers wishing to enable SEV on their hosts.
22:05:16 efried can you do the uefi thing in nova.conf too?
22:05:25 sean-k-mooney no
22:05:28 efried bummer
22:05:35 sean-k-mooney at least not that im aware of
22:05:41 aspiers that might be a nice feature to add
22:06:05 sean-k-mooney eventrually we do want to change the defualt machie type to Q35 by the way
22:06:08 aspiers that would actually be really easy
22:06:24 aspiers sean-k-mooney: yeah I mentioned that too
22:06:26 sean-k-mooney adding the bios/uefi
22:06:33 sean-k-mooney ya it should not be hard
22:06:54 aspiers sean-k-mooney: https://bugs.launchpad.net/nova/+bug/1780138
22:06:55 openstack Launchpad bug 1780138 in OpenStack Compute (nova) "Don't assume the guest machine type to be of 'pc'" [Medium,Confirmed] - Assigned to Kashyap Chamarthy (kashyapc)
22:07:12 sean-k-mooney although you do need to have the firmware ectra but i can see people wanting to eventurally mvoe the Q35+uefi only
22:08:31 sean-k-mooney ya we shoudl avoid assumign pc but its also wrong to assume q35 currently
22:09:22 sean-k-mooney we have a function that returns the correct machine type taking into account the default + config + image
22:09:43 sean-k-mooney if we dont know what to use we should call that rather then guessing
22:10:48 sean-k-mooney aspiers: by the way https://review.opendev.org/#/c/666915/16/nova/tests/functional/libvirt/test_report_cpu_traits.py were the changes i need to make the sev code work with my series
22:11:37 aspiers sean-k-mooney: resetting the traits cache makes sense
22:11:46 aspiers sean-k-mooney: two questions about resetting domain caps cache
22:11:56 aspiers 1. how did my tests pass without that before?
22:12:01 sean-k-mooney technically i think you should have always been doing self.compute.driver._host._domain_caps = None in your orginal code
22:12:11 sean-k-mooney aspiers: i think it should not or it was a race
22:12:14 aspiers 2. why not do it on both tests?
22:12:44 aspiers AFAICS it can't be a race because we're only talking about a single thread here
22:12:53 sean-k-mooney the second test does not modify the capablities but it would be valid to do it for both
22:12:57 openstackgerrit Merged openstack/nova master: libvirt: Fold in argument to '_update_provider_tree_for_vgpu' https://review.opendev.org/676729
22:13:25 sean-k-mooney aspiers: i ment between fucntional test and the order they execute in
22:13:34 aspiers oh ok the second test doesn't look up caps because it gives up as soon as it sees the kernel parameter file missing
22:13:42 sean-k-mooney yep
22:14:07 aspiers but the compute host isn't shared between the tests surely?
22:14:26 sean-k-mooney it depens on how your mocking.
22:14:39 sean-k-mooney it should not be but i have seen wierd issue
22:15:19 aspiers start_compute() is called by each test
22:15:37 aspiers and creates a fresh compute service
22:16:18 sean-k-mooney when i ran it under the debugger i found you were not geting to the updated domain caps because of https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L759-L760
22:16:37 aspiers right
22:17:12 aspiers er
22:17:48 sean-k-mooney i dont really know why it passed before but self.compute.driver._host._domain_caps = None seams to fix it
22:18:09 aspiers actually I think it's https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1191
22:18:23 aspiers if the kernel param file is missing, it won't cache any caps
22:18:45 aspiers so when it appears later, it will call the fake getDomCaps API for the first time
22:18:47 sean-k-mooney no i steped throug it in the debugger
22:18:53 aspiers that's in the off_on case
22:19:26 sean-k-mooney it might be becasue of my traits code actully
22:19:32 aspiers in the on_off case, it wouldn't even care what caps were cached when it gets to off
22:19:53 aspiers but technically I think it's good to reset both caches in both tests
22:20:09 aspiers in case the order of tests ever got changed in the future
22:20:23 sean-k-mooney im calling this https://review.opendev.org/#/c/666915/16/nova/virt/libvirt/driver.py@6832
22:20:38 aspiers sorry, I mean the order of these https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1188-L1193
22:21:06 sean-k-mooney sure ill add that
22:21:28 sean-k-mooney it make it symetic and really we dont want the order of those check to break this
22:21:40 aspiers right
22:21:51 aspiers that static_traits looks fine to me
22:22:00 aspiers except for the blank line after the def ;-)
22:22:13 sean-k-mooney anyway the change looks sane to you?
22:22:17 aspiers yes
22:22:30 aspiers I didn't review the whole thing yet, but that file does
22:22:52 aspiers oh, according to Gerrit I did review the whole thing
22:22:54 aspiers I forgot :)
22:22:55 sean-k-mooney there is no other change to that form the previous versin
22:22:58 sean-k-mooney yes
22:23:05 sean-k-mooney a week or so ago
22:23:08 aspiers k
22:23:26 sean-k-mooney it been wait since and the only change was resolving the merge conflcit with the sev code
22:23:34 aspiers not since PS9
22:23:58 aspiers I hate how this old Gerrit makes it so hard to compare patchsets after rebase
22:24:11 aspiers that's the #1 thing I want a new Gerrit for
22:27:34 openstackgerrit sean mooney proposed openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775
22:27:34 openstackgerrit sean mooney proposed openstack/nova master: libvirt: use domain capabilities to get supported device models https://review.opendev.org/666915
#openstack-nova - 2019-09-04
00:27:03 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606

Earlier   Later