Earlier  
Posted Nick Remark
#openstack-nova - 2017-06-05
21:05:57 mriedem anyway, i'm not proposing we tackle testing mixed-level cells in ci right now
21:06:37 dansmith mriedem: I will keep looking for my multicell service lookup patch in a couple dead end branches, but if I don't find it I'll just write it.. it's not hard
21:06:56 mriedem sure - you want to take that bug?
21:07:02 mriedem or just going to provide the helper?
21:07:31 dansmith mriedem: lemme write the helper and I'll survey the other uses... might be easy for me to just fix them uses too
21:07:50 mriedem dansmith: here it is https://review.openstack.org/#/c/442787/1/nova/objects/service.py
21:08:05 dansmith haha
21:08:15 dansmith no wonder I couldn't find it. that patch is dead to me
21:08:19 mriedem i know
21:08:24 dansmith lol
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/

Earlier   Later