| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 13:58:42 | openstackgerrit | sahid proposed openstack/nova master: libvirt: configure trust mode for vfs https://review.openstack.org/458514 | |
| 13:58:53 | mdbooth | sfinucan johnthetubaguy mriedem: I'd really like to push this one over the line: https://review.openstack.org/#/c/383859/ . Appreciate it's a pretty complex, though. I was thinking about splitting out the code which resets on libvirt reconnection into a separate patch. I still think this code adds robustness, but splitting it out might make it simpler to review the most important part of the patch. Worth it? | |
| 13:59:26 | mriedem | i wouldn't split it | |
| 13:59:31 | sfinucan | I wouldn't | |
| 13:59:41 | mdbooth | Glad I asked :) | |
| 14:00:09 | sfinucan | Yeah, I'm generally happy with it as is. The tests are by far the most complex piece but copious comments do help | |
| 14:00:34 | johnthetubaguy | yeah, I think I am quite close to +2 on that now, its dropped off my radar for a bit | |
| 14:00:43 | mdbooth | Yeah, the tests are a nightmare | |
| 14:00:43 | johnthetubaguy | will try taking another peak today | |
| 14:01:04 | mdbooth | Ok, thanks. I'll update with sfinucan's comments, but otherwise leave it alone. | |
| 14:02:25 | sfinucan | (y) | |
| 14:21:30 | traskat | hey, what i have to do to apply extra specs to running VMs for examaple disk IO limitations without restart the VM? | |
| 14:25:13 | efried | traskat Resize? | |
| 14:25:31 | efried | (I don't know the answer, just an idea.) | |
| 14:29:05 | traskat | efried, you mean to another flavor ? | |
| 14:29:32 | efried | traskat I would think so. Where else would you get the new extra_specs? | |
| 14:30:09 | efried | traskat Prolly depends on the driver whether the resize can happen without a reboot... | |
| 14:30:42 | efried | But then, the extra_specs will be driver-specific anyway, não é? | |
| 14:39:26 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add ability to signal and perform online volume size change https://review.openstack.org/454322 | |
| 14:40:40 | imacdonn | mdbooth, around ? | |
| 14:40:50 | mdbooth | imacdonn: Yep | |
| 14:41:01 | mriedem | takashin: alex_xu: ok did you want to talk about the server migrations API? | |
| 14:41:06 | imacdonn | hi! Could you take a peek at https://bugs.launchpad.net/nova/+bug/1683972 ? | |
| 14:41:06 | openstack | Launchpad bug 1662483 in OpenStack Compute (nova) "duplicate for #1683972 detach_volume races with delete" [Undecided,In progress] - Assigned to Matthew Booth (mbooth-9) | |
| 14:41:06 | mriedem | or wait for me to write something up? | |
| 14:41:08 | alex_xu | mriedem: yea | |
| 14:41:23 | takashin | mriedem: yes. | |
| 14:41:25 | alex_xu | I just didn't remember why we only show in-progress initially | |
| 14:41:46 | mriedem | alex_xu: because the api was written solely to cancel in-progress live migratoins | |
| 14:41:48 | mriedem | *migrations | |
| 14:41:53 | mdbooth | imacdonn: Looking | |
| 14:41:58 | imacdonn | mdbooth, thanks | |
| 14:42:04 | takashin | alex_xu: There is os-migrations API. | |
| 14:42:26 | takashin | alex_xu: We can use it to list all migrations. | |
| 14:42:29 | mriedem | alex_xu: so rather than just having to validate when a request is made to cancel a live migration, we filtered out the migrations you could pick from to do the cancel | |
| 14:43:25 | takashin | mriedem: Yes. To cancel a live migration. | |
| 14:44:06 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/migrations.py#L62 | |
| 14:44:21 | mriedem | so before 2.23, we can list all migrations, but we don't expose the type | |
| 14:44:26 | mdbooth | imacdonn: Fun | |
| 14:44:28 | mriedem | because add_link defaults to False | |
| 14:44:35 | alex_xu | mriedem: that works for me. I guess initialy we think there is only one migration, and we didn't want to trouble with user to find out which migration is in-progress. | |
| 14:44:37 | imacdonn | mdbooth :) indeed | |
| 14:45:17 | alex_xu | with filter, we can reach the same goal | |
| 14:45:21 | mriedem | at microversion >= 2.23 with os-migrations, we provide a link and migration_type in the response for an in-progress live migration, otherwise just the migration_type | |
| 14:45:35 | mriedem | alex_xu: note that os-migrations also takes a filter parameter | |
| 14:45:40 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/migrations.py#L77 | |
| 14:45:47 | alex_xu | so i'm ok with that. and the user have one more way to query all the migraitons for a specific servers, so nothing bad at here | |
| 14:45:50 | mriedem | it's not validated, but it's there | |
| 14:46:21 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Canary test against multi-cell devstack https://review.openstack.org/458537 | |
| 14:46:42 | mdbooth | imacdonn: Let me think about that for a bit. lyarwood may also be interested in https://bugs.launchpad.net/nova/+bug/1683972 | |
| 14:46:42 | openstack | Launchpad bug 1662483 in OpenStack Compute (nova) "duplicate for #1683972 detach_volume races with delete" [Undecided,In progress] - Assigned to Matthew Booth (mbooth-9) | |
| 14:47:22 | alex_xu | takashin: what would you think? | |
| 14:47:43 | mriedem | alex_xu: this is how os-migrations gets the migrations from the db + filters https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4789 | |
| 14:47:47 | imacdonn | mdbooth, OK, will check that one out too. I think my problem is a bit more extensive than 1683972, but maybe we can find a solution that covers both | |
| 14:48:00 | mdbooth | imacdonn: It's the same bug, just repeating for lyarwood | |
| 14:48:16 | mdbooth | I just unlinked the bug it was marked as a dup of, btw | |
| 14:48:17 | mriedem | alex_xu: so technically you could get all migration types via os-migrations v2.1 and filter on instance_uuid | |
| 14:48:28 | imacdonn | mdbooth, oh, duh .. still working on my first coffee :) | |
| 14:48:32 | traskat | efried, are there other options then changing the flavor. i tried now a reboot but that didn't work either. the flavor i use for the rebooted instance has the extra specs before. | |
| 14:48:34 | takashin | alex_xu: So I would like to make new APIs to show in-progress migrations only. | |
| 14:48:42 | mdbooth | I don't think they're the same, although they're cousins | |
| 14:48:48 | imacdonn | mdbooth, agreed | |
| 14:49:51 | zioproto | dansmith: I upgraded to 14.0.5 but there are still some strange different messages https://pastebin.com/vLNWdiRZ | |
| 14:49:56 | mriedem | takashin: why would you rather filter all migrations and return only in-progress migrations without any option for the user to choose the filtering? | |
| 14:50:05 | zioproto | looks like a problem with the keypairs | |
| 14:50:08 | efried | traskat Sorry, I'm not familiar enough with this to be of help. The resize was just an off-the-cuff idea. But you probably want to take this discussion to the #openstack channel (see channel topic). | |
| 14:50:38 | traskat | ok thanks | |
| 14:50:51 | mriedem | takashin: that seems inconsistent behavior with the rest of our apis, which if you list resources, you get undeleted resources in their current state, | |
| 14:51:02 | mriedem | you can take actions on those resources, and those actions may be invalid for the state that the resource is in, | |
| 14:51:09 | mriedem | in which case, the api will tell you that with a 400 response | |
| 14:51:10 | dansmith | zioproto: hmm | |
| 14:51:27 | takashin | mriedem: Because we have os-migrations API and there should be a difference between new APIs and old one. | |
| 14:51:43 | lyarwood | mdbooth: thanks, I missed that you had a change up to add locking to detach_volume in the compute layer somehow | |
| 14:51:48 | zioproto | dansmith: Some instances are still missing keypair information. Unable to run keypair migration at this time. | |
| 14:52:19 | dansmith | zioproto: yeah, but the problem is the "error attempting to run ..." one | |
| 14:52:21 | dansmith | zioproto: just a sec | |
| 14:52:24 | mdbooth | lyarwood: Not related to this issue. I should probably abandon that patch as it's more complex than I was going for. | |
| 14:52:30 | mriedem | takashin: so if i understand your point, if i want to list all migrations, regardless of type or status, i should use os-migrations | |
| 14:52:38 | alex_xu | takashin: /servers/{id}/migrations is more easy for user to query migration for specific server | |
| 14:52:53 | mriedem | takashin: but if i want to list migrations for a specific server in order to cancel the migration, i should use /servers/server_id/migrations | |
| 14:52:55 | zioproto | dansmith: I wait no problem :) | |
| 14:53:32 | takashin | mriedem: alex_xu: Yes. In specific state. | |
| 14:53:50 | mriedem | takashin: the thing that worries me about saying just use os-migrations, is the docs have a warning on it https://developer.openstack.org/api-ref/compute/#migrations-os-migrations-frozen | |
| 14:53:54 | alex_xu | takashin: From the ML, operators have use-case to query specific server's migrations. so | |
| 14:53:56 | mriedem | basically saying, don't use this, use the other API | |
| 14:54:02 | lyarwood | mdbooth: is it not? wouldn't locking against instance.uuid for both attach and detach help here? | |
| 14:54:12 | dansmith | zioproto: well, I was looking to see if we had any updates to that migration but I don't think we have | |
| 14:54:19 | mriedem | alex_xu: argubally they could, i.e. GET /os-migrations?instance_uuid=1234 | |
| 14:54:23 | dansmith | zioproto: you can make a code tweak to dump more info since this is a test right? | |
| 14:54:35 | mdbooth | lyarwood: Don't think so. This is racing with cinder internals. | |
| 14:54:43 | imacdonn | lyarwood, mdbooth, my problem is that the iSCSI connection termination is separate from detach | |
| 14:55:28 | mdbooth | imacdonn: If it rescans again before the lun has been reallocated, does the device go away? | |
| 14:55:40 | dansmith | zioproto: can you do this and see what it shows us? https://pastebin.com/3RJdsJE1 | |
| 14:55:44 | lyarwood | imacdonn: it shouldn't be anymore, we've moved everything into the bdm.detach() call | |
| 14:55:57 | alex_xu | mriedem: but more straight forward with /servers/{id}/migrations? but actually we should think about more how the API user write the client code | |
| 14:56:01 | lyarwood | imacdonn: so locking that against instance.uuid should help avoid this now unless I'm missing something | |
| 14:56:03 | mdbooth | lyarwood: terminate connection isn't atomic, right? | |
| 14:56:27 | mdbooth | lyarwood: So on completion of the call to terminate connection, the connection hasn't necessarily been terminated? | |
| 14:56:46 | mdbooth | atomic is the wrong word. Synchronous. | |
| 14:56:48 | lyarwood | mdbooth: yeah good point | |