8:27 a.m.
Inspired-by: Laine Stump <laine(a)laine.org>
Day-of-the-week: Friday <6>
Ján Tomko (16):
vboxDumpSharedFolders: rename non-standard label
vboxDumpSharedFolders: remove pointless comment
vboxDumpSharedFolders: return a value
vboxDumpNetwork: add temp variable for current network
vboxDumpNetwork: rename to vboxDumpNetworks
vboxDumpNetwork: re-introduce this function
vboxDumpNetworks: reduce indentation level
vboxDumpNetwork: allocate the network too
vboxDumpNetworks: delete pointless comment
vboxDumpNetworks: do not allocate def->nets upfront
vboxDumpNetwork: use virMacAddrParseHex
vboxDumpNetwork: Use a single utf16 variable
vboxDumpNetwork: Use a single utf8 temp variable
vboxDumpNetwork: use a switch for attachmentType
vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP
vboxDumpNetwork: use switch for adapterType
src/vbox/vbox_common.c | 243 ++++++++++++++++++++++++-------------------------
1 file changed, 120 insertions(+), 123 deletions(-)
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 01/16] vboxDumpSharedFolders: rename non-standard label
s/sharedFoldersCleanup/cleanup/
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 07f430878..e1629b07f 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3640,10 +3640,10 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
gVBoxAPI.UArray.handleMachineGetSharedFolders(machine));
if (sharedFolders.count <= 0)
- goto sharedFoldersCleanup;
+ goto cleanup;
if (VIR_ALLOC_N(def->fss, sharedFolders.count) < 0)
- goto sharedFoldersCleanup;
+ goto cleanup;
for (i = 0; i < sharedFolders.count; i++) {
ISharedFolder *sharedFolder = sharedFolders.items[i];
@@ -3654,7 +3654,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
PRBool writable = PR_FALSE;
if (VIR_ALLOC(def->fss[i]) < 0)
- goto sharedFoldersCleanup;
+ goto cleanup;
def->fss[i]->type = VIR_DOMAIN_FS_TYPE_MOUNT;
@@ -3663,7 +3663,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
if (VIR_STRDUP(def->fss[i]->src->path, hostPath) < 0) {
VBOX_UTF8_FREE(hostPath);
VBOX_UTF16_FREE(hostPathUtf16);
- goto sharedFoldersCleanup;
+ goto cleanup;
}
VBOX_UTF8_FREE(hostPath);
VBOX_UTF16_FREE(hostPathUtf16);
@@ -3673,7 +3673,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
if (VIR_STRDUP(def->fss[i]->dst, name) < 0) {
VBOX_UTF8_FREE(name);
VBOX_UTF16_FREE(nameUtf16);
- goto sharedFoldersCleanup;
+ goto cleanup;
}
VBOX_UTF8_FREE(name);
VBOX_UTF16_FREE(nameUtf16);
@@ -3684,7 +3684,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
++def->nfss;
}
- sharedFoldersCleanup:
+ cleanup:
gVBoxAPI.UArray.vboxArrayRelease(&sharedFolders);
}
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 02/16] vboxDumpSharedFolders: remove pointless comment
Now that the functions are separate, we no longer need comment
separators.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index e1629b07f..afd00a91a 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3630,7 +3630,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine)
static void
vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
{
- /* shared folders */
vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER;
size_t i = 0;
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 03/16] vboxDumpSharedFolders: return a value
The allocation errors in this function are already handled by jumping
to a cleanup label.
Change the return type from void to int and return -1 on error.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index afd00a91a..cc7772f25 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3627,19 +3627,23 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine)
return ret;
}
-static void
+static int
vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
{
vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER;
size_t i = 0;
+ int ret = -1;
def->nfss = 0;
gVBoxAPI.UArray.vboxArrayGet(&sharedFolders, machine,
gVBoxAPI.UArray.handleMachineGetSharedFolders(machine));
- if (sharedFolders.count <= 0)
+ if (sharedFolders.count <= 0) {
+ if (sharedFolders.count == 0)
+ ret = 0;
goto cleanup;
+ }
if (VIR_ALLOC_N(def->fss, sharedFolders.count) < 0)
goto cleanup;
@@ -3683,8 +3687,11 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
++def->nfss;
}
+ ret = 0;
+
cleanup:
gVBoxAPI.UArray.vboxArrayRelease(&sharedFolders);
+ return ret;
}
static void
@@ -4179,7 +4186,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
if (vboxDumpDisks(def, data, machine) < 0)
goto cleanup;
- vboxDumpSharedFolders(def, data, machine);
+ if (vboxDumpSharedFolders(def, data, machine) < 0)
+ goto cleanup;
vboxDumpNetwork(def, data, machine, networkAdapterCount);
vboxDumpAudio(def, data, machine);
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 04/16] vboxDumpNetwork: add temp variable for current network
Instead of using def->nets every time, use a temporary pointer.
This will allow splitting out the per-adapter code.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index cc7772f25..052655ca7 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3726,6 +3726,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
/* Now get the details about the network cards here */
for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount;
i++) {
INetworkAdapter *adapter = NULL;
+ virDomainNetDefPtr net = def->nets[netAdpIncCnt];
gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter);
if (adapter) {
@@ -3742,18 +3743,18 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter,
&attachmentType);
if (attachmentType == NetworkAttachmentType_NAT) {
- def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_USER;
+ net->type = VIR_DOMAIN_NET_TYPE_USER;
} else if (attachmentType == NetworkAttachmentType_Bridged) {
PRUnichar *hostIntUtf16 = NULL;
char *hostInt = NULL;
- def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter,
&hostIntUtf16);
VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
-
ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.bridge.brname, hostInt));
+ ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
VBOX_UTF8_FREE(hostInt);
VBOX_UTF16_FREE(hostIntUtf16);
@@ -3762,12 +3763,12 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
PRUnichar *intNetUtf16 = NULL;
char *intNet = NULL;
- def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
+ net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter,
&intNetUtf16);
VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet);
-
ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.internal.name, intNet));
+ ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
VBOX_UTF8_FREE(intNet);
VBOX_UTF16_FREE(intNetUtf16);
@@ -3776,12 +3777,12 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
PRUnichar *hostIntUtf16 = NULL;
char *hostInt = NULL;
- def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_NETWORK;
+ net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter,
&hostIntUtf16);
VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
-
ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.network.name, hostInt));
+ ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
VBOX_UTF8_FREE(hostInt);
VBOX_UTF16_FREE(hostIntUtf16);
@@ -3790,24 +3791,24 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
/* default to user type i.e. NAT in VirtualBox if this
* dump is ever used to create a machine.
*/
- def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_USER;
+ net->type = VIR_DOMAIN_NET_TYPE_USER;
}
gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType);
if (adapterType == NetworkAdapterType_Am79C970A) {
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"Am79C970A"));
+ ignore_value(VIR_STRDUP(net->model, "Am79C970A"));
} else if (adapterType == NetworkAdapterType_Am79C973) {
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"Am79C973"));
+ ignore_value(VIR_STRDUP(net->model, "Am79C973"));
} else if (adapterType == NetworkAdapterType_I82540EM) {
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"82540EM"));
+ ignore_value(VIR_STRDUP(net->model, "82540EM"));
} else if (adapterType == NetworkAdapterType_I82545EM) {
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"82545EM"));
+ ignore_value(VIR_STRDUP(net->model, "82545EM"));
} else if (adapterType == NetworkAdapterType_I82543GC) {
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"82543GC"));
+ ignore_value(VIR_STRDUP(net->model, "82543GC"));
} else if (gVBoxAPI.APIVersion >= 3000051 &&
adapterType == NetworkAdapterType_Virtio) {
/* Only vbox 3.1 and later support NetworkAdapterType_Virto */
- ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model,
"virtio"));
+ ignore_value(VIR_STRDUP(net->model, "virtio"));
}
gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16);
@@ -3819,8 +3820,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRUi
MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]);
/* XXX some real error handling here some day ... */
- ignore_value(virMacAddrParse(macaddr,
- &def->nets[netAdpIncCnt]->mac));
+ ignore_value(virMacAddrParse(macaddr, &net->mac));
netAdpIncCnt++;
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 05/16] vboxDumpNetwork: rename to vboxDumpNetworks
Free up 'vboxDumpNetwork' for dumping single network.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 052655ca7..03266557a 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3695,7 +3695,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
}
static void
-vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32
networkAdapterCount)
+vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32
networkAdapterCount)
{
PRUint32 netAdpIncCnt = 0;
size_t i = 0;
@@ -4188,7 +4188,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
if (vboxDumpSharedFolders(def, data, machine) < 0)
goto cleanup;
- vboxDumpNetwork(def, data, machine, networkAdapterCount);
+ vboxDumpNetworks(def, data, machine, networkAdapterCount);
vboxDumpAudio(def, data, machine);
if (vboxDumpSerial(def, data, machine, serialPortCount) < 0)
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 06/16] vboxDumpNetwork: re-introduce this function
Split out per-adapter code from vboxDumpNetworks.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 186 +++++++++++++++++++++++++------------------------
1 file changed, 96 insertions(+), 90 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 03266557a..295f48376 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3695,6 +3695,101 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
}
static void
+vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data, INetworkAdapter *adapter)
+{
+ PRUint32 attachmentType = NetworkAttachmentType_Null;
+ PRUint32 adapterType = NetworkAdapterType_Null;
+ PRUnichar *MACAddressUtf16 = NULL;
+ char *MACAddress = NULL;
+ char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
+
+ gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType);
+ if (attachmentType == NetworkAttachmentType_NAT) {
+
+ net->type = VIR_DOMAIN_NET_TYPE_USER;
+
+ } else if (attachmentType == NetworkAttachmentType_Bridged) {
+ PRUnichar *hostIntUtf16 = NULL;
+ char *hostInt = NULL;
+
+ net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
+ gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16);
+
+ VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
+ ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
+
+ VBOX_UTF8_FREE(hostInt);
+ VBOX_UTF16_FREE(hostIntUtf16);
+
+ } else if (attachmentType == NetworkAttachmentType_Internal) {
+ PRUnichar *intNetUtf16 = NULL;
+ char *intNet = NULL;
+
+ net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
+
+ gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16);
+
+ VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet);
+ ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
+
+ VBOX_UTF8_FREE(intNet);
+ VBOX_UTF16_FREE(intNetUtf16);
+
+ } else if (attachmentType == NetworkAttachmentType_HostOnly) {
+ PRUnichar *hostIntUtf16 = NULL;
+ char *hostInt = NULL;
+
+ net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
+
+ gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16);
+
+ VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
+ ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
+
+ VBOX_UTF8_FREE(hostInt);
+ VBOX_UTF16_FREE(hostIntUtf16);
+
+ } else {
+ /* default to user type i.e. NAT in VirtualBox if this
+ * dump is ever used to create a machine.
+ */
+ net->type = VIR_DOMAIN_NET_TYPE_USER;
+ }
+
+ gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType);
+ if (adapterType == NetworkAdapterType_Am79C970A) {
+ ignore_value(VIR_STRDUP(net->model, "Am79C970A"));
+ } else if (adapterType == NetworkAdapterType_Am79C973) {
+ ignore_value(VIR_STRDUP(net->model, "Am79C973"));
+ } else if (adapterType == NetworkAdapterType_I82540EM) {
+ ignore_value(VIR_STRDUP(net->model, "82540EM"));
+ } else if (adapterType == NetworkAdapterType_I82545EM) {
+ ignore_value(VIR_STRDUP(net->model, "82545EM"));
+ } else if (adapterType == NetworkAdapterType_I82543GC) {
+ ignore_value(VIR_STRDUP(net->model, "82543GC"));
+ } else if (gVBoxAPI.APIVersion >= 3000051 &&
+ adapterType == NetworkAdapterType_Virtio) {
+ /* Only vbox 3.1 and later support NetworkAdapterType_Virto */
+ ignore_value(VIR_STRDUP(net->model, "virtio"));
+ }
+
+ gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16);
+ VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress);
+ snprintf(macaddr, VIR_MAC_STRING_BUFLEN,
+ "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c",
+ MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3],
+ MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7],
+ MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]);
+
+ /* XXX some real error handling here some day ... */
+ ignore_value(virMacAddrParse(macaddr, &net->mac));
+
+ VBOX_UTF16_FREE(MACAddressUtf16);
+ VBOX_UTF8_FREE(MACAddress);
+}
+
+static void
vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32
networkAdapterCount)
{
PRUint32 netAdpIncCnt = 0;
@@ -3734,98 +3829,9 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled);
if (enabled) {
- PRUint32 attachmentType = NetworkAttachmentType_Null;
- PRUint32 adapterType = NetworkAdapterType_Null;
- PRUnichar *MACAddressUtf16 = NULL;
- char *MACAddress = NULL;
- char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
-
- gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter,
&attachmentType);
- if (attachmentType == NetworkAttachmentType_NAT) {
-
- net->type = VIR_DOMAIN_NET_TYPE_USER;
-
- } else if (attachmentType == NetworkAttachmentType_Bridged) {
- PRUnichar *hostIntUtf16 = NULL;
- char *hostInt = NULL;
-
- net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-
- gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter,
&hostIntUtf16);
-
- VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
- ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
-
- VBOX_UTF8_FREE(hostInt);
- VBOX_UTF16_FREE(hostIntUtf16);
-
- } else if (attachmentType == NetworkAttachmentType_Internal) {
- PRUnichar *intNetUtf16 = NULL;
- char *intNet = NULL;
-
- net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
-
- gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter,
&intNetUtf16);
-
- VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet);
- ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
-
- VBOX_UTF8_FREE(intNet);
- VBOX_UTF16_FREE(intNetUtf16);
-
- } else if (attachmentType == NetworkAttachmentType_HostOnly) {
- PRUnichar *hostIntUtf16 = NULL;
- char *hostInt = NULL;
-
- net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-
- gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter,
&hostIntUtf16);
-
- VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
- ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
-
- VBOX_UTF8_FREE(hostInt);
- VBOX_UTF16_FREE(hostIntUtf16);
-
- } else {
- /* default to user type i.e. NAT in VirtualBox if this
- * dump is ever used to create a machine.
- */
- net->type = VIR_DOMAIN_NET_TYPE_USER;
- }
-
- gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType);
- if (adapterType == NetworkAdapterType_Am79C970A) {
- ignore_value(VIR_STRDUP(net->model, "Am79C970A"));
- } else if (adapterType == NetworkAdapterType_Am79C973) {
- ignore_value(VIR_STRDUP(net->model, "Am79C973"));
- } else if (adapterType == NetworkAdapterType_I82540EM) {
- ignore_value(VIR_STRDUP(net->model, "82540EM"));
- } else if (adapterType == NetworkAdapterType_I82545EM) {
- ignore_value(VIR_STRDUP(net->model, "82545EM"));
- } else if (adapterType == NetworkAdapterType_I82543GC) {
- ignore_value(VIR_STRDUP(net->model, "82543GC"));
- } else if (gVBoxAPI.APIVersion >= 3000051 &&
- adapterType == NetworkAdapterType_Virtio) {
- /* Only vbox 3.1 and later support NetworkAdapterType_Virto */
- ignore_value(VIR_STRDUP(net->model, "virtio"));
- }
-
- gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16);
- VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress);
- snprintf(macaddr, VIR_MAC_STRING_BUFLEN,
- "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c",
- MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3],
- MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7],
- MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]);
-
- /* XXX some real error handling here some day ... */
- ignore_value(virMacAddrParse(macaddr, &net->mac));
+ vboxDumpNetwork(net, data, adapter);
netAdpIncCnt++;
-
- VBOX_UTF16_FREE(MACAddressUtf16);
- VBOX_UTF8_FREE(MACAddress);
}
VBOX_RELEASE(adapter);
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 07/16] vboxDumpNetworks: reduce indentation level
The 'enabled' bool is initialized to false, there is no need to nest the
conditions.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 295f48376..1d38001f9 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3822,20 +3822,19 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount;
i++) {
INetworkAdapter *adapter = NULL;
virDomainNetDefPtr net = def->nets[netAdpIncCnt];
+ PRBool enabled = PR_FALSE;
gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter);
- if (adapter) {
- PRBool enabled = PR_FALSE;
-
+ if (adapter)
gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled);
- if (enabled) {
- vboxDumpNetwork(net, data, adapter);
- netAdpIncCnt++;
- }
+ if (enabled) {
+ vboxDumpNetwork(net, data, adapter);
- VBOX_RELEASE(adapter);
+ netAdpIncCnt++;
}
+
+ VBOX_RELEASE(adapter);
}
}
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 08/16] vboxDumpNetwork: allocate the network too
Move the allocation from vboxDumpNetworks inside vboxDumpNetwork.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 1d38001f9..2eb7af4ba 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3694,14 +3694,18 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
return ret;
}
-static void
-vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data, INetworkAdapter *adapter)
+static virDomainNetDefPtr
+vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
{
PRUint32 attachmentType = NetworkAttachmentType_Null;
PRUint32 adapterType = NetworkAdapterType_Null;
PRUnichar *MACAddressUtf16 = NULL;
char *MACAddress = NULL;
char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
+ virDomainNetDefPtr net = NULL;
+
+ if (VIR_ALLOC(net) < 0)
+ return NULL;
gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType);
if (attachmentType == NetworkAttachmentType_NAT) {
@@ -3787,6 +3791,7 @@ vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data,
INetworkAdapter *ada
VBOX_UTF16_FREE(MACAddressUtf16);
VBOX_UTF8_FREE(MACAddress);
+ return net;
}
static void
@@ -3813,15 +3818,13 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
}
/* Allocate memory for the networkcards which are enabled */
- if ((def->nnets > 0) && (VIR_ALLOC_N(def->nets, def->nnets) >=
0)) {
- for (i = 0; i < def->nnets; i++)
- ignore_value(VIR_ALLOC(def->nets[i]));
- }
+ if (def->nnets > 0)
+ ignore_value(VIR_ALLOC_N(def->nets, def->nnets));
/* Now get the details about the network cards here */
for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount;
i++) {
INetworkAdapter *adapter = NULL;
- virDomainNetDefPtr net = def->nets[netAdpIncCnt];
+ virDomainNetDefPtr net = NULL;
PRBool enabled = PR_FALSE;
gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter);
@@ -3829,9 +3832,8 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled);
if (enabled) {
- vboxDumpNetwork(net, data, adapter);
-
- netAdpIncCnt++;
+ net = vboxDumpNetwork(data, adapter);
+ def->nets[netAdpIncCnt++] = net;
}
VBOX_RELEASE(adapter);
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 09/16] vboxDumpNetworks: delete pointless comment
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 2eb7af4ba..c730c0cc0 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3799,7 +3799,7 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
{
PRUint32 netAdpIncCnt = 0;
size_t i = 0;
- /* dump network cards if present */
+
def->nnets = 0;
/* Get which network cards are enabled */
for (i = 0; i < networkAdapterCount; i++) {
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 10/16] vboxDumpNetworks: do not allocate def->nets upfront
Use VIR_APPEND_ELEMENT instead and change the return type
to int to catch allocation errors.
This removes the need to figure out the adapter count
upfront.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 35 +++++++++--------------------------
1 file changed, 9 insertions(+), 26 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index c730c0cc0..c807d2965 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3794,36 +3794,13 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
return net;
}
-static void
+static int
vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32
networkAdapterCount)
{
- PRUint32 netAdpIncCnt = 0;
size_t i = 0;
- def->nnets = 0;
- /* Get which network cards are enabled */
for (i = 0; i < networkAdapterCount; i++) {
INetworkAdapter *adapter = NULL;
-
- gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter);
- if (adapter) {
- PRBool enabled = PR_FALSE;
-
- gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled);
- if (enabled)
- def->nnets++;
-
- VBOX_RELEASE(adapter);
- }
- }
-
- /* Allocate memory for the networkcards which are enabled */
- if (def->nnets > 0)
- ignore_value(VIR_ALLOC_N(def->nets, def->nnets));
-
- /* Now get the details about the network cards here */
- for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount;
i++) {
- INetworkAdapter *adapter = NULL;
virDomainNetDefPtr net = NULL;
PRBool enabled = PR_FALSE;
@@ -3833,11 +3810,16 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine, PRU
if (enabled) {
net = vboxDumpNetwork(data, adapter);
- def->nets[netAdpIncCnt++] = net;
+ if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) {
+ VBOX_RELEASE(adapter);
+ return -1;
+ }
}
VBOX_RELEASE(adapter);
}
+
+ return 0;
}
static void
@@ -4195,7 +4177,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
if (vboxDumpSharedFolders(def, data, machine) < 0)
goto cleanup;
- vboxDumpNetworks(def, data, machine, networkAdapterCount);
+ if (vboxDumpNetworks(def, data, machine, networkAdapterCount) < 0)
+ goto cleanup;
vboxDumpAudio(def, data, machine);
if (vboxDumpSerial(def, data, machine, serialPortCount) < 0)
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 11/16] vboxDumpNetwork: use virMacAddrParseHex
Use the virMacAddrParse helper that does not require colon-separated
values instead of using extra code to format it that way.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index c807d2965..3099e20c5 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3701,7 +3701,6 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
PRUint32 adapterType = NetworkAdapterType_Null;
PRUnichar *MACAddressUtf16 = NULL;
char *MACAddress = NULL;
- char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
virDomainNetDefPtr net = NULL;
if (VIR_ALLOC(net) < 0)
@@ -3780,18 +3779,19 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16);
VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress);
- snprintf(macaddr, VIR_MAC_STRING_BUFLEN,
- "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c",
- MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3],
- MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7],
- MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]);
+ VBOX_UTF16_FREE(MACAddressUtf16);
- /* XXX some real error handling here some day ... */
- ignore_value(virMacAddrParse(macaddr, &net->mac));
+ if (virMacAddrParseHex(MACAddress, &net->mac) < 0) {
+ VBOX_UTF8_FREE(MACAddress);
+ goto error;
+ }
- VBOX_UTF16_FREE(MACAddressUtf16);
VBOX_UTF8_FREE(MACAddress);
return net;
+
+ error:
+ virDomainNetDefFree(net);
+ return NULL;
}
static int
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 12/16] vboxDumpNetwork: Use a single utf16 variable
There is a pattern of using two temporary utf16/utf8 variables
for every value we get from VirtualBox and put in the domain
definition right away.
Reuse the same variable name to improve the chances of getting
the function on one screen.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 3099e20c5..dc12bc662 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3699,7 +3699,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
{
PRUint32 attachmentType = NetworkAttachmentType_Null;
PRUint32 adapterType = NetworkAdapterType_Null;
- PRUnichar *MACAddressUtf16 = NULL;
+ PRUnichar *utf16 = NULL;
char *MACAddress = NULL;
virDomainNetDefPtr net = NULL;
@@ -3712,46 +3712,43 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
net->type = VIR_DOMAIN_NET_TYPE_USER;
} else if (attachmentType == NetworkAttachmentType_Bridged) {
- PRUnichar *hostIntUtf16 = NULL;
char *hostInt = NULL;
net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
- gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16);
+ gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
+ VBOX_UTF16_TO_UTF8(utf16, &hostInt);
ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
VBOX_UTF8_FREE(hostInt);
- VBOX_UTF16_FREE(hostIntUtf16);
+ VBOX_UTF16_FREE(utf16);
} else if (attachmentType == NetworkAttachmentType_Internal) {
- PRUnichar *intNetUtf16 = NULL;
char *intNet = NULL;
net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
- gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16);
+ gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet);
+ VBOX_UTF16_TO_UTF8(utf16, &intNet);
ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
VBOX_UTF8_FREE(intNet);
- VBOX_UTF16_FREE(intNetUtf16);
+ VBOX_UTF16_FREE(utf16);
} else if (attachmentType == NetworkAttachmentType_HostOnly) {
- PRUnichar *hostIntUtf16 = NULL;
char *hostInt = NULL;
net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
- gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16);
+ gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt);
+ VBOX_UTF16_TO_UTF8(utf16, &hostInt);
ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
VBOX_UTF8_FREE(hostInt);
- VBOX_UTF16_FREE(hostIntUtf16);
+ VBOX_UTF16_FREE(utf16);
} else {
/* default to user type i.e. NAT in VirtualBox if this
@@ -3777,9 +3774,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
ignore_value(VIR_STRDUP(net->model, "virtio"));
}
- gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16);
- VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress);
- VBOX_UTF16_FREE(MACAddressUtf16);
+ gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16);
+ VBOX_UTF16_TO_UTF8(utf16, &MACAddress);
+ VBOX_UTF16_FREE(utf16);
if (virMacAddrParseHex(MACAddress, &net->mac) < 0) {
VBOX_UTF8_FREE(MACAddress);
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 13/16] vboxDumpNetwork: Use a single utf8 temp variable
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index dc12bc662..2943c534d 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3700,7 +3700,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
PRUint32 attachmentType = NetworkAttachmentType_Null;
PRUint32 adapterType = NetworkAdapterType_Null;
PRUnichar *utf16 = NULL;
- char *MACAddress = NULL;
+ char *utf8 = NULL;
virDomainNetDefPtr net = NULL;
if (VIR_ALLOC(net) < 0)
@@ -3712,42 +3712,36 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
net->type = VIR_DOMAIN_NET_TYPE_USER;
} else if (attachmentType == NetworkAttachmentType_Bridged) {
- char *hostInt = NULL;
-
net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(utf16, &hostInt);
- ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
+ VBOX_UTF16_TO_UTF8(utf16, &utf8);
+ ignore_value(VIR_STRDUP(net->data.bridge.brname, utf8));
- VBOX_UTF8_FREE(hostInt);
+ VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
} else if (attachmentType == NetworkAttachmentType_Internal) {
- char *intNet = NULL;
-
net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(utf16, &intNet);
- ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
+ VBOX_UTF16_TO_UTF8(utf16, &utf8);
+ ignore_value(VIR_STRDUP(net->data.internal.name, utf8));
- VBOX_UTF8_FREE(intNet);
+ VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
} else if (attachmentType == NetworkAttachmentType_HostOnly) {
- char *hostInt = NULL;
-
net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(utf16, &hostInt);
- ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
+ VBOX_UTF16_TO_UTF8(utf16, &utf8);
+ ignore_value(VIR_STRDUP(net->data.network.name, utf8));
- VBOX_UTF8_FREE(hostInt);
+ VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
} else {
@@ -3775,15 +3769,15 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
}
gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16);
- VBOX_UTF16_TO_UTF8(utf16, &MACAddress);
+ VBOX_UTF16_TO_UTF8(utf16, &utf8);
VBOX_UTF16_FREE(utf16);
- if (virMacAddrParseHex(MACAddress, &net->mac) < 0) {
- VBOX_UTF8_FREE(MACAddress);
+ if (virMacAddrParseHex(utf8, &net->mac) < 0) {
+ VBOX_UTF8_FREE(utf8);
goto error;
}
- VBOX_UTF8_FREE(MACAddress);
+ VBOX_UTF8_FREE(utf8);
return net;
error:
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 14/16] vboxDumpNetwork: use a switch for attachmentType
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 2943c534d..8f5f04efb 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3707,11 +3707,13 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
return NULL;
gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType);
- if (attachmentType == NetworkAttachmentType_NAT) {
+ switch (attachmentType) {
+ case NetworkAttachmentType_NAT:
net->type = VIR_DOMAIN_NET_TYPE_USER;
+ break;
- } else if (attachmentType == NetworkAttachmentType_Bridged) {
+ case NetworkAttachmentType_Bridged:
net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16);
@@ -3721,8 +3723,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
+ break;
- } else if (attachmentType == NetworkAttachmentType_Internal) {
+ case NetworkAttachmentType_Internal:
net->type = VIR_DOMAIN_NET_TYPE_INTERNAL;
gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16);
@@ -3732,8 +3735,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
+ break;
- } else if (attachmentType == NetworkAttachmentType_HostOnly) {
+ case NetworkAttachmentType_HostOnly:
net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16);
@@ -3743,8 +3747,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
VBOX_UTF8_FREE(utf8);
VBOX_UTF16_FREE(utf16);
+ break;
- } else {
+ default:
/* default to user type i.e. NAT in VirtualBox if this
* dump is ever used to create a machine.
*/
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 15/16] vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP
We can steal the strings instead of creating more copies.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 8f5f04efb..1a413e4ac 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3719,9 +3719,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16);
VBOX_UTF16_TO_UTF8(utf16, &utf8);
- ignore_value(VIR_STRDUP(net->data.bridge.brname, utf8));
-
- VBOX_UTF8_FREE(utf8);
+ VIR_STEAL_PTR(net->data.bridge.brname, utf8);
VBOX_UTF16_FREE(utf16);
break;
@@ -3731,9 +3729,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16);
VBOX_UTF16_TO_UTF8(utf16, &utf8);
- ignore_value(VIR_STRDUP(net->data.internal.name, utf8));
-
- VBOX_UTF8_FREE(utf8);
+ VIR_STEAL_PTR(net->data.internal.name, utf8);
VBOX_UTF16_FREE(utf16);
break;
@@ -3743,9 +3739,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16);
VBOX_UTF16_TO_UTF8(utf16, &utf8);
- ignore_value(VIR_STRDUP(net->data.network.name, utf8));
-
- VBOX_UTF8_FREE(utf8);
+ VIR_STEAL_PTR(net->data.network.name, utf8);
VBOX_UTF16_FREE(utf16);
break;
--
2.13.6
8:27 a.m.
New subject: [libvirt] [PATCH 16/16] vboxDumpNetwork: use switch for adapterType
Also return an error when VIR_STRDUP fails.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 1a413e4ac..3bcca43d3 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3699,6 +3699,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
{
PRUint32 attachmentType = NetworkAttachmentType_Null;
PRUint32 adapterType = NetworkAdapterType_Null;
+ const char *model = NULL;
PRUnichar *utf16 = NULL;
char *utf8 = NULL;
virDomainNetDefPtr net = NULL;
@@ -3751,21 +3752,30 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
}
gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType);
- if (adapterType == NetworkAdapterType_Am79C970A) {
- ignore_value(VIR_STRDUP(net->model, "Am79C970A"));
- } else if (adapterType == NetworkAdapterType_Am79C973) {
- ignore_value(VIR_STRDUP(net->model, "Am79C973"));
- } else if (adapterType == NetworkAdapterType_I82540EM) {
- ignore_value(VIR_STRDUP(net->model, "82540EM"));
- } else if (adapterType == NetworkAdapterType_I82545EM) {
- ignore_value(VIR_STRDUP(net->model, "82545EM"));
- } else if (adapterType == NetworkAdapterType_I82543GC) {
- ignore_value(VIR_STRDUP(net->model, "82543GC"));
- } else if (gVBoxAPI.APIVersion >= 3000051 &&
- adapterType == NetworkAdapterType_Virtio) {
+ switch (adapterType) {
+ case NetworkAdapterType_Am79C970A:
+ model = "Am79C970A";
+ break;
+ case NetworkAdapterType_Am79C973:
+ model = "Am79C973";
+ break;
+ case NetworkAdapterType_I82540EM:
+ model = "82540EM";
+ break;
+ case NetworkAdapterType_I82545EM:
+ model = "82545EM";
+ break;
+ case NetworkAdapterType_I82543GC:
+ model = "82543GC";
+ break;
+ case NetworkAdapterType_Virtio:
/* Only vbox 3.1 and later support NetworkAdapterType_Virto */
- ignore_value(VIR_STRDUP(net->model, "virtio"));
+ if (gVBoxAPI.APIVersion >= 3000051)
+ model = "virtio";
+ break;
}
+ if (VIR_STRDUP(net->model, model) < 0)
+ goto error;
gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16);
VBOX_UTF16_TO_UTF8(utf16, &utf8);
--
2.13.6
10:47 a.m.
On 02/23/2018 09:27 AM, Ján Tomko wrote:
Inspired-by: Laine Stump <laine(a)laine.org>
How can I *not* at least look at the patches when you call me out like this!
Day-of-the-week: Friday <6>
Ján Tomko (16):
vboxDumpSharedFolders: rename non-standard label
vboxDumpSharedFolders: remove pointless comment
vboxDumpSharedFolders: return a value
vboxDumpNetwork: add temp variable for current network
vboxDumpNetwork: rename to vboxDumpNetworks
vboxDumpNetwork: re-introduce this function
vboxDumpNetworks: reduce indentation level
vboxDumpNetwork: allocate the network too
vboxDumpNetworks: delete pointless comment
vboxDumpNetworks: do not allocate def->nets upfront
vboxDumpNetwork: use virMacAddrParseHex
vboxDumpNetwork: Use a single utf16 variable
vboxDumpNetwork: Use a single utf8 temp variable
vboxDumpNetwork: use a switch for attachmentType
vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP
vboxDumpNetwork: use switch for adapterType
src/vbox/vbox_common.c | 243 ++++++++++++++++++++++++-------------------------
1 file changed, 120 insertions(+), 123 deletions(-)
Nice. Where I had whined, you actually took action! :-)
I'm unable to test, but I looked through and each patch looks
straightforward and sane (there were bits I didn't like (e.g.
perpetuating ignore_value() uses), but they were removed in subsequent
patches, so all is good. You say that you've actually tested the code,
so as long as you've also run make syntax-check and make check:
ACK series
https://tinyurl.com/y8hxgcg
2539
days inactive
2539
days old
17 comments
2 participants
participants (2)
-
Ján Tomko -
Laine Stump