| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-25 | |||
| 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. | |
| 13:56:09 | zhubx007 | mdbooth: yes, force_raw_images is False | |
| 13:56:38 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Add native implementation OVSDB API https://review.openstack.org/482226 | |
| 13:57:13 | yonglihe | call for review, thanks, it's small one: https://review.openstack.org/#/c/621474/22 | |
| 13:57:47 | mdbooth | zhubx007: Cool, that's your problem. | |
| 13:58:09 | mdbooth | zhubx007: As I say it does feel wrong that the we're making it the operator's responsibility to get that right. | |
| 13:58:17 | alex_xu | sean-k-mooney: agree, and we can transulate hw:.... to a required trait in placement request | |
| 13:58:40 | mdbooth | zhubx007: But that would be a much lower priority issue :) Patches welcome. | |
| 13:59:20 | sean-k-mooney | yep we could. ill write something up for train and add you to the reviews list. | |
| 13:59:36 | alex_xu | cool | |
| 13:59:37 | cdent | scheduler placement meeting in #openstack-meeting-alt now ish | |
| 14:01:12 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: DNM: Testing nova legacy jobs on bionic https://review.openstack.org/639017 | |
| 14:01:41 | zhubx007 | mdbooth: yeah, you are right. but when setting force_raw_images to False, I think it is still an issue. | |
| 14:09:48 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099 | |
| 14:10:16 | mdbooth | zhubx007: Right. That setting is incompatible with rbd imagebackend. It would be nice to, for eg, give an error at startup pointing it out. Or perhaps just switching it off automatically and logging a warning. | |
| 14:11:01 | mdbooth | zhubx007: In fact it's only useful for flat and qcow2 imagebackends. LVM would also have a problem with it. | |
| 14:12:57 | stephenfin | jangutter: Some small comments in that patch ^ | |
| 14:15:56 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099 | |
| 14:16:13 | jangutter | stephenfin: ask and ye shall receive. | |
| 14:16:20 | sean-k-mooney | jangutter: damb looks like i missed leaving the comment by a few seconds | |
| 14:16:54 | sean-k-mooney | jangutter: can ou add the comment too? thanks for doing this by the way | |
| 14:17:23 | jangutter | sean-k-mooney: hahaha, no worries. I don't have my Nagle algorithm turned on today. | |
| 14:17:34 | stephenfin | jangutter: is that correct? | |
| 14:17:48 | stephenfin | x, = (1, 2) gives a ValueError | |
| 14:17:59 | stephenfin | You'd need 'x, _' | |
| 14:18:20 | jangutter | stephenfin: frack, you're right. | |