| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-28 | |||
| 11:46:16 | cdent | and this is a test that’s also been languishing, adds a bit more coverage: https://review.openstack.org/#/c/485209/ | |
| 11:46:37 | cdent | gibi: you too on both of those | |
| 11:51:31 | openstackgerrit | John Garbutt proposed openstack/nova master: WIP: Send traits to ironic on server boot https://review.openstack.org/508116 | |
| 12:05:23 | sdague | ok, the qemu >= 2.10 patch should really really be ready this time - https://review.openstack.org/#/c/505673/ - stephenfin, bauzas, johnthetubaguy if anyone wants to take a quick look | |
| 12:10:04 | mdbooth | Any chance of some eyes on this: https://review.openstack.org/#/c/507202/ | |
| 12:10:20 | mdbooth | It's a data corruptor with no current mitigation other than "don't do that" | |
| 12:10:57 | mdbooth | The dependent patch is an annoying python 3-ism | |
| 12:11:46 | mdbooth | That's unfair, it's a bug in the libvirt python bindings, and a questionable default choice in the lxml python bindings | |
| 12:20:14 | cdent | mdbooth: that etree default stuff has bit me so many times | |
| 12:20:24 | cdent | (not in an openstack context, but elsewhere) | |
| 12:21:10 | cdent | sdague: no one will believe you until you use a 3rd “really” | |
| 12:21:29 | mdbooth | cdent: It bit me, so I did my usual trick of fixing the whole problem rather than just my bit. Ironically, that decision usually bites me. | |
| 12:39:07 | stephenfin | mdbooth: Comments left. One of us is missing something :) | |
| 12:39:24 | mdbooth | stephenfin: Thanks. Look in a bit. | |
| 12:48:39 | openstackgerrit | konstantin proposed openstack/nova master: switch from filesystem to disk for parallels containers https://review.openstack.org/506687 | |
| 12:48:40 | openstackgerrit | konstantin proposed openstack/nova master: don't add device address if there is no any units https://review.openstack.org/506686 | |
| 12:54:55 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/ocata: Updated from global requirements https://review.openstack.org/490256 | |
| 12:57:05 | cdent | thanks stephenfin for the robust review | |
| 13:04:14 | openstackgerrit | Artom Lifshitz proposed openstack/nova stable/ocata: Test InstanceNotFound handling in 'nova usage' https://review.openstack.org/482219 | |
| 13:09:32 | openstackgerrit | Artom Lifshitz proposed openstack/nova stable/pike: Test InstanceNotFound handling in 'nova usage' https://review.openstack.org/499208 | |
| 13:14:18 | alex_xu | johnthetubaguy: yea, we are work on that together | |
| 13:15:54 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Update the placement deployment instructions https://review.openstack.org/469048 | |
| 13:22:12 | johnthetubaguy | alex_xu: cool, I was just thinking that spec didn't use the flavor extra specs mapping dansmith was talking about at the PTG | |
| 13:22:59 | johnthetubaguy | alex_xu: I added comments on the spec anyways | |
| 13:25:11 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/ocata: Updated from global requirements https://review.openstack.org/490256 | |
| 13:25:18 | alex_xu | johnthetubaguy: yea, I just read that, thanks | |
| 13:29:00 | stephenfin | cdent: Some more comments left on that patch from stuff I missed first go round. After those are addressed, I'm +2 | |
| 13:29:15 | cdent | stephenfin: ✔ | |
| 13:35:21 | mdbooth | stephenfin: https://review.openstack.org/#/c/507488/2/nova/tests/unit/virt/libvirt/test_guest.py | |
| 13:35:21 | openstackgerrit | Alex Xu proposed openstack/nova-specs master: Add trait support in the allocation candidates API https://review.openstack.org/497713 | |
| 13:35:33 | mdbooth | stephenfin: They won't be converted to byte strings? | |
| 13:35:40 | mdbooth | I don't understand. | |
| 13:35:52 | mdbooth | We don't need to convert anything to bytestring. | |
| 13:36:01 | stephenfin | mdbooth: Isn't that what this is doing? https://review.openstack.org/#/c/507488/2/nova/virt/libvirt/guest.py | |
| 13:36:21 | stephenfin | if they're unicode values, convert to bytestrings (str in Python 2.7) | |
| 13:36:23 | openstackgerrit | Bhagyashri Shewale proposed openstack/nova master: Don't create instance backup image if rotation is 0 https://review.openstack.org/409644 | |
| 13:36:37 | mdbooth | Not bytestrings, regular strings | |
| 13:36:43 | mdbooth | But only for python2 | |
| 13:36:59 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/newton: Updated from global requirements https://review.openstack.org/373293 | |
| 13:37:11 | mdbooth | We're not interested in what happens to bytestrings, because we don't expect to receive them | |
| 13:37:56 | mdbooth | Just a sec, lemme dig out the libvirt python bindings again... | |
| 13:38:02 | stephenfin | mdbooth: I'm using the wrong terminology so | |
| 13:38:21 | alex_xu | johnthetubaguy: ^ try to address your comment | |
| 13:38:30 | stephenfin | Whatever the default string type in Python3 is | |
| 13:38:34 | stephenfin | *Python 2 | |
| 13:38:46 | stephenfin | i.e. six.binary_type | |
| 13:39:22 | mdbooth | stephenfin: The default string type in Python3 is unicode | |
| 13:39:32 | mdbooth | Oh, python2 | |
| 13:39:40 | mdbooth | It's not binary type | |
| 13:39:49 | mdbooth | b'foo' != 'foo' | |
| 13:40:12 | stephenfin | mdbooth: Yeah it is, in Python 3 | |
| 13:40:14 | stephenfin | *Python 2 | |
| 13:40:16 | stephenfin | dammit | |
| 13:40:19 | mdbooth | Hehe | |
| 13:40:20 | stephenfin | True | |
| 13:40:20 | stephenfin | >>> b'test' == 'test' | |
| 13:40:29 | mdbooth | Ah, so it is | |
| 13:40:37 | mdbooth | In that case, we're already testing that, no? | |
| 13:40:55 | mdbooth | Because we're testing a bare string | |
| 13:40:55 | stephenfin | In Python 2, 'str' == binary strings. In Python 3, 'str' == unicode strings | |
| 13:41:02 | alex_xu | johnthetubaguy: in the allocation_candidate API, we said the parameter named as 'requires', so I guess the extra spec should be "HW_CPU_X86_AVX=require", not the `required`? | |
| 13:41:18 | stephenfin | mdbooth: Yeah, but you're not testing what passing a unicode string into 'migrate' does in Python 3 | |
| 13:41:21 | stephenfin | *Python 2 | |
| 13:41:34 | johnthetubaguy | alex_xu: yeah, keeping those consistent would be a good idea | |
| 13:41:34 | stephenfin | (I'm working on my laptop keyboard and keep missing 2) | |
| 13:41:41 | mdbooth | stephenfin: Yes we do, because the other test already does that | |
| 13:41:48 | mdbooth | because in python 3 u'foo' == 'foo' | |
| 13:41:51 | mdbooth | And we test 'foo' | |
| 13:41:57 | openstackgerrit | Bhagyashri Shewale proposed openstack/nova master: Don't create instance backup image if rotation is 0 https://review.openstack.org/409644 | |
| 13:42:01 | alex_xu | johnthetubaguy: cool, got it | |
| 13:42:42 | mdbooth | stephenfin: So I actually wrote the test, and I changed 'v1' to be u'v1' | |
| 13:42:49 | mdbooth | And asserted that's what we ended up with | |
| 13:42:58 | mdbooth | Then I realised that they're the same thing | |
| 13:43:03 | mdbooth | So actually I hadn't changed anything | |
| 13:43:28 | stephenfin | mdbooth: But what about the other way round | |
| 13:43:42 | stephenfin | If you called 'self.guest.migrate' with a "u'test'" param value | |
| 13:43:58 | mdbooth | test_migrate_v3 already tests that | |
| 13:44:01 | stephenfin | I'd expect the call to 'self.domain.migrateToURI3' to be called with "b'test'" | |
| 13:44:05 | stephenfin | But only for Python 2 | |
| 13:44:18 | mdbooth | test_migrate_v3_unicode tests that | |
| 13:44:32 | mdbooth | Because it passes u'v1' and asserts 'v1' | |
| 13:44:36 | stephenfin | Oh FFS | |
| 13:44:46 | stephenfin | I read 'not six.PY2' as 'not six.PY3' | |
| 13:44:57 | stephenfin | and thought that test was only for Python 3. Sorry! | |
| 13:44:59 | mdbooth | HAHA, sorry! | |
| 13:45:14 | mdbooth | -ESTUPIDNEGATIONS | |
| 13:45:33 | stephenfin | :D | |
| 13:45:54 | stephenfin | mdbooth: OK, let me give that one more spin through so. Sorry for the confusion :) | |
| 13:46:02 | mdbooth | NP, thanks again | |
| 13:47:21 | mdbooth | FWIW I wrote it that way because I way being anal. | |
| 13:47:50 | mriedem | https://review.openstack.org/#/c/507938/ needs final +2, regression in pike | |
| 13:48:14 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/newton: Updated from global requirements https://review.openstack.org/373293 | |
| 13:48:51 | mdbooth | mriedem: https://review.openstack.org/#/c/507202/ libvirt data corruptor since forever | |
| 13:52:48 | mriedem | mdbooth: ok i'm going to put a patch on top of that to test live block migration with something like this https://review.openstack.org/#/c/481290/ | |
| 13:53:25 | mdbooth | mriedem: kk | |
| 13:54:00 | kashyap | mriedem: Oh cool. Funnily enough, I was just checking an hour ago if we do (non-shared) live block migration in the Gate | |
| 13:54:17 | mdbooth | Although the bug itself is really confined to the calling conventions of migrateToURI3 () | |
| 13:54:52 | mdbooth | We're going to have to backport it a long way, btw. | |
| 13:56:51 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: Run test_volume_backed_live_migration and iscsi test https://review.openstack.org/508163 | |
| 13:56:55 | mriedem | ^ | |