Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-06-11
15:41:57 mriedem return ran and 1 or 0
15:41:59 efried mriedem: Right, that's the sentence that's prompting my question. "We've done all the ones we can do." So... what about the rest of them?
15:42:02 mriedem ran is based on done
15:42:12 dansmith mriedem: not really.. if done is zero it just means you're not able to do more work, but it doesn't mean everything is migrated
15:42:41 dansmith efried: it depends
15:43:00 dansmith efried: in my instance example, it means "try later once those instances are no longer transitioning state"
15:43:02 mriedem dansmith: then that's not clear from the command help https://docs.openstack.org/nova/latest/cli/nova-manage.html#nova-database
15:43:08 mriedem "Perform data migration to update all live data. Return exit code 0 if migrations were successful or exit code 1 for partial updates. This command should be called after upgrading database schema and nova services on all controller nodes. If the command exits with partial updates (exit code 1) the command will need to be called again."
15:43:35 dansmith mriedem: it's a minor loop vs. major loop type of thing
15:43:45 mriedem so,
15:43:47 efried ah, that indicates that (!0, 0) means "keep doing this until (0, 0)"
15:43:50 dansmith mriedem: nonzero means ansible should call again immediately.. zero means there's no more you can do right now
15:43:59 dansmith I can update those words too
15:44:02 mriedem it sounds like what you'd really need, if you were automating this, is rc=2 == total != 0, but done == 0
15:44:30 efried no, it sounds like rc=1 means that already.
15:44:41 mriedem but rc=1 is only based on "done != 0"
15:44:54 efried oh, okay. So yeah.
15:44:58 efried what you said.
15:45:20 dansmith mriedem: we could, but anyone currently just checking for nonzero would get stuck, because it'd be changing what zero really means
15:45:23 mriedem otherwise if there is no other return code for the total != 0, done == 0 case, i can only know that by manually running things
15:45:41 dansmith early on in this, we didn't have migrations that couldn't complete fully, until we did, so..
15:45:45 mriedem dansmith: sure, osa uses [0, 4] for things like that
15:46:00 mriedem i realize it would be backward incompatible, and upgrade release note, etc
15:46:14 dansmith mriedem: I don't know what that means
15:46:26 mriedem which part?
15:46:33 dansmith oh you mean [0,4] for either return value, sure,
15:46:36 dansmith but it has to change
15:46:40 mriedem right
15:46:56 mriedem if you're just saying != 0 in your tooling, that's kind of on you
15:47:06 mriedem anyway, i don't mean to open a can of worms
15:47:13 dansmith well, you did
15:47:16 dansmith worms. everywhere.
15:47:50 dansmith although efried is like the child that just starts throwing cans on the floor of the supermarket aisle
15:47:59 dansmith denting cans left and right until one opens
15:48:10 efried lol
15:48:20 efried like a child in my ignorance, at least.
15:48:51 efried My vote is that we merge this as is and (maybe) figure out what to do about that other thing later.
15:49:14 mriedem sure, throw a TODO in there about the rc=2 idea i guess
15:50:29 openstackgerrit Dan Smith proposed openstack/nova master: Document the internal online_migrations function behaviors https://review.openstack.org/574268
15:50:34 mriedem efried: can you double check https://review.openstack.org/#/c/486475/36/doc/source/admin/configuration/hypervisor-xen-api.rst and if i'm right, i'll update the patch for my comments and respin so we can flush it
15:50:53 efried ...
15:51:04 mriedem flush from the runways spot i mean
15:53:46 efried mriedem: You're right. History there was that they were going to switch the default, but I said nay. They wound it back, but missed a couple spots. Good eye.
15:54:01 mriedem ok, i figured that was the case
15:54:05 mriedem want to default to the new shiny
15:55:11 efried mriedem: Was I right to disallow that?
15:56:00 mriedem shrug
15:56:05 mriedem we do change defaults from time to time
15:56:21 mriedem if you were explicitly using the old thing, it's no upgrade issue, otherwise it is if you don't want that new default
15:56:30 mriedem but new installs get the new thing out of the box
15:56:37 efried mm
15:56:45 openstackgerrit Zack Cornelius proposed openstack/nova master: Refactor libvirt get_memory_used_mb() https://review.openstack.org/571030
15:56:46 openstackgerrit Zack Cornelius proposed openstack/nova master: Implement file backed memory for instances in libvirt https://review.openstack.org/567876
15:56:49 mriedem if the xenapi team wants the vdi_remote_stream stuff to be the default at some point, their docs should probably say something to that effect, like "this is the recommended image handler"
15:57:01 mriedem maybe direct is just the simplest / easiest to setup?
15:57:30 efried Well, reading back (PS29..34) it looks like it was more of an internal decision to revert the default. But I registered a not-having-liked-it comment too.
15:57:51 efried in any case, we're at not changing the default at this point, so your comment is apt.
16:00:35 openstackgerrit Jan Gutter proposed openstack/os-vif master: [WIP] Add support for generic offload representors https://review.openstack.org/572081
16:00:59 openstackgerrit Matt Riedemann proposed openstack/nova master: XenAPI: define a new image handler to use vdi streaming https://review.openstack.org/486475
16:01:00 openstackgerrit Matt Riedemann proposed openstack/nova master: XenAPI: update the document related to vdi streaming https://review.openstack.org/568444
16:01:01 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: default [xenserver]/image_handler to vdi_remote_stream https://review.openstack.org/574318
16:01:23 mriedem efried: ok i'll wait to +2 until that top test patch passes the 3rd party ci
16:01:46 efried mriedem: ack, poke me then and I'll +A
16:04:52 mriedem dansmith: on the consumer allocs patch, i think you were asking for this test https://review.openstack.org/#/c/567678/11/nova/tests/functional/api/openstack/placement/gabbits/ensure-consumer.yaml
16:05:03 mriedem creates an allocation at 1.7 so the consumer is auto-generated
16:05:06 mriedem using the conf
16:05:16 dansmith yup
16:05:57 mriedem ok +W
16:14:47 openstackgerrit Curt Moore proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.openstack.org/574301
16:15:33 openstackgerrit Curt Moore proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.openstack.org/574301
16:17:32 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix nits from change Id609789ef6b4a4c745550cde80dd49cabe03869a https://review.openstack.org/574324
16:20:19 openstackgerrit Chris Dent proposed openstack/nova stable/pike: Ensure resource class cache when listing usages https://review.openstack.org/574327
16:20:37 mriedem dansmith: don't think i didn't see you give me that todo
16:57:34 dansmith mriedem: I was hoping you would notice :)
17:22:33 mdbooth mriedem: Saw your note about access_mode, but note that we're testing attach_mode. Started looking at cinder to see how robustly they're related but didn't finish.
17:26:46 mriedem there are likely glaring inconsistencies; access_mode is what comes back in the connection_info dict,
17:27:00 mriedem attach_mode is something on the attachment record itself, but you can't set it when creating or updating an attachment
17:27:05 mriedem https://developer.openstack.org/api-ref/block-storage/v3/#create-attachment
17:27:08 mriedem not directly anyway
17:27:25 mriedem https://developer.openstack.org/api-ref/block-storage/v3/#update-an-attachment
17:27:43 mriedem today read-only access for attachments is controlled via the readonly status on the volume,
17:27:47 mriedem which is via it's volume admin metadata
17:27:52 mriedem which is what i did in the dependent tempest change
17:28:45 mriedem yeah you can't create or update an attachment record and specify the attach_mode directly https://github.com/openstack/cinder/blob/master/cinder/api/schemas/attachments.py
17:29:04 mriedem however, you can set it's attach_mode via the 'mode' key in the 'connector' dict when creating/updating the attachment
17:29:07 mriedem it's all not very REST-y
17:29:23 mriedem https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L4376
17:29:43 openstackgerrit Artom Lifshitz proposed openstack/nova master: Add InstanceNUMATopology to LibvirtLiveMigrateData https://review.openstack.org/566398
17:29:44 openstackgerrit Artom Lifshitz proposed openstack/nova master: WIP: LM: re-fit topology and claim resources on dest https://review.openstack.org/567242
17:29:45 openstackgerrit Artom Lifshitz proposed openstack/nova master: Service version check for NUMA live migration https://review.openstack.org/566723
17:30:10 mriedem mdbooth: i planned on cleaning up the _translate_attachment_ref stuff in a follow up
17:30:15 mriedem since it's not necessary for the backport
17:31:34 mdbooth mriedem: I was thinking of the issue of whether ro attachments are really ro.
17:31:45 mdbooth Which they might be, but it's not at all obvious.
17:32:10 mriedem if they aren't, that's a bug in cinder yes?
17:32:10 mdbooth Bizarrely, that code is adding attach_mode to connection_info, but I couldn't find any use of it.
17:32:23 mriedem right, attach_mode doesn't need to be in the connection_info
17:32:27 mdbooth But as you pointed out, we *are* using access_mode in nova/os-brick.
17:32:29 mriedem since access_mode is there and that's what the volume driver uses
17:32:33 mriedem yes

Earlier   Later