Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-21
11:19:39 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1322
11:19:51 sean-k-mooney so that is not actully calling libvirt anymore
11:19:53 gibi yes but the call coming from the libvirt driver
11:20:05 gibi so the nova-compute does not depend on the sysfs in that case
11:20:10 gibi as the virtdirver hides it
11:20:37 sean-k-mooney yes this is genergating the virt independent view of the pci devices that is pass to the pci tracker
11:20:49 sean-k-mooney yes
11:21:33 sean-k-mooney gibi: so at one point i think i proposed storing the mac and vf number in addtion to the serial in the pci dev extra info
11:21:46 sean-k-mooney we could do that and remove the need to do this lookup entirly
11:21:55 sean-k-mooney in the neutorn module
11:22:50 gibi yeah my question is do we want two interfaces from compute manager towards the hypervisor host
11:23:10 gibi as today we have the virt interface and the pci_utils "interface"
11:23:40 sean-k-mooney thats fair i know os-brick also looks at the host filesystem
11:23:47 sean-k-mooney i guess that is hidden?
11:24:00 gibi do we call os-brick outside of the virt driver?
11:24:03 sean-k-mooney is os-brick only used via the virt dirver i assume so
11:24:19 sean-k-mooney ya i dont think we call it in the generic code
11:24:27 sean-k-mooney i was just trying to think what other indrections we have
11:24:48 sean-k-mooney i generally did not consider the nuetorn module to be part fo the compute manager
11:24:53 sean-k-mooney but it is call by it
11:25:03 sean-k-mooney i was thinking of it like the pci module
11:25:04 gibi I grepped now, os_brick only imported from under nova.virt (and in nova-manage)_
11:25:10 sean-k-mooney ack
11:25:43 sean-k-mooney gibi: so i think we could file this as a bug and adress it without much extra work
11:25:54 gibi OK. I will file a bug.
11:26:36 gibi and probably I have too do the fixing as well as I need this to make the PF MAC address reporting to neutron
11:27:07 gibi as the current patch for that is also calling pci_utils from nova's neutron code instead of relying on the pci_info
11:27:10 gibi from the virt driver
11:27:52 sean-k-mooney ya it was an exsiting pattern but i agree the tight coupleing shoudl not exist between linux and the neutron module
11:28:57 gibi I think the pattern was established ~ 2015 when the first fix for the PF MAC address problem was added :D
11:29:06 gibi so everything is connected :D
11:29:17 sean-k-mooney there is one think to consider however
11:29:37 sean-k-mooney the pci tracker tracks pools of similar devices
11:30:11 sean-k-mooney so for PF mac and PF name that logicaly maps ok to pool fo the PF's VFs
11:30:42 sean-k-mooney the vf number will it can be tracked per device its not an atribute of the pool
11:31:10 sean-k-mooney so some of this info coudl be store in the db and some will have to be provided by the virt dirver per device
11:31:43 sean-k-mooney it can do that just by calling the existing fucntion when it creates the device object
11:32:09 sean-k-mooney but just pointing out that we might not be abel to hide this in the common pci module code
11:32:16 sean-k-mooney we might need to do this in the virt driver part
11:33:09 sean-k-mooney in any case it should be relitivly trivial to extend https://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L121
11:33:49 gibi sean-k-mooney: currently the parent_ifname is part of the extra_info
11:33:51 sean-k-mooney in fact we could store the info in extra_info for backwards compatiablity without changing the object
11:33:56 sean-k-mooney ya
11:33:57 gibi yeah
11:34:14 gibi lets try that
11:34:24 sean-k-mooney so we can add the other field there and keep object compat and perhaps add properties for access
11:34:31 gibi yepp
11:34:50 gibi and that way I can still use that in my PF MAC address bugfix and keep it backportable
11:34:59 sean-k-mooney yes
11:35:37 gibi cool, thanks for the brainstroming
11:35:54 gibi :D
11:36:15 gibi yeah I feel the mental pain due to that json blobs in the db
11:36:26 gibi but the bugfix backporting pain is bigger
11:36:31 gibi so that won
11:40:22 sean-k-mooney honestly the only thing i woudl replace it with is a key value mapping table
11:40:26 sean-k-mooney like instance extra
11:40:38 sean-k-mooney * instance_system_metadata
11:40:58 gibi if the OVO model is just a Dict then changing the SQL model does not help architecturally
11:41:18 sean-k-mooney but that would not buy use much since we dont really query on the sub fileds
11:41:25 sean-k-mooney ya it doesnt really
11:42:53 sean-k-mooney if you work on this ping me and ill happily review i dont know if dmitriis woudl have time to work on it as a followup/techdebt cleanup
11:43:32 gibi sean-k-mooney: thanks. I will work on it as I need it for the PF MAC bugfix anyhow
11:44:35 sean-k-mooney :) ping me if you want me to review that too. speaking of which i should go look at the rest of your placement series
11:45:03 sean-k-mooney gibi: we defintly want to backport the PF fix if it ends up being posible by the way
11:45:25 gibi sean-k-mooney: I think it will be possible
11:45:40 sean-k-mooney i kind of feel like the backporatble fix and final fix would be different however
11:45:49 gibi sean-k-mooney: yeah, if you are done with the placement series then I will switch to that to fix the small thing in a followup
11:46:25 gibi sean-k-mooney: let's see about the backportability of the PF MAC
11:46:26 sean-k-mooney as in final fix would invovle multipel port bdingin but backporable would just unbind the port for the vm update the mac and reattach it or something like that
11:46:59 gibi sean-k-mooney: nope, the plan is to add the PF MAC to the binding:profile and let neutron do the overwrite of port.mac_addres based on that
11:47:10 sean-k-mooney oh right yes
11:47:20 sean-k-mooney you said that last week
11:47:32 gibi the neutron side is basically ready in https://review.opendev.org/c/openstack/neutron/+/829247
11:47:41 gibi (I need to fix comments there)
11:47:49 sean-k-mooney are neutron ok to backport that?
11:47:57 sean-k-mooney or will we have to workaround that in the nova backport
11:48:07 gibi I think they are OK to backport that
11:48:14 gibi no issue was raise so far on that
11:48:30 sean-k-mooney ack then that makes our life simpler
11:48:35 gibi the nova side needs a redo as per the pci_utils / pci_info refactor https://review.opendev.org/c/openstack/nova/+/829248
11:48:38 sean-k-mooney i was assuming they would not backport that change
11:48:58 gibi sean-k-mooney: at least they haven't indicated that it is not backportable
11:49:04 gibi I will double check with them
11:49:41 sean-k-mooney ack it looks pretty small and i dont think it would break existing usage
11:49:46 sean-k-mooney so it shoudl be safe to backport
11:49:55 gibi yepp that is my view too
11:50:44 sean-k-mooney its kind of a feature however but i would be look very favorably towards framing it as a bugfix as you have done in the release note
11:50:55 sean-k-mooney ill finish reviewing that since i have it open now
11:51:37 gibi yeah it is a bug in nova, that needs some featury thing from neutron :)
11:51:46 sean-k-mooney gibi: there is no way to detect this form the api right
11:51:50 sean-k-mooney e.g. that this is supproted
11:52:07 gibi it cannot be detected
11:52:09 sean-k-mooney just thining that on the nova side we could eventually move to only setting the PF port via this eventully
11:52:10 gibi but it is safe from upgrade
11:52:35 gibi yeah I left a note in the nove code to only remove the current PF MAC settings logic after couple of releases
11:52:44 sean-k-mooney perfect
12:00:17 gibi sean-k-mooney: btw the arch OVO backport is also crazyness https://review.opendev.org/c/openstack/nova/+/829989 but I have a totally differnt proposal as a possible direction as a comment within that patch
12:00:59 sean-k-mooney what exactly is that doing?
12:01:25 gibi so the Arch enum got new values
12:01:34 gibi due to the emulation support feature

Earlier   Later