| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-04 | |||
| 14:58:57 | sean-k-mooney | ya | |
| 14:59:19 | sean-k-mooney | if it was a python lib i would say we should be capping it in upper constratits | |
| 14:59:48 | johnthetubaguy | we are using the interface wrong here, I dislike libosinfo's interface (but honestly, this is minor in my general dislike of the interface)... but we don't have anything better, we did try feedback this dislike when we first added support. | |
| 14:59:53 | sean-k-mooney | but anyway i guess we need to normalise it but im not sure what value this code adds | |
| 15:00:25 | johnthetubaguy | I think this was probably broken when we added it, just clearly no one seems to use it | |
| 15:00:42 | johnthetubaguy | appart from me, when I tried to squash a bunch of warning logs in production... | |
| 15:00:49 | johnthetubaguy | (facepalm) | |
| 15:01:54 | johnthetubaguy | magnum actually requires an osdistro value that isn't supported by libosinfo... but that is just as well else the instance wouldn't boot | |
| 15:02:27 | johnthetubaguy | (they want fedora atomic only, for their fedora atomic driver) | |
| 15:02:49 | sean-k-mooney | so i think this is fundementally broken as it also ignores what disk buses are supporte by the hypervior | |
| 15:04:03 | sean-k-mooney | i am adding some related logic for one of my spec | |
| 15:04:06 | sean-k-mooney | https://review.opendev.org/#/c/666915/2/nova/virt/libvirt/driver.py@9659 | |
| 15:04:40 | sean-k-mooney | there i am using the new domain capablities api to get the disk models supoted by the hypervior | |
| 15:05:11 | sean-k-mooney | your current patch just checks there in the supported list but does not take account of the vrit type | |
| 15:05:49 | sean-k-mooney | so if the virt type was lxc you could end up retruning virtio which is wrong | |
| 15:06:32 | sean-k-mooney | osinfo also should not take preference overe hw_disk_bus | |
| 15:06:49 | sean-k-mooney | if the user specfically set it we shoudl use that | |
| 15:06:55 | sean-k-mooney | so there are several bugs with this | |
| 15:07:24 | sean-k-mooney | actully never mind that last point | |
| 15:07:32 | sean-k-mooney | it handel that internaly | |
| 15:07:43 | sean-k-mooney | that is is why we need to pass the image metadata | |
| 15:08:11 | sean-k-mooney | to the hardware properties class but we dont pass the virt type | |
| 15:10:17 | sean-k-mooney | i dont know maybe im wrong but it feels odd teh way its currently layed out | |
| 15:10:39 | sean-k-mooney | i have not gone back and read the commit that added it maybe it explained why the current form is used | |
| 15:10:50 | sean-k-mooney | * order | |
| 15:20:10 | johnthetubaguy | sean-k-mooney: its feels stupid the way it currently is, particular in reference to this logic: https://github.com/openstack/nova/blob/61558f274842b149044a14bbe7537b9f278035fd/nova/virt/libvirt/blockinfo.py#L219 and https://github.com/openstack/nova/blob/ff0f33cf7e44c5d01f3fb999295eff4508c24a14/nova/virt/libvirt/vif.py#L137 | |
| 15:20:28 | johnthetubaguy | I know stephenfin just did a bunch of improvements in there, but yeah, its a bit stupid in its current form | |
| 15:24:02 | stephenfin | Yeah, it's on my list. tbh, 'get_base_config' just needs to disappear | |
| 15:24:29 | stephenfin | It doesn't really provide a consistent base for anything at this point | |
| 15:26:34 | johnthetubaguy | so I liked the libosinfo plan, specify os_distro and you get a best default for your chosen hypervisor... but yeah, it doesn't do that today. | |
| 15:31:16 | kashyap | johnthetubaguy or anyone: If you want a change of scenery, some easy patches with "negative code": https://review.opendev.org/#/q/topic:Bump_min_libvirt_and_QEMU_for_Stein+(status:open) | |
| 15:31:54 | johnthetubaguy | yeah, I keep meaning to look, but I have to fix some customer stuff at the moment | |
| 15:33:19 | kashyap | johnthetubaguy: Sure, this can wait | |
| 15:33:30 | kashyap | Go for the other one; more important :-) | |
| 15:41:50 | stephenfin | kashyap: Looks like we've got some duplication between that series above and my one ending here https://review.opendev.org/#/c/666814/ | |
| 15:42:00 | kashyap | stephenfin: Whoops | |
| 15:42:40 | stephenfin | kashyap: Specifically on this one https://review.opendev.org/#/c/669171/ Think you could do some rebasing one way or the other (or just drop that patch) | |
| 15:43:03 | stephenfin | Also, have questions on the cmt patch. Rest is +" | |
| 15:43:05 | stephenfin | *+2 | |
| 15:43:24 | kashyap | stephenfin: Darn the 'vif' thing is indeed a duplicate. Sorry; I'll abandon that | |
| 15:45:33 | kashyap | Abandoned; will check the question on the CMT one. | |
| 15:45:41 | kashyap | Thanks for review. | |
| 15:45:58 | kashyap | s/for/"for the"/ | |
| 16:30:55 | sean-k-mooney | johnthetubaguy: regarding the osinfo issue we coudl merge you small fix as a stepping stone but i think we should file a seperate bug to adress the larger issue later | |
| 16:31:15 | sean-k-mooney | e.g. we should unbreak your customers and then figure out where to go from there. | |
| 16:31:24 | johnthetubaguy | sean-k-mooney: cool, that aligns with my thinking | |
| 16:32:21 | johnthetubaguy | certainly, a heap of issues in there... its tempting to just drop the libosinfo stuff honestly, I am not sure its helping us any more, if we always send "virtio" expect for very old distros | |
| 16:33:36 | sean-k-mooney | ya i considerd that too but was not sure why it was originally added e.g. what would we be regressing if we did | |
| 16:38:06 | sean-k-mooney | by the way os_distro=centos7.0 is not correct it should be os_distro=centos and os_version=7 | |
| 16:38:18 | sean-k-mooney | see https://github.com/openstack/glance/blob/master/etc/metadefs/operating-system.json | |
| 16:39:36 | sean-k-mooney | which i now realise that module does not handel correctly either | |
| 16:43:34 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/osinfo.py#L123-L124 should be checking both values and technically we should be rejecting centos7.0 as an invalde disto name | |
| 16:44:52 | sean-k-mooney | but we treat it as a sting filed here https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L456 since we dont know the full set of os we could be running | |
| 16:53:02 | openstackgerrit | Merged openstack/nova master: vif: Skip most of 'get_base_config' if not using virtio https://review.opendev.org/666629 | |
| 17:10:52 | openstackgerrit | Merged openstack/nova stable/queens: Init HostState.failed_builds https://review.opendev.org/668911 | |
| 17:24:04 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP:Introduce scope_types in os-services https://review.opendev.org/645427 | |
| 18:00:20 | johnthetubaguy | sean-k-mooney: right, it only seems to work with centos7.0 right now, else it gets rejected, go figure | |
| 18:00:52 | sean-k-mooney | right but the os_disto is ment to be the os name without the version | |
| 18:00:59 | sean-k-mooney | so it was broken from the start | |
| 18:01:47 | johnthetubaguy | maybe, it spits out a warning unless you smoosh them together | |
| 18:02:12 | sean-k-mooney | libosinfo? | |
| 18:02:28 | sean-k-mooney | looking at the spec the spec was using it wrongly form the start https://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/libvirt-hardware-policy-from-libosinfo.html | |
| 18:02:30 | johnthetubaguy | no, Nova | |
| 18:02:43 | sean-k-mooney | nova should not spit out an error | |
| 18:02:46 | johnthetubaguy | the spec wasn't updated vs the implementation | |
| 18:03:05 | sean-k-mooney | os_distro=centos is the correct form | |
| 18:03:08 | johnthetubaguy | it always, had, well only in the operators logs as a warning | |
| 18:03:14 | johnthetubaguy | saying osdistro not found | |
| 18:03:23 | openstack | bug 1825537 in OpenStack Compute (nova) stein "finish_resize failures incorrectly revert allocations" [Medium,In progress] https://launchpad.net/bugs/1825537 - Assigned to Matt Riedemann (mriedem) | |
| 18:03:23 | openstackgerrit | Merged openstack/nova stable/stein: Add functional recreate test for regression bug 1825537 https://review.opendev.org/666959 | |
| 18:03:28 | openstackgerrit | Merged openstack/nova stable/stein: Drop source node allocations if finish_resize fails https://review.opendev.org/667155 | |
| 18:03:34 | openstackgerrit | Merged openstack/nova stable/rocky: Delete resource providers for all nodes when deleting compute service https://review.opendev.org/666854 | |
| 18:03:38 | johnthetubaguy | sorry, I need ot run | |
| 18:03:39 | sean-k-mooney | yes but os_distro was not added for libosinfo | |
| 18:03:41 | openstackgerrit | Merged openstack/nova stable/rocky: Disable limit if affinity(anti)/same(different)host is requested https://review.opendev.org/659246 | |
| 18:03:49 | sean-k-mooney | johnthetubaguy: sure o/ | |
| 18:03:59 | johnthetubaguy | I thought it existed before | |
| 18:04:05 | sean-k-mooney | it did | |
| 18:04:16 | sean-k-mooney | for other drivers for other things that expect jsut the os name | |
| 18:04:25 | sean-k-mooney | not the os name + version | |
| 18:07:05 | openstackgerrit | Merged openstack/nova master: Refactor bandwidth related functional tests https://review.opendev.org/641679 | |
| #openstack-nova - 2019-07-05 | |||
| 01:01:06 | openstackgerrit | Merged openstack/nova stable/stein: Ignore hw_vif_type for direct, direct-physical vNIC types https://review.opendev.org/667354 | |
| 02:18:11 | openstackgerrit | Boxiang Zhu proposed openstack/nova master: Make evacuation respects anti-affinity rule https://review.opendev.org/649963 | |
| 03:13:03 | lewe | Hi, Maybe someone can help me, I am trying to cold migrate an instance but it does not seems to work right. The instance was newly created on the destination hosts rather than copying the original disk from the instance original host. Thank you. | |
| 03:36:16 | openstackgerrit | wangwei1 proposed openstack/nova master: fix spelling error in nova/api/validation/__init__.py https://review.opendev.org/669244 | |
| 04:26:06 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: api-ref: Fix a broken link https://review.opendev.org/669248 | |
| 04:45:38 | openstackgerrit | Seyeong Kim proposed openstack/nova stable/queens: Share snapshot image membership with instance owner https://review.opendev.org/661667 | |
| 06:00:33 | openstackgerrit | wangwei1 proposed openstack/nova stable/stein: fix spelling error in nova/api/validation/__init__.py https://review.opendev.org/669251 | |
| 06:32:07 | openstackgerrit | Ghanshyam Mann proposed openstack/nova-specs master: Fix the cross_az_attach value in AZ-unshelved spec https://review.opendev.org/669261 | |
| 06:39:10 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021 | |
| 07:18:21 | openstackgerrit | Yongli He proposed openstack/nova master: clean up orphan instances https://review.opendev.org/627765 | |
| 07:18:21 | openstackgerrit | Yongli He proposed openstack/nova master: Clean up orphan instances virt driver https://review.opendev.org/648912 | |
| 07:24:02 | openstackgerrit | Yongli He proposed openstack/nova master: clean up orphan instances https://review.opendev.org/627765 | |
| 08:12:55 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova-specs master: Fix indentation in the "Secure Boot for KVM ... guests" spec https://review.opendev.org/669284 | |
| 08:13:24 | kashyap | stephenfin: johnthetubaguy: A trivial cosmetic fix --^ (rST syntax related) | |
| 08:25:32 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Get rid of args to RBDDriver.__init__() https://review.opendev.org/668564 | |
| 08:35:40 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove native LUKS compat code https://review.opendev.org/669121 | |
| 09:12:29 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Fix spelling error in 'nova/api/validation/__init__.py' https://review.opendev.org/669244 | |
| 09:14:08 | openstackgerrit | Stephen Finucane proposed openstack/nova stable/stein: Fix spelling error in 'nova/api/validation/__init__.py' https://review.opendev.org/669251 | |
| 09:15:41 | kashyap | stephenfin: Hope I answered your question there: https://review.opendev.org/#/c/669129/ | |