| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-24 | |||
| 18:15:38 | sean-k-mooney | cdent: thanks im out of office tomrow but ill print out the current version and review tomrow. | |
| 18:15:41 | cdent | thanks efried | |
| 18:15:55 | sean-k-mooney | efried: im glad ye found a path forward | |
| 18:15:56 | cdent | woah, colors finally merged | |
| 18:16:30 | efried | cdent: No, I think that's just because it's in our CI env and I have that patched in there (cause that's where I was testing it) | |
| 18:16:30 | sean-k-mooney | cdent: colors? | |
| 18:17:07 | efried | cdent: But you should definitely tell everybody how much you like it in https://review.openstack.org/524744 so they might merge it. | |
| 18:17:33 | cdent | oh, sorry, didn't recognize that was your ci efried | |
| 18:17:49 | melwitt | cdent: I had wanted to bring this bug to your attention, in case you might have some idea why the test_wsgi stuff would fail with webob 1.8.1 https://bugs.launchpad.net/nova/+bug/1765748 | |
| 18:17:49 | openstack | Launchpad bug 1765748 in OpenStack Global Requirements "webob-1.8.1 breaks projects" [High,In progress] - Assigned to Matthew Thode (prometheanfire) | |
| 18:18:23 | efried | cdent: Anyway, the deal here is that powervm has a utility method that accepts either an integer ID or a UUID. It detects which one it is and then behaves accordingly. The code that does that hasn't changed in yeeears. | |
| 18:18:47 | efried | The code path that calls it is when we're disconnecting a disk, which (I think) also hasn't changed in yeeeears. | |
| 18:19:20 | sean-k-mooney | melwitt: the stack trace look like one that we get when we need to cap a dependcy in upper constrataint because of an incompatible change | |
| 18:19:35 | cdent | melwitt: hmmm. webob often breaks such stuff, but that looks like a bad mock? is there an in-nova example? | |
| 18:19:37 | efried | cdent: But suddenly on Friday afternoon, our CI started failing with that error. I don't actually know what type it thinks it has in there - just know that isinstance(str) is failing where it didn't before. | |
| 18:19:55 | mriedem | sean-k-mooney: totally unrelated to what you're talking about, but i'm a bit lost in https://review.openstack.org/#/c/551370/16/nova/virt/libvirt/migration.py@229 and looking for some feedback when you get a chance | |
| 18:20:03 | cdent | efried: still parsing | |
| 18:21:03 | sean-k-mooney | mriedem: in the generation of the updated xml for the vif on the destination node | |
| 18:21:45 | mriedem | sean-k-mooney: yeah, i'm looking at how _update_volume_xml works and it's basically an update of any volume disks on the source using the destination block device config | |
| 18:21:55 | mriedem | it does that by keying off the volume 'serial' which is the volume id | |
| 18:22:09 | mriedem | we don't have something like that for the interface config in a domain xml | |
| 18:22:25 | sean-k-mooney | mriedem: we could posibly key of the interface mac adress | |
| 18:22:28 | mriedem | i'm not really even sure if i should be updating the interface xml entries, or replacing them | |
| 18:23:01 | mriedem | i.e. i'm trying to handle the case that you're live migrating from a host using ovs to a host using ovn, or linuxbridge, or vhostuser, et al | |
| 18:23:06 | sean-k-mooney | that said im not sure if we can have a vm with two port with the same mac.. | |
| 18:23:52 | mriedem | i've been trying to write a test for this for a couple of hours and it just seems like doing a vif type conversion like this down in the bowels of the xml code is going to be....not reasy | |
| 18:23:53 | mriedem | *easy | |
| 18:23:55 | sean-k-mooney | mriedem: right in that case we are replaceing them but we likely want to keep the guest pci adress the same on both ends | |
| 18:24:33 | mriedem | yeah the only thing that _update_volume_xml doesn't change is the address | |
| 18:25:09 | cdent | efried: so "ValueError: invalid literal for int() with base 10: '4E27E1E6-6A24-4F0A-8E7B-2BBE7B4A28BA'" is caused by a conditional faililng somewhere that wasn't before? I'm unable to grep isinstance anywhere in the log? | |
| 18:25:59 | efried | cdent: It's caused by this: https://github.com/powervm/pypowervm/blob/master/pypowervm/utils/uuid.py#L50 | |
| 18:26:21 | efried | That isinstance() fails, so we try to int() the UUID on L55, leading to the ValueError. | |
| 18:26:48 | mriedem | efried: have you looked at https://github.com/openstack/oslo.versionedobjects/compare/1.32.0...1.33.1 ? | |
| 18:27:04 | mriedem | https://github.com/openstack/oslo.versionedobjects/commit/b1d0b5d886afef8c08330bf3c2291e180aa1f534 | |
| 18:27:09 | efried | cdent: or, shit, I guess it's possible the regex match could be failing. But when I str(instance.uuid) up the stack, it succeeds. | |
| 18:27:11 | efried | mriedem: looking... | |
| 18:27:14 | sean-k-mooney | mriedem: so if we can assume that the vm cannont have 2 intefaces with the same mac then i think that is likely the best way to approch. if we had a way to store the neutron port uuid in the xml that would help alot but i dont think we can do that with libvirt | |
| 18:27:50 | efried | mriedem: Bingo. When did we subsume that req? | |
| 18:27:53 | efried | in nova | |
| 18:27:57 | efried | in queens | |
| 18:28:00 | efried | cause I looked for that | |
| 18:28:06 | mriedem | efried: upper-constraints on friday for rocky | |
| 18:28:14 | mriedem | https://github.com/openstack/requirements/commit/87540884100650cfd1a67f05163a724906efb46f#diff-0bdd949ed8a7fdd4f95240bd951779c8 | |
| 18:28:15 | efried | ahhhh, upper-constraints. | |
| 18:28:28 | efried | Yup, that'd do it. Thanks mriedem | |
| 18:28:37 | efried | I knew you would come through for me. | |
| 18:30:03 | efried | mriedem: That same thing must have gone into queens somehow. | |
| 18:30:32 | mriedem | or your CI is using the wrong upper-constraints? | |
| 18:30:41 | mriedem | your queens CI is likely pointing at master | |
| 18:31:25 | mriedem | although maybe not https://github.com/openstack/oslo.versionedobjects/commit/e918eb976fb5a6f9fa7b7644d5a10d383fcfcf21 | |
| 18:31:38 | mriedem | https://review.openstack.org/#/q/Ic6b6308fb1960ec40407e6efde30137b64543e72 | |
| 18:32:04 | mriedem | that's not released yet on stable though | |
| 18:33:46 | cdent | efried: even if that change isn't your problem, it may be the clue. are you in py2? | |
| 18:33:57 | mriedem | sean-k-mooney: so do you think this is safe https://review.openstack.org/#/c/551370/16/nova/virt/libvirt/migration.py@229 where it's basically taking the source vif and just overwriting whatever we got from the dest to get the new config xml? | |
| 18:34:02 | efried | cdent: Probably | |
| 18:34:21 | cdent | so if you've got a unicode there (for whatever random reason) | |
| 18:35:10 | efried | cdent: Our "official" fix is going to be using is_uuid_like. But that's going to require changes to pypowervm, which is going to need a requirements bump, which I'm not sure if we can swing in stable. Because we're going to have the same problem in nova (right esberglu?) | |
| 18:36:37 | cdent | I'm totally riffing at this point (because multitasking), but does six.text_type make a difference? | |
| 18:36:45 | sean-k-mooney | mriedem: my concern is how to you merge the two so that if i have a vm with 2 interfaces of the same type they dont swap places e.g. eth0 becomes eth1 and vice versa. | |
| 18:36:56 | efried | and pike | |
| 18:37:25 | esberglu | efried: That wasn't hitting queens | |
| 18:37:29 | esberglu | Only master | |
| 18:37:33 | efried | esberglu: See email - it is now. | |
| 18:37:47 | efried | and based on https://review.openstack.org/#/q/Ic6b6308fb1960ec40407e6efde30137b64543e72 it'll hit pike as soon as that percolates down. | |
| 18:37:52 | cdent | melwitt: okay, I've repeated fhe failures so can look more sensibly now | |
| 18:38:03 | sean-k-mooney | mriedem: i think you will have to loop over each interface element in the original xml and find the corresponding vif based on mac and then update the xml with the new atributes but maintain the order of the interfaces and maintianing the guest pci adress | |
| 18:39:34 | mriedem | sean-k-mooney: but wouldn't it we be weird to have interface xmls from the guest with certain source bridge and target dev values which are then unchanged on the dest host, but things like the vif type and vnic type could change? | |
| 18:39:42 | melwitt | thanks cdent | |
| 18:40:14 | melwitt | mriedem: pike https://review.openstack.org/#/c/562879 and ocata https://review.openstack.org/#/c/564044 release reviews for your perusal | |
| 18:40:15 | sean-k-mooney | mriedem: the bridge name might change on the dest as might hte vhost-user socekt path. | |
| 18:40:47 | sean-k-mooney | mriedem: centos use /run/openvswitch/... ubuntu uses /var/run/openvswitch/... | |
| 18:40:59 | mriedem | sean-k-mooney: but we don't have the bridge name in the migrate data object from the dest when we're munging the guest xml on the source | |
| 18:41:46 | sean-k-mooney | mriedem: you should have that as we have already created the binding on the dest host but not activated it | |
| 18:41:54 | sean-k-mooney | mriedem: it will be in the respocne from neutron | |
| 18:42:13 | mriedem | is that in the details or profile dict? | |
| 18:42:27 | sean-k-mooney | its in the vif binding_details | |
| 18:42:36 | sean-k-mooney | i belive the key is just bridge_name | |
| 18:43:05 | sean-k-mooney | mriedem: yep https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/portbindings.py#L53 | |
| 18:43:28 | cdent | melwitt: looks like a google summoer of code project totally revamped accept* handling in webob 1.8.*. still digging up details | |
| 18:43:41 | mriedem | is the vhostuser socket patch also in the vif binding details dict? | |
| 18:43:45 | mriedem | *path | |
| 18:43:53 | sean-k-mooney | yes | |
| 18:43:56 | mriedem | ok | |
| 18:44:11 | sean-k-mooney | get_vif_config(vif=vif) is the same fuction we use when spawning a vm right? | |
| 18:44:21 | mriedem | yes | |
| 18:44:26 | sean-k-mooney | if so it should be able to extract that info from the vif object | |
| 18:44:31 | sean-k-mooney | ok cool | |
| 18:46:35 | mriedem | sean-k-mooney: when you say "maintain the guest pci address" i don't see anything in the interface elements in the xml, e.g. http://logs.openstack.org/95/563995/1/check/neutron-tempest-linuxbridge/a3015a9/logs/screen-n-cpu.txt#_Apr_24_16_42_59_327342 | |
| 18:46:51 | mriedem | oh, is that only for sriov ports? | |
| 18:47:10 | mriedem | like vnic_type='direct'? | |
| 18:47:47 | sean-k-mooney | no the libvirt xml for the running vm is not the same as the one we hand to libvirt. it adds pci adress for each device itself | |
| 18:48:33 | sean-k-mooney | at least i think it does let me dump the xml from a running vm | |
| 18:51:07 | sean-k-mooney | mriedem: so this is a running vm xml http://paste.openstack.org/show/719855/ | |
| 18:51:29 | sean-k-mooney | the interfaces have an addtional <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> element added which is the guest virtual pci adresss | |
| 18:52:04 | sean-k-mooney | when we generate the xml fragment using get_vif_config we do not set that | |
| 18:52:26 | mriedem | right | |
| 18:52:29 | sean-k-mooney | so if you just merged the generated fagment with the exising interface and done touch the adress element it will be fine | |
| 18:52:33 | mriedem | we can parse it out of the running domain | |
| 18:52:42 | sean-k-mooney | yep | |
| 18:53:11 | mriedem | so here is another wrinkle wrt merging, | |