Skip to content

More vmware image#572

Draft
fwiesel wants to merge 1 commit intostable/2023.2-m3from
more_vmware_image
Draft

More vmware image#572
fwiesel wants to merge 1 commit intostable/2023.2-m3from
more_vmware_image

Conversation

@fwiesel
Copy link
Member

@fwiesel fwiesel commented Aug 7, 2025

No description provided.

@fwiesel
Copy link
Member Author

fwiesel commented Aug 7, 2025

Follow up to #571. I am not 100% sure about the impact, hence rather a draft.

Copy link

@joker-at-work joker-at-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use _get_instance_config_info() instead? It seems to return the vi we see often used with vi.ii.

@fwiesel fwiesel force-pushed the more_vmware_image branch from 7536894 to 900db19 Compare August 7, 2025 09:14
@fwiesel
Copy link
Member Author

fwiesel commented Aug 7, 2025

_get_instance_config_info also calls _get_vm_config_info which involves many other calls.
I think, we can probably refactor some more code where we need the datastore to be selected, but again, I'd rather would do that in a separate change. It is easily changed in the code, but the tests need adopted as well.

@joker-at-work
Copy link

Another separate change? I thought this PR was the separate change already :D

@fwiesel
Copy link
Member Author

fwiesel commented Aug 7, 2025

Well, it also was the one with VMwareImage. That change I can do easily, as it is a minimal change.
_get_instance_config_info looks much more involved, and I am not sure, I can actually implement it quickly, and I might not even do it before my PTO.

@joker-at-work
Copy link

joker-at-work commented Aug 7, 2025

We have a quick-fix in, so I'd be fine with a proper fix later instead of another one next week which we have to update again.

If we say we go with VMwareImage directly, that's fine by me, too. I wanted to raise awareness how other code does it and if that's maybe the better option.

@fwiesel fwiesel force-pushed the more_vmware_image branch from 900db19 to e845816 Compare August 7, 2025 10:03
By using VMwareImage we have less places of normalization
of defaults.

Followup to Change-Id: I28c439dc612bc2ddae3b564c0d2cae8f5207bfc4

Change-Id: I7b1f251ffaa90beec768a2f044e15a39aaf9527f
@fwiesel
Copy link
Member Author

fwiesel commented Aug 7, 2025

I agree, it is likely better in several places, because there they also do the same calls as _get_instance_config_info, but not necessarily all of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants