Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-21
09:11:36 openstackgerrit Nguyen Hung Phuong proposed openstack/python-novaclient master: Clean imports in code https://review.openstack.org/546543
09:24:00 jehane thanks
09:59:02 openstackgerrit sahid proposed openstack/nova-specs master: update: introducing isolate emulthreads on host https://review.openstack.org/511188
10:10:43 openstackgerrit sahid proposed openstack/nova-specs master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/539605
10:37:41 openstack Launchpad bug 1670628 in OpenStack Compute (nova) "nova-compute will try to re-plug the vif even if it exists for vhostuser port." [Undecided,Opinion]
10:37:41 sahid sean-k-mooney: any idea whether we should fix it or not? https://bugs.launchpad.net/nova/+bug/1670628
11:09:18 openstackgerrit Rajesh Tailor proposed openstack/nova master: Reset instance status on invalid host https://review.openstack.org/401009
11:23:35 openstackgerrit Shoham Peller proposed openstack/nova master: Handle spawning error on unshelving https://review.openstack.org/378009
11:31:13 openstackgerrit Shoham Peller proposed openstack/nova master: Handle spawning error on unshelving https://review.openstack.org/378009
12:19:48 sean-k-mooney sahid: just looking now
12:20:49 sean-k-mooney sahid: first taught is that restarting nova-compute should not result in plug being called but still reading
12:24:11 sahid sean-k-mooney: that it's too big change, we don't know the impact
12:24:35 sahid currently we know that, deleting the port is affecting dpdkvhostuser
12:24:38 sean-k-mooney sahid: but that is a bug in its own right plug is only ment to be called on boot or interface attach
12:25:02 sahid it depends on the vif type
12:25:08 sean-k-mooney restarting the nova-compute agent should never effect the vm datapath
12:25:47 sahid i have no idea why we have this code in nova to call plug vifs for all instanes after a restart
12:25:53 sean-k-mooney in its current form upgrading the nova compute deamon on a host with ovs-dpdk will breack connectivity
12:25:58 sahid the point i'm not going to remove that since i do not know the impact
12:26:45 sahid it's something that can be work for a long term perspective
12:26:51 sean-k-mooney the code in os-vif was written that way to allow ports to be moved to different bridge fov the vlan aware vms spec
12:27:38 sean-k-mooney thats why its not jsut add-port --may-exists and instead we do a del-port followed by add-port in a singel transaction
12:27:58 sahid yes but that is not going to work for a type dpdkvhostuser
12:28:25 sahid the instance is going to lost the connecctivity if we delete the socket
12:28:28 sean-k-mooney it should as ovs used to do this atomically and not remove and read the port ovs was already in the desired state
12:29:18 sahid not sure i understand what you are saying
12:29:28 sahid do you understand the issue here?
12:29:33 sean-k-mooney well the socket is not useed for vm traffic but ovs is not ment to be deleting the socket in this case.
12:29:47 sean-k-mooney sahid: yes i understand the issue but not why its happening
12:30:43 sean-k-mooney one of the requirement i had for our ovs team when vhost-user was added was that doing a del-port followed by add port of the same port in a trasaction would not impact the datapath and become a noop
12:31:06 sahid i see your point now
12:33:05 sean-k-mooney we could try chainging to ovs-vsctl add-port --may-exist instead but not sure if that will prevent changing neuton port into trunk ports introduced in the vlan aware vms specs
12:33:31 sean-k-mooney that is the only thing i can think of that could be effect by that change and it would resolve the issue
12:34:36 sean-k-mooney we do have patchs form rodolfo that may or may not have merged that uses the python ovs client lib which may not have this issue also
12:34:39 sahid sean-k-mooney: in your idea this issue is only for dpdkvhostuser or so you think we have the same for dpdkvhostuserclient?
12:35:15 sean-k-mooney i think we have the same issue for dpdkvhostuserclient but vhost reconnect is masking it.
12:35:41 sean-k-mooney dpdkvhostuser dose not suppport recoonect so it needs a vm reboot to fix
12:36:13 sean-k-mooney for dpdkvhostuserclient the unix socket is created by qemu so it is definetly not deleted
12:36:25 sahid so this issue does not exist for dpdkvhostuserclient
12:37:03 sean-k-mooney sahid: i cant say 100% with out testing but i think it would be significatly less likely to happen
12:37:22 sahid that is also my thinking
12:37:36 sahid so i don't know whether we really want to fix that
12:37:46 sahid starts to ocata we are using dpdkvhostuserclient
12:37:56 sahid and i think dpdkvhostuser is deprecated, right?
12:38:09 sean-k-mooney sahid: well we are using dpdkvhostuserclient if ovs is new enough
12:38:25 sean-k-mooney dpdkvhostuser is not but dpdkvhostcuse was and has been removed
12:38:57 sean-k-mooney dpdkvhostuser is just not recommended but no one has pushed for removal/deprecation yet as far as i know
12:39:01 sahid oh yes we are looking at the capabilities to know which one to user
12:39:03 sahid use
12:39:23 sahid ok so let fix it
12:39:58 sean-k-mooney for ovs-vsctl its should be a 2 line fix. want me to trow up a patch
12:40:57 sahid no worries i will do i have also a env to test it
12:41:27 sahid sean-k-mooney: will ping you when it's sent upstrea
12:42:19 sean-k-mooney cool it should just be changing "'--if-exists', 'del-port', dev, '--','add-port'" -> "'add-port','--may-exist'" https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/linux_net.py#L68 and maybe updating some unit tests
12:43:02 sean-k-mooney actully the --may-exists possibly goes before add-port but youll figure it out
12:44:40 openstackgerrit Surya Seetharaman proposed openstack/nova master: Unmap compute nodes when deleting host mappings in delete cell operation https://review.openstack.org/542964
13:15:13 openstackgerrit sahid proposed openstack/os-vif master: ovs: do not delete port if already exists https://review.openstack.org/546588
13:20:19 sahid sean-k-mooney: ^ i still need to test it but if you want to have a llok
13:46:53 openstackgerrit Shoham Peller proposed openstack/nova master: Handle spawning error on unshelving https://review.openstack.org/378009
14:15:15 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: [WIP] libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/534384
14:20:52 mriedem mnaser: some comments / thoughts in https://review.openstack.org/#/c/546398/
14:24:06 mnaser mriedem: if i bdm.attachment_id = None .. that means that i can drop 'attachment_id' in bdm ?
14:24:25 mnaser and i can change the test to make sure that i always get an attachment that is =None i guess?
14:25:10 mnaser that does feel like a much cleaner fix because then we won't carry the extra cruft of an issue that exists in the old flow of attachment which will one day disappear
14:26:30 mriedem mnaser: i think that is true...
14:26:54 mnaser mriedem: i have a functional test so ill make the change and see if it does address it
14:27:06 mriedem the only time we do a local delete in the api and the bdm.attachment_id wouldn't be set is this pre-scheduling thing where we delete the instance via the build_request
14:27:42 mnaser mriedem: and if i understand correctly, the bdms we get from the build_request are not persisted in db so they don't have an attachment_id
14:27:44 mriedem if the bdm does exist in a cell database, when the api loads it out, it will set bdm.attachment_id b/c of whatever is in the db record
14:28:03 mriedem they aren't persisted in a cell block_device_mappings table,
14:28:20 mriedem they are serialized as a json blob (like the instance) and stored in the nova_api.build_requests table
14:28:24 mnaser gotcha
14:28:33 mriedem which means, when we load them into the object, we don't set any default field values from the actual db record
14:28:41 sahid mriedem: any chance you ack this: https://review.openstack.org/#/c/515008/ ?
14:28:54 mriedem like if we load up a bdm record created 3 years ago, the attachment_id entry will be null and we'll set that on the object
14:29:17 mnaser mriedem: ok, so but if we did change the behaviour to set a default value, i assume that would involve bumping the object version which wouldn't make it very backportable
14:29:34 mriedem mnaser: no i'm not suggesting we change the object at all
14:29:45 mnaser mriedem: oh i know, i'm just thinking out loud
14:29:48 mriedem ok
14:29:50 mnaser trying to understand nova's codebase a tad bit more
14:30:14 mnaser anyways im going to test out your idea and see if it passes locally
14:34:26 efried_omalley Ye gods, even reading just the *titles* of 882 summit talks is a labor of hours.
14:35:59 sean-k-mooney sahid: im not sure if the intel nfv ci is triggering on os-vif. it should be triggering on any change to nova,neutron,os-vif,networking-ovs-dpdk and devstack but if it is that will do some validation of https://review.openstack.org/#/c/546588/
14:37:14 openstackgerrit Rajesh Tailor proposed openstack/nova master: Reset instance status on invalid host https://review.openstack.org/401009
14:37:46 sean-k-mooney sahid: zuul seams to be happy with it so that is a good sign.
14:38:33 mriedem lyarwood: since sahid left, question in https://review.openstack.org/#/c/515008/
14:40:07 mriedem lyarwood: i mean, i guess the patch is ok in that if one thing fails the user has to retry (either updating the guest xml or the disconnect from the host) and either thing is now idempotent
14:40:19 mriedem just seems like you'd want to try the cleanup thing that's more likely to fail first
14:41:50 dansmith mriedem: trivial review to correct a stack trace in logs that confused multiple support people: https://review.openstack.org/#/c/546423/
14:42:19 jaypipes efried_omalley: I can shortcut that for you. "Edge", "NFV", "Hyperconverged", "Kubernetes". Rinse and repeat.
14:42:31 mriedem dansmith: timely given i'm asking about https://review.openstack.org/#/c/515008/
14:42:58 dansmith hah
14:43:04 efried_omalley jaypipes: Yeah, would be nice to have a list of checkboxes like Amazon does, where I could exclude those terms and filter down to the 20 or so talks that might interest me.
14:44:03 dansmith jaypipes: lol
14:44:10 dansmith jaypipes: early or late?
14:44:30 jaypipes dansmith: early of course.
14:44:38 dansmith jaypipes: good, just checking.
14:44:59 jaypipes dansmith: you know... pre-cloud.
14:45:16 mnaser wow i just realized this bug has 3 paths, < BFV_RESERVE_MIN_COMPUTE_VERSION --> _check_attach (reserves), >= BFV_RESERVE_MIN_COMPUTE_VERSION -> _check_attach_and_reserve_volume (can reserve if cinder version is too old), >= CINDER_V3_ATTACH_MIN_COMPUTE_VERSION -> _check_attach_and_reserve_volume (will create attachment)
14:45:18 dansmith jaypipes: even the ultra-modern starship days were pre-cloud, so I had to ask

Earlier   Later