| opendevreview | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/851337 | 04:02 |
|---|---|---|
| opendevreview | Amit Uniyal proposed openstack/nova master: For evacuation, ignore if task_state is not None https://review.opendev.org/c/openstack/nova/+/848886 | 04:53 |
| gibi | good morning | 07:22 |
| Uggla | gibi, o/ | 07:33 |
| opendevreview | Merged openstack/nova master: Updated Suspend definition in server concepts doc https://review.opendev.org/c/openstack/nova/+/851511 | 07:33 |
| gibi | \o/ the unittest.mock series landed \o/ | 07:35 |
| gibi | (hope nobody too mad about the merge conflicts we generated with that :D) | 07:35 |
| gibi | sean-k-mooney[m]: left feedback in the vdpa patch https://review.opendev.org/c/openstack/nova/+/832330 | 07:47 |
| gibi | sean-k-mooney[m]: could you look at https://review.opendev.org/c/openstack/nova/+/851909 it is needed for the sdk 0.100 release here https://review.opendev.org/c/openstack/requirements/+/849986 | 08:04 |
| sean-k-mooney[m] | im just about to have a quick call downstream but ill look at it shortly thanks for reviewing the vdpa patch ill adress your feedback when im adressing stephens | 08:12 |
| sean-k-mooney[m] | if i can get the first patch at least to a point where it can merge today that would be ideall then i can focus on just the ones that wont be backported and thos can hopefully merge seperately as a group | 08:13 |
| gibi | sean-k-mooney[m]: sure. ping me and I will re-review the vdpa patch | 08:15 |
| gibi | if stephenfin is also available the we can land that today | 08:16 |
| opendevreview | Balazs Gibizer proposed openstack/placement master: Func test for os-traits and os-resource-classes lib sync https://review.opendev.org/c/openstack/placement/+/851966 | 08:36 |
| gibi | sean-k-mooney[m], melwitt: as we agreed on the Zed PTG I change the how placement tests for the lib sync so that we don't need to do the test disable/enable dance any more at a lib release ^^ | 08:37 |
| sean-k-mooney[m] | ack almost done with internal call | 08:41 |
| sean-k-mooney[m] | ill look at that next | 08:42 |
| sean-k-mooney[m] | +2 on the safeconnect fix | 08:42 |
| *** tosky_ is now known as tosky | 08:47 | |
| gibi | thanks | 08:58 |
| sean-k-mooney[m] | i was going to ask for the placement change to be done slightly differntly | 09:04 |
| sean-k-mooney[m] | but i realise now that you are loading the tratis/resouce classes directly form the lib and assertign the api returns the same content | 09:04 |
| sean-k-mooney[m] | which should always be in sync | 09:04 |
| sean-k-mooney[m] | i was going to ask that you check that the api respocne contains the lib content and the could was greater or equal | 09:05 |
| sean-k-mooney[m] | but the exact comparison should work as ultimately they have the same data source | 09:05 |
| sean-k-mooney[m] | the lib | 09:05 |
| sean-k-mooney[m] | the only test coverage we currntly loose by not doing the >= check is if we acidentally delete a trait | 09:09 |
| sean-k-mooney[m] | but we are aware that that is not allowed so im not really concerned by that | 09:09 |
| gibi | do you mean accidentally deleting a trait from os-traits? yeah that is something we need to cover elsewhere | 09:14 |
| gibi | today we also not checking for that. Also a >= would not cover that as a os-traits lib version bump migh add 10 new traits and remove 2 but the change is net positive so >= would pass | 09:15 |
| sean-k-mooney[m] | we were checking for it indirectly by asserting the exact number | 09:16 |
| sean-k-mooney[m] | but ya | 09:16 |
| sean-k-mooney[m] | we could have added 2 and removed 2 | 09:16 |
| sean-k-mooney[m] | i think core review is enough to catch that honestly | 09:16 |
| gibi | yes, I hope so | 09:16 |
| sean-k-mooney[m] | we all know that we cant ever remove traits | 09:16 |
| gibi | if not then we need a test in os-traits for it | 09:17 |
| sean-k-mooney[m] | if we need too i would put a hacking test in or similar | 09:17 |
| gibi | as the placement test would be too late to catch it | 09:17 |
| gibi | yeah, hacking would work too | 09:17 |
| sean-k-mooney[m] | ok ill be back in about 10 mins | 09:19 |
| gibi | ack | 09:19 |
| sean-k-mooney[m] | just going to check on fryea and what she is barking at | 09:19 |
| * gibi needs a cat | 09:20 | |
| amorin | hello nova team, when shelving an instance, the port binding in neutron is "staying" on the host, I was expecting it to be unbound | 09:34 |
| amorin | am I wrong? | 09:34 |
| sean-k-mooney[m] | amorin: no your are not wrong its a know issue | 09:34 |
| amorin | nice! | 09:35 |
| sean-k-mooney[m] | your expectation alines with mine but we have never actully done it | 09:35 |
| amorin | do you have a launchpad bug already reported for this? | 09:35 |
| sean-k-mooney[m] | https://review.opendev.org/c/openstack/nova/+/832330/7/nova/tests/functional/libvirt/test_pci_sriov_servers.py#1286 | 09:36 |
| sean-k-mooney[m] | amorin no not currently | 09:36 |
| sean-k-mooney[m] | i just came across it whlie wrting functional tests for something else | 09:36 |
| sean-k-mooney[m] | so if you want to file one and or adress it please do | 09:36 |
| sean-k-mooney[m] | amorin: this should not break anything today | 09:36 |
| amorin | ok, I will create the bug report at least | 09:37 |
| sean-k-mooney[m] | ack | 09:37 |
| amorin | do you know if neutron already implement such API? | 09:37 |
| amorin | that would allow nova to unbound the port? | 09:37 |
| sean-k-mooney[m] | yes it does | 09:37 |
| sean-k-mooney[m] | and nova has some code to do it too | 09:38 |
| amorin | in the binding extension probably | 09:38 |
| sean-k-mooney[m] | it currently however does too much | 09:38 |
| sean-k-mooney[m] | well unbinding is jsut doing 2 things | 09:38 |
| amorin | oh nice, any hint where it's located in nova code? | 09:38 |
| sean-k-mooney[m] | one settign bind-host=None | 09:38 |
| sean-k-mooney[m] | and second clearing the files we set in the binding profile | 09:38 |
| sean-k-mooney[m] | what we shoudl not do is clear the device id | 09:38 |
| sean-k-mooney[m] | that is the bit our current unbind code is doing that it should not be | 09:39 |
| sean-k-mooney[m] | our current unbind is only used for detaching a port or when deleteing a vm | 09:39 |
| amorin | ack, yes, the device_id should stay | 09:39 |
| sean-k-mooney[m] | https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L616 | 09:40 |
| amorin | perfect, that where I was also looking :) | 09:41 |
| sean-k-mooney[m] | so this should not be part of unbind https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L642 | 09:41 |
| sean-k-mooney[m] | 'device_id': '', | 09:41 |
| sean-k-mooney[m] | 'device_owner': '', | 09:41 |
| sean-k-mooney[m] | is acttully detach | 09:41 |
| amorin | ack, we should maybe split that in 2 separate function | 09:42 |
| amorin | where detach would call unbind | 09:42 |
| sean-k-mooney[m] | that or add a detach kwarg | 09:42 |
| amorin | ok, maybe easier | 09:42 |
| amorin | I think I have all info, I'll do some tests, open the bug, and come back | 09:43 |
| sean-k-mooney[m] | did this cause you any issues or did you just notice it and find it ood | 09:43 |
| amorin | it's causing us issues because we are monitoring the ports attached to a compute, and we found some bound ports without instances | 09:43 |
| amorin | and then, digging into that, we were thinking that it's odd to keep the binding :) | 09:44 |
| sean-k-mooney[m] | that should not result in the ports being bound on the compute node | 09:44 |
| sean-k-mooney[m] | sorry not bound | 09:44 |
| sean-k-mooney[m] | but present on ovs | 09:44 |
| amorin | it's not anymore | 09:44 |
| amorin | so i'ts good | 09:44 |
| sean-k-mooney[m] | and or "plugged" | 09:44 |
| sean-k-mooney[m] | ya okj | 09:45 |
| sean-k-mooney[m] | just making sure we were not leaking the neutron ports | 09:45 |
| sean-k-mooney[m] | we acctuly do call unplug_vifs | 09:45 |
| sean-k-mooney[m] | during shelve offload | 09:45 |
| amorin | yes, everything is actually fine on the compute | 09:45 |
| sean-k-mooney[m] | incidentally shelve offload is where it should be unbound | 09:46 |
| sean-k-mooney[m] | not shelve itslef | 09:46 |
| amorin | it's just that the database (ml2_port_binding) does not reflect what is on the compute | 09:46 |
| amorin | yes | 09:46 |
| sean-k-mooney[m] | by defualt we automaticaly shelve offload with a time out of 0 seconds | 09:46 |
| sean-k-mooney[m] | yep that makes sense | 09:46 |
| amorin | true, we kept this parameter to 0 here | 09:46 |
| sean-k-mooney[m] | personally i would prefer to eventurlaly remove the config option and always just offload but that proably wont happen | 09:47 |
| amorin | what's the purpose of delaying the offload? | 09:48 |
| sean-k-mooney[m] | in rare cases if you unshleve quickly enough it can sometimes be useful to delay. the use case was for bustable instances | 09:48 |
| sean-k-mooney[m] | basically in some cases where shelve/unshele is used to scale in/out | 09:49 |
| sean-k-mooney[m] | its nice to have say a 15min delay incase the load spike | 09:49 |
| sean-k-mooney[m] | as its faster to unshleve if the vms is still in place | 09:49 |
| sean-k-mooney[m] | in partice that was only ever useful for vms without ceph or boot form volume | 09:50 |
| amorin | ok, that's sound like a weird usecase to me | 09:50 |
| amorin | shelve / unshelve is not suppose to happen a quick way | 09:50 |
| sean-k-mooney[m] | well that depends | 09:50 |
| sean-k-mooney[m] | shelve should be very very quick | 09:50 |
| sean-k-mooney[m] | if you are using rbd or boot form volume | 09:50 |
| sean-k-mooney[m] | sicne there is no data to copy | 09:50 |
| sean-k-mooney[m] | but ya it was a pretty niche usecase which is why i would prefer to simplify the code | 09:51 |
| amorin | ack | 09:51 |
| sean-k-mooney[m] | that or remove the config option but make it an api parmater | 09:51 |
| sean-k-mooney[m] | so the user can say delay for up to x time | 09:51 |
| sean-k-mooney[m] | i do not like config driven api behavior shich this currently is | 09:52 |
| opendevreview | Merged openstack/nova master: Remove the PowerVM driver https://review.opendev.org/c/openstack/nova/+/850346 | 10:47 |
| *** bhagyashris is now known as bhagyashris|ruck | 11:11 | |
| sean-k-mooney[m] | stephenfin: your lines of code stats will never not be negitive ^ | 11:23 |
| gibi | don't encourage him, he will find a way to replace the content of the nova repo with a simple readme file :D | 11:23 |
| sean-k-mooney[m] | cells v1, nova networks xen and power vm. | 11:23 |
| sean-k-mooney[m] | whats next on the list :) | 11:24 |
| sean-k-mooney[m] | oh i for got the docs project :) | 11:24 |
| sean-k-mooney[m] | @gibi: that would be one way to close out all the bugs | 11:25 |
| gibi | what an elegant way :) | 11:25 |
| sean-k-mooney[m] | @gibi speaking of which i marked the custom device_owner bug as invalid | 11:25 |
| gibi | heh, I guessed you would | 11:26 |
| gibi | and I agree | 11:26 |
| gibi | their use case can be solved without the requested change | 11:28 |
| sean-k-mooney[m] | if we wanted to support it we should do it properly as a minor feature with the filtering of events on the neutron side and filtering of port on the nova side so taht they are never stored in our netrowk info cache ectra | 11:30 |
| opendevreview | Merged openstack/nova master: [trivial] Simplify dict get call by removing unused default https://review.opendev.org/c/openstack/nova/+/850450 | 11:30 |
| gibi | sean-k-mooney[m]: I agree | 11:37 |
| gibi | during live migration what triggers the creation of the domain on the dest host? Is it the call to migrateToURI3 from the source host? or nova somehow pre-create a domain on the dest host? | 12:12 |
| sean-k-mooney[m] | yes | 12:12 |
| sean-k-mooney[m] | its created by libvirt as a result of our call to libvirt on the source node | 12:13 |
| gibi | thanks | 12:13 |
| sean-k-mooney[m] | via the migrateToURI3 function | 12:13 |
| gibi | so even though our func test env simulate libvirt it does not simulate that 14:12 < gibi> during live migration what triggers the creation of the domain on the dest host? Is it the call to migrateToURI3 from the source host? or nova somehow pre-create a domain on the dest host? | 12:13 |
| gibi | 14:12 < sean-k-mooney[m]> yes | 12:13 |
| gibi | again | 12:13 |
| gibi | sorry | 12:13 |
| sean-k-mooney[m] | correct | 12:13 |
| sean-k-mooney[m] | we just mock out that call | 12:14 |
| gibi | so even though our func test env simulate libvirt it does not simulate that migrateToURI3 triggers the domain creation on the dest | 12:14 |
| gibi | ack | 12:14 |
| gibi | that explains some func test failures I see with https://review.opendev.org/c/openstack/nova/+/851832/3 | 12:14 |
| sean-k-mooney[m] | we dont really simulate the domins in general in the libvirt fixture | 12:14 |
| sean-k-mooney[m] | at least not for the move ops like that | 12:14 |
| sean-k-mooney[m] | we proably could mock it differntly adn have it do that to be fair | 12:15 |
| gibi | three is some vm tracking in the fixture but yes, that seems to be incomplete | 12:15 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Do not mutate migration context for rollback_live_migration_at_destination https://review.opendev.org/c/openstack/nova/+/851832 | 13:13 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Prevent instance.save() under mutated migration context https://review.opendev.org/c/openstack/nova/+/850746 | 13:13 |
| *** dasm|off is now known as dasm | 13:18 | |
| opendevreview | Balazs Gibizer proposed openstack/placement master: Clarify trait filtering in the API doc https://review.opendev.org/c/openstack/placement/+/825501 | 13:22 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/ussuri: Reproduce bug 1944759 https://review.opendev.org/c/openstack/nova/+/836733 | 13:25 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/ussuri: Store old_flavor already on source host during resize https://review.opendev.org/c/openstack/nova/+/836734 | 13:25 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Poison /sys access via various calls in test https://review.opendev.org/c/openstack/nova/+/844627 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Add compute restart capability for libvirt func tests https://review.opendev.org/c/openstack/nova/+/850510 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rename [pci]passthrough_whitelist to device_spec https://review.opendev.org/c/openstack/nova/+/843834 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rename exception.PciConfigInvalidWhitelist to PciConfigInvalidSpec https://review.opendev.org/c/openstack/nova/+/843861 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rename whitelist in tests https://review.opendev.org/c/openstack/nova/+/843862 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Basics for PCI Placement reporting https://review.opendev.org/c/openstack/nova/+/846187 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Extend device_spec with resource_class and traits https://review.opendev.org/c/openstack/nova/+/846218 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Reject PCI dependent device config https://review.opendev.org/c/openstack/nova/+/846435 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Reject mixed VF rc and trait config https://review.opendev.org/c/openstack/nova/+/846436 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Ignore PCI devs with physical_network tag https://review.opendev.org/c/openstack/nova/+/846219 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Reject devname based device_spec config https://review.opendev.org/c/openstack/nova/+/846466 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support [pci]device_spec reconfiguration https://review.opendev.org/c/openstack/nova/+/846470 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Stop if tracking is disable after it was enabled before https://review.opendev.org/c/openstack/nova/+/847009 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Move provider_tree RP creation to PciResourceProvider https://review.opendev.org/c/openstack/nova/+/850546 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Retry /reshape at provider generation conflict https://review.opendev.org/c/openstack/nova/+/851358 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Heal missing PCI allocation in the resource tracker https://review.opendev.org/c/openstack/nova/+/851359 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | 13:31 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Add more test coverage for devname base dev spec https://review.opendev.org/c/openstack/nova/+/844625 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Extra tests for remote managed dev spec https://review.opendev.org/c/openstack/nova/+/844626 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Unparent PciDeviceSpec from PciAddressSpec https://review.opendev.org/c/openstack/nova/+/844491 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Fix PciAddressSpec descendants to call super.__init__ https://review.opendev.org/c/openstack/nova/+/844565 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Remove dead code from PhysicalPciAddress https://review.opendev.org/c/openstack/nova/+/844628 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Clean up mapping input to address spec types https://review.opendev.org/c/openstack/nova/+/845765 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Remove unused PF checking from get_function_by_ifname https://review.opendev.org/c/openstack/nova/+/845775 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Fix type annotation of pci.Whitelist class https://review.opendev.org/c/openstack/nova/+/845780 | 13:38 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Move __str__ to the PciAddressSpec base class https://review.opendev.org/c/openstack/nova/+/845781 | 13:38 |
| gibi | pheww rebased out the pci serieses from the merge conflict | 13:39 |
| *** lifeless_ is now known as lifeless | 13:51 | |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Reproducer for bug 1982497 https://review.opendev.org/c/openstack/nova/+/850672 | 14:41 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Do not mutate migration context for rollback_live_migration_at_destination https://review.opendev.org/c/openstack/nova/+/851832 | 14:41 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Prevent instance.save() under mutated migration context https://review.opendev.org/c/openstack/nova/+/850746 | 14:41 |
| melwitt | gibi: ack, will look | 15:29 |
| gibi | thanks | 15:31 |
| sean-k-mooney[m] | gibi are you going with dans suggetion to just remove all the save or proceeding with the decorator for now | 15:51 |
| sean-k-mooney[m] | or both | 15:51 |
| sean-k-mooney[m] | backport decorator adn then remove saves on master only? | 15:51 |
| gibi | sean-k-mooney[m]: I proposed a decorator now and fixed the rebuild case by hand with apply and revert | 15:51 |
| sean-k-mooney[m] | ok | 15:52 |
| gibi | I feel that changing the contract between the compute manager and the virt drivers is a bit much for me for now | 15:52 |
| sean-k-mooney[m] | ack | 15:52 |
| gibi | the decorator will catch all the problematic cases | 15:52 |
| gibi | and we have two of those one that is fixed by the bugfix and the other was as simple one with rebuild | 15:53 |
| sean-k-mooney[m] | ill review tomorrow but i assume you are just logging the issue for now and not saving | 15:53 |
| gibi | wait | 15:53 |
| gibi | so for the rollback live migration | 15:53 |
| gibi | I was able to remove the mutated migration context and keep the save | 15:53 |
| sean-k-mooney[m] | ok | 15:54 |
| gibi | the vpmem cleanup that needed the mutation contex can be done based on the domain and not based on the instance.resources storage | 15:54 |
| gibi | this solves our current bug I believe | 15:54 |
| gibi | the decorator is added in a followup to catch new cases by raising at save() if it is called under a mutated context | 15:54 |
| gibi | and that revealed a second place where we do this, at rebuild | 15:55 |
| gibi | but that was fairly simple to fix | 15:55 |
| sean-k-mooney[m] | oh ok so you have resolved the current incorrect uses | 15:56 |
| sean-k-mooney[m] | and new incorrect uses will raise | 15:56 |
| gibi | yes | 15:56 |
| gibi | yes | 15:56 |
| sean-k-mooney[m] | so we wont merge it | 15:57 |
| sean-k-mooney[m] | ok | 15:57 |
| sean-k-mooney[m] | that a liit more context then im prepared to digest to review today | 15:57 |
| sean-k-mooney[m] | but ill look back on this tomorrow | 15:57 |
| gibi | ack | 15:58 |
| gibi | artom and dansmith are also on it to keep me honest :) | 15:58 |
| dansmith | yeah will try to look a bit later | 15:58 |
| sean-k-mooney[m] | ok if dansmith and melwitt are able to review it i would prefer not to add another context switch to my current list | 15:59 |
| sean-k-mooney[m] | but if this is urgent i can | 16:00 |
| melwitt | sean-k-mooney[m]: sure, I can look | 16:01 |
| gibi | I don't think that it is super urgent | 16:01 |
| gibi | thank you all | 16:03 |
| frickler | dansmith: melwitt: could you also have a look at https://review.opendev.org/c/openstack/nova/+/851909 pls? would be good to be able to have wider testing of latest sdk, currently this is blocking getting it into u-c | 16:37 |
| dansmith | hmm, I guess that's needed because of when we talk to placement | 16:39 |
| dansmith | frickler: +W | 16:42 |
| frickler | thx | 16:50 |
| opendevreview | Jan Hartkopf proposed openstack/nova master: add support for updating server's user_data https://review.opendev.org/c/openstack/nova/+/816157 | 17:26 |
| opendevreview | Merged openstack/placement master: Func test for os-traits and os-resource-classes lib sync https://review.opendev.org/c/openstack/placement/+/851966 | 17:44 |
| opendevreview | Merged openstack/nova master: Fix mocking SafeConnectedTestCase https://review.opendev.org/c/openstack/nova/+/851909 | 17:44 |
| stephenfin | gibi: Is that PCI series ready for review? | 17:49 |
| spatel | does nova support sound adaptor ? | 18:59 |
| wolsen[m] | gibi: bauzas: sorry I've been slow to respond on that patch set (the mdev devices libvirt 7.x one - https://review.opendev.org/c/openstack/nova/+/838976) - I 'm reading the backscroll now and see you commented that you need something for me. I'm jugging various things at the moment but will review feedback shortly | 19:19 |
| wolsen[m] | looks like an update around the FUP patch that bauzas had posted | 19:21 |
| *** EugenMayer4 is now known as EugenMayer | 21:39 | |
| *** dasm is now known as dasm|off | 23:26 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!