| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-08 | |||
| 16:58:32 | whoami-rajat | I'm kind of confused with all the conditions ... | |
| 16:58:48 | stephenfin | sec, code is probably easier :) | |
| 16:58:53 | whoami-rajat | we've 1) confirm_reimage check 2) microversion check 3) image check | |
| 16:59:07 | whoami-rajat | yeah would be helpful that way :) | |
| 17:08:02 | stephenfin | whoami-rajat: https://paste.opendev.org/show/bpmNKNYMoKN736cxOa7e/ | |
| 17:10:10 | stephenfin | whoami-rajat: that should do the trick. Personally though, I'd really rethink the need for this. People know rebuild is a destructive operation and they'd have to opt-in to the new microversion to even take advantage of this | |
| 17:10:19 | stephenfin | I'm sure the cinder team have discussed this at length though | |
| 17:13:11 | whoami-rajat | I might be thinking too much but if a rebuild related MV is added in future, the first "if" case of the "else" case becomes invalid, Eg: we are passing 2.94 for rebuilding an image backed instance with a new field and this check will fail that operation | |
| 17:15:44 | whoami-rajat | stephenfin, ^ | |
| 17:15:53 | whoami-rajat | stephenfin, yeah the spec discussion has been going on for several releases (when I wasn't working on it) and the general comments from both nova and cinder side were to have an additional check | |
| 17:16:25 | sean-k-mooney | with specifci decenters | |
| 17:16:33 | sean-k-mooney | ie. i did not want the check in nova or the client | |
| 17:16:39 | sean-k-mooney | but i can live with it in the client | |
| 17:17:05 | sean-k-mooney | rebuild is ment to be distructive | |
| 17:17:13 | stephenfin | whoami-rajat: Oh yeah, I need to check if it's BFV in both branches of the else https://paste.opendev.org/show/bveR7gYr02CL4XLBbvVe/ | |
| 17:17:31 | sean-k-mooney | if we want to supporot the other useage we shoudl have explit way to do that that works for all vms not just bfv | |
| 17:17:56 | whoami-rajat | also a bootable volume can still exist when an instance is destroyed so it's kind of different from the rebuild we refer to for ephemeral cases, at least from a cinder perspective | |
| 17:18:33 | sean-k-mooney | form a nova persecvitve not really if you want to save the root data after a vm is delete just snapshot it | |
| 17:18:46 | sean-k-mooney | i dont coniser the nova root disk to be ephemeral | |
| 17:19:06 | sean-k-mooney | nova has a seperate thing called ephmearl disks in addtion to the root disk | |
| 17:19:49 | sean-k-mooney | nova vms by can have 4 types of storage at the same time | |
| 17:20:25 | sean-k-mooney | disk_gb is the root disk, ephemeral_gb is 0-n addtional disk, swap and cinder volumes | |
| 17:20:38 | whoami-rajat | stephenfin, that looks good, will update it | |
| 17:21:09 | sean-k-mooney | ah yes if server.image is None: | |
| 17:21:24 | sean-k-mooney | one slight optimisation | |
| 17:21:30 | sean-k-mooney | can you change the else and if | |
| 17:21:32 | sean-k-mooney | to an elif | |
| 17:22:39 | whoami-rajat | ack, don't have much insights in nova but cinder folks would be really angry without that check :D | |
| 17:22:47 | sean-k-mooney | https://paste.opendev.org/show/befzjPz2izxuGZtwr1Vs/ | |
| 17:23:39 | sean-k-mooney | whoami-rajat: as long as its not in nova im ok with it but i really hate that we allow the non distrutive case and am sad we have to support it | |
| 17:24:26 | sean-k-mooney | we cant change history but allowing the metadat to be updated via rebuidl to same image for BFV instance should not have been done | |
| 17:24:37 | stephenfin | sean-k-mooney: I usually avoid doing that to indicate that the two checks (microversion and "is it volume backed?" aren't totally related but that would be less nesting, yeah | |
| 17:25:19 | sean-k-mooney | honestly i dont mind etither way i guss | |
| 17:25:35 | sean-k-mooney | i just dislike wraping on 80 charters so avoid nesting if i can | |
| 17:26:02 | sean-k-mooney | in this case it does not matter as its only impacting the commnet | |
| 17:26:44 | stephenfin | true | |
| 17:26:54 | sean-k-mooney | whoami-rajat: stephenfin has the +2 rights so follw there prefernce on this | |
| 17:27:13 | stephenfin | for my own notes, attempting to rebuild a volume-backed server without --image fails currently | |
| 17:27:15 | stephenfin | ❯ openstack server rebuild test-server-bfv | |
| 17:27:15 | stephenfin | 'str' object has no attribute 'get' | |
| 17:27:43 | sean-k-mooney | that proably a bug | |
| 17:27:57 | sean-k-mooney | since rebuild without an image specified is ment to default to same image | |
| 17:27:57 | stephenfin | 100% | |
| 17:28:03 | stephenfin | in OSC | |
| 17:28:21 | sean-k-mooney | let me just check if in the api | |
| 17:28:25 | whoami-rajat | yeah, I kind of like stephenfin approach too, we can ignore saving one LOC for readability | |
| 17:28:46 | whoami-rajat | will update the patch | |
| 17:29:01 | sean-k-mooney | stephenfin: its not | |
| 17:29:06 | sean-k-mooney | well there is a osc bug | |
| 17:29:09 | sean-k-mooney | but image is required | |
| 17:29:13 | sean-k-mooney | in the api | |
| 17:29:24 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/?expanded=rebuild-server-rebuild-action-detail#rebuild-server-rebuild-action | |
| 17:29:40 | sean-k-mooney | stephenfin: so in osc image should be required | |
| 17:29:59 | sean-k-mooney | unless this was implemted in osc /nova client | |
| 17:30:04 | stephenfin | yeah, it was | |
| 17:30:10 | sean-k-mooney | ah ok | |
| 17:30:13 | stephenfin | if you don't specify it, we use the same image | |
| 17:30:21 | sean-k-mooney | right but that is client side | |
| 17:30:29 | stephenfin | which is why you're used to that behaviour, I guess | |
| 17:30:31 | stephenfin | sean-k-mooney: yup | |
| 17:30:38 | sean-k-mooney | ack | |
| 17:31:38 | stephenfin | whoami-rajat: one point: annoyingly either nova or novaclient returns the empty string if the server is volume-backed, so that 'if server.image is None' needs to be simply 'if not server.image' :( | |
| 17:32:20 | stephenfin | https://paste.opendev.org/show/bcl82mF5NEjC4T1uhbnu/ | |
| 17:32:30 | stephenfin | yeah, it's nova itself. Ugh | |
| 17:32:40 | whoami-rajat | ah ok, will update that part | |
| 17:33:06 | sean-k-mooney | The UUID and links for the image for your server instance. The image object will be an empty string when you boot the server from a volume. | |
| 17:33:10 | sean-k-mooney | its in the api ref | |
| 17:33:27 | stephenfin | Just because it's documented doesn't make it right :-P | |
| 17:33:29 | sean-k-mooney | stephenfin: so its not a bug that just what we chose as the sentinel | |
| 17:33:44 | sean-k-mooney | yes but its part of the api contract | |
| 17:34:06 | stephenfin | oh, I'm not suggesting changing it. Merely complaining | |
| 17:34:30 | sean-k-mooney | :) | |
| 17:35:08 | stephenfin | yup, confirmed that rebuilding a BFV instance with the same image works, but using a different image results in a failure | |
| 17:35:14 | stephenfin | ❯ openstack server rebuild --image cirros-0.5.1-x86_64-disk test-server-bfv | |
| 17:35:14 | stephenfin | Image 781c8aa6-210f-4f55-9964-b9fe5ef07749 is unacceptable: Unable to rebuild with a different image for a volume-backed server. (HTTP 400) (Request-ID: req-945cc80d-129e-448a-bde9-bce5f3a8be88) | |
| 17:35:32 | stephenfin | ❯ openstack server rebuild --image cirros-0.5.2-x86_64-disk test-server-bfv | |
| 17:35:32 | stephenfin | | Field | Value | | |
| 17:35:32 | stephenfin | +-------------------+----------------------------------------------------------+ | |
| 17:35:34 | stephenfin | ... | |
| 17:36:03 | stephenfin | whoami-rajat: I think we're on the same page. I'll review that again in the morning. Late for me now o/ | |
| 17:36:13 | stephenfin | Cheers for the input, sean-k-mooney | |
| 17:36:37 | sean-k-mooney | :) | |
| 17:37:11 | whoami-rajat | thanks stephenfin and sean-k-mooney | |
| #openstack-nova - 2022-09-09 | |||
| 08:55:06 | kashyap | Does anyone recall if use "virtio-blk" or "virtio-scsi" by default for in Wallaby? | |
| 09:09:38 | kashyap | virtio-blk | |
| 09:15:50 | bauzas | sean-k-mooney: fwiw, as you being the release liaison, please see the etherpad I wrote for managing RC1 patches that are on duty https://etherpad.opendev.org/p/nova-zed-rc-potential | |
| 09:35:55 | sean-k-mooney | bauzas: thanks looking | |
| 09:50:10 | kashyap | sean-k-mooney: Hey | |
| 09:50:23 | kashyap | sean-k-mooney: Do you recall what's the limit to no. of disks when using 'virtio-blk'? | |
| 09:50:29 | kashyap | In OpenStack... | |
| 09:50:43 | sean-k-mooney | in openstack we dont have one | |
| 09:50:47 | sean-k-mooney | but in reality | |
| 09:50:51 | sean-k-mooney | each virtio-blk device | |
| 09:50:58 | sean-k-mooney | consumes a pci slot | |
| 09:51:07 | sean-k-mooney | so ~20ish | |
| 09:51:14 | kashyap | Right; some 28 disks, IIRC | |
| 09:51:32 | sean-k-mooney | if you need more you need to use virtio-scsi | |
| 09:52:01 | kashyap | Yep | |
| 09:52:23 | sean-k-mooney | there uses to be a limit of 26 because we ran out of drive letters but we fix that a few cycles ago | |
| 09:52:23 | kashyap | I was more interested in the no. of disks limitation for virtio-blk; and the QEMU blog-post confirms it is 28 (https://www.qemu.org/2021/01/19/virtio-blk-scsi-configuration/) | |