Skip to content

common: make sure to prepend remote iso path#388

Merged
nywilken merged 1 commit intovmware:mainfrom
Hi-Angel:fix-386
Mar 19, 2024
Merged

common: make sure to prepend remote iso path#388
nywilken merged 1 commit intovmware:mainfrom
Hi-Angel:fix-386

Conversation

@Hi-Angel
Copy link
Copy Markdown
Contributor

@Hi-Angel Hi-Angel commented Mar 18, 2024

This fixes regression introduced by the commit

 "common: deduplicate AddCdrom call by appending path to ISOPaths"

that was squashed into:

 "refactor: only add / remove minimul number of cdroms (#355)"

It explained that the order is not guaranteed and does not matter, however the documentation says:

If the "iso_url" is defined in addition to the "iso_paths", the
"iso_url" is added to the VM first. This keeps the "iso_url" first
in the boot order by default allowing the boot iso being defined by
the iso_url and the vmware tools iso added from the datastore.

So fix the regression.

Fixes: #386

This fixes regression introduced by the commit

     "common: deduplicate AddCdrom call by appending path to ISOPaths"

that was squashed into:

     "refactor: only add / remove minimul number of cdroms (vmware#355)"

It assumed that the order is not guaranteed and does not matter,
however the documentation says:

> If the "iso_url" is defined in addition to the "iso_paths", the
> "iso_url" is added to the VM first. This keeps the "iso_url" first
> in the boot order by default allowing the boot iso being defined by
> the iso_url and the vmware tools iso added from the datastore.

So fix the regression.

Fixes: vmware#386

Signed-off-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
@Hi-Angel
Copy link
Copy Markdown
Contributor Author

Hi-Angel commented Mar 18, 2024

upd: added the S-b to the commit message

@tenthirtyam
Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam added bug Bug regression Regression labels Mar 18, 2024
@tenthirtyam tenthirtyam added this to the v1.2.6 milestone Mar 18, 2024
@tenthirtyam tenthirtyam requested a review from nywilken March 18, 2024 17:07
Copy link
Copy Markdown
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround on this bug @Hi-Angel the changes look good to me. I see the previous version added a CDROM for the remote path to start but that is now handled later in the code when iterating through s.Config.ISOPaths. So all is good.

@nywilken nywilken merged commit b5b901b into vmware:main Mar 19, 2024
@vmware vmware locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Bug regression Regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1.2.5 No longer builds Windows 2019 & 2022

3 participants