-
Notifications
You must be signed in to change notification settings - Fork 111
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
Use the libvirt API to obtain domain IP address #40
base: master
Are you sure you want to change the base?
Conversation
@@ -1,4 +1,4 @@ | |||
default: build | |||
|
|||
build: | |||
GOGC=off go build -i -o docker-machine-driver-kvm | |||
GOGC=off go build -tags libvirt.1.2.14 -i -o docker-machine-driver-kvm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I don't have a physical ubuntu 14.04 box (or other older LTS distro) to try this out at the moment. Was this alone sufficient to have this all hold together where it both compiles and doesn't do something nasty at runtime when the underlying API isn't present? Maybe enough time has passed that we can ignore the older LTS distros, but I was hoping to keep this compatible for a while if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have access to such a machine. However latest Ubuntu LTS (xenial 16.04LTS) has version 1.3.1 available.
Personally I would ignore 14.04.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been meaning to improve the build a bit... and this helped motivate me to get some extra build support in place to generate binaries for various host systems. f328c4b
I think we should be able to at least partially answer the compatibility question on this PR by seeing if it builds on ubuntu14.04. Then we could try to do a create in a container, which should fail due to the missing libvirt socket, but as long as it errors that way and not with some obscure C lib link problem, then I think we're good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased my changes against your master, this is the error I get when I try to build the code inside of 14.04:
# github.com/dhiltgen/docker-machine-kvm
./kvm.go:596: cannot use uint(libvirt.DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE) (type uint) as type libvirt.DomainInterfaceAddressesSource in argument to d.VM.ListAllInterfaceAddresses
Do you think it's acceptable?
Requires libvirt >= 1.2.6. The tag libvirt.1.2.14 is required by libvirt-go to enable its use, since a wider range of libvirt versions are supported there. Initial patch provided by Casey Marshall <[email protected]>
Changed the code to use the native libvirt API provided by the official Go bindings. Signed-off-by: Flavio Castelli <[email protected]>
ccf9e5a
to
f720110
Compare
This is a complete revamp of #2.
The code has been changed to use the right API provided by the official libvirt-go bindings. The previous PR was using a different Go binding library.