rgbkrk/libvirt-go

add virDomainInterfaceAddresses

vtolstov opened this issue · 12 comments

Does it possible to add virDomainInterfaceAddresses api call ? i'm try myself but stuck at representation of provided data in go

type VirDomainInterface struct {
        ptr *C.virDomainInterfacePtr
        n   int
}

func (d *VirDomain) InterfaceAddresses(src uint) (VirDomainInterface, error) {
        var ptr *C.virDomainInterfacePtr
        vdi := VirDomainInterface{}

        n := C.virDomainInterfaceAddresses(d.ptr, (**C.virDomainInterfacePtr)(unsafe.Pointer(ptr)), C.uint(src), 0)
        if n == -1 {
                return vdi, GetLastError()
        }

        vdi.ptr = ptr
        vdi.n = int(n)

        return vdi, nil
}

func (i *VirDomainInterface) Free() {
        for k := 0; k < i.n; k++ {
                C.virDomainInterfaceFree(unsafe.Pointer(i.ptr[k]))
        }
        C.free(unsafe.Pointer(i.ptr))
}

above code does not build,

invalid operation: i.ptr[k] (type *C.virDomainInterfacePtr does not support indexing)

Looks like that API call was added in 1.2.14 so it won't be included in this Go package, because it won't build against various distro-supplied libvirt (as per README, we are pinned against 1.2.2). In fact I don't have 1.2.14 at all so I can't test your implementation.

But, from what I can see,

  • Take a look at libvirt.go:ListAllInterfaces to see how to work with C pointers and go slices.
  • VirInterface is already defined in the package, so no need to declare VirDomainInterface.
  • virDomainInterfaceFree just takes a virDomainInterfacePtr, which is not an array (hence the indexing error you get)

@alexzorin I need to implement this API for https://github.com/dmacvicar/terraform-provider-libvirt so I will add it to my fork https://github.com/dmacvicar/libvirt-go soon.

What do you think that we create a domain_foo.go interface_foo.go etc?, foo being an arbitrary tag that we can //+build foo in the file for conditional compilation. Either opting in APIs that are > 1.2.14 or opting in support for old distros stuck with ancient libvirt versions.

(side note, VirInterface represent an interface on the host, VirDomainInterface which is the one @vtolstov was trying to add, represents an interface on the guest, and they are different structs used for different function calls)

cc @rgbkrk I'm not sure what the approach with new versions/features is anymore

I'd rather us move forward supporting newer libvirt. The interface additions sound good to me.

/cc @hinesmr

Ok, I will prepare a pull request with this API call. Thanks everyone.

Newer libvirt is always good. @rgbkrk Can we branch and tag master at the supported version and still let people backport pull-requests to that branch if they need it?

Yeah, that sounds good. We can also maintain those branches on gopkg.in for the releases, as has been done in the past.

I'd actually love to have you take the lead in general @hinesmr, because my role has largely changed for work and I'm about to make a transition.

@rgbkrk I appreciate the offer, but my golang experience is pretty young - so I don't think I would qualify. We are using these bindings at DigitalOcean, though, so I'm happy to play a supporting role, as we will keep using them.

Any other takers on being a maintainer? I'm happy to help vet our "libvirt policies" whenever patches come in.....

All good, figured it was worth asking. Happy to have you.