| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-23 | |||
| 18:05:47 | aspiers | so we get a few None keys in dict returned by get_domain_capabilities | |
| 18:07:27 | sean-k-mooney | jamesdenton: it looks like it was previously commented out https://github.com/openstack/os-vif/blob/stable/ocata/vif_plug_linux_bridge/linux_net.py#L132 | |
| 18:07:59 | sean-k-mooney | aspiers: i know that is not an issue | |
| 18:08:08 | sean-k-mooney | i have mips installed locally | |
| 18:08:39 | sean-k-mooney | i didnt add an example for mip to force it t use the dynamic version which is used for x86_64 | |
| 18:09:15 | sean-k-mooney | aspiers: we really should not be provideing default machine types in nova IMO, instead we should leave libvirt choose the defualt | |
| 18:10:01 | jamesdenton | sean-k-mooney even as recently as Rocky, it looks like. undocumented comment. love it. | |
| 18:10:46 | sean-k-mooney | ya so this looks like it was an unitntional change. ill see if there is a commit that warrented it. | |
| 18:11:18 | sean-k-mooney | jamesdenton: i know that shaid wanted to set it to 0 for a live migration edge cases at one point | |
| 18:11:35 | sean-k-mooney | that might be valid for ovs hybrid plug | |
| 18:11:43 | sean-k-mooney | but for linux bridge | |
| 18:11:55 | sean-k-mooney | i think we should rever back to the default | |
| 18:12:02 | jamesdenton | agreed. | |
| 18:15:08 | jamesdenton | sean-k-mooney Using common code now? See OVS https://github.com/openstack/os-vif/blob/stable/rocky/vif_plug_ovs/linux_net.py#L180 vs LXB https://github.com/openstack/os-vif/blob/stable/rocky/vif_plug_linux_bridge/linux_net.py#L148 | |
| 18:15:08 | sean-k-mooney | jamesdenton: this was the patch that changed it originally https://github.com/openstack/os-vif/commit/fa4ff64b86e6e1b6399f7250eadbee9775c22d32#diff-f55bc78ffb4c10000bbf81b88bf68673 | |
| 18:16:23 | sean-k-mooney | ya we are using common code now and i did ovs first so i guess we ended up with the ovs default | |
| 18:16:37 | jamesdenton | makes sense | |
| 18:16:45 | sean-k-mooney | ok ill go fix this shortly and we can get this backported | |
| 18:16:50 | jamesdenton | thank you | |
| 18:17:05 | sean-k-mooney | thanks for reporting it. ill update the bug | |
| 18:17:10 | jamesdenton | right on | |
| 18:17:27 | aspiers | sean-k-mooney: which bit of libvirt provides defaults? caps or domcaps or something else? | |
| 18:18:02 | aspiers | sean-k-mooney: I guess /domainCapabilities/machine ? | |
| 18:18:36 | aspiers | oh no, that's dependent on the API parameter | |
| 18:19:20 | aspiers | although I now remember that the CLI supported a default and the API didn't which was a bug they fixed after I reported it | |
| 18:19:26 | aspiers | amazing how much I forgot in a month :-/ | |
| 18:22:13 | sean-k-mooney | aspiers: ya so how i have re written it we now pass none to the libvirt python bindings and libvirt interally select a machine type and we parse it from the result | |
| 18:22:28 | sean-k-mooney | aspiers: i can add mips and other example if you like | |
| 18:22:51 | sean-k-mooney | its just another contstant in the fake data file | |
| 18:24:06 | sean-k-mooney | aspiers: i personally think however that we might want to allow passing the arch and or machine type to the funtion have it lookup the value form libvirt if its not already cached | |
| 18:24:35 | sean-k-mooney | but i currently dont have a usecase where that is strictly needed | |
| 18:24:50 | sean-k-mooney | which is why i havent changed it | |
| 18:28:41 | mriedem | efried: i saw your questions on https://review.opendev.org/#/c/665138/ | |
| 18:28:59 | mriedem | i put a bunch of notes on it - i think it's correct for a narrow window, but i'd like to see a functional test as well | |
| 18:29:12 | mriedem | it's basically https://review.opendev.org/#/c/641806/ but revert instead of confirm | |
| 18:35:58 | efried | mriedem: did you see the func test I tried to write? | |
| 18:36:22 | efried | https://review.opendev.org/#/c/665253/ | |
| 18:36:53 | efried | which I suspect was totally stolen from the one you mention | |
| 18:38:32 | mriedem | no i didn't | |
| 18:39:08 | mriedem | might want to compare your test to what my scenario is in the commit message on the other patch | |
| 18:55:39 | aspiers | sean-k-mooney: I finally found it https://www.mail-archive.com/libvir-list@redhat.com/msg182746.html | |
| 18:55:50 | aspiers | https://github.com/libvirt/libvirt-python/commit/5004b4ad532ffa93ed4b62c8021eaa9210855209 | |
| 18:58:09 | sean-k-mooney | aspiers: ah ok so we need to libvirt-python version to be bumped to 5.3 | |
| 18:58:34 | aspiers | sean-k-mooney: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-26.log.html#t2019-04-26T11:30:18 | |
| 18:59:17 | sean-k-mooney | aspiers: ya that vaguely rings a bell | |
| 18:59:25 | aspiers | sean-k-mooney: IIUC the workaround was to explicitly pass None to the API | |
| 18:59:44 | aspiers | We don't want to introduce a dependency on newer libvirt-python unless we really have to | |
| 19:00:14 | sean-k-mooney | if explctily passing none works then we dont need too | |
| 19:00:25 | sean-k-mooney | that is what we are doing in my current patch | |
| 19:00:27 | aspiers | right | |
| 19:00:45 | aspiers | but still the nested dict gets broken | |
| 19:01:42 | sean-k-mooney | how so? | |
| 19:02:02 | sean-k-mooney | i never allow a key or value to be none in the current code | |
| 19:02:08 | aspiers | 'ppc': {'None': <nova.virt.libvirt.config.LibvirtConfigDomainCaps object at 0x7f243d3ec7b8>} | |
| 19:02:19 | aspiers | I see entries like that in the domcaps dict | |
| 19:02:24 | aspiers | using your patch | |
| 19:02:27 | sean-k-mooney | that should not happen maybe i missed something | |
| 19:02:48 | aspiers | there is no guarantee that machine_type is not None | |
| 19:02:55 | aspiers | since you removed the "or 'q35'" | |
| 19:03:07 | sean-k-mooney | i was depending on qemu not returing none | |
| 19:03:33 | sean-k-mooney | we cant just default to q35 but we can register default for as many acrhs as we care to support | |
| 19:03:33 | aspiers | where is it supposed to be getting the default from? | |
| 19:04:03 | aspiers | it's currently None for mips, mipsel, and ppc | |
| 19:04:56 | sean-k-mooney | https://review.opendev.org/#/c/670189/4/nova/virt/libvirt/host.py@763 | |
| 19:05:09 | sean-k-mooney | we use the value we got back from libvirt as the key | |
| 19:05:20 | aspiers | back from which libvirt call? | |
| 19:05:34 | sean-k-mooney | getDomainCapabilities | |
| 19:05:35 | aspiers | getCapabilities or getDomainCapabilities? | |
| 19:06:16 | aspiers | OK I'll try to track it down | |
| 19:06:21 | sean-k-mooney | so if we dont have a defualt for the arch we call getDomainCapabilities with machine_type=None | |
| 19:07:28 | aspiers | right and then look at the machine type which was returned in the XML | |
| 19:07:33 | sean-k-mooney | then i added https://review.opendev.org/#/c/670189/4/nova/virt/libvirt/config.py@132 to parse the machine type and store it in the object | |
| 19:08:08 | sean-k-mooney | ya so i was assuming that there would aways be a machine type returned in the xml | |
| 19:08:21 | aspiers | I think there is | |
| 19:08:40 | sean-k-mooney | Maybe we are getting the default from https://review.opendev.org/#/c/670189/4/nova/virt/libvirt/config.py@121 | |
| 19:09:24 | sean-k-mooney | that would cause it to be none if we did not get a machine type in the xml | |
| 19:09:40 | sean-k-mooney | i could just skip storing it if we get none back | |
| 19:10:18 | aspiers | yeah but I'm not yet convinced that's the issue | |
| 19:12:27 | aspiers | sean-k-mooney: OK if I tweak your patch with a few extra comments and typo fixes? | |
| 19:13:23 | redkrieg | lyarwood: I've come across your work on rescue mode and it's exactly what I need to offer ISO based OS installs for my clients. I see it's Abandoned for Train, but I was wondering if you knew off-hand what else needs to be done. https://blueprints.launchpad.net/nova/+spec/virt-rescue-stable-disk-devices | |
| 19:19:50 | aspiers | sean-k-mooney: the bug is here https://review.opendev.org/#/c/670189/4/nova/tests/unit/virt/libvirt/fakelibvirt.py@1369 | |
| 19:20:32 | aspiers | e.g. for mips we don't have a default machine type, so the x86_64 template gets used with mtype: None | |
| 19:20:55 | aspiers | also, it shouldn't be using the x86_64 template for mips arch | |
| 19:22:17 | aspiers | sean-k-mooney: recommendations welcome on how we should fix this. Ideally I guess we would get fake domcaps for all the architectures returned by the fakelibvirt getCapabilities - currently we are missing mips/mipsel/ppc | |
| 19:22:42 | aspiers | efried: do we have access to mips/mipsel/ppc boxes to grab that XML from? | |
| 19:25:20 | sean-k-mooney | aspiers: sure feel free too | |
| 19:25:24 | efried | aspiers: I sure don't. edleafe, do you know who might be able to spin up kvm on Power (or PowerKVM, or whatever tf it's called) for this purpose? | |
| 19:25:53 | sean-k-mooney | aspiers: efried i can just generate them locally | |
| 19:26:07 | sean-k-mooney | aspiers: efried i have the emultators installed for all of them | |
| 19:27:10 | edleafe | efried: sorry, no | |
| 19:27:12 | sean-k-mooney | you can just use the virsh cli and specify the arch and it will give you the domain caps for any emulator you have installed | |
| 19:29:20 | sean-k-mooney | aspiers: here are the xmls for the ppc/mips/mipsel http://paste.openstack.org/show/754771/ | |
| 19:31:12 | sean-k-mooney | aspiers: ah and ya the bug makes sesne gald it was just in the tests | |
| 19:43:30 | aspiers | sean-k-mooney: cool thanks! how did you set up the emulators? | |
| 19:45:55 | sean-k-mooney | aspiers: if you install qemu, qemu-kvm and libvirt on ubunut 18.04 it pulls in all the qemu emulators that are available by default | |
| 19:46:14 | sean-k-mooney | on fedora/centos you have to install them one by one | |
| 19:46:46 | sean-k-mooney | so in my ubunut vm i have all of them on my centos one i only have x86 and x86_64 | |
| 20:04:50 | aspiers | interesting | |
| 20:19:04 | mriedem | efried: i'm tickling your ksa/sdk funny bone here https://review.opendev.org/#/c/664842/7/ironic/common/nova.py@13 | |
| 20:25:08 | sean-k-mooney | aspiers: looks like we have other issue too. if manually set virt_type=kvm in the nova.conf it also breaks on archittures where its not supported | |