| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-01 | |||
| 11:13:04 | opendevreview | Stephen Finucane proposed openstack/nova master: tests: Merge 'test_utils', 'test_scheduler_utils' https://review.opendev.org/c/openstack/nova/+/773646 | |
| 11:13:04 | opendevreview | Stephen Finucane proposed openstack/nova master: scheduler: Merge driver into manager https://review.opendev.org/c/openstack/nova/+/773644 | |
| 11:13:05 | opendevreview | Stephen Finucane proposed openstack/nova master: conf: Remove deprecated aliases https://review.opendev.org/c/openstack/nova/+/773647 | |
| 11:38:57 | opendevreview | norman shen proposed openstack/nova master: Saving security group to info_cache https://review.opendev.org/c/openstack/nova/+/786348 | |
| 11:42:15 | stephenfin | dead simple Python 3.10 prep patch here if anyone has 2 mins https://review.opendev.org/c/openstack/nova/+/790405 | |
| 11:45:15 | gibi | stephenfin: done | |
| 11:45:18 | stephenfin | ta | |
| 12:15:20 | opendevreview | Lee Yarwood proposed openstack/nova stable/wallaby: hardware: Use image_meta.id within get_mem_encryption_constraint https://review.opendev.org/c/openstack/nova/+/793956 | |
| 12:16:29 | opendevreview | Lee Yarwood proposed openstack/nova stable/victoria: hardware: Use image_meta.id within get_mem_encryption_constraint https://review.opendev.org/c/openstack/nova/+/793957 | |
| 12:17:15 | opendevreview | Lee Yarwood proposed openstack/nova stable/ussuri: hardware: Use image_meta.id within get_mem_encryption_constraint https://review.opendev.org/c/openstack/nova/+/793958 | |
| 13:07:29 | ganso | Hi nova folks! Not sure if you're familiar with this behavior, whether it is a known bug or limitation (I didn't find any launchpad entry for it), but it is very easy to reproduce: if you try attach a volume with hw_disk_bus property that is different from the root disk's hw_disk_bus one, it gets ignored. Example, root disk's is ide, volume's is virtio, and it gets attached as ide. | |
| 13:08:16 | ganso | using pure kvm through virt-manager, it is possible to have mixed ide+virtio disks, but nova doesn't seem to be allowing that | |
| 13:08:32 | sean-k-mooney | ganso: yes that is expected behavior | |
| 13:08:44 | sean-k-mooney | ganso: the image metadta is only use form the root disk | |
| 13:08:57 | sean-k-mooney | we do not use image metadta form any other disk | |
| 13:09:11 | sean-k-mooney | ganso: you might be able to enable this using the block device mappings api | |
| 13:09:39 | ganso | sean-k-mooney: is there a reason why you would want to keep it this way? | |
| 13:10:28 | sean-k-mooney | well if the bus is not already present in the vm at a minitum we woudl have to also attach a contole for that bus be it ide/stata whatever | |
| 13:10:38 | sean-k-mooney | and then all attach the volume to that new contoler | |
| 13:10:55 | sean-k-mooney | that could fail for a number of reasons | |
| 13:11:24 | sean-k-mooney | in general image metadata was intened to only be used form teh root disk | |
| 13:11:56 | sean-k-mooney | when not using cinder it does not really make sense to have multipel image metadta soruce | |
| 13:12:14 | ganso | sean-k-mooney: hmmm I see, so this is like a new feature, to handle independent disk_bus values for each new volume, something nova doesn't do today | |
| 13:12:34 | sean-k-mooney | if we allowed it to work for cinder there is an issue with confilt between volumens that we would have to determin how to handel | |
| 13:12:40 | ganso | sean-k-mooney: I was able to hack the code just to test it, I replaced the variable with "virtio" here: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2018 | |
| 13:13:04 | sean-k-mooney | ganso: yes it would basiclaly be a new feature | |
| 13:13:06 | ganso | and it worked, upon attach it added a virtio disk beside an ide one (the root disk) | |
| 13:13:28 | sean-k-mooney | yes there is a virtio contoler present by defualt for the nic | |
| 13:13:37 | sean-k-mooney | if you put stata it will fail | |
| 13:13:55 | sean-k-mooney | /stata/sata | |
| 13:14:53 | ganso | sean-k-mooney: so, adding a controller during attach is something risk you say? would it be 100% safe if it requires the VM to be shutoff? | |
| 13:15:15 | sean-k-mooney | it may or may not fail | |
| 13:15:43 | sean-k-mooney | and it may depend on the version fo libvirt if the contoler is hot pluggable or not | |
| 13:16:20 | ganso | sean-k-mooney: I understand. I will do some testing around this, thank you for the clarification! | |
| 13:16:24 | sean-k-mooney | for example when using q35 machine type libvirt only alloctes 1 spare pcie port for hotplug devices | |
| 13:16:37 | sean-k-mooney | so you would not have enough to add the contoler and a volume | |
| 13:16:54 | sean-k-mooney | there is one way you might be able to do this today | |
| 13:17:05 | sean-k-mooney | but only for new vms | |
| 13:17:07 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/?expanded=create-server-detail#create-server | |
| 13:17:25 | sean-k-mooney | server create allows you to specify the block device mappings block_device_mapping_v2 (Optional) | |
| 13:17:31 | sean-k-mooney | "block_device_mapping_v2": [{ | |
| 13:17:33 | sean-k-mooney | "boot_index": "0", | |
| 13:17:35 | sean-k-mooney | "uuid": "ac408821-c95a-448f-9292-73986c790911", | |
| 13:17:37 | sean-k-mooney | "source_type": "image", | |
| 13:17:38 | ganso | sean-k-mooney: oh I see, it would depend on that as well, so depending on how I test it, I may be avoiding a problem (like testing with just 1 extra disk instead of 2 or 3, as it could run out of devices) | |
| 13:17:39 | sean-k-mooney | "volume_size": "25", | |
| 13:17:41 | sean-k-mooney | "destination_type": "volume", | |
| 13:17:43 | sean-k-mooney | "delete_on_termination": true, | |
| 13:17:45 | sean-k-mooney | "tag": "disk1", | |
| 13:17:47 | sean-k-mooney | "disk_bus": "scsi"}] | |
| 13:17:49 | sean-k-mooney | which has the disk bus | |
| 13:18:07 | sean-k-mooney | ganso: yes although we have a config option for how may free port to allcote | |
| 13:18:18 | ganso | sean-k-mooney: I will try that! could just work out for my use case! thank you! | |
| 13:18:47 | sean-k-mooney | ganso: if that works and you can set the disk bus per volume when creatign a new server | |
| 13:18:50 | sean-k-mooney | which the api imples | |
| 13:19:03 | sean-k-mooney | then we could consider if that would be extanded to port attach | |
| 13:20:01 | sean-k-mooney | that coudl be done by either alowing the disk bus to be specified on attach or possibel adding support for a subset of data to be stored in teh block device mapping form the volume metadtata | |
| 13:20:41 | sean-k-mooney | e.g. copy just the hw_disk_bus into the bdm that we create on attach | |
| 13:21:08 | ganso | sean-k-mooney: well, even if that works for the "create" operation, it is a more limited one, as you said, when creating the VM it is a safer scenario (not hotpluggable, can determine the required controllers before-hand). The attach operation is less safe in this regard and could fail in many ways, as you said | |
| 13:21:14 | sean-k-mooney | ganso: this is a feature that would require a spec and someone more expert in the strage layware like lyarwood to review | |
| 13:22:32 | sean-k-mooney | ganso: the failure could actully be handeled internally in the driver just roleing back and rejecting the attach but we would have to code with that in mind and think about the ux and error message we give to a user | |
| 13:23:05 | sean-k-mooney | whihc is one of the reason i think this woudl need a spec rather then trateing it as a bugfix | |
| 13:24:10 | ganso | sean-k-mooney: yea, just like the hotplugged error message for IDE devices, I was confused at first when the attach didn't work while the request was accepted, but I had no obvious error message | |
| 13:24:20 | sean-k-mooney | ganso: the driver code in general assumes there is only one value for hw_disk_bus for the entire vm so thre are proably edgcases like rescue and move operations that might break. | |
| 13:24:21 | ganso | sean-k-mooney: I agree! +1 | |
| 13:30:10 | ganso | sean-k-mooney: btw, if you have a minute, that patch you previously reviewed already has a +2. I was wondering if you could please take another look at it when you have a minute, since you're already familiar with it: https://review.opendev.org/c/openstack/nova/+/784166 | |
| 13:36:11 | sean-k-mooney | ah yes am i do not have +2 rights on nova but ill take a look now and maybe poke some of the nova cores to review when im done | |
| 13:49:14 | opendevreview | Merged openstack/nova master: Add unit test for importing ed25519 ssh key https://review.opendev.org/c/openstack/nova/+/789642 | |
| 13:50:12 | opendevreview | Merged openstack/nova master: Change minversion of tox to 3.18.0 https://review.opendev.org/c/openstack/nova/+/791968 | |
| 13:51:11 | sean-k-mooney | bauzas: you like anit-afintiy groups right :P care to review ganso's patch https://review.opendev.org/c/openstack/nova/+/784166 | |
| 13:51:53 | opendevreview | Merged openstack/nova master: Remove unused DeleteFromSelect db api method https://review.opendev.org/c/openstack/nova/+/784528 | |
| 13:52:14 | bauzas | sean-k-mooney: not sure it's correct to say "I like" for the instance groups but sure ;) | |
| 13:52:50 | sean-k-mooney | hehe that was ment to be drowning in sarcasim but i for got to anotate that :) | |
| 13:56:27 | sean-k-mooney | do we have quota for how may revision of a patch you are allow to have without unit tests passing... | |
| 13:57:28 | sean-k-mooney | i fell like if we did its less then 51 in 6 weeks | |
| 14:01:00 | bauzas | ganso: I provided another +2 but please look at my comment in https://review.opendev.org/c/openstack/nova/+/784166 | |
| 14:01:49 | bauzas | once you look at it, tell me and then I'll +W your change | |
| 14:02:30 | sean-k-mooney | bauzas: yes adding an explaintion would be nice to avoid the tribal knoladge that live migration never reschdules | |
| 14:02:44 | bauzas | can be a follow-ip | |
| 14:02:46 | bauzas | up* | |
| 14:03:09 | sean-k-mooney | also yes i agree it can be a followup | |
| 14:03:19 | bauzas | ganso: do you want to backport your change, actually ? | |
| 14:03:38 | bauzas | if so, maybe we would need to have a new revision for the relnote | |
| 14:03:54 | sean-k-mooney | we can backport the followup too | |
| 14:04:01 | sean-k-mooney | just and fyi | |
| 14:04:03 | bauzas | or squashing both when backporting but... | |
| 14:04:05 | sean-k-mooney | reno is ok with that | |
| 14:04:13 | sean-k-mooney | no need to squash | |
| 14:04:30 | sean-k-mooney | we could but you can edit renos after the fact | |
| 14:04:41 | bauzas | sure, but I don't like to have two patches when backporting when one of them is just a FUP | |
| 14:04:49 | sean-k-mooney | fair | |
| 14:05:23 | bauzas | either way, wanting ganso to reply... | |
| 14:05:31 | ganso | reading scrollback now | |
| 14:07:20 | ganso | sean-k-mooney: loved the term tribal knowledge! didnt know it! will now use it for many things lol! | |
| 14:07:30 | ganso | bauzas: yes, I plan to backport. Will look at the patch now | |
| 14:08:02 | bauzas | ganso: okay, then please provide a new PS and I'll directly +2/+W | |
| 14:08:13 | opendevreview | Balazs Gibizer proposed openstack/placement master: [doc] Redirect people to #openstack-nova https://review.opendev.org/c/openstack/placement/+/793832 | |
| 14:08:58 | ganso | bauzas: great! will work on that now! thank you! | |
| 14:09:12 | bauzas | ganso: ping me when you're done | |
| 14:13:38 | gibi | bauzas, sean-k-mooney, melwitt: when you have time please check back to the placement re-parenting spec. https://review.opendev.org/c/openstack/nova-specs/+/788243 | |