Skip to content

Commit

Permalink
vm: let libvirt choose the MAC address for us
Browse files Browse the repository at this point in the history
There appear to be some weird conflicts within libvirt when creating a
VM with a MAC address that already has a DHCP lease. To work around
this, make sure to always get a fresh MAC address by letting libvirt
choose one for us.

Also adapt the tests; the fake libvirt implementation should "allocate"
a MAC address for us on DomainDefineXML. We also don't want to assert
the correctness of the returned MAC address anymore, because we don't
deal with that value anymore.
  • Loading branch information
chrboe committed Jul 28, 2020
1 parent 1332299 commit 1915d11
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 14 deletions.
7 changes: 0 additions & 7 deletions internal/virter/dhcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ func ipToID(ipnet net.IPNet, ip net.IP) (uint, error) {
return v - sv, nil
}

func qemuMAC(id uint) string {
id0 := byte((id >> 16) & 0xFF)
id1 := byte((id >> 8) & 0xFF)
id2 := byte(id & 0xFF)
return fmt.Sprintf("52:54:00:%02x:%02x:%02x", id0, id1, id2)
}

func (v *Virter) getIPNet(network libvirt.Network) (net.IPNet, error) {
ipNet := net.IPNet{}

Expand Down
5 changes: 5 additions & 0 deletions internal/virter/libvirt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ func (l *FakeLibvirtConnection) DomainDefineXML(XML string) (rDom libvirt.Domain
if err := description.Unmarshal(XML); err != nil {
return libvirt.Domain{}, fmt.Errorf("invalid domain XML: %w", err)
}
if description.Devices.Interfaces[0].MAC == nil {
description.Devices.Interfaces[0].MAC = &libvirtxml.DomainInterfaceMAC{
Address: "00:11:22:33:44:55",
}
}
l.domains[description.Name] = &FakeLibvirtDomain{
description: description,
persistent: true,
Expand Down
5 changes: 1 addition & 4 deletions internal/virter/libvirtxml.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func vmDisksToLibvirtDisks(vmDisks []VMDisk) ([]lx.DomainDisk, error) {
return result, nil
}

func (v *Virter) vmXML(poolName string, vm VMConfig, mac string) (string, error) {
func (v *Virter) vmXML(poolName string, vm VMConfig) (string, error) {
vmDisks := []VMDisk{
VMDisk{device: VMDiskDeviceDisk, poolName: poolName, volumeName: vm.Name, bus: "virtio", format: "qcow2"},
VMDisk{device: VMDiskDeviceCDROM, poolName: poolName, volumeName: ciDataVolumeName(vm.Name), bus: "ide", format: "raw"},
Expand Down Expand Up @@ -158,9 +158,6 @@ func (v *Virter) vmXML(poolName string, vm VMConfig, mac string) (string, error)
},
Interfaces: []lx.DomainInterface{
lx.DomainInterface{
MAC: &lx.DomainInterfaceMAC{
Address: mac,
},
Source: &lx.DomainInterfaceSource{
Network: &lx.DomainInterfaceSourceNetwork{
Network: v.networkName,
Expand Down
17 changes: 15 additions & 2 deletions internal/virter/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/LINBIT/virter/pkg/netcopy"
libvirtxml "github.com/libvirt/libvirt-go-xml"
"github.com/rck/unit"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -187,8 +188,7 @@ func diskVolumeName(vmName string, diskName string) string {
}

func (v *Virter) createVM(sp libvirt.StoragePool, vmConfig VMConfig) (net.IP, error) {
mac := qemuMAC(vmConfig.ID)
xml, err := v.vmXML(sp.Name, vmConfig, mac)
xml, err := v.vmXML(sp.Name, vmConfig)
if err != nil {
return nil, err
}
Expand All @@ -201,6 +201,19 @@ func (v *Virter) createVM(sp libvirt.StoragePool, vmConfig VMConfig) (net.IP, er
return nil, fmt.Errorf("could not define domain: %w", err)
}

domainXML, err := v.libvirt.DomainGetXMLDesc(d, 0)
if err != nil {
return nil, err
}

domcfg := &libvirtxml.Domain{}
err = domcfg.Unmarshal(domainXML)
if err != nil {
return nil, err
}

mac := domcfg.Devices.Interfaces[0].MAC.Address

// Add DHCP entry after defining the VM to ensure that it can be
// removed when removing the VM, but before starting it to ensure that
// it gets the correct IP address
Expand Down
1 change: 0 additions & 1 deletion internal/virter/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ func TestVMRun(t *testing.T) {
assert.Empty(t, l.vols[vmName].content)

host := l.network.description.IPs[0].DHCP.Hosts[0]
assert.Equal(t, "52:54:00:00:00:2a", host.MAC)
assert.Equal(t, "192.168.122.42", host.IP)

domain := l.domains[vmName]
Expand Down

0 comments on commit 1915d11

Please sign in to comment.