| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-05 | |||
| 21:08:42 | mriedem | i was looking at it on friday considering taking it over | |
| 21:09:04 | mriedem | anyway, we should split that piece out into a separate change, and then the bug fix stuff on top probably | |
| 21:09:07 | dansmith | I've been working on sql injection attacks at gerrit so I can reassign it to you | |
| 21:09:08 | dansmith | yeah | |
| 21:09:10 | dansmith | I shall | |
| 21:28:49 | edleafe | jaypipes: dansmith: cdent: Started a thread on the ML: http://lists.openstack.org/pipermail/openstack-dev/2017-June/117913.html | |
| 21:29:02 | jaypipes | edleafe: thx ed | |
| 21:35:24 | openstackgerrit | Steve Noyes proposed openstack/nova master: WIP: update live migration to use v3 cinder api https://review.openstack.org/463987 | |
| 21:39:35 | openstackgerrit | Steve Noyes proposed openstack/nova master: objects: Add old_attachment_id to BlockDeviceMapping https://review.openstack.org/471142 | |
| 21:40:24 | mriedem | stvnoyes: that's not going to work | |
| 21:40:32 | stvnoyes | no? | |
| 21:40:42 | mriedem | primarily because you haven't added a column to the block_device_mappings table | |
| 21:41:28 | mriedem | however, we should talk to johnthetubaguy about this since i'd really hope we could avoid something like this | |
| 21:41:43 | stvnoyes | I was following this pattern that did somethng about the same - https://review.openstack.org/#/c/437665/ | |
| 21:42:04 | mriedem | right but you missed https://review.openstack.org/#/c/437597/ | |
| 21:42:22 | stvnoyes | ah ok. one of john's comments on the initial migrate review was for a place to keep the old attach id | |
| 21:43:09 | mriedem | john garbutt? | |
| 21:43:11 | mriedem | or griffith? | |
| 21:43:13 | stvnoyes | ah. ok (i am impressed that you remember all these things) | |
| 21:43:16 | stvnoyes | garbutt | |
| 21:43:35 | mriedem | hmm, i'd like to avoid an old/new attachment id column in the bdms table/object | |
| 21:44:01 | mriedem | initial thought if we have 2 bdms, one for the source host attachment and one for the dest host attachment | |
| 21:44:06 | mriedem | but that might break other things | |
| 21:44:40 | stvnoyes | did you have another idea? I originally tried to do without i, by creating a new bdm. but had problems down in libvirt driver distinguishing between what was source and destination | |
| 21:44:50 | stvnoyes | without 'it' | |
| 21:44:53 | mriedem | right that's one of the issues | |
| 21:45:31 | mriedem | the other problem is we have a BlockDeviceMapping.get_by_volume_and_instance method | |
| 21:45:32 | stvnoyes | so after going thru it a number of times, this came out to be less new code | |
| 21:45:46 | mriedem | which expects to get 0 or 1 results | |
| 21:46:11 | jgriffith | any way to do a "new/old" bdm? | |
| 21:46:42 | mriedem | probably not in a way that's any less bad than a single bdm with new/old attachmnets | |
| 21:46:43 | mriedem | *attachments | |
| 21:46:57 | mriedem | and like i said, a bunch of the code assumes volume id + instance id is uniqueu | |
| 21:47:01 | mriedem | *unique even | |
| 21:48:34 | stvnoyes | the other stringe thing I found is that when you have a connector present in the bd, the attachment_id is in the bdm twice (bdm.attachment_id, and in the connection_info string) | |
| 21:48:44 | stvnoyes | that's not terrific | |
| 21:49:12 | mriedem | stvnoyes: long-term i think we want to stop setting connection_info | |
| 21:49:15 | mriedem | when we have attachment_id | |
| 21:49:20 | mriedem | because cinder will store the connection_info for us | |
| 21:49:41 | mriedem | that probably happens in the attach patch | |
| 21:49:45 | jgriffith | stvnoyes that's because the attachment_id comes form that | |
| 21:49:52 | stvnoyes | that seems like a good idea (later) | |
| 21:50:07 | mriedem | nova wants to not store the connection_info | |
| 21:50:12 | mriedem | b/c it can be stale | |
| 21:50:43 | mriedem | and, well, we just don't want to duplicate all of this state modeling in two services | |
| 21:51:23 | stvnoyes | so how would you like to resolve whether we want to go with old_attachment_id? talk with John Garbutt? I can update the rv in the meantime with that missing change. | |
| 21:53:05 | mriedem | stvnoyes: yeah i'd say do what you're doing now, fix the db api thing, and then we talk to john to see if he had other idea | |
| 21:53:06 | mriedem | *ideas | |
| 21:53:21 | stvnoyes | ok cool | |
| 21:53:22 | mriedem | i don't like it but it's probably the lesser of many evils | |
| 21:53:48 | mriedem | unless of course dansmith has bright ideas he'd like to share about BDMs modeling and multi-attach | |
| 21:53:49 | mriedem | ... | |
| 21:54:12 | dansmith | barf | |
| 21:54:32 | jgriffith | You guys running some kinda vomitorium in here? | |
| 21:54:51 | mriedem | vomiting on oneself is a common reaction to things in this channel | |
| 21:55:01 | jgriffith | small world | |
| 21:55:14 | jgriffith | seems were all more alike than we thought | |
| 21:57:21 | stvnoyes | I found that libvirt driver needs to know what bdm is what and getting that info down in there was incredibly ugly. This way libvirt driver hardly needs to change at all. | |
| 21:58:57 | mriedem | old_attachment_id vs attachment_id also gets weird, | |
| 21:59:05 | mriedem | because we have to assume "old" means source host | |
| 21:59:15 | mriedem | and ties it to migrations in the bdm object | |
| 21:59:26 | mriedem | but, we have old/new flavors too for the same thing (resizes) | |
| 21:59:58 | stvnoyes | using john Gr's attach code, I did get this working ok. I could migrate back and forth successfully | |
| 22:00:13 | mriedem | actually, flavors gives me an idea here | |
| 22:00:31 | mriedem | nova.objects.Migration has an old_instance_type_id and new_instance_type_id field | |
| 22:00:53 | mriedem | i'm wondering if we can store the old/new attachment id information in the migration object | |
| 22:01:29 | mriedem | however, an instance has 1 flavor, | |
| 22:01:36 | mriedem | but possibly many volume attachments | |
| 22:01:39 | mriedem | so it'd have to be list | |
| 22:01:44 | stvnoyes | hmmm, ok i can take a look at that. is that object persisted and available to both hosts? | |
| 22:01:50 | mriedem | yes | |
| 22:02:02 | mriedem | that would be a cleaner interface if we can make it work | |
| 22:02:21 | stvnoyes | agree, i'm not a big fan of putting new fields in the bdm. | |
| 22:02:40 | stvnoyes | also, a migrate object seems more appropriate. | |
| 22:03:17 | stvnoyes | i wasn't aware of that object. new things to learn. | |
| 22:04:35 | openstackgerrit | Felipe Monteiro proposed openstack/nova-specs master: Patrole (RBAC) Keystone Gating https://review.openstack.org/471145 | |
| 22:10:42 | openstackgerrit | Eric Fried proposed openstack/nova master: Trim the fat from InstanceInfo https://review.openstack.org/471146 | |
| 22:10:58 | mriedem | cfriesen_: your release notes are fixed | |
| 22:11:09 | efried | mriedem ^^ | |
| 22:11:30 | mriedem | efried: ok probably for tomorrow at this rate | |
| 22:11:32 | cfriesen_ | mriedem: sweet, thanks for the recheck too | |
| 22:11:43 | efried | mriedem Oh, for sure. | |
| 22:25:40 | openstackgerrit | Eric Fried proposed openstack/nova master: Trim the fat from InstanceInfo https://review.openstack.org/471146 | |
| 22:49:33 | openstackgerrit | Takashi NATSUME proposed openstack/nova-specs master: Uncap sphinx version https://review.openstack.org/471153 | |
| 23:26:45 | mriedem | cfriesen_: why does anything in https://review.openstack.org/#/c/435141/10/novaclient/tests/unit/test_shell.py need to change? | |
| 23:26:52 | mriedem | i.e. what's up with all of the list -> flavor-list changes? | |
| 23:28:33 | mriedem | i see in the commit message now, | |
| 23:28:46 | mriedem | but still, seems we can set a default api_version in whatever fixture is used for those tests so you don't need to change them all | |
| 23:33:28 | openstackgerrit | Merged openstack/nova master: Completed implementation of instance diagnostics for Xen https://review.openstack.org/399613 | |
| 23:55:12 | mriedem | artom: some questions inline before i approve https://review.openstack.org/#/c/465205/ | |
| 23:55:32 | artom | mriedem, man of your word. Much thanks, let me take a look | |
| 23:57:54 | mriedem | i have to do something during girls gymnastics class (besides watch and support of course) | |
| 23:58:24 | artom | Well crap, now I feel guilty for taking away father-daughter time | |
| #openstack-nova - 2017-06-06 | |||
| 00:02:18 | mriedem | no i'm up in a viewing area and she's down refusing to flip | |
| 00:02:26 | mriedem | i'm with the screaming siblings | |
| 00:03:50 | mriedem | preparing for the soul-crushing conversation on the car ride home about, "why didn't you do the thing we talked about you doing" | |
| 00:04:10 | artom | Who's asking that question of whom? | |
| 00:05:08 | mriedem | ha | |
| 01:03:25 | mriedem | alex_xu: you should probably take a look at https://review.openstack.org/#/c/469578/ | |
| 01:23:52 | mriedem | jaypipes: i just reviewed the os-traits sync thing https://review.openstack.org/#/c/469578/ and there is a comment in there saying it's synced on start of the process, but i don't see where/how that happens | |