Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hyperv: Add option to use legacy network adapter #4656

Closed
wants to merge 1 commit into from
Closed

hyperv: Add option to use legacy network adapter #4656

wants to merge 1 commit into from

Conversation

gh2k
Copy link

@gh2k gh2k commented Mar 10, 2017

Add an option to allow hyperv to use a legacy network adapter instead of the default.

@gh2k
Copy link
Author

gh2k commented Mar 13, 2017

I was trying to fix #4404 with this, but it turns out that this doesn't actually help matters. My belief that having a legacy adapter would correctly report the IP address was, I think, based on faulty tests.

Nevertheless I'll leave this open as it might benefit someone and it doesn't do any harm.

@mwhooker
Copy link
Contributor

mwhooker commented Jun 5, 2017

Thanks very much for this. I appreciate leaving the PR open, but I don't think we'll merge this right away unless it's requested. Untargeting

@mwhooker mwhooker removed this from the v1.0.1 milestone Jun 5, 2017
@taliesins
Copy link
Collaborator

Can we close this PR?

@mwhooker
Copy link
Contributor

mwhooker commented Jun 6, 2017

@taliesins up to you

@puneetloya
Copy link

Is there a plan to merge this to main packer?

@Justin-DynamicD
Copy link

it's odd that it doesn't help matters as it should (I can install ubuntu using a legacy adapter and not need integration drivers). While it may not "solve" the problem entirely I imagine it at least helps.

@matthew-hickok
Copy link

This would still be a great option to have.

@tuxillo
Copy link

tuxillo commented Mar 7, 2018

Hi,

Why isn't this merged? It is not possible to use packer to build images of OSes that don't suport synthetic network adapters.

Before finding this PR I have done something similar (but much less elegant I think tuxillo@f67c265) and it's not reporting the IP back either, @gh2k did you figure out why?

Thanks,
Antonio Huete

@tuxillo
Copy link

tuxillo commented Mar 7, 2018

The way the IP is retrieved in

func IpAddress(mac string) (string, error) {

seems to return "" for legacy network adapters when the guest is not running integration services:

PS C:\Users\Administrator\go\bin> Hyper-V\Get-Vm | %{$_.NetworkAdapters}

Name                   IsManagementOs VMName SwitchName                                       MacAddress   Status IPAddresses
----                   -------------- ------ ----------                                       ----------   ------ -----
Legacy Network Adapter False          test   Red Hat VirtIO Ethernet Adapter - Virtual Switch 0155D792809 {Ok}   {}

I created the VM manually to test this, therefore it is not a replica, which is the only thing that would show an empty "IpAddresses" according to my google searches :)

Regards,
Antonio Huete

@SwampDragons
Copy link
Contributor

Closing this due to old age and questions as to whether it ever worked. If someone wants to pick up development of this feature, I'll gladly review a new PR.

@tuxillo
Copy link

tuxillo commented Apr 5, 2018

Hey wait, I'm still working on this! Sorry for the delay :)

@tuxillo
Copy link

tuxillo commented Apr 5, 2018

All the work will likely happen in #5993, ignore my previous comment.

@ghost
Copy link

ghost commented Apr 1, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants