Rework NetworkUpdate workaround (#950)

* Switch from github.com/libvirt/libvirt-go-xml to libvirt.org/go/libvirtxml

This is the same go module, it was renamed so that it can start
following go module versioning (only change major version if there are
API/ABI breakages).
github.com/libvirt/libvirt-go-xml is no longer maintained/updated.

* Remove NetworkUpdate workaround

The libvirt version check which is currently to decide whether to swap
libvirt.NetworkUpdate arguments is not enough. The fix was backported to
older libvirt in RHEL/CentOS (for example in
libvirt-6.0.0-37.1.module+el8.5.0+13858+39fdc467 ), and the current
version checking code would be swapping the arguments when it's no
longer needed.

This commit uses NetworkUpdateCompat which was recently introduced in
digitalocean/go-libvirt, and which takes care of this by checking
libvirt connection features.

* update golangci-lint

Co-authored-by: Duncan Mac-Vicar P <duncan@mac-vicar.eu>

.github/workflows/test.yml

@@ -27,7 +27,7 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.43.0
+          version: v1.46.2
           only-new-issues: true
   build:
     name: Build

go.mod

@@ -4,13 +4,12 @@ require (
 	github.com/c4milo/gotoolkit v0.0.0-20170704181456-e37eeabad07e // indirect
 	github.com/coreos/go-semver v0.3.0 // indirect
 	github.com/davecgh/go-spew v1.1.1
-	github.com/digitalocean/go-libvirt v0.0.0-20210723161134-761cfeeb5968
+	github.com/digitalocean/go-libvirt v0.0.0-20220616141158-7ed4ed4decd9
 	github.com/fatih/color v1.10.0 // indirect
 	github.com/google/uuid v1.1.2
 	github.com/hashicorp/terraform-plugin-sdk v1.9.0
 	github.com/hooklift/assert v0.0.0-20170704181755-9d1defd6d214 // indirect
 	github.com/hooklift/iso9660 v1.0.0
-	github.com/libvirt/libvirt-go-xml v7.4.0+incompatible
 	github.com/mattn/goveralls v0.0.2
 	github.com/pborman/uuid v1.2.0 // indirect
 	github.com/stretchr/testify v1.7.0
@@ -19,6 +18,7 @@ require (
 	golang.org/x/crypto v0.0.0-20220112180741-5e0467b6c7ce
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b
 	gopkg.in/yaml.v2 v2.4.0 // indirect
+	libvirt.org/go/libvirtxml v1.8003.0
 )
 
 replace git.apache.org/thrift.git => github.com/apache/thrift v0.0.0-20180902110319-2566ecd5d999

go.sum

@@ -63,6 +63,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/digitalocean/go-libvirt v0.0.0-20210723161134-761cfeeb5968 h1:ZdYBqLPrXioo+1Z97PWaTK4+jRcS45BI6JlepKtkPKI=
 github.com/digitalocean/go-libvirt v0.0.0-20210723161134-761cfeeb5968/go.mod h1:o129ljs6alsIQTc8d6eweihqpmmrbxZ2g1jhgjhPykI=
+github.com/digitalocean/go-libvirt v0.0.0-20220616141158-7ed4ed4decd9 h1:sORhX0pmJTbieCx5NMHcpTESQTQIJry+OGlSDZO7yKQ=
+github.com/digitalocean/go-libvirt v0.0.0-20220616141158-7ed4ed4decd9/go.mod h1:o129ljs6alsIQTc8d6eweihqpmmrbxZ2g1jhgjhPykI=
 github.com/dmacvicar/golang-x-crypto v0.0.0-20220126233154-a96af8f07497 h1:LBVsZYaxAt1v07hCDdhjL7F13238aw7ZLUOypcj/Uh8=
 github.com/dmacvicar/golang-x-crypto v0.0.0-20220126233154-a96af8f07497/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
@@ -180,8 +182,6 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
-github.com/libvirt/libvirt-go-xml v7.4.0+incompatible h1:+BBo2XjlT8pAK4pm+aSX8mC/6nc/rdRac10ZukpW31U=
-github.com/libvirt/libvirt-go-xml v7.4.0+incompatible/go.mod h1:oBlgD3xOA01ihiK5stbhFzvieyW+jVS6kbbsMVF623A=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
@@ -448,4 +448,6 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
+libvirt.org/go/libvirtxml v1.8003.0 h1:7mwIQNem9/2xaRZONhUUh6z3W1do/6SqEHWlv5B6tes=
+libvirt.org/go/libvirtxml v1.8003.0/go.mod h1:7Oq2BLDstLr/XtoQD8Fr3mfDNrzlI3utYKySXF2xkng=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=

libvirt/disk_def.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"math/rand"
 
-	"github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 const oui = "05abcd"

libvirt/domain.go

@@ -15,7 +15,7 @@ import (
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 const domWaitLeaseStillWaiting = "waiting-addresses"

libvirt/domain_def.go

@@ -7,7 +7,7 @@ import (
 
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 // from existing domain return its  XMLdefintion

libvirt/helpers_test.go

@@ -13,8 +13,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/terraform"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
 	"github.com/terraform-providers/terraform-provider-ignition/ignition"
+	"libvirt.org/go/libvirtxml"
 )
 
 // This file contain function helpers used for testsuite/testacc

libvirt/network.go

@@ -9,7 +9,7 @@ import (
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func waitForNetworkActive(virConn *libvirt.Libvirt, network libvirt.Network) resource.StateRefreshFunc {

libvirt/network_def.go

@@ -7,7 +7,7 @@ import (
 	"net"
 
 	libvirt "github.com/digitalocean/go-libvirt"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 // HasDHCP checks if the network has a DHCP server managed by libvirt
@@ -87,9 +87,9 @@ func addHost(virConn *libvirt.Libvirt, n libvirt.Network, ip, mac, name string,
 	log.Printf("Adding host with XML:\n%s", xmlDesc)
 	// From https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkUpdateFlags
 	// Update live and config for hosts to make update permanent across reboots
-	//
-	// See networkUpdateWorkAroundLibvirt for more information about why this wrapper method exists
-	return (&networkUpdateWorkaroundLibvirt{virConn}).NetworkUpdate(n, uint32(libvirt.NetworkUpdateCommandAddLast), uint32(libvirt.NetworkSectionIPDhcpHost), int32(xmlIdx), xmlDesc, libvirt.NetworkUpdateAffectConfig|libvirt.NetworkUpdateAffectLive)
+	return virConn.NetworkUpdateCompat(n, libvirt.NetworkUpdateCommandAddLast,
+		libvirt.NetworkSectionIPDhcpHost, int32(xmlIdx), xmlDesc,
+		libvirt.NetworkUpdateAffectConfig|libvirt.NetworkUpdateAffectLive)
 }
 
 // Update a static host from the network
@@ -98,9 +98,9 @@ func updateHost(virConn *libvirt.Libvirt, n libvirt.Network, ip, mac, name strin
 	log.Printf("Updating host with XML:\n%s", xmlDesc)
 	// From https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkUpdateFlags
 	// Update live and config for hosts to make update permanent across reboots
-	//
-	// See networkUpdateWorkAroundLibvirt for more information about why this wrapper method exists
-	return (&networkUpdateWorkaroundLibvirt{virConn}).NetworkUpdate(n, uint32(libvirt.NetworkUpdateCommandModify), uint32(libvirt.NetworkSectionIPDhcpHost), int32(xmlIdx), xmlDesc, libvirt.NetworkUpdateAffectConfig|libvirt.NetworkUpdateAffectLive)
+	return virConn.NetworkUpdateCompat(n, libvirt.NetworkUpdateCommandModify,
+		libvirt.NetworkSectionIPDhcpHost, int32(xmlIdx), xmlDesc,
+		libvirt.NetworkUpdateAffectConfig|libvirt.NetworkUpdateAffectLive)
 }
 
 // Get the network index of the target network

libvirt/network_def_test.go

@@ -6,7 +6,7 @@ import (
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func init() {

libvirt/network_dns.go

@@ -10,7 +10,7 @@ import (
 
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 // updateDNSHosts detects changes in the DNS hosts entries
@@ -50,8 +50,8 @@ func updateDNSHosts(d *schema.ResourceData, meta interface{}, network libvirt.Ne
 				return fmt.Errorf("serialize update: %s", err)
 			}
 
-			// See networkUpdateWorkAroundLibvirt for more information about why this wrapper method exists
-			err = (&networkUpdateWorkaroundLibvirt{virConn}).NetworkUpdate(network, uint32(libvirt.NetworkUpdateCommandDelete), uint32(libvirt.NetworkSectionDNSHost), -1, data, libvirt.NetworkUpdateAffectLive|libvirt.NetworkUpdateAffectConfig)
+			err = virConn.NetworkUpdateCompat(network, libvirt.NetworkUpdateCommandDelete,
+				libvirt.NetworkSectionDNSHost, -1, data, libvirt.NetworkUpdateAffectLive|libvirt.NetworkUpdateAffectConfig)
 			if err != nil {
 				return fmt.Errorf("delete %s: %s", oldEntry.IP, err)
 			}
@@ -75,8 +75,8 @@ func updateDNSHosts(d *schema.ResourceData, meta interface{}, network libvirt.Ne
 				return fmt.Errorf("serialize update: %s", err)
 			}
 
-			// See networkUpdateWorkAroundLibvirt for more information about why this wrapper method exists
-			err = (&networkUpdateWorkaroundLibvirt{virConn}).NetworkUpdate(network, uint32(libvirt.NetworkUpdateCommandAddLast), uint32(libvirt.NetworkSectionDNSHost), -1, data, libvirt.NetworkUpdateAffectLive|libvirt.NetworkUpdateAffectConfig)
+			err = virConn.NetworkUpdateCompat(network, libvirt.NetworkUpdateCommandAddLast,
+				libvirt.NetworkSectionDNSHost, -1, data, libvirt.NetworkUpdateAffectLive|libvirt.NetworkUpdateAffectConfig)
 			if err != nil {
 				return fmt.Errorf("add %v: %s", newEntry, err)
 			}

libvirt/network_routes.go

@@ -6,7 +6,7 @@ import (
 	"net"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	"github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 // getRoutesFromResource gets the libvirt network routes from a network definition

libvirt/resource_libvirt_domain.go

@@ -13,7 +13,7 @@ import (
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/dmacvicar/terraform-provider-libvirt/libvirt/helper/suppress"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 type pendingMapping struct {

libvirt/resource_libvirt_domain_test.go

@@ -15,7 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/terraform"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func TestAccLibvirtDomain_Basic(t *testing.T) {

libvirt/resource_libvirt_network.go

@@ -10,7 +10,7 @@ import (
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 const (

libvirt/resource_libvirt_network_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/terraform"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func TestAccLibvirtNetwork_Addresses(t *testing.T) {

libvirt/resource_libvirt_pool.go

@@ -7,7 +7,7 @@ import (
 
 	libvirt "github.com/digitalocean/go-libvirt"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func resourceLibvirtPool() *schema.Resource {

libvirt/utils_domain_def.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 
 	libvirt "github.com/digitalocean/go-libvirt"
-	libvirtxml "github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func getGuestForArchType(caps libvirtxml.Caps, arch string, virttype string) (libvirtxml.CapsGuest, error) {

libvirt/utils_net.go

@@ -5,39 +5,12 @@ import (
 	"math/rand"
 	"net"
 	"time"
-
-	libvirt "github.com/digitalocean/go-libvirt"
 )
 
 const (
 	maxIfaceNum = 100
 )
 
-// Wrapper to work-around the NetworkUpdate swapper parameters bug.
-// Unfortunately, as we don't know if the remote dispatcher is fixed,
-// we need to check the version.
-// the official libvirt client does some introspection and swaps internally as well.
-//
-// See https://listman.redhat.com/archives/libvir-list/2021-March/msg00054.html
-// and https://listman.redhat.com/archives/libvir-list/2021-March/msg00760.html
-type networkUpdateWorkaroundLibvirt struct {
-	*libvirt.Libvirt
-}
-
-func (l *networkUpdateWorkaroundLibvirt) NetworkUpdate(Net libvirt.Network, Command uint32, Section uint32, ParentIndex int32, XML string, Flags libvirt.NetworkUpdateFlags) (err error) {
-	version, err := l.ConnectGetLibVersion()
-	if err != nil {
-		return fmt.Errorf("failed to retrieve libvirt version: %w", err)
-	}
-
-	// https://gitlab.com/libvirt/libvirt/-/commit/b0f78d626a18bcecae3a4d165540ab88bfbfc9ee
-	// order is fixed since 7.2.0
-	if version < 7002000 {
-		return l.Libvirt.NetworkUpdate(Net, Section, Command, ParentIndex, XML, Flags)
-	}
-	return l.Libvirt.NetworkUpdate(Net, Command, Section, ParentIndex, XML, Flags)
-}
-
 // randomMACAddress returns a randomized MAC address
 // with libvirt prefix
 func randomMACAddress() (string, error) {

libvirt/volume_def.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 
 	libvirt "github.com/digitalocean/go-libvirt"
-	"github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 func newDefVolume() libvirtxml.StorageVolume {

libvirt/volume_image.go

@@ -14,7 +14,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/libvirt/libvirt-go-xml"
+	"libvirt.org/go/libvirtxml"
 )
 
 // network transparent image

libvirt/volume_image_test.go

@@ -14,9 +14,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/libvirt/libvirt-go-xml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"libvirt.org/go/libvirtxml"
 )
 
 func TestNewImage(t *testing.T) {