Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-08-25
15:34:05 auniyal yes CI passing :)
15:47:48 dansmith auniyal: one very small (but weird) formatting nit
15:47:58 dansmith auniyal: if you can fix that real quick I'll +2
15:48:46 opendevreview Dan Smith proposed openstack/nova master: VERYWIP: Set libvirt device alias for volumes https://review.opendev.org/c/openstack/nova/+/892801
15:48:46 opendevreview Dan Smith proposed openstack/nova master: VERYWIP: Support setting alias on libvirt disks https://review.opendev.org/c/openstack/nova/+/892800
15:49:00 dansmith oh I guess kashyap isn't here
15:56:11 sean-k-mooney any reason for the ua- prefix for the volume id
15:56:19 dansmith auniyal: comments on the reproducer too
15:56:28 dansmith sean-k-mooney: required by libvirt (as noted in the commit message)
15:56:47 dansmith sean-k-mooney: interestingly if you omit that, it just ignores what you gave it and uses its own value with no warning :/
15:57:07 sean-k-mooney .. of couse it does
15:57:48 sean-k-mooney im debating in my head the value fo doint that in the call site as you did vs prefixing it here https://review.opendev.org/c/openstack/nova/+/892800/1/nova/virt/libvirt/config.py#1205
15:58:38 dansmith well, one reason is that libvirt gives everything auto-generated aliases if you don't specify one,
15:59:06 dansmith and so we'd need to strip out the ua- from the parsing if we do it on the input, and then there's no way to tell from the higher layers if it's one we set or libvirt set
15:59:23 dansmith <alias name='virtio-disk0'/>
15:59:23 dansmith default looks like:
15:59:36 sean-k-mooney ok thats good to know.
16:00:20 sean-k-mooney then one suggestion would be add a is a uuid_to_alais funciton
16:00:28 sean-k-mooney so we can just use taht everywhere
16:00:32 sean-k-mooney but thats a minor nit
16:01:06 sean-k-mooney i guess we wont alwasy have a uuid
16:01:13 dansmith right
16:01:18 sean-k-mooney for pci devie form flavor alaises for example
16:01:29 dansmith and root disk, if we wanted to set that
16:01:46 dansmith anyway, I mostly just wanted to make sure we could plumb that through and it would stick in libvirt
16:01:58 sean-k-mooney im not sure if we dont because we dont supprot detaching that but it would help for swap on reize perhaps
16:02:36 sean-k-mooney ya its good to get this basic proof of concept and find the "ua-" issue
16:02:37 dansmith I don't know that we need it at all, I'm just saying if we set the alias for those devices, we won't have a uuid
16:03:02 sean-k-mooney related question
16:03:20 sean-k-mooney do we know if libvirt has an api to get/remove a device via the alais
16:03:29 sean-k-mooney it would be nice if it did but probaly does not
16:03:45 dansmith idk, but it's not required I think
16:04:29 sean-k-mooney ya its likely not but it would be nice just to detach a volume by doing libvir.detach_device(alias=<volume uuid>)
16:04:39 dansmith https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDeviceAlias
16:04:49 dansmith that will be *very* nice, IMHO
16:04:52 sean-k-mooney oh perfect
16:05:01 sean-k-mooney ya so that is what i was hoping for i think
16:05:32 sean-k-mooney it also renforice the idea that this is how libvirt want use to name/specify things to detatch ectra
16:05:43 dansmith up
16:05:48 dansmith also yup
16:07:01 sean-k-mooney so this was added in 3.9.0 which was released in late 2017 so when we first started lookign at teh lun issue it did not exist and when we looed at them last i dont know if we have a high enough min libvirt
16:07:16 sean-k-mooney but for master this looks like a nice cleanup over all
16:07:33 sean-k-mooney thanks for starting a poc of this
16:08:53 dansmith yeah, I'll get going on a spec just to lay some of this otu
16:09:10 kashyap dansmith: Hi! I hear you were looking for me :)
16:09:11 dansmith I'll have to survey where all we look up devices to see where we need to convert them to use this
16:09:26 dansmith kashyap: was just going to point you at this: https://review.opendev.org/q/topic:libvirt-dev-alias
16:09:35 dansmith and sean-k-mooney and I were chatting about it briefly just now
16:10:11 kashyap Yeah, just found 'em
16:13:46 kashyap dansmith: Just added a small comment here - https://review.opendev.org/c/openstack/nova/+/892800
16:14:00 kashyap My guess is you already know it
16:14:12 dansmith kashyap: it's in the commit message
16:15:43 dansmith well, the ua- part is anyway
16:17:01 kashyap Ohh, I read the commit message, but I didn't see the "ua-" part explicitly called out. Blind me
16:19:10 kashyap (Oh, it's in the other commit message. Just saw it)
16:21:28 auniyal dansmith, ack, will update , thanks
16:22:18 sean-k-mooney dansmith: looking out our downstream multi attach volume docs it says "Live migration of multi-attach volumes is not available" that cinder volume migration right
16:22:36 sean-k-mooney ill check our upstrema docs but as far as i am aware we supprot live migration of vms with multi attach
16:22:38 dansmith sean-k-mooney: no idea
16:23:37 kashyap sean-k-mooney: dansmith: I just read your brief discussion on the upstream log. (I agree that we should indeed retain the prefix or else ... the odd behavior Dan noted is not nice.)
16:28:16 opendevreview Merged openstack/nova master: introduce global greenpool https://review.opendev.org/c/openstack/nova/+/873061
16:33:28 opendevreview Dan Smith proposed openstack/nova-specs master: WIP: Add libvirt-dev-alias spec https://review.opendev.org/c/openstack/nova-specs/+/892806
16:34:07 dansmith sean-k-mooney: kashyap^ just brain dump level to get it started
16:41:11 kashyap Nice; the outline looks already quite good to me. Cced myself, will do a concetrated read on Mon
16:41:37 kashyap Heh, how plainly you write the alternative "We could maintain our own mapping in our database for those device types"
16:41:47 kashyap (I mean, I like the matter-of-factly stating)
16:46:14 opendevreview Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947
16:46:14 opendevreview Amit Uniyal proposed openstack/nova master: Add function to get all attachments in Cinder.API module https://review.opendev.org/c/openstack/nova/+/881457
16:46:15 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
16:50:26 dansmith auniyal: looks like you committed a bunch of extra changes into that base patch?
16:52:25 auniyal yes, that was a pep8 fix I have to do after rebase
16:52:37 auniyal I added a note in commit msg for same
16:52:46 dansmith auniyal: they're in files you haven't touched
16:53:07 dansmith they should not be in that patch
16:53:40 auniyal ack, will remove them,
17:03:14 opendevreview Amit Uniyal proposed openstack/nova master: Add function to get all attachments in Cinder.API module https://review.opendev.org/c/openstack/nova/+/881457
17:03:15 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
17:03:15 opendevreview Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947
17:11:28 auniyal thanks dansmith
17:24:04 sean-k-mooney dansmith: this wont shock you but since we are discussin cinder voluems in varios place i cam acroos an unsupproted edgecase a while back
17:24:37 sean-k-mooney if you have an image of format=iso you can both a vm form it and then install to a cinder data volume
17:24:49 sean-k-mooney however you cant do that if you try to do bfv
17:25:14 sean-k-mooney as we assume all bfv case are block devices not "cd-roms"
17:25:43 sean-k-mooney so we try to use the volume contianign the iso data as a block device
17:25:57 sean-k-mooney so wehn the vm boot it cant actully load the bootload/root filesytem
17:26:39 sean-k-mooney i feel like that should be simple to fix if we ever had time to do it
17:27:44 sean-k-mooney as a result you cant actully use cidner volumes as "dirver disks" or install media today without somehow converting the iso to a bootable filesystem
17:30:27 dansmith why would you bfv with the root disk as the iso?
17:30:49 dansmith you want bfv with the root disk as a volume and the iso as a secondary non-root device right? same for driver disks
17:31:28 sean-k-mooney well that also wotn work volume created form iso just dont work when attach to a vm
17:31:54 sean-k-mooney but my usecase was installing openshift on openstack using the installer iso
17:32:11 sean-k-mooney my home cloud is configured ot only do bfv
17:32:51 sean-k-mooney i worked aroudn it by temproy addign a flavor with some local storage and then did the install to a cienr volume and then booting another vm form that data volume after the fact
17:33:08 dansmith yeah I think the iso-on-volume thing is known,
17:33:26 dansmith Im just not sure that it has anything to do with bfv necessarily except that's the situation you need it for more I uess
17:33:46 sean-k-mooney it actully woudl be fairly simple to fix just add metadata on teh volume to say it not a block device but rather a cd/removal media
17:34:00 sean-k-mooney oh its not its just if you dont do boot form volume
17:34:12 sean-k-mooney the workflow of boot form iso and then install do data volume works
17:34:42 sean-k-mooney the gap/bug is just we cant take a cinder volume and attach it as a cd to the vm
17:35:11 sean-k-mooney that is totally fixable with metadta if we wanted too

Earlier   Later