| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-20 | |||
| 11:50:36 | aj_mailing | join #openstack | |
| 21:59:03 | openstackgerrit | Merged openstack/nova stable/rocky: compute: refactor volume bdm rollback error handling https://review.opendev.org/749225 | |
| 21:59:11 | openstackgerrit | Merged openstack/nova stable/rocky: Add regression tests for bug #1889108 https://review.opendev.org/749226 | |
| 21:59:11 | openstack | bug 1889108 in OpenStack Compute (nova) rocky "failures during driver.pre_live_migration remove source attachments during rollback" [Undecided,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood) | |
| 21:59:14 | openstackgerrit | Merged openstack/nova stable/rocky: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/749227 | |
| #openstack-nova - 2020-09-21 | |||
| 01:10:54 | openstackgerrit | Yao wang proposed openstack/nova master: Use VIR_DOMAIN_XML_INACTIVE for detach volume https://review.opendev.org/749489 | |
| 01:40:33 | openstackgerrit | Yingji Sun proposed openstack/nova stable/train: Set different VirtualDevice.key https://review.opendev.org/752843 | |
| 02:37:18 | openstackgerrit | Yao wang proposed openstack/nova master: Use VIR_DOMAIN_XML_INACTIVE for detach volume https://review.opendev.org/749489 | |
| 08:01:16 | bauzas | good morning Nova | |
| 08:10:47 | gibi | good morning bauzas | |
| 08:23:12 | openstackgerrit | Elod Illes proposed openstack/nova stable/pike: libvirt: Provide VIR_MIGRATE_PARAM_PERSIST_XML during live migration https://review.opendev.org/747978 | |
| 08:27:39 | kashyap | bauzas: When is the deadline? 01-Oct? | |
| 08:27:47 | bauzas | yes... | |
| 08:35:59 | brinzhang | gibi: can you review this patch https://review.opendev.org/#/c/749052? | |
| 09:09:03 | kashyap | lyarwood: Morning, I've tried a rapid succession of attaching 20 disks (via `attach-device`) while there's consistent 40% of load on the CPUs ... can't reproduce so far. | |
| 09:09:18 | kashyap | lyarwood: If you don't mind holding your nose tightly ... here's the repro: https://kashyapc.fedorapeople.org/device-attach-and-detach.bash | |
| 09:10:24 | kashyap | (Fun aside: changing the target dev from sdX to vdX increases the execution of the above script by at least 20 seconds. So attaching 'virtio' disks seems to "slow"? Can't be so) | |
| 09:20:37 | kashyap | That said, I should rewrite it in Python and make it as close to what Nova is doing as possible | |
| 09:23:13 | kashyap | Because, I learnt that if we suspsect a timing issue, 'virsh' has a lot more overhead. A naive connection-based approach is better | |
| 09:50:06 | hemanth_n | sean-k-mooney: Appreciate if you can have a final look on this patch https://review.opendev.org/#/c/749175/ .. thank you | |
| 09:55:26 | lyarwood | kashyap:k~. | |
| 09:55:37 | lyarwood | ops weird connection issue | |
| 09:56:05 | lyarwood | kashyap: ack yeah that sounds like a sane approach, I don't think it's the rapid attach/detach that's the issue btw, just the overall load on the underlying host and libvirtd | |
| 09:58:42 | kashyap | lyarwood: Oh, right; perhaps I should increase the load to 80% and do a detach? | |
| 09:58:55 | kashyap | I'll vary the load (by measuring it in parallel with `sar -u 5`) and see | |
| 10:00:52 | lyarwood | yup sounds good | |
| 10:01:57 | tosky | lyarwood: nice work on the live migration/evacuate stuff, it seems that part is fine | |
| 10:02:05 | tosky | now only the grenade one is missing | |
| 10:10:26 | lyarwood | tosky: np thanks for leading the effort :) | |
| 10:10:53 | lyarwood | gibi / stephenfin ; https://review.opendev.org/#/q/owner:self+topic:native-zuulv3-migration+status:open+project:openstack/nova ^ re the above, the live-migration and new evacuate job should be ready for review now. | |
| 10:14:25 | lyarwood | actually let me update the TODO for the ceph jobs | |
| 10:14:46 | openstackgerrit | Lee Yarwood proposed openstack/nova master: zuul: Introduce nova-evacuate https://review.opendev.org/744883 | |
| 10:14:47 | openstackgerrit | Lee Yarwood proposed openstack/nova master: zuul: Replace nova-live-migration with zuulv3 jobs https://review.opendev.org/752557 | |
| 10:15:19 | lyarwood | there we go, should be good now if the gate behaves | |
| 10:16:16 | tosky | gates and Monday, uhm | |
| 10:16:40 | tosky | there are only 90 (check) + 16 (gate) jobs, so we may be lucky | |
| 10:19:50 | lyarwood | hmmm I wonder if it's actually worth merging the nova-live-migration and nova-evacuate job now that the playbook is written | |
| 10:32:24 | tosky | up to you and the time needed to run the tests vs the time needed to deploy | |
| 10:41:07 | gibi | lyarwood: I have some nits in https://review.opendev.org/#/c/744883 | |
| 10:41:18 | gibi | lyarwood: will you do a follow up or respin it quickly? | |
| 10:42:43 | gibi | brinzhang: sorry I think that refactor needs to wait for W | |
| 10:44:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Enable 'vmcoreinfo' feature by default https://review.opendev.org/752912 | |
| 11:01:28 | sean-k-mooney | hemanth_n: sure ill go over it later today | |
| 11:01:41 | hemanth_n | sean-k-mooney: thanks | |
| 11:01:55 | lyarwood | gibi: ack can do | |
| 11:09:57 | openstackgerrit | Lee Yarwood proposed openstack/nova master: zuul: Introduce nova-evacuate https://review.opendev.org/744883 | |
| 11:09:58 | openstackgerrit | Lee Yarwood proposed openstack/nova master: zuul: Replace nova-live-migration with zuulv3 jobs https://review.opendev.org/752557 | |
| 11:22:21 | gibi | lyarwood: thanks | |
| 11:38:29 | openstackgerrit | Yao wang proposed openstack/nova master: Use VIR_DOMAIN_XML_INACTIVE for detach volume https://review.opendev.org/749489 | |
| 11:43:11 | sean-k-mooney | gibi: stephenfin i have figured out why the func test are failing. one issue is i was not adding the 0x prefix if you use "%(func)o" or "%(func)x" it does not genreate the prefix, the main issue however is device detach was only implemented for disk in the fake libvirt driver | |
| 11:44:21 | sean-k-mooney | so im really not sure how the sriov detach func test actully are passing on master but once i fixed the 0x prefix and added a valid impleamtion of detach the tests seam to be passing. now i just need to run all the other func and unit tests before pushing | |
| 11:48:29 | gibi | I guess it only verifies the compute.manager and resource_tracker part of the functionality | |
| 11:49:04 | sean-k-mooney | ya it must never check the doamin xml. anyway that should now be correct too. | |
| 11:49:59 | sean-k-mooney | i dont really know why my minimal chages to the test code caused it to now fail but im going to fix this in a seperate patch below my actul change | |
| 11:50:35 | sean-k-mooney | although maybe i cant actully | |
| 11:50:50 | sean-k-mooney | since im changin some code im adding in my patch | |
| 11:50:59 | sean-k-mooney | and some code that was already there. | |
| 11:51:23 | gibi | I did see that nova and libvirt generates the func number differnetly I think libvirt adds the prefix while nova doesn't | |
| 11:51:38 | gibi | and libvirt accepts both | |
| 11:53:17 | sean-k-mooney | we strip the prfix when generating the adress doing [2:] | |
| 11:53:41 | sean-k-mooney | i was basically passing an unprefixed string and that endup passing the empty stiring to oslo | |
| 11:54:15 | sean-k-mooney | oslo pciadress field then rejected it since the func part of the pci adress was empty | |
| 11:57:37 | openstackgerrit | Claudiu Belu proposed openstack/nova master: hyperv: Configures chassis asset tags for VMs. https://review.opendev.org/752723 | |
| 12:53:09 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: delegate ovs plug to os-vif https://review.opendev.org/602432 | |
| 13:37:19 | gibi | sean-k-mooney: please remind me which patch is the one where you faced this pci function issue? | |
| 13:40:11 | sean-k-mooney | https://review.opendev.org/#/c/602432/18 | |
| 13:40:22 | sean-k-mooney | the followup to the one you jsut reviewd | |
| 13:40:47 | sean-k-mooney | this is the detach impleamtion https://review.opendev.org/#/c/602432/18/nova/tests/unit/virt/libvirt/fakelibvirt.py@1084 | |
| 13:40:51 | gibi | sean-k-mooney: thanks. then I stoppped to early in my review on that series | |
| 13:40:54 | gibi | continuing now | |
| 13:41:36 | sean-k-mooney | the second hunk in that file is where i had dropped the 0x prefix | |
| 13:41:49 | sean-k-mooney | that was the second issue | |
| 13:44:53 | sean-k-mooney | gibi: the first patch really is a fix for a different bug but i just happen to need that to maintain the same behavior for my own patch since libvirt would nolonger be unplugging the vifs in this case now that os-vif handels the pluging. | |
| 13:48:05 | sean-k-mooney | gibi: by the way i have already accpeted that these wont qualify for rc1 and they will be backported after wards so while i am glad for the review if something else needs review for rc1 then that obviously is higher priority. thanks in anycase. | |
| 13:50:52 | gibi | sean-k-mooney: ack | |
| 13:51:08 | gibi | at the moment I'm not tracking anything for RC1 that needs review | |
| 13:52:22 | sean-k-mooney | ya same. i think lees ubuntu cloud archive patches have also merged in devstack at least the ussuri one i think so the libvirt bumps are really the only think im mindful off that we should still try to land | |
| 13:52:36 | sean-k-mooney | i think we are in a pretty good state overall | |
| 13:53:19 | gibi | yepp | |
| 13:55:05 | sean-k-mooney | gmann: care to send https://review.opendev.org/#/c/747123/ through the gate now that the ussuri version has merged | |
| 13:58:06 | gmann | sean-k-mooney: done | |
| 13:58:36 | sean-k-mooney | :) | |
| 14:14:13 | openstackgerrit | Merged openstack/nova stable/train: Correctly disable greendns https://review.opendev.org/751622 | |
| 14:31:07 | dansmith | artom: I didn't see any discussion of a test for this: https://review.opendev.org/#/c/751302/ | |
| 14:31:26 | artom | dansmith, there wasn't | |
| 14:31:37 | dansmith | artom: since we've had a few ordering things changed in there in recent memory, can't we have a test that pokes at eventlet internals or something to check that things happen like we expect? | |
| 14:31:37 | artom | How would we test that thing specifically? | |
| 14:31:51 | artom | dansmith, ah I see what you mean | |
| 14:31:59 | artom | dansmith, yeah, could be done | |
| 14:32:01 | artom | I guess? | |
| 14:32:39 | dansmith | well, it would make it easier to validate that you've really changed behavior.. clearly the original code didn't think the ordering mattered, so if it does, it'd be nice if we could, you know, validate that | |
| 14:33:28 | artom | dansmith, I agree with your point. Originally I 1. couldn't think of a way to test specifically this and 2. figured the NOTE I left was the best we could do to prevent future reorderings | |
| 14:33:56 | dansmith | but I'm guessing the reviewers just assumed that it does matter and that your change fixed it sufficiently, | |
| 14:34:31 | dansmith | and it'd just be nice to have a test where you can revert the code change and make sure the test validates some wrong behavior, which is (or used to be) common double checking for obscure fixes like this | |
| 14:36:23 | artom | dansmith, I think it still is - we try to do the "regression test that assers the wrong thing -> fix + test fix" thing when we can | |
| 14:36:38 | artom | I suppose in this case we all assumed it wasn't realistic | |
| 14:37:50 | dansmith | assumed but didn't even discuss or question, it seems... just sayin', this is pretty fundamental code, merged pretty quick and backported quick as well.. would be nice to have something to validate it | |
| 14:38:09 | dansmith | like, maybe even the version of eventlet on stable/train behaves a little different or something | |
| 14:39:55 | artom | dansmith, yep, I'mma try to hax something | |
| 14:41:11 | dansmith | I think maybe one tricky thing is going to be that a test will run after eventlet has already been imported and this monkey patching has been done, which means you just need to have the test check that greendns is disabled in the internals or whatever, | |