Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-25
11:08:43 kashyap sean-k-mooney: Yep, noted
11:13:55 lyarwood mdbooth: https://review.openstack.org/#/c/637527/ - If you have time today would you mind going over this again given our discussion on Friday?
11:18:14 mdbooth lyarwood: Yep, sure. Will do it now.
11:18:48 lyarwood mdbooth: thanks
11:19:44 mdbooth lyarwood: Although... Hmm. I wonder if my comments are still relevant, despite my previous misunderstanding.
11:20:33 mdbooth lyarwood: Backend_names != hosts, right?
11:21:13 lyarwood mdbooth: they should form part of the hostname itself
11:21:19 lyarwood mdbooth: host@backend#type
11:22:02 mdbooth Ok, so if there's 1 c-vol and 2 backend_names then there are 2 'hosts' as reported by list_hosts()?
11:22:37 lyarwood correct
11:23:11 lyarwood c-api is all over the place tbh
11:25:57 mdbooth https://developer.openstack.org/api-ref/block-storage/v3/index.html?expanded=list-all-hosts-for-a-project-detail#hosts-extension-os-hosts
11:26:19 mdbooth The 'cinder-volume' in that case has host@backend
11:29:04 lyarwood mdbooth: right sorry, src_host is coming from os-vol-host-attr:host on the volume and that's host@backend#type
11:29:17 lyarwood mdbooth: or at least that can be host@backend#type
11:29:50 lyarwood mdbooth: thus the host_name not in src_host check when trying to find a destination host
11:33:59 mdbooth lyarwood: Does host *always* have #type appended?
11:34:14 mdbooth Because you're doing a straight string comparison
11:39:46 lyarwood mdbooth: volume['os-vol-host-attr:host'] appears to, again I was wrong about host['host_name'], it doesn't contain #type
11:44:47 mdbooth lyarwood: Ok, does that mean you need to parse #type out of the former?
11:45:09 mdbooth It would be nice if this was consistent
11:46:14 lyarwood mdbooth: I don't think so, we only care about the host@backend part here for migrations
11:46:45 lyarwood mdbooth: we are just checking that the host@backend isn't hosting the volume at present
11:46:57 lyarwood mdbooth: the type isn't a factor here
11:47:05 mdbooth lyarwood: Right, but if I've understood what you've just told me, you're doing a string comparson between node@backend#type and node@backend, which is going to fail
11:47:36 lyarwood mdbooth: other way around, node@backend not in node@backend#type
11:48:05 mdbooth Ah, you're doing a not in, not a string comparison...
11:48:22 mdbooth Eww
11:48:28 mdbooth But I see
11:49:14 lyarwood yeah I can spell that out more in a comment
11:56:26 mdbooth lyarwood: Updated.
11:56:39 lyarwood mdbooth: thanks, would you mind hitting the actual nova bugfix while you're at it
11:56:45 lyarwood mdbooth: https://review.openstack.org/#/c/637224/
11:56:57 mdbooth lyarwood: I've asked for additional assertions, but I think you can cut/paste them out of my old patch.
11:57:57 openstackgerrit Zhenyu Zheng proposed openstack/nova master: Detach/Attach root volume API changes https://review.openstack.org/623981
11:58:04 lyarwood mdbooth: ack thanks
11:58:34 openstackgerrit Merged openstack/nova stable/rocky: Avoid redundant initialize_connection on source post live migration https://review.openstack.org/636895
11:58:47 lyarwood finally
11:58:56 mdbooth lyarwood: Thinking about it, you might need to split up the tempest test into the familiar 'assert incorrect behaviour', 'fix incorrect behaviour' 2 part series.
11:59:04 mdbooth lyarwood: Nice :)
11:59:33 lyarwood mdbooth: I don't think we do that with tempest test additions tbh
12:00:04 mdbooth Because I think we want the nova fix to Depends-On the change which asserts the correct behaviour in retype so as to avoid a regression.
12:00:47 mdbooth But once you've landed the assertions I asked for against migration, the tempest test will fail until the nova fix lands.
12:01:23 mdbooth (Which highlights why the assertions are required, btw)
12:02:12 lyarwood mdbooth: The tempest changes will depend on the Nova fix
12:02:43 lyarwood mdbooth: we can land some updates to the existing retype assertions outside of that
12:02:46 mdbooth lyarwood: So you want to add the assertions to *retype* before landing the Nova change, because that asserts that you're not regressing retype
12:03:03 lyarwood mdbooth: yeah that's simple enough to land first
12:04:00 lyarwood mdbooth: so retype assertions update, nova fix and finally tempest migration test right?
12:04:07 mdbooth lyarwood: Yeah
12:05:07 mdbooth So the Nova change will depends-on the retype tempest addition. The full tempest test will depends-on the nova change.
12:05:18 lyarwood correct
12:05:44 lyarwood zuul++
12:40:27 mdbooth lyarwood: Done. -1 is for discussed tempest change. Unit test change could be a nit, but one I really think we should fix. I'd like to see +1 from a cinder dev before anybody +2s it.
12:48:47 openstackgerrit Zhenyu Zheng proposed openstack/nova master: Detach/Attach root volume API changes https://review.openstack.org/623981
13:02:59 openstackgerrit Merged openstack/os-vif master: remove brctl from vif_plug_ovs https://review.openstack.org/636821
13:10:36 zhubx007 Can anyone help to take a look at this https://review.openstack.org/#/c/638080/? A bug fix. Thanks.
13:19:54 jaypipes sean-k-mooney: trying to get through all os-vif patches you list in your email today.
13:19:54 mdbooth zhubx007: That isn't going to fly for a few reasons. One of which is that by using automatic format detection it opens a potential security hole in image import.
13:20:40 mdbooth zhubx007: In fact I'm almost certain that would open a major security hole.
13:20:48 sean-k-mooney jaypipes: thanks. im respinning "make functional tests run on python 3" based on the feedback
13:21:29 mdbooth zhubx007: Allowing any authenticated tenant to read all data from the compute host.
13:21:57 jangutter jaypipes: I think that bridge+routing code is the classic monkeys+bananas thing.
13:24:18 sean-k-mooney jaypipes: does the timeline and porities make sense. once the final brctl patch merges we have reached the mvp for stien the rest are nice to have but dont block features
13:31:02 gibi jaypipes: hi! the next couple of patches in the bandwidth series (staring here https://review.openstack.org/#/c/616240) has mriedem's +2 on it. I'm working on a fup to fix his comments. So if you have time could you look at the series again?
13:34:27 jaypipes sean-k-mooney: I'd like to get the native ovslib stuff in if possible.
13:34:36 jaypipes gibi: yup
13:34:42 gibi jaypipes: thanks a lot
13:37:35 alex_xu sean-k-mooney: stephenfin something need your expert https://review.openstack.org/#/c/634828/24/nova/virt/libvirt/driver.py@6783 :)
13:37:37 sean-k-mooney jaypipes: yep that is why its in the prefer section :) its not required but im happy to wait a couple of extra hours to try and get it over the line
13:39:08 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Rework 'EBUSY' (SIGKILL) error handling code path https://review.openstack.org/639091
13:40:29 artom alex_xu, hah, was just about to ask :)
13:40:39 sean-k-mooney alex_xu that is a good question. i dont really think there is a good usecase for different values
13:40:41 alex_xu artom: hah
13:40:43 artom alex_xu, thanks for taking a thorough look at that series, by the way :)
13:40:55 alex_xu np :)
13:40:55 sean-k-mooney alex_xu: that said since it can be set per node it proably should be sent back
13:41:31 alex_xu sean-k-mooney: ok, thanks
13:41:33 openstackgerrit Merged openstack/os-vif master: remove use of brctl from vif_plug_linux_bridge https://review.openstack.org/636822
13:41:45 artom sean-k-mooney, noted, thanks
13:42:44 sean-k-mooney if there was a good usecase for different values we should be schduling on it and we cant today except via host aggregates
13:44:15 sean-k-mooney we could expose it via a custom trait at some point but i think this is too virtdirer specific to warrent it.
13:45:33 openstackgerrit Merged openstack/os-resource-classes master: Add normalize_name utility https://review.openstack.org/634258
13:45:33 sean-k-mooney technically different prioties of realtime tasks is a thing but i dont think it should be a thing in cloud. im much more comfortable with a boolean form that point of view. e.g. it supprot realtime instance or it doesnt
13:47:14 zhubx007 mdbooth: Got your review. Is there any suggestion to fix the bug? If do not convert the qcow2 to raw here, the instance failed to boot. Thanks.
13:47:32 mdbooth zhubx007: I just added another comment on the review about that.
13:48:15 mdbooth I don't think you need to fix it, because if you leave force_raw_images with its default value of True then the image should be (safely) converted for you anyway.
13:48:31 alex_xu sean-k-mooney: that priorities is also indicated the host whether support realtime? so you mean at least there is way to find out a host support realtime
13:49:24 alex_xu ah, I see now, we won't fix realtime and normal workload on the same host, right? sean-k-mooney
13:49:30 alex_xu s/fix/mix
13:51:07 mdbooth zhubx007: Are you setting force_raw_images to false?
13:51:43 sean-k-mooney alex_xu: well right now you have to use host aggrates to split them
13:51:53 mdbooth If force_raw_images is True and you're hitting this, I think we should go back and open a bug about this to try to work out why before attempting a fix.
13:51:57 sean-k-mooney alex_xu: mainly because we dont know which host have a realtime kernel
13:52:22 alex_xu sean-k-mooney: I see now
13:53:46 zhubx007 mdbooth: :) wait for a minute. I check nova.conf file now.
13:53:50 sean-k-mooney if https://github.com/openstack/nova/blob/master/nova/conf/libvirt.py#L759-L763 did not have a default we could use the presence or absence of that config value to know if we should add a realtime trait to the compute node
13:54:01 sean-k-mooney but since it default to 1 we cant
13:54:32 sean-k-mooney we might want to change that in the future but not in stien
13:55:30 sean-k-mooney alex_xu: ^ likely we would have to add another config to not break backwards compatiblity but the detail should praobly be discussed in a short spec.

Earlier   Later