| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-03 | |||
| 16:00:22 | mriedem | specifying the device on create and attach | |
| 16:00:26 | bauzas | jaypipes: humpf, reading thru your comments on the claims spec, do you accept now given our IRC meeting that we could POST allocations not at the same time with GET /rps, ie. after filters running ? | |
| 16:00:30 | mriedem | https://review.openstack.org/#/c/452546/ | |
| 16:00:43 | jaypipes | bauzas: yes | |
| 16:00:47 | mriedem | artom: turns out there is an ec2 use case that we've been supporting for years, | |
| 16:00:50 | mriedem | that no one really knew existed | |
| 16:00:55 | bauzas | jaypipes: I know it's not what you want, but I think it's a trade-off so that sooner or later, we could merge both calls | |
| 16:00:56 | mriedem | and now i can't break it | |
| 16:01:10 | bauzas | jaypipes: okay, thanks for the clarification | |
| 16:01:11 | jaypipes | bauzas: yes. | |
| 16:01:16 | artom | mriedem, and actually I think it would break device tagging as well | |
| 16:01:26 | mriedem | artom: why? | |
| 16:01:38 | artom | mriedem, https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7696 | |
| 16:01:50 | artom | We use device name to match the XML to the bdm | |
| 16:01:57 | oomichi | alex_xu: can you take a look at https://review.openstack.org/#/c/452284 ? | |
| 16:02:03 | bauzas | jaypipes: and sorry, I thought you were in favor of a "ticket" (let's call it UUID), at least that's what I remember from some very old midcycle | |
| 16:02:13 | mriedem | artom: the libvirt driver completely ignores any user-supplied device name, and calculates it's own | |
| 16:02:13 | artom | (I'm right in saying that it's the device name the user passed in, right?) | |
| 16:02:46 | jaypipes | bauzas: no, I think the APIs we have will work fine as-is for the moment. | |
| 16:03:18 | bauzas | jaypipes: good with me | |
| 16:03:28 | mriedem | artom: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7833 | |
| 16:03:29 | artom | mriedem, fair enough, I need to refresh myself on how the above link works then :/ | |
| 16:03:32 | mriedem | ^ been there since liberty | |
| 16:03:59 | mriedem | artom: i think this code is what sets the device names on the bdm objects https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7840 | |
| 16:04:08 | artom | Ah, so there's still a matching name between the bdm and XML | |
| 16:04:16 | artom | But it's libvirt that sets it, not the user | |
| 16:04:19 | mriedem | correct | |
| 16:04:29 | mriedem | it's all very confusing where libvirt sets it, because the libvirt driver doesn't actually set it | |
| 16:04:33 | mriedem | block_device.default_device_names does | |
| 16:04:49 | mriedem | like most of the bdm code, it's a lot of smoke and mirrors | |
| 16:05:14 | artom | Any sentence that matches /.*block.*device.*confusing.*/ is a tautology | |
| 16:05:37 | mriedem | nvm i guess it is specific to the libvirt driver https://github.com/openstack/nova/blob/master/nova/virt/libvirt/blockinfo.py#L466 | |
| 16:05:54 | mriedem | it has those driver bdm wrapper objects | |
| 16:06:02 | mriedem | so it can mutate the underlying bdm object, and then update it in the db | |
| 16:06:05 | mriedem | using driver_bdm.save() | |
| 16:06:20 | mriedem | and get_disk_info(virt_type, instance, image_meta, block_device_info) is not read-only :) | |
| 16:07:02 | artom | Well, get_guest_xml() isn't read-only either IIRC, so it's at least internally consistent | |
| 16:07:21 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/blockinfo.py#L488 | |
| 16:07:50 | mriedem | yeah anyway, point is, the libvirt driver sets the device name for the bdm, the user doesn't | |
| 16:08:43 | mriedem | sdague: johnthetubaguy: dansmith: did you read that thread in the ML on removing user-supplied bdm devices from requests in the API? i had no idea people were using our crappy data model for overriding image bdms | |
| 16:09:12 | openstack | Launchpad bug 1679223 in OpenStack Compute (nova) "tempest.api.compute.servers.test_server_tags.ServerTagsTestJSON fail on centos7 nodes" [Undecided,New] | |
| 16:09:12 | markus_z | felipemonteiro: Would be cool if you could take a look at: https://bugs.launchpad.net/nova/+bug/1679223 | |
| 16:10:43 | dansmith | mriedem: I saw it, didn't read it | |
| 16:11:00 | dansmith | mriedem: but yeah, I knew | |
| 16:11:01 | sdague | mriedem: it didn't fully sink in, I'll have to revisit | |
| 16:14:21 | bauzas | mriedem: jaypipes: I have a thought, and I'd love your feedback about claims | |
| 16:14:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data (2) https://review.openstack.org/452819 | |
| 16:14:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data https://review.openstack.org/452818 | |
| 16:14:29 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Factor out duplicated notification sample data (3) https://review.openstack.org/452820 | |
| 16:14:44 | bauzas | mriedem: jaypipes: I have a long standing BP targeting to pass the Spec object down to the compute, mostly for rescheduling needs | |
| 16:14:53 | jaypipes | bauzas: I'll trade ya for a review of https://review.openstack.org/#/c/448282/? :) | |
| 16:15:31 | bauzas | mriedem: jaypipes: if we would pass the allocations as part of the Spec object down to the compute, it would prevent us to have a very specific RPC modification for something abstract enough | |
| 16:16:04 | jaypipes | bauzas: yup, makes sense to me. | |
| 16:16:05 | bauzas | mriedem: jaypipes: ie. the conductor gets the Spec object, pass it to the scheduler, and gives back a list of allocations (because the scheduler would claim) | |
| 16:16:07 | mriedem | i'm out of the loop on the claims stuff. | |
| 16:16:21 | bauzas | then, it would pass it down to the compute by setting those claims | |
| 16:16:30 | mriedem | what is the compute going to do with them? | |
| 16:16:50 | mriedem | isn't the whole point that the compute no longer needs to care about setting allocations in the RT? | |
| 16:16:50 | bauzas | jaypipes: it would allow us to change the RPC interface to just accept a spec_obj attribute and read the allocations UUIDs | |
| 16:16:58 | bauzas | mriedem: in case something goes wrong | |
| 16:17:03 | mriedem | what does that mean? | |
| 16:17:06 | bauzas | mriedem: the compute would DELETE those allocations | |
| 16:17:06 | jaypipes | mriedem: if the compute sees the allocations in the request spec, it wouldn't need to do the claims.Claim() build. | |
| 16:17:35 | bauzas | and it would trigger the reschedule by passing back the Spec | |
| 16:17:54 | mriedem | i thought the scheduler was going to delete the allocations if something failed on a given compute host? | |
| 16:18:03 | bauzas | jaypipes: yeah, the Spec object would signal if we support new-style of claiming | |
| 16:18:24 | bauzas | mriedem: the scheduler would POST allocations | |
| 16:18:30 | jaypipes | bauzas: yeah, the presence of allocation objects in the RequestSpec would be that signal. | |
| 16:18:30 | bauzas | mriedem: and get the UUIDs | |
| 16:18:35 | mriedem | jaypipes: so we bump the service version when computes start checking for claims in the scheduler (via the allocations in the request spec?) and then use that service version to determine when the scheduler can start doing this? once all computes are upgraded or what? | |
| 16:19:07 | bauzas | mriedem: I was considering that approach | |
| 16:19:21 | bauzas | mriedem: because we suppose a fully-upgraded cloud | |
| 16:19:24 | jaypipes | mriedem: ya | |
| 16:19:32 | mriedem | i'm not sure i like the compute deleting the allocations | |
| 16:19:36 | mriedem | on a retry | |
| 16:19:49 | dansmith | definitely not | |
| 16:19:55 | mriedem | wouldn't it be better to have the allocation create and cleanup in the same service? | |
| 16:20:01 | bauzas | I dunno | |
| 16:20:02 | mriedem | if we want to just pass a flag down to the compute, sure | |
| 16:20:10 | bauzas | I'm not opiniated | |
| 16:20:24 | dansmith | compute shouldn't be doing that once we're claiming in the scheduler, methinks | |
| 16:20:24 | bauzas | so | |
| 16:20:30 | mriedem | dansmith: agree | |
| 16:20:31 | bauzas | if we say no to that idea | |
| 16:20:39 | bauzas | then, we would just send a signal | |
| 16:20:42 | mriedem | at a high level, we want the compute out of the business of doing this stuff | |
| 16:20:52 | bauzas | but the compute would pass down the Spec by signaling against the signal :) | |
| 16:20:52 | mriedem | yeah send a flag in the reqspec object i guess? | |
| 16:21:04 | bauzas | I mean | |
| 16:21:07 | mriedem | do we pass the reqspec today to the compute | |
| 16:21:08 | mriedem | ? | |
| 16:21:17 | bauzas | mriedem: not yet, but I was planning to | |
| 16:21:22 | mriedem | oy | |
| 16:21:26 | bauzas | it's a specless bp | |
| 16:21:30 | mriedem | why not just pass a simple boolean on the rpc method? | |
| 16:21:35 | dansmith | mriedem: it's leftovers from that whole process still :/ | |
| 16:21:38 | mriedem | do_claims=False | |
| 16:21:54 | dansmith | mriedem: +1.. I think encoding the state in the reqspec is kinda bad | |
| 16:22:00 | bauzas | mriedem: I'd rather prefer to pass a Spec object because we could need it for rescheduling needs | |
| 16:22:01 | dansmith | it's not guaranteed to be right | |