| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-29 | |||
| 16:39:24 | sean-k-mooney | stephenfin: just the normal action | |
| 16:39:33 | sean-k-mooney | so no rules required | |
| 16:39:50 | sean-k-mooney | on the neutron side if we wanted to proceed there woudl need to be some qos changes for ovn | |
| 16:39:57 | sean-k-mooney | so that will be covered by a spec | |
| 16:40:24 | sean-k-mooney | if we are ok with this on the nova side i would like to track the capablity as a bug against os-vif | |
| 16:40:48 | sean-k-mooney | so we can backport the ablity to opt in tothis behavor but not use it by default for stable branches | |
| 16:40:59 | stephenfin | excellent, so we'll pre-populate a flow in the br-int for the new patch port, and then the comms from the other side of the patch port to the VM don't need anything explicit bar the normal action | |
| 16:41:07 | stephenfin | that wfm, personally | |
| 16:41:26 | stephenfin | certainly seems better than re-adding hybrid plug with the OVS -> linux bridge -> VM dance | |
| 16:41:47 | sean-k-mooney | i guess may main question is bug blueprint or spec for this | |
| 16:43:01 | stephenfin | I would like to see some high level docs on this _somewhere_ | |
| 16:43:02 | gibi | hm, if this requires a neutron spec, then why do you need to backport the os-vif change to stable? | |
| 16:43:12 | sean-k-mooney | personally i would prefer to leave this bake for a cycle and enable it by default next cycle | |
| 16:43:23 | sean-k-mooney | gibi: the neutorn spec is to fix QOS support | |
| 16:43:29 | stephenfin | it could be a blueprint but docs in the neutron tree might be better | |
| 16:43:37 | gibi | sean-k-mooney: ahh OK I see | |
| 16:43:40 | sean-k-mooney | it would be useful for those that dont need qos without that | |
| 16:43:48 | gibi | yepp now I got it | |
| 16:44:07 | gibi | this is a bugfix for os-vif to support live migration with OVN | |
| 16:44:23 | gibi | or more preciesly fix a race in live migration | |
| 16:44:28 | gibi | I can live with this as a bugfix | |
| 16:44:37 | sean-k-mooney | yes basically | |
| 16:45:13 | sean-k-mooney | and thats also why we woudl default this to off intially and then enable it by default in the future | |
| 16:45:30 | gibi | any objection? | |
| 16:45:39 | sean-k-mooney | operators can opt in early if they want but not change any behavior by default | |
| 16:46:16 | stephenfin | I'm good. Can't speak for others tho | |
| 16:46:27 | gibi | I don't see any hands raised :) | |
| 16:46:33 | sean-k-mooney | we can defer if peopel want to think about it more | |
| 16:46:38 | sean-k-mooney | im still working on the poc | |
| 16:47:09 | sean-k-mooney | my main concern is m2 and spec freeze | |
| 16:47:14 | gibi | it is accepted as a bug now, here. If somebody later has an objection the we can rediscuss but until that this is a bug | |
| 16:47:34 | gibi | Is there any other topic for today | |
| 16:48:31 | sean-k-mooney | not form me | |
| 16:49:00 | gibi | then let's close this | |
| 16:49:04 | gibi | thanks for joining | |
| 16:49:07 | gibi | #endmeeting | |
| 16:49:07 | opendevmeet | Meeting ended Tue Jun 29 16:49:07 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | |
| 16:49:07 | opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2021/nova.2021-06-29-16.00.html | |
| 16:49:07 | opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2021/nova.2021-06-29-16.00.txt | |
| 16:49:07 | opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2021/nova.2021-06-29-16.00.log.html | |
| 16:49:11 | elodilles | o/ | |
| 16:49:21 | gibi | feel free to continue the video mode discussion | |
| 16:49:30 | gibi | I will drop now but will read back tomorrow | |
| 16:49:42 | sean-k-mooney | well quickly the live migration issue https://review.opendev.org/c/openstack/nova/+/742180 should have fixed that | |
| 16:50:03 | sean-k-mooney | this is what previously broke live migration without binding-extended | |
| 16:50:32 | sean-k-mooney | but that is still fully supported so droping binding-extended form ml2/ovn shoudl not result in job failures | |
| 16:50:45 | sean-k-mooney | unless we are talking about cross-cell migration | |
| 16:50:51 | kashyap | Thanks for running it, gibi. | |
| 16:50:54 | sean-k-mooney | *cross-cell resize | |
| 16:51:57 | dansmith | sean-k-mooney: so, looking at the build process, it surely seems like the libvirt driver could just look at vm_state==BUILDING in spawn and annotate the desired default going forward | |
| 16:52:05 | dansmith | doesn't seem overly complicated to me | |
| 16:52:16 | dansmith | unless I'm missing elsewhere that we might be building but not want to do that | |
| 16:52:24 | sean-k-mooney | dansmith: ya i was thinking about that later in the meeting | |
| 16:52:40 | sean-k-mooney | i think your right we can detech inital spwan | |
| 16:53:14 | dansmith | yeah, so IMHO that'd be the way to go | |
| 16:53:54 | sean-k-mooney | dansmith: the quistion that i have is wether we can detect it in a place that is within the virt driver that also has the required info | |
| 16:54:15 | opendevreview | Rodrigo Barbieri proposed openstack/nova stable/train: Error anti-affinity violation on migrations https://review.opendev.org/c/openstack/nova/+/798717 | |
| 16:54:17 | sean-k-mooney | i dont belive it will still be in building when we are generating the xml | |
| 16:54:23 | dansmith | well, it's set to building right before we get our spawn called | |
| 16:54:55 | sean-k-mooney | yes but we might need to pass down a flag internally in the driver | |
| 16:54:59 | dansmith | sean-k-mooney: we don't need to detect it while building the xml do we? we can go ahead and annotate the instance right in spawn() so it's there later for the xml building no? | |
| 16:55:22 | sean-k-mooney | dansmith: the default depens on the image and flavor and config values | |
| 16:55:23 | dansmith | if instance.vm_state == building: instance.system_metadata['image_hw_whatever'] = $default; instance.save() | |
| 16:55:35 | sean-k-mooney | dansmith: e.g. if different based on architrues and a few other things | |
| 16:55:35 | dansmith | sean-k-mooney: it does? | |
| 16:55:42 | dansmith | oh sure, okay | |
| 16:55:53 | dansmith | but still, I think you have all that in spawn I would guess | |
| 16:56:16 | sean-k-mooney | yes proably let me check quickly | |
| 16:56:30 | dansmith | yeah we actually build the xml right in spawn, | |
| 16:56:53 | dansmith | so I think we should be fine, even if you want to pass a flag to get_guest_xml() from there instead of having it look or something | |
| 16:57:05 | sean-k-mooney | its basically decided here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5912-L5951 | |
| 16:57:14 | sean-k-mooney | in _add_video_driver | |
| 16:57:55 | dansmith | sure, so we could break out the "which video" part from the actual xml bit and just call it to get the model name we need to use separate from the xml part | |
| 16:58:20 | sean-k-mooney | yep or just call that directly | |
| 16:58:38 | dansmith | it returns an xml node or something doesn't it? | |
| 16:58:39 | sean-k-mooney | but ok | |
| 16:58:51 | dansmith | anyway, regardless.. I think it's not hard | |
| 16:58:54 | sean-k-mooney | yes well it returns one of our config objects | |
| 16:59:37 | sean-k-mooney | LibvirtConfigGuestVideo i think https://github.com/openstack/nova/blob/25e218484990b41485973fab86adf5afc21dd476/nova/virt/libvirt/config.py#L2052 | |
| 17:00:05 | dansmith | yeah | |
| 17:00:09 | sean-k-mooney | so we can jsut get the type filed value if we need too | |
| 17:00:20 | sean-k-mooney | so as a general pattern you would advise | |
| 17:00:43 | sean-k-mooney | update the default on new instance creation and recored in instance_system_metadata | |
| 17:00:53 | sean-k-mooney | and then just use that value in all other spawn cases | |
| 17:01:20 | dansmith | yup | |
| 17:01:50 | sean-k-mooney | in this case though virtio will work in all configuration i belive | |
| 17:02:02 | sean-k-mooney | so we really just need to check if tis build and if tis set in the image | |
| 17:02:10 | sean-k-mooney | if not set it in the image metada copy we have | |
| 17:03:59 | dansmith | well, I'd really say we should avoid breaking sensitive windows vms by changing anything much | |
| 17:04:03 | sean-k-mooney | so basiclly image_meta.properties.set('hw_video_model', image_meta.properties.get('hw_video_model')) | |
| 17:04:26 | sean-k-mooney | * image_meta.properties.set('hw_video_model', image_meta.properties.get('hw_video_model', 'virtio')) | |
| 17:04:53 | dansmith | https://www.howson.pro/content/images/2016/07/sound-popped-up-after-fi.png | |
| 17:04:57 | dansmith | don't want that on your cloud instance :P | |
| 17:05:23 | sean-k-mooney | hehe no that would be awkward | |
| 17:06:00 | sean-k-mooney | i mean you can attach a cinder volume as a driver disk but it sucks | |
| 17:06:56 | dansmith | let us not go there :) | |
| 17:08:03 | sean-k-mooney | you can kind of do that today actully but its really inovlved and convulted | |
| 17:09:11 | dansmith | lol | |
| 17:10:16 | sean-k-mooney | dansmith: so 1.) set new default in image metata copy on new spawn. 2.) keep current logic in driver which will use the image vaule preferencally if set. 3.) recored value if not in image after its calulated for exising instance? | |
| 17:10:38 | sean-k-mooney | 3.) would only run once per instance the first time they hard reboot | |