[libvirt] [PATCH 0/3] Refactor formatting of MAC addresses and domif-getlink virsh command
This series refactors libvirt XML code generator to create uniform mac addresses with lowercase hex characters and refactors the domif-getlink command to avoid a bug and too complex code. Peter Krempa (3): util: Change virMacAddrFormat to lowercase hex characters conf: Use virMacAddrFormat while generating domain XML files virsh-domain-monitor: Refactor cmdDomIfGetLink src/conf/domain_conf.c | 7 +- src/util/virmacaddr.c | 2 +- tests/networkxml2xmlout/bandwidth-network.xml | 2 +- .../networkxml2xmlout/dhcp6host-routed-network.xml | 2 +- tests/networkxml2xmlout/empty-allow-ipv6.xml | 2 +- tests/networkxml2xmlout/isolated-network.xml | 2 +- tests/networkxml2xmlout/routed-network.xml | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx | 2 +- tools/virsh-domain-monitor.c | 100 ++++++++------------- 21 files changed, 62 insertions(+), 87 deletions(-) -- 1.8.1.5
The domain XML generator creates the mac addres strings with lowercase strings with a separate piece of code. This patch changes the formating helper to do the same stuff to allow using it to normalize a string provided by the user. After this change some of the tests that are outputing the mac address will need to be changed. --- src/util/virmacaddr.c | 2 +- tests/networkxml2xmlout/bandwidth-network.xml | 2 +- tests/networkxml2xmlout/dhcp6host-routed-network.xml | 2 +- tests/networkxml2xmlout/empty-allow-ipv6.xml | 2 +- tests/networkxml2xmlout/isolated-network.xml | 2 +- tests/networkxml2xmlout/routed-network.xml | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx | 4 ++-- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx | 4 ++-- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx | 2 +- tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx | 2 +- 19 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c index 9d6ab7c..c4ca0a8 100644 --- a/src/util/virmacaddr.c +++ b/src/util/virmacaddr.c @@ -191,7 +191,7 @@ virMacAddrFormat(const virMacAddrPtr addr, char *str) { snprintf(str, VIR_MAC_STRING_BUFLEN, - "%02X:%02X:%02X:%02X:%02X:%02X", + "%02x:%02x:%02x:%02x:%02x:%02x", addr->addr[0], addr->addr[1], addr->addr[2], addr->addr[3], addr->addr[4], addr->addr[5]); str[VIR_MAC_STRING_BUFLEN-1] = '\0'; diff --git a/tests/networkxml2xmlout/bandwidth-network.xml b/tests/networkxml2xmlout/bandwidth-network.xml index 555ee18..c0c8ac3 100644 --- a/tests/networkxml2xmlout/bandwidth-network.xml +++ b/tests/networkxml2xmlout/bandwidth-network.xml @@ -3,7 +3,7 @@ <uuid>986fed9e-a488-186d-ef2d-17ebfd1993f8</uuid> <forward mode='nat'/> <bridge name='virbr1' stp='on' delay='0' /> - <mac address='52:54:00:E6:A2:C9'/> + <mac address='52:54:00:e6:a2:c9'/> <bandwidth> <inbound average='1000' peak='2000' burst='1024'/> <outbound average='2000'/> diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml b/tests/networkxml2xmlout/dhcp6host-routed-network.xml index 7305043..1d3035b 100644 --- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml +++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml @@ -5,7 +5,7 @@ <interface dev='eth1'/> </forward> <bridge name='virbr1' stp='on' delay='0' /> - <mac address='12:34:56:78:9A:BC'/> + <mac address='12:34:56:78:9a:bc'/> <ip address='192.168.122.1' netmask='255.255.255.0'> <dhcp> <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10' /> diff --git a/tests/networkxml2xmlout/empty-allow-ipv6.xml b/tests/networkxml2xmlout/empty-allow-ipv6.xml index 6ee55ad..53e4fa7 100644 --- a/tests/networkxml2xmlout/empty-allow-ipv6.xml +++ b/tests/networkxml2xmlout/empty-allow-ipv6.xml @@ -2,5 +2,5 @@ <name>empty</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> <bridge name='virbr7' stp='on' delay='0' /> - <mac address='52:54:00:17:3F:47'/> + <mac address='52:54:00:17:3f:47'/> </network> diff --git a/tests/networkxml2xmlout/isolated-network.xml b/tests/networkxml2xmlout/isolated-network.xml index cc320a9..9ff1f55 100644 --- a/tests/networkxml2xmlout/isolated-network.xml +++ b/tests/networkxml2xmlout/isolated-network.xml @@ -2,7 +2,7 @@ <name>private</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> <bridge name='virbr2' stp='on' delay='0' /> - <mac address='52:54:00:17:3F:37'/> + <mac address='52:54:00:17:3f:37'/> <ip address='192.168.152.1' netmask='255.255.255.0'> <dhcp> <range start='192.168.152.2' end='192.168.152.254' /> diff --git a/tests/networkxml2xmlout/routed-network.xml b/tests/networkxml2xmlout/routed-network.xml index 9235e15..cb81bae 100644 --- a/tests/networkxml2xmlout/routed-network.xml +++ b/tests/networkxml2xmlout/routed-network.xml @@ -5,7 +5,7 @@ <interface dev='eth1'/> </forward> <bridge name='virbr1' stp='on' delay='0' /> - <mac address='12:34:56:78:9A:BC'/> + <mac address='12:34:56:78:9a:bc'/> <ip address='192.168.122.1' netmask='255.255.255.0'> </ip> </network> diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx index 9059197..b2b5668 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.vmx @@ -18,4 +18,4 @@ ethernet0.present = "true" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "vpx" -ethernet0.generatedAddress = "00:50:56:91:48:C7" +ethernet0.generatedAddress = "00:50:56:91:48:c7" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx index 68f069b..a0199cc 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.vmx @@ -37,5 +37,5 @@ ethernet0.virtualDev = "vlance" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:3C:98:3E" +ethernet0.generatedAddress = "00:0c:29:3c:98:3e" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx index e09c694..4ab2574 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.vmx @@ -23,5 +23,5 @@ ethernet0.present = "true" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:F5:C3:0C" +ethernet0.generatedAddress = "00:0c:29:f5:c3:0c" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx index 504997f..a2d679f 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.vmx @@ -18,12 +18,12 @@ ethernet0.present = "true" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "vpx" -ethernet0.generatedAddress = "00:50:56:91:66:D4" +ethernet0.generatedAddress = "00:50:56:91:66:d4" ethernet1.present = "true" ethernet1.networkName = "VM Switch 2" ethernet1.connectionType = "bridged" ethernet1.addressType = "vpx" -ethernet1.generatedAddress = "00:50:56:91:0C:51" +ethernet1.generatedAddress = "00:50:56:91:0c:51" serial0.present = "true" serial0.fileType = "file" serial0.fileName = "/vmfs/volumes/498076b2-02796c1a-ef5b-000ae484a6a3/virtMonServ1/serial1.file" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx index 2e3b856..192867e 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx @@ -24,4 +24,4 @@ ethernet0.virtualDev = "e1000" ethernet0.networkName = "VM-LAN" ethernet0.connectionType = "bridged" ethernet0.addressType = "vpx" -ethernet0.generatedAddress = "00:50:56:BE:00:15" +ethernet0.generatedAddress = "00:50:56:be:00:15" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx index 1f29ae5..bc6a835 100644 --- a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx @@ -21,6 +21,6 @@ ethernet0.virtualDev = "vmxnet3" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:2C:3A:FC" +ethernet0.generatedAddress = "00:0c:29:2c:3a:fc" ethernet0.generatedAddressOffset = "0" svga.vramSize = "8388608" diff --git a/tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx b/tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx index 4f5c47b..18a6b67 100644 --- a/tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx +++ b/tests/xml2vmxdata/xml2vmx-ethernet-generated.vmx @@ -12,5 +12,5 @@ ethernet0.present = "true" ethernet0.networkName = "VM Network" ethernet0.connectionType = "bridged" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:11:22:33" +ethernet0.generatedAddress = "00:0c:29:11:22:33" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx index 48476d6..9893595 100644 --- a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx +++ b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.vmx @@ -16,5 +16,5 @@ ethernet0.networkName = "net1" ethernet0.connectionType = "custom" ethernet0.vnet = "/dev/vmnet1" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:D6:2B:D3" +ethernet0.generatedAddress = "00:0c:29:d6:2b:d3" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx index 757083a..c597807 100644 --- a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx +++ b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.vmx @@ -16,5 +16,5 @@ ethernet0.networkName = "net1" ethernet0.connectionType = "custom" ethernet0.vnet = "/dev/vmnet1" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:D6:CB:A4" +ethernet0.generatedAddress = "00:0c:29:d6:cb:a4" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx index 74cd47d..a07ecfe 100644 --- a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx +++ b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.vmx @@ -16,12 +16,12 @@ ethernet0.networkName = "net1" ethernet0.connectionType = "custom" ethernet0.vnet = "/dev/vmnet1" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:C4:BE:5A" +ethernet0.generatedAddress = "00:0c:29:c4:be:5a" ethernet0.generatedAddressOffset = "0" ethernet1.present = "true" ethernet1.networkName = "net2" ethernet1.connectionType = "custom" ethernet1.vnet = "/dev/vmnet2" ethernet1.addressType = "generated" -ethernet1.generatedAddress = "00:0C:29:C4:BE:64" +ethernet1.generatedAddress = "00:0c:29:c4:be:64" ethernet1.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx index c9da229..1361954 100644 --- a/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx +++ b/tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.vmx @@ -16,5 +16,5 @@ ethernet0.networkName = "net2" ethernet0.connectionType = "custom" ethernet0.vnet = "/dev/vmnet2" ethernet0.addressType = "generated" -ethernet0.generatedAddress = "00:0C:29:C5:E3:5D" +ethernet0.generatedAddress = "00:0c:29:c5:e3:5d" ethernet0.generatedAddressOffset = "0" diff --git a/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx b/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx index 539a371..4211a67 100644 --- a/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx +++ b/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.vmx @@ -17,6 +17,6 @@ ethernet0.present = "true" ethernet0.virtualDev = "e1000" ethernet0.connectionType = "nat" ethernet0.addressType = "static" -ethernet0.address = "00:90:B9:DC:EA:81" +ethernet0.address = "00:90:b9:dc:ea:81" ethernet0.checkMACAddress = "false" svga.vramSize = "4194304" diff --git a/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx b/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx index 7035ac3..ad68df9 100644 --- a/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx +++ b/tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx @@ -17,6 +17,6 @@ ethernet0.present = "true" ethernet0.virtualDev = "e1000" ethernet0.connectionType = "bridged" ethernet0.addressType = "static" -ethernet0.address = "00:90:B9:DC:EA:81" +ethernet0.address = "00:90:b9:dc:ea:81" ethernet0.checkMACAddress = "false" svga.vramSize = "4194304" -- 1.8.1.5
On 03/26/2013 12:26 PM, Peter Krempa wrote:
The domain XML generator creates the mac addres strings with lowercase strings with a separate piece of code. This patch changes the formating helper to do the same stuff to allow using it to normalize a string provided by the user. After this change some of the tests that are outputing the mac address will need to be changed. ---
ACK, I see no problem with changing this to lowercase, especially when it brings us easier use of matching mac addresses in some cases. Martin
Format the address using the helper to avoid code duplication. --- src/conf/domain_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4cae0d3..40dfc99 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13354,6 +13354,7 @@ virDomainNetDefFormat(virBufferPtr buf, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + char macstr[VIR_MAC_STRING_BUFLEN]; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -13369,10 +13370,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 6); - virBufferAsprintf(buf, - "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(buf, "<mac address='%s'/>\n", + virMacAddrFormat(&def->mac, macstr)); switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: -- 1.8.1.5
On 03/26/2013 12:26 PM, Peter Krempa wrote:
Format the address using the helper to avoid code duplication. --- src/conf/domain_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4cae0d3..40dfc99 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13354,6 +13354,7 @@ virDomainNetDefFormat(virBufferPtr buf, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + char macstr[VIR_MAC_STRING_BUFLEN];
if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -13369,10 +13370,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 6); - virBufferAsprintf(buf, - "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(buf, "<mac address='%s'/>\n", + virMacAddrFormat(&def->mac, macstr));
switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK:
I'd ACK that, fine use for the cleanup, but pity it's fixed on only one place. How about squashing in the following? diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 40dfc99..ba3bb59 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11486,14 +11486,15 @@ static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src, virDomainNetDefPtr dst) { + char srcmac[VIR_MAC_STRING_BUFLEN]; + char dstmac[VIR_MAC_STRING_BUFLEN]; + if (virMacAddrCmp(&src->mac, &dst->mac) != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target network card mac %02x:%02x:%02x:%02x:%02x:%02x" - " does not match source %02x:%02x:%02x:%02x:%02x:%02x"), - dst->mac.addr[0], dst->mac.addr[1], dst->mac.addr[2], - dst->mac.addr[3], dst->mac.addr[4], dst->mac.addr[5], - src->mac.addr[0], src->mac.addr[1], src->mac.addr[2], - src->mac.addr[3], src->mac.addr[4], src->mac.addr[5]); + _("Target network card mac %s" + " does not match source %s"), + virMacAddrFormat(&dst->mac, dstmac), + virMacAddrFormat(&src->mac, srcmac)); return false; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a0c278f..d539dbd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3635,12 +3635,12 @@ qemuBuildNicStr(virDomainNetDefPtr net, int vlan) { char *str; + char macaddr[VIR_MAC_STRING_BUFLEN]; + if (virAsprintf(&str, - "%smacaddr=%02x:%02x:%02x:%02x:%02x:%02x,vlan=%d%s%s%s%s", + "%smacaddr=%s,vlan=%d%s%s%s%s", prefix ? prefix : "", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5], + virMacAddrFormat(&net->mac, macaddr), vlan, (net->model ? ",model=" : ""), (net->model ? net->model : ""), @@ -3663,6 +3663,7 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, virBuffer buf = VIR_BUFFER_INITIALIZER; const char *nic; bool usingVirtio = false; + char macaddr[VIR_MAC_STRING_BUFLEN]; if (!net->model) { nic = "rtl8139"; @@ -3719,10 +3720,8 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, else virBufferAsprintf(&buf, ",vlan=%d", vlan); virBufferAsprintf(&buf, ",id=%s", net->info.alias); - virBufferAsprintf(&buf, ",mac=%02x:%02x:%02x:%02x:%02x:%02x", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5]); + virBufferAsprintf(&buf, ",mac=%s", + virMacAddrFormat(&net->mac, macaddr)); if (qemuBuildDeviceAddressStr(&buf, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info, qemuCaps) < 0) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index b3ac326..0fe59fb 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -164,6 +164,7 @@ umlBuildCommandLineNet(virConnectPtr conn, int idx) { virBuffer buf = VIR_BUFFER_INITIALIZER; + char macaddr[VIR_MAC_STRING_BUFLEN]; /* General format: ethNN=type,options */ @@ -264,9 +265,7 @@ umlBuildCommandLineNet(virConnectPtr conn, goto error; } - virBufferAsprintf(&buf, ",%02x:%02x:%02x:%02x:%02x:%02x", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(&buf, ",%s", virMacAddrFormat(&def->mac, macaddr)); if (def->type == VIR_DOMAIN_NET_TYPE_MCAST) { virBufferAsprintf(&buf, ",%s,%d", diff --git a/src/util/virebtables.c b/src/util/virebtables.c index ded62d7..e1c18f0 100644 --- a/src/util/virebtables.c +++ b/src/util/virebtables.c @@ -1,7 +1,7 @@ /* * virebtables.c: Helper APIs for managing ebtables * - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * Copyright (C) 2009 IBM Corp. * * This library is free software; you can redistribute it and/or @@ -447,14 +447,10 @@ ebtablesAddForwardAllowIn(ebtablesContext *ctx, const virMacAddrPtr mac) { char *macaddr; - - if (virAsprintf(&macaddr, - "%02x:%02x:%02x:%02x:%02x:%02x", - mac->addr[0], mac->addr[1], - mac->addr[2], mac->addr[3], - mac->addr[4], mac->addr[5]) < 0) { + if (VIR_ALLOC_N(macaddr, VIR_MAC_STRING_BUFLEN) < 0) return -1; - } + + virMacAddrFormat(mac, macaddr); return ebtablesForwardAllowIn(ctx, iface, macaddr, ADD); } @@ -476,13 +472,9 @@ ebtablesRemoveForwardAllowIn(ebtablesContext *ctx, const virMacAddrPtr mac) { char *macaddr; + if (VIR_ALLOC_N(macaddr, VIR_MAC_STRING_BUFLEN) < 0) + return -1; - if (virAsprintf(&macaddr, - "%02x:%02x:%02x:%02x:%02x:%02x", - mac->addr[0], mac->addr[1], - mac->addr[2], mac->addr[3], - mac->addr[4], mac->addr[5]) < 0) { - return -1; - } + virMacAddrFormat(mac, macaddr); return ebtablesForwardAllowIn(ctx, iface, macaddr, REMOVE); } diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index ddea11f..2578ff0 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2010-2012 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -518,6 +518,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, virNetlinkCallbackDataPtr calld = opaque; pid_t lldpad_pid = 0; pid_t virip_pid = 0; + char macaddr[VIR_MAC_STRING_BUFLEN]; hdr = (struct nlmsghdr *) msg; data = nlmsg_data(hdr); @@ -707,11 +708,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, VIR_INFO("Re-send 802.1qbg associate request:"); VIR_INFO(" if: %s", calld->cr_ifname); VIR_INFO(" lf: %s", calld->linkdev); - VIR_INFO(" mac: %02x:%02x:%02x:%02x:%02x:%02x", - calld->macaddress.addr[0], calld->macaddress.addr[1], - calld->macaddress.addr[2], calld->macaddress.addr[3], - calld->macaddress.addr[4], calld->macaddress.addr[5]); - + VIR_INFO(" mac: %s", virMacAddrFormat(&calld->macaddress, macaddr)); ignore_value(virNetDevVPortProfileAssociate(calld->cr_ifname, calld->virtPortProfile, &calld->macaddress, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index a884de1..871376e 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -286,6 +286,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, unsigned int flags) { virMacAddr tapmac; + char macaddrstr[VIR_MAC_STRING_BUFLEN]; if (virNetDevTapCreate(ifname, tapfd, flags) < 0) return -1; @@ -306,10 +307,8 @@ int virNetDevTapCreateInBridgePort(const char *brname, */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unable to use MAC address starting with " - "reserved value 0xFE - '%02X:%02X:%02X:%02X:%02X:%02X' - "), - macaddr->addr[0], macaddr->addr[1], - macaddr->addr[2], macaddr->addr[3], - macaddr->addr[4], macaddr->addr[5]); + "reserved value 0xFE - '%s' - "), + virMacAddrFormat(macaddr, macaddrstr)); goto error; } tapmac.addr[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 398da0d..835296a 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3730,17 +3730,14 @@ virDomainXMLDevID(virDomainPtr domain, if (tmp == NULL) return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - char mac[30]; + char mac[VIR_MAC_STRING_BUFLEN]; virDomainNetDefPtr def = dev->data.net; - snprintf(mac, sizeof(mac), "%02x:%02x:%02x:%02x:%02x:%02x", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virMacAddrFormat(&def->mac, mac)); strcpy(class, "vif"); xenUnifiedLock(priv); - xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, - mac); + xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, mac); xenUnifiedUnlock(priv); if (xref == NULL) return -1; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 83b7c74..cc4225b 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1,7 +1,7 @@ /* * xen_sxpr.c: Xen SEXPR parsing functions * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * @@ -1914,6 +1914,7 @@ xenFormatSxprNet(virConnectPtr conn, int isAttach) { const char *script = DEFAULT_VIF_SCRIPT; + char macaddr[VIR_MAC_STRING_BUFLEN]; if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && def->type != VIR_DOMAIN_NET_TYPE_NETWORK && @@ -1936,10 +1937,7 @@ xenFormatSxprNet(virConnectPtr conn, virBufferAddLit(buf, "(vif "); - virBufferAsprintf(buf, - "(mac '%02x:%02x:%02x:%02x:%02x:%02x')", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(buf, "(mac '%s')", virMacAddrFormat(&def->mac, macaddr)); switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 73ba06b..405ebf3 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,7 +1,7 @@ /* * xen_xm.c: Xen XM parsing functions * - * Copyright (C) 2006-2007, 2009-2010, 2012 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2010, 2012, 2013 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2006 Daniel P. Berrange * @@ -1335,11 +1335,9 @@ static int xenFormatXMNet(virConnectPtr conn, { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; + char macaddr[VIR_MAC_STRING_BUFLEN]; - virBufferAsprintf(&buf, "mac=%02x:%02x:%02x:%02x:%02x:%02x", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5]); + virBufferAsprintf(&buf, "mac=%s", virMacAddrFormat(&net->mac, macaddr)); switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: -- Martin
On 03/26/2013 08:06 AM, Martin Kletzander wrote:
On 03/26/2013 12:26 PM, Peter Krempa wrote:
Format the address using the helper to avoid code duplication. --- src/conf/domain_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
I'd ACK that, fine use for the cleanup, but pity it's fixed on only one place. How about squashing in the following?
The followup also looks fine. Is this something you were planning on pushing in 1.0.4? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/27/13 22:45, Eric Blake wrote:
On 03/26/2013 08:06 AM, Martin Kletzander wrote:
On 03/26/2013 12:26 PM, Peter Krempa wrote:
Format the address using the helper to avoid code duplication. --- src/conf/domain_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
I'd ACK that, fine use for the cleanup, but pity it's fixed on only one place. How about squashing in the following?
The followup also looks fine. Is this something you were planning on pushing in 1.0.4?
No, I'll rather wait until the release is out. Peter
Format the address using the helper instead of having similar code in multiple places. This patch also fixes leak of the MAC address string in ebtablesRemoveForwardAllowIn() and ebtablesAddForwardAllowIn() in src/util/virebtables.c --- src/conf/domain_conf.c | 20 ++++++++++---------- src/qemu/qemu_command.c | 15 +++++++-------- src/uml/uml_conf.c | 7 +++---- src/util/virebtables.c | 26 +++++++------------------- src/util/virnetdevmacvlan.c | 9 +++------ src/util/virnetdevtap.c | 9 ++++----- src/xen/xend_internal.c | 9 +++------ src/xenxs/xen_sxpr.c | 8 +++----- src/xenxs/xen_xm.c | 8 +++----- 9 files changed, 43 insertions(+), 68 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 371d80c..cc26f21 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11521,14 +11521,15 @@ static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src, virDomainNetDefPtr dst) { + char srcmac[VIR_MAC_STRING_BUFLEN]; + char dstmac[VIR_MAC_STRING_BUFLEN]; + if (virMacAddrCmp(&src->mac, &dst->mac) != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target network card mac %02x:%02x:%02x:%02x:%02x:%02x" - " does not match source %02x:%02x:%02x:%02x:%02x:%02x"), - dst->mac.addr[0], dst->mac.addr[1], dst->mac.addr[2], - dst->mac.addr[3], dst->mac.addr[4], dst->mac.addr[5], - src->mac.addr[0], src->mac.addr[1], src->mac.addr[2], - src->mac.addr[3], src->mac.addr[4], src->mac.addr[5]); + _("Target network card mac %s" + " does not match source %s"), + virMacAddrFormat(&dst->mac, dstmac), + virMacAddrFormat(&src->mac, srcmac)); return false; } @@ -13389,6 +13390,7 @@ virDomainNetDefFormat(virBufferPtr buf, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + char macstr[VIR_MAC_STRING_BUFLEN]; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -13404,10 +13406,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 6); - virBufferAsprintf(buf, - "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(buf, "<mac address='%s'/>\n", + virMacAddrFormat(&def->mac, macstr)); switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c80218d..17b0a9f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3638,12 +3638,12 @@ qemuBuildNicStr(virDomainNetDefPtr net, int vlan) { char *str; + char macaddr[VIR_MAC_STRING_BUFLEN]; + if (virAsprintf(&str, - "%smacaddr=%02x:%02x:%02x:%02x:%02x:%02x,vlan=%d%s%s%s%s", + "%smacaddr=%s,vlan=%d%s%s%s%s", prefix ? prefix : "", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5], + virMacAddrFormat(&net->mac, macaddr), vlan, (net->model ? ",model=" : ""), (net->model ? net->model : ""), @@ -3666,6 +3666,7 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, virBuffer buf = VIR_BUFFER_INITIALIZER; const char *nic; bool usingVirtio = false; + char macaddr[VIR_MAC_STRING_BUFLEN]; if (!net->model) { nic = "rtl8139"; @@ -3722,10 +3723,8 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, else virBufferAsprintf(&buf, ",vlan=%d", vlan); virBufferAsprintf(&buf, ",id=%s", net->info.alias); - virBufferAsprintf(&buf, ",mac=%02x:%02x:%02x:%02x:%02x:%02x", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5]); + virBufferAsprintf(&buf, ",mac=%s", + virMacAddrFormat(&net->mac, macaddr)); if (qemuBuildDeviceAddressStr(&buf, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info, qemuCaps) < 0) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index b3ac326..0fe59fb 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -164,6 +164,7 @@ umlBuildCommandLineNet(virConnectPtr conn, int idx) { virBuffer buf = VIR_BUFFER_INITIALIZER; + char macaddr[VIR_MAC_STRING_BUFLEN]; /* General format: ethNN=type,options */ @@ -264,9 +265,7 @@ umlBuildCommandLineNet(virConnectPtr conn, goto error; } - virBufferAsprintf(&buf, ",%02x:%02x:%02x:%02x:%02x:%02x", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(&buf, ",%s", virMacAddrFormat(&def->mac, macaddr)); if (def->type == VIR_DOMAIN_NET_TYPE_MCAST) { virBufferAsprintf(&buf, ",%s,%d", diff --git a/src/util/virebtables.c b/src/util/virebtables.c index ded62d7..65c0ac3 100644 --- a/src/util/virebtables.c +++ b/src/util/virebtables.c @@ -1,7 +1,7 @@ /* * virebtables.c: Helper APIs for managing ebtables * - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * Copyright (C) 2009 IBM Corp. * * This library is free software; you can redistribute it and/or @@ -446,15 +446,9 @@ ebtablesAddForwardAllowIn(ebtablesContext *ctx, const char *iface, const virMacAddrPtr mac) { - char *macaddr; - - if (virAsprintf(&macaddr, - "%02x:%02x:%02x:%02x:%02x:%02x", - mac->addr[0], mac->addr[1], - mac->addr[2], mac->addr[3], - mac->addr[4], mac->addr[5]) < 0) { - return -1; - } + char macaddr[VIR_MAC_STRING_BUFLEN]; + + virMacAddrFormat(mac, macaddr); return ebtablesForwardAllowIn(ctx, iface, macaddr, ADD); } @@ -475,14 +469,8 @@ ebtablesRemoveForwardAllowIn(ebtablesContext *ctx, const char *iface, const virMacAddrPtr mac) { - char *macaddr; - - if (virAsprintf(&macaddr, - "%02x:%02x:%02x:%02x:%02x:%02x", - mac->addr[0], mac->addr[1], - mac->addr[2], mac->addr[3], - mac->addr[4], mac->addr[5]) < 0) { - return -1; - } + char macaddr[VIR_MAC_STRING_BUFLEN]; + + virMacAddrFormat(mac, macaddr); return ebtablesForwardAllowIn(ctx, iface, macaddr, REMOVE); } diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index ddea11f..2578ff0 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2010-2012 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -518,6 +518,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, virNetlinkCallbackDataPtr calld = opaque; pid_t lldpad_pid = 0; pid_t virip_pid = 0; + char macaddr[VIR_MAC_STRING_BUFLEN]; hdr = (struct nlmsghdr *) msg; data = nlmsg_data(hdr); @@ -707,11 +708,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, VIR_INFO("Re-send 802.1qbg associate request:"); VIR_INFO(" if: %s", calld->cr_ifname); VIR_INFO(" lf: %s", calld->linkdev); - VIR_INFO(" mac: %02x:%02x:%02x:%02x:%02x:%02x", - calld->macaddress.addr[0], calld->macaddress.addr[1], - calld->macaddress.addr[2], calld->macaddress.addr[3], - calld->macaddress.addr[4], calld->macaddress.addr[5]); - + VIR_INFO(" mac: %s", virMacAddrFormat(&calld->macaddress, macaddr)); ignore_value(virNetDevVPortProfileAssociate(calld->cr_ifname, calld->virtPortProfile, &calld->macaddress, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index a884de1..871376e 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -286,6 +286,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, unsigned int flags) { virMacAddr tapmac; + char macaddrstr[VIR_MAC_STRING_BUFLEN]; if (virNetDevTapCreate(ifname, tapfd, flags) < 0) return -1; @@ -306,10 +307,8 @@ int virNetDevTapCreateInBridgePort(const char *brname, */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unable to use MAC address starting with " - "reserved value 0xFE - '%02X:%02X:%02X:%02X:%02X:%02X' - "), - macaddr->addr[0], macaddr->addr[1], - macaddr->addr[2], macaddr->addr[3], - macaddr->addr[4], macaddr->addr[5]); + "reserved value 0xFE - '%s' - "), + virMacAddrFormat(macaddr, macaddrstr)); goto error; } tapmac.addr[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 398da0d..7abc030 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3730,17 +3730,14 @@ virDomainXMLDevID(virDomainPtr domain, if (tmp == NULL) return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - char mac[30]; + char mac[VIR_MAC_STRING_BUFLEN]; virDomainNetDefPtr def = dev->data.net; - snprintf(mac, sizeof(mac), "%02x:%02x:%02x:%02x:%02x:%02x", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virMacAddrFormat(&def->mac, mac); strcpy(class, "vif"); xenUnifiedLock(priv); - xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, - mac); + xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, mac); xenUnifiedUnlock(priv); if (xref == NULL) return -1; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 83b7c74..cc4225b 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1,7 +1,7 @@ /* * xen_sxpr.c: Xen SEXPR parsing functions * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * @@ -1914,6 +1914,7 @@ xenFormatSxprNet(virConnectPtr conn, int isAttach) { const char *script = DEFAULT_VIF_SCRIPT; + char macaddr[VIR_MAC_STRING_BUFLEN]; if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && def->type != VIR_DOMAIN_NET_TYPE_NETWORK && @@ -1936,10 +1937,7 @@ xenFormatSxprNet(virConnectPtr conn, virBufferAddLit(buf, "(vif "); - virBufferAsprintf(buf, - "(mac '%02x:%02x:%02x:%02x:%02x:%02x')", - def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], - def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); + virBufferAsprintf(buf, "(mac '%s')", virMacAddrFormat(&def->mac, macaddr)); switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 73ba06b..405ebf3 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,7 +1,7 @@ /* * xen_xm.c: Xen XM parsing functions * - * Copyright (C) 2006-2007, 2009-2010, 2012 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2010, 2012, 2013 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2006 Daniel P. Berrange * @@ -1335,11 +1335,9 @@ static int xenFormatXMNet(virConnectPtr conn, { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; + char macaddr[VIR_MAC_STRING_BUFLEN]; - virBufferAsprintf(&buf, "mac=%02x:%02x:%02x:%02x:%02x:%02x", - net->mac.addr[0], net->mac.addr[1], - net->mac.addr[2], net->mac.addr[3], - net->mac.addr[4], net->mac.addr[5]); + virBufferAsprintf(&buf, "mac=%s", virMacAddrFormat(&net->mac, macaddr)); switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: -- 1.8.1.5
On 04/02/2013 03:37 PM, Peter Krempa wrote:
Format the address using the helper instead of having similar code in multiple places.
This patch also fixes leak of the MAC address string in ebtablesRemoveForwardAllowIn() and ebtablesAddForwardAllowIn() in src/util/virebtables.c --- [...] diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 73ba06b..405ebf3 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,7 +1,7 @@ /* * xen_xm.c: Xen XM parsing functions * - * Copyright (C) 2006-2007, 2009-2010, 2012 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2010, 2012, 2013 Red Hat, Inc.
s/2012, 2013/2012-2013/ and this is my fault, sorry for that. ACK with this fixed. Martin
The domif-getlink command did not terminate successfully when the interface state was found. As the code used old and too complex approach to do the job, this patch refactors it and fixed the bug. --- tools/virsh-domain-monitor.c | 100 ++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 62 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d34878f..f7297f5 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -663,25 +663,23 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *iface = NULL; - int flags = 0; char *state = NULL; - char *value = NULL; + char *xpath = NULL; virMacAddr macaddr; - const char *element; - const char *attr; - bool ret = false; - int i; - char *desc; + char macstr[VIR_MAC_STRING_BUFLEN] = ""; + char *desc = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; - xmlNodePtr cur = NULL; - xmlXPathObjectPtr obj = NULL; + xmlNodePtr *interfaces = NULL; + int ninterfaces; + unsigned int flags = 0; + bool ret = false; - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0) - goto cleanup; + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; if (vshCommandOptBool(cmd, "config")) flags = VIR_DOMAIN_XML_INACTIVE; @@ -691,72 +689,50 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - xml = virXMLParseStringCtxt(desc, _("(domain_definition)"), &ctxt); - VIR_FREE(desc); - if (!xml) { + if (!(xml = virXMLParseStringCtxt(desc, _("(domain_definition)"), &ctxt))) { vshError(ctl, _("Failed to parse domain description xml")); goto cleanup; } - obj = xmlXPathEval(BAD_CAST "/domain/devices/interface", ctxt); - if (obj == NULL || obj->type != XPATH_NODESET || - obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) { - vshError(ctl, _("Failed to extract interface information or no interfaces found")); + /* normalize the mac addr */ + if (virMacAddrParse(iface, &macaddr) == 0) + virMacAddrFormat(&macaddr, macstr); + + if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address = '%s') or " + " (target/@dev = '%s')]", + macstr, iface) < 0) { + virReportOOMError(); goto cleanup; } - if (virMacAddrParse(iface, &macaddr) == 0) { - element = "mac"; - attr = "address"; - } else { - element = "target"; - attr = "dev"; + if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0) { + vshError(ctl, _("Failed to extract interface information")); + goto cleanup; } - /* find interface with matching mac addr */ - for (i = 0; i < obj->nodesetval->nodeNr; i++) { - cur = obj->nodesetval->nodeTab[i]->children; - - while (cur) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST element)) { - - value = virXMLPropString(cur, attr); + if (ninterfaces != 1) { + if (macstr[0]) + vshError(ctl, _("Interface (mac: %s) not found."), macstr); + else + vshError(ctl, _("Interface (dev: %s) not found."), iface); - if (STRCASEEQ(value, iface)) { - VIR_FREE(value); - goto hit; - } - VIR_FREE(value); - } - cur = cur->next; - } + goto cleanup; } - vshError(ctl, _("Interface (%s: %s) not found."), element, iface); - goto cleanup; - -hit: - cur = obj->nodesetval->nodeTab[i]->children; - while (cur) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "link")) { - - state = virXMLPropString(cur, "state"); - vshPrint(ctl, "%s %s", iface, state); - VIR_FREE(state); + ctxt->node = interfaces[0]; - goto cleanup; - } - cur = cur->next; - } - - /* attribute not found */ - vshPrint(ctl, "%s default", iface); + if ((state = virXPathString("string(./link/@state)", ctxt))) + vshPrint(ctl, "%s %s", iface, state); + else + vshPrint(ctl, "%s default", iface); ret = true; + cleanup: - xmlXPathFreeObject(obj); + VIR_FREE(desc); + VIR_FREE(state); + VIR_FREE(interfaces); + VIR_FREE(xpath); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); virDomainFree(dom); -- 1.8.1.5
On 03/26/2013 12:26 PM, Peter Krempa wrote:
The domif-getlink command did not terminate successfully when the interface state was found. As the code used old and too complex approach to do the job, this patch refactors it and fixed the bug.
s/fixed/fixes/ ACK, the only change this patch makes is that reporting of unknown devices is now: error: Interface (dev: asdf) not found. instead of: error: Interface (target: asdf) not found. And that change makes sense. Martin
On 03/26/13 15:43, Martin Kletzander wrote:
On 03/26/2013 12:26 PM, Peter Krempa wrote:
The domif-getlink command did not terminate successfully when the interface state was found. As the code used old and too complex approach to do the job, this patch refactors it and fixed the bug.
s/fixed/fixes/
ACK,
Thanks for the reviews. I've just pushed this series. Peter
participants (3)
-
Eric Blake -
Martin Kletzander -
Peter Krempa