Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-26
17:59:52 sean-k-mooney yonglihe: efried just reviewing https://review.opendev.org/#/c/648912/14 but why are we looking up instance by name?
18:01:58 efried sean-k-mooney: I haven't the foggiest. I'm involved here in an administrative capacity :)
18:02:43 sean-k-mooney the domain xml has the instance uuid stored in the uuid field for several release now so im wondering why we would use the instance domain name instead
18:03:43 melwitt sean-k-mooney: been meaning to get to that review. after what we briefly discussed at the ptg, that change would be best to piggyback onto the existing periodic for handling deleted instances. I don't know why it would be looking up instances by name
18:03:45 efried k, hopefully yonglihe will be able to answer on the review. Thanks sean-k-mooney.
18:04:20 sean-k-mooney melwitt: it is piggybacking on that task
18:04:42 melwitt but I see a lot of new methods
18:04:42 sean-k-mooney efried: ok im trying to find where it gets teh list of suspected instances
18:05:16 sean-k-mooney melwitt: ya there are. im not sure if they are all needed.
18:05:36 melwitt yeah, I would think there should be none new needed
18:06:18 sean-k-mooney well we need a new method to query the driver for the instance that are runnin on the host but not in the db
18:07:07 sean-k-mooney and then we can call the old meethods that implement the policy. e.g. reap or log or do nothing whatever you have set in the config
18:07:31 melwitt why? there's a self._get_instances_on_driver method already
18:08:08 sean-k-mooney that is a good question :)
18:08:46 sean-k-mooney i have only properly looked at https://review.opendev.org/#/c/648912/14 whic is doing driver change but i need to look at how that is being used in https://review.opendev.org/#/c/627765/28
18:08:51 melwitt anyway, just saying skimming those patches I don't see why they're so large
18:09:29 melwitt or rather, I expected not such a large patch for this
18:13:16 sean-k-mooney melwitt: this seam oververly complex https://review.opendev.org/#/c/627765/28/nova/compute/manager.py@8884
18:14:32 sean-k-mooney also _destroy_orphan_instances is not a greate name for that since it might not destroy anything .
18:15:02 sean-k-mooney melwitt: i was wrong however its adding a new periodici task not extending the existing tasks
18:41:20 melwitt sean-k-mooney: yeah, that's what I had thought. it should get much simpler if it's changed to extend the existing periodic. and I think the suggestion at the ptg from dansmith was to add another enumerated choice to the existing conf option that is something like "reap-unknown" which will reap both known deleted and unknown guests. and otherwise just log unknowns
18:43:20 openstackgerrit Merged openstack/nova master: Fix update_provider_tree signature in reference docs https://review.opendev.org/667408
18:43:52 openstackgerrit Eric Fried proposed openstack/nova-specs master: grammar fix for show-server-numa-topology spec https://review.opendev.org/667487
18:44:18 mriedem sean-k-mooney: it's named _destroy* because it's similar to the existing _destroy_running_instances
18:44:21 mriedem which might also not destroy anything
18:44:38 sean-k-mooney mriedem: ah ok
18:45:12 sean-k-mooney my main issue with this is it will only really work for the libvirt dirver
18:45:17 sean-k-mooney and the fake driver
18:53:52 mriedem yup
18:54:01 mriedem i believe i noted something along those lines last time i did a deep review on it
18:54:07 openstackgerrit Miguel Ángel Herranz Trillo proposed openstack/nova master: Fix type error on call to mount device https://review.opendev.org/659780
18:54:13 mriedem i seem to remember why they needed to lookup by name at the time, and it was libvirt-specific
18:54:35 sean-k-mooney ya im looking at it again
18:55:02 sean-k-mooney its because we can have libvirt domains that are for instance that nova created but have been deleted form the db
18:55:40 sean-k-mooney so to destory the running guest they need to use the libvirt domain name
18:55:58 mriedem having said all that, i'm not opposed to starting with something that could eventually be implemented by other drivers
18:56:07 mriedem as long as it's graceful about other drivers not implementing the necessary interface
18:56:31 sean-k-mooney ya it handels the not implmetned excetion correctly in the manager
18:56:58 sean-k-mooney im think of asking them to use the uuid instead of the name however
18:57:36 sean-k-mooney the instance uuid is set in the libvirt domains uuid field
18:57:57 mriedem yeah uuid is ideal if we can use it
18:58:14 sean-k-mooney but i libvirt allows us to deleate by uuid
18:58:32 sean-k-mooney me might just be pushing the translation into the dirver
18:59:48 sean-k-mooney im also wondering how to deal with the fact we could be leaking plugged interfaces
19:03:55 mriedem heh,
19:04:01 mriedem well we could be leaking all sorts of things
19:04:07 mriedem storage connections
19:04:42 sean-k-mooney will undefineing the domin delete the root disk?
19:04:55 sean-k-mooney or other disks
19:05:26 mriedem i doubt it
19:05:46 sean-k-mooney if we are reaping the orpahn vms that were created by nova but are deleted in the db we really should be cleaning up all there local resouces in that case which this current patch does not attemtp to do
19:06:03 mriedem otherwise we wouldn't need separate calls during driver.destroy to cleanup the volumes via brick and unplug vifs via os-vif
19:06:19 mriedem at some point this could also be cyborg devices and such couldn't it?
19:06:20 sean-k-mooney ya that is a good point
19:06:26 sean-k-mooney ya
19:07:19 sean-k-mooney so if we are going to reap these we need to try and clean up as much of the resouces as we can or just support powering down the instance but not reaping them?
19:07:50 openstackgerrit Merged openstack/nova-specs master: grammar fix for show-server-numa-topology spec https://review.opendev.org/667487
19:07:57 sean-k-mooney if we just delete the domain the operator has noting ot go on to figure out what they need to clean up manually
19:09:11 sean-k-mooney its tricky however as from the domain we dont know what nova create or not but i think we should at lesast try to do some cleanup
19:12:52 sean-k-mooney anyway im going to get something to eat
19:13:05 sean-k-mooney o/
19:19:29 mriedem there is meta in the domain that tells us if nova created it
19:19:42 sean-k-mooney yes there is
19:19:48 mriedem for libvirt anyway
19:20:05 sean-k-mooney which i asked yonglihe to check when deleteing the domian
19:20:13 sean-k-mooney in the libvirt case that is
19:20:48 efried mriedem: +1 on the ML note, thanks for that
19:21:24 mriedem np
19:22:36 sean-k-mooney that is what https://review.opendev.org/#/c/648912/14/nova/virt/libvirt/driver.py@9695 is doing and its used to filer the domains here https://review.opendev.org/#/c/627765/28/nova/compute/manager.py@8886
19:59:33 efried mriedem: took me three days to go through all the specs, but http://lists.openstack.org/pipermail/openstack-discuss/2019-June/007381.html
20:10:01 mriedem efried: huh https://review.opendev.org/#/q/project:openstack/nova-specs+status:open+path:%255Especs/train/approved/.*+reviewedby:self seems to not work properly, it's only showing me https://review.opendev.org/#/c/631154/ but i've clearly commented on https://review.opendev.org/#/c/648686/ as well
20:10:17 mriedem maybe reviewedby is only on the latest PS?
20:10:55 mriedem aha
20:10:56 mriedem https://review.opendev.org/#/q/project:openstack/nova-specs+status:open+path:%255Especs/train/approved/.*+reviewer:self
20:11:00 mriedem reviewer:self, not reviewedby:self
20:14:14 efried whoops, thanks
21:31:07 melwitt mnaser, imacdonn: hi, as responders to the ML thread awhile back, I have a spec up for showing server status as UNKNOWN if host status is UNKNOWN that has been receiving some reviews. your reviews would be helpful for deciding whether it goes forward https://review.opendev.org/666181
21:33:10 mriedem dansmith: you may want to drop your +2 to a +1 or 0 https://review.opendev.org/#/c/457886/ until i get the ceph job results on it
21:41:44 openstackgerrit Miguel Ángel Herranz Trillo proposed openstack/nova master: Fix type error on call to mount device https://review.opendev.org/659780
21:44:11 mriedem dansmith: nvm, lyarwood already had a patch up to test that
22:14:40 mriedem efried: are you ok with me just pushing up this test change and +2ing? https://review.opendev.org/#/c/659780/3/nova/tests/unit/virt/disk/mount/test_nbd.py
22:14:51 efried ...
22:15:58 efried mriedem: yes, and I'll +A
22:16:52 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix type error on call to mount device https://review.opendev.org/659780
22:17:37 mriedem done
22:18:58 efried +A
22:25:39 mnaser melwitt: left a comment thanks :D
22:43:44 melwitt thanks
#openstack-nova - 2019-06-27
13:13:51 gibi mriedem: the previous patches just refactored the heal allocation code path
13:14:10 gibi to make it easy to add the port healing
13:15:30 gibi mriedem: the basic structure of final patch
13:18:00 gibi mriedem: during _heal_allocations_for_instance first we get if the instance has ports that needs healing
13:18:26 gibi this _get_port_allocations_to_heal is the majority of the added code there
13:19:01 gibi as it detect is there is any port that needs healing and generates the allocation fragment for that port
13:19:33 mriedem ok i'm assuming just checking if the port has a resource request (if hitting neutron directly) or checking the info cache for a bound port with the allocatoin in the binding profile?
13:19:52 gibi we are hitting neutron directly
13:20:02 mriedem hmm, that's kind of expensive isn't it?
13:20:19 mriedem why can't we use the info cache to tell if the vif has a port allocation?
13:20:49 mriedem or is that some kind of chicken and egg issue? or we don't trust the cache?
13:20:55 gibi we need to hit neutron to see if there is resource request on the port

Earlier   Later