[libvirt] [PATCH] util: xml: Don't conflict with other libxml2 user callbacks

[Cole Robinson]

lxml is a popular python XML processing library. It uses libxml2
behind the scenes, and registers custom callbacks via
xmlSetExternalEntityLoader. However this can cause crashes if
if an app uses both lxml and libxml2 together in the same process.

This is a known limitation of lxml and libxml2 generally. It also
prevents us from using lxml in virt-manager:

https://bugzilla.redhat.com/show_bug.cgi?id=1544019

However it's easy enough to work around in libvirt, by unsetting the
EntityLoader callback to a known state before we ask libxml2 to
parse a file from disk.

Signed-off-by: Cole Robinson 
---
 src/util/virxml.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/util/virxml.c b/src/util/virxml.c
index 6e87605ea..3e01794f9 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -810,9 +810,14 @@ virXMLParseHelper(int domcode,
 pctxt->sax->error = catchXMLError;
 
 if (filename) {
+/* Reset any libxml2 file callbacks, other libs (like python lxml)
+ * may have set their own which can get crashy */
+xmlExternalEntityLoader origloader = xmlGetExternalEntityLoader();
+xmlSetExternalEntityLoader(xmlNoNetExternalEntityLoader);
 xml = xmlCtxtReadFile(pctxt, filename, NULL,
   XML_PARSE_NONET |
   XML_PARSE_NOWARNING);
+xmlSetExternalEntityLoader(origloader);
 } else {
 xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL,
  XML_PARSE_NONET |
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] test: Implement virConnectListAllNodeDevices

[Cole Robinson]

Signed-off-by: Cole Robinson 
---
 src/test/test_driver.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 39784c9fa..844e99dd7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5385,6 +5385,18 @@ testNodeListDevices(virConnectPtr conn,
 cap, names, maxnames);
 }
 
+static int
+testConnectListAllNodeDevices(virConnectPtr conn,
+  virNodeDevicePtr **devices,
+  unsigned int flags)
+{
+testDriverPtr driver = conn->privateData;
+
+virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP, -1);
+
+return virNodeDeviceObjListExport(conn, driver->devs, devices,
+  NULL, flags);
+}
 
 static virNodeDevicePtr
 testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
@@ -7022,6 +7034,7 @@ static virStorageDriver testStorageDriver = {
 };
 
 static virNodeDeviceDriver testNodeDeviceDriver = {
+.connectListAllNodeDevices = testConnectListAllNodeDevices, /* 4.1.0 */
 .connectNodeDeviceEventRegisterAny = 
testConnectNodeDeviceEventRegisterAny, /* 2.2.0 */
 .connectNodeDeviceEventDeregisterAny = 
testConnectNodeDeviceEventDeregisterAny, /* 2.2.0 */
 .nodeNumOfDevices = testNodeNumOfDevices, /* 0.7.2 */
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] test: Implement virConnectListAllNodeDevices

[Cole Robinson]

First patch is prep work to teach generic nodedev code not
to access host info for the test driver, which is needed to
give correct results for patch 2

Cole Robinson (2):
  conf: nodedev: Don't refresh host caps in testdriver
  test: Implement virConnectListAllNodeDevices

 src/conf/node_device_conf.c |  3 +++
 src/conf/node_device_conf.h |  1 +
 src/test/test_driver.c  | 15 +++
 3 files changed, 19 insertions(+)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] conf: nodedev: Don't refresh host caps in testdriver

[Cole Robinson]

Add a 'testdriver' bool that we set for test_driver.c nodedevs
which will skip accessing host resources via virNodeDeviceUpdateCaps

Signed-off-by: Cole Robinson 
---
 src/conf/node_device_conf.c | 3 +++
 src/conf/node_device_conf.h | 1 +
 src/test/test_driver.c  | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index fd8f4e4a9..90c940f11 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2425,6 +2425,9 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
 {
 virNodeDevCapsDefPtr cap = def->caps;
 
+if (def->testdriver)
+return 0;
+
 while (cap) {
 switch (cap->data.type) {
 case VIR_NODE_DEV_CAP_SCSI_HOST:
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 685ae3034..665f766e2 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -316,6 +316,7 @@ struct _virNodeDeviceDef {
 char *driver;   /* optional driver name */
 char *devnode;  /* /dev path */
 char **devlinks;/* /dev links */
+bool testdriver;/* if true, skip host checks */
 virNodeDevCapsDefPtr caps;  /* optional device capabilities */
 };
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 043caa976..39784c9fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1165,6 +1165,7 @@ testParseNodedevs(testDriverPtr privconn,
 if (!def)
 goto error;
 
+def->testdriver = true;
 if (!(obj = virNodeDeviceObjListAssignDef(privconn->devs, def))) {
 virNodeDeviceDefFree(def);
 goto error;
@@ -5565,6 +5566,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver,
 caps = caps->next;
 }
 
+def->testdriver = true;
 if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
 goto cleanup;
 def = NULL;
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 01/12] tests: Add some tests for PCI controller options

[Laine Stump]

On 02/21/2018 09:14 AM, Andrea Bolognani wrote:
> The input configurations set all existing options for all PCI
> controllers, even those that are not valid for the controller.
> As we implement validation for PCI controller options, we expect
> these test to start failing.

Oh, and s/test/tests/.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 01/12] tests: Add some tests for PCI controller options

[Laine Stump]

On 02/21/2018 09:14 AM, Andrea Bolognani wrote:
> The input configurations set all existing options for all PCI
> controllers, even those that are not valid for the controller.
> As we implement validation for PCI controller options, we expect
> these test to start failing.

A noble cause, but since multiple options are being tested for multiple
controllers in the same file, once you have all the proper checks in
place the tests won't actually be verifying all of the negative tests -
only the first failure will be noticed - if one of the others is missed,
it won't "fail extra hard" or anything.

Although I hate to explode the number of tests, I think if you want to
have proper negative testing for every options that doesn't belong on a
particular controller, then you'll need a separate test case for each
combination of option and controller model. And since that would make
for a *lot* of test cases if we tried to extrapolate to all other
options for all other elements, I don't know that it's worth going down
that rabbit hole.

(What we really need is a more intelligent test rig that would allow
specifying a single test with a list of variations/diffs and a note of
whether the test should succeed or fail with that difference. It would
end up taking the same amount of time to run the tests, but at least the
amount of xml would be drastically reduced.)


>
> Signed-off-by: Andrea Bolognani 
> ---
>  .../i440fx-controllers-pciopts.args| 24 +++
>  .../i440fx-controllers-pciopts.xml | 36 ++
>  .../pseries-controllers-pciopts.args   | 22 +++
>  .../pseries-controllers-pciopts.xml| 35 ++
>  .../qemuxml2argvdata/q35-controllers-pciopts.args  | 28 
>  tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +
>  tests/qemuxml2argvtest.c   | 17 +
>  .../i440fx-controllers-pciopts.xml | 45 +
>  .../pseries-controllers-pciopts.xml| 43 
>  .../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 76 
> ++
>  tests/qemuxml2xmltest.c| 17 +
>  11 files changed, 403 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
>  create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
>  create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args
>  create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
>  create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args
>  create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml
>  create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
>  create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
>  create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
>
> diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args 
> b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
> new file mode 100644
> index 00..d85fae5c96
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
> @@ -0,0 +1,24 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest \
> +-S \
> +-M pc \
> +-m 1024 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-numa node,nodeid=0,cpus=0,mem=1024 \
> +-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
> +-nographic \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=readline \
> +-no-acpi \
> +-boot c \
> +-global i440FX-pcihost.pci-hole64-size=1024K \
> +-device pci-bridge,chassis_nr=2,id=pci.1,bus=pci.0,addr=0x3 \
> +-device pxb,bus_nr=3,id=pci.2,numa_node=0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml 
> b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
> new file mode 100644
> index 00..06008b7338
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
> @@ -0,0 +1,36 @@
> +
> +  guest
> +  496d7ea8-9739-544b-4ebd-ef08be936e8b
> +  1048576
> +  1
> +  
> +hvm
> +  
> +  
> +
> +  
> +
> +  
> +  
> +/usr/bin/qemu-system-x86_64
> +
> +  
> +  
> +  1024
> +
> +
> +  
> +  
> +0
> +  
> +
> +
> +  
> +  
> +0
> +  
> +
> +
> +
> +  
> +
> diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.args 
> b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
> new file mode 100644
> index 00..5f1edfc833
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
> @@ -0,0 +1,22 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> 

Re: [libvirt] [PATCH 00/16] some more vboxDump cleanups

[Laine Stump]

On 02/23/2018 09:27 AM, Ján Tomko wrote:
> Inspired-by: Laine Stump 

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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] nwfilter: save error from DHCP snoop thread to report in main thread

[Laine Stump]

A problem encountered due to a bug in libpcap was reported to the caller as:

   An error occurred, but the cause is unknown

This was because the error had been logged in the DHCPSnoop
thread. The worker thread handling the API call to start a domain
spins up the DHCPSnoop thread which watches for dhcp packets with
libpcap, then uses virCondSignal() to notify the worker thread (which
has been waiting with virCondWait()). The worker thread knows that
there was an error (because threadStatus != THREAD_STATUS_OK), but the
error info had been stored in thread-specific storage for the other
thread, so the worker thread can only report that there was a failure,
but it doesn't know why.

The solution is to save the error that was logged (with
virErrorPreserveLast() into the object the is used to share info
between the threads, then we can set the error in the worker thread
using virErrorRestore().

In the case of the error I was looking at, this changed the "unknown"
message into:

internal error: pcap_setfilter: can't remove kernel filter:
Bad file descriptor

Signed-off-by: Laine Stump 
---
 src/nwfilter/nwfilter_dhcpsnoop.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index 8e955150fa..6069e70460 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -147,6 +147,7 @@ struct _virNWFilterSnoopReq {
 virNWFilterSnoopIPLeasePtr   start;
 virNWFilterSnoopIPLeasePtr   end;
 char*threadkey;
+virErrorPtr  threadError;
 
 virNWFilterSnoopThreadStatus threadStatus;
 virCond  threadStatusCond;
@@ -639,6 +640,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req)
 
 virMutexDestroy(>lock);
 virCondDestroy(>threadStatusCond);
+virFreeError(req->threadError);
 
 VIR_FREE(req);
 }
@@ -1404,10 +1406,12 @@ virNWFilterDHCPSnoopThread(void *req0)
 
 /* let creator know how well we initialized */
 if (error || !threadkey || tmp < 0 || !worker ||
-ifindex != req->ifindex)
+ifindex != req->ifindex) {
+virErrorPreserveLast(>threadError);
 req->threadStatus = THREAD_STATUS_FAIL;
-else
+} else {
 req->threadStatus = THREAD_STATUS_OK;
+}
 
 virCondSignal(>threadStatusCond);
 
@@ -1713,9 +1717,16 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr 
techdriver,
 }
 
 /* sync with thread */
-if (virCondWait(>threadStatusCond, >lock) < 0 ||
-req->threadStatus != THREAD_STATUS_OK)
+if (virCondWait(>threadStatusCond, >lock) < 0) {
+virReportSystemError(errno, "%s",
+ _("unable to wait on dhcp snoop thread"));
 goto exit_snoop_cancel;
+}
+
+if (req->threadStatus != THREAD_STATUS_OK) {
+virErrorRestore(>threadError);
+goto exit_snoop_cancel;
+}
 
 virNWFilterSnoopReqUnlock(req);
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 15/16] vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP

[Ján Tomko]

We can steal the strings instead of creating more copies.

Signed-off-by: Ján Tomko 
---
 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, );
 
 VBOX_UTF16_TO_UTF8(utf16, );
-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, );
 
 VBOX_UTF16_TO_UTF8(utf16, );
-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, );
 
 VBOX_UTF16_TO_UTF8(utf16, );
-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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 02/16] vboxDumpSharedFolders: remove pointless comment

[Ján Tomko]

Now that the functions are separate, we no longer need comment
separators.

Signed-off-by: Ján Tomko 
---
 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 04/16] vboxDumpNetwork: add temp variable for current network

[Ján Tomko]

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 
---
 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, );
 if (adapter) {
@@ -3742,18 +3743,18 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr 
data, IMachine *machine, PRUi
 gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, 
);
 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, 
);
 
 VBOX_UTF16_TO_UTF8(hostIntUtf16, );
-
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, 
);
 
 VBOX_UTF16_TO_UTF8(intNetUtf16, );
-
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, 
);
 
 VBOX_UTF16_TO_UTF8(hostIntUtf16, );
-
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, 
);
 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 

[libvirt] [PATCH 08/16] vboxDumpNetwork: allocate the network too

[Ján Tomko]

Move the allocation from vboxDumpNetworks inside vboxDumpNetwork.

Signed-off-by: Ján Tomko 
---
 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, );
 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, );
@@ -3829,9 +3832,8 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, 
IMachine *machine, PRU
 gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, );
 
 if (enabled) {
-vboxDumpNetwork(net, data, adapter);
-
-netAdpIncCnt++;
+net = vboxDumpNetwork(data, adapter);
+def->nets[netAdpIncCnt++] = net;
 }
 
 VBOX_RELEASE(adapter);
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 05/16] vboxDumpNetwork: rename to vboxDumpNetworks

[Ján Tomko]

Free up 'vboxDumpNetwork' for dumping single network.

Signed-off-by: Ján Tomko 
---
 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 16/16] vboxDumpNetwork: use switch for adapterType

[Ján Tomko]

Also return an error when VIR_STRDUP fails.

Signed-off-by: Ján Tomko 
---
 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, );
-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 >= 351 &&
-   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 >= 351)
+model = "virtio";
+break;
 }
+if (VIR_STRDUP(net->model, model) < 0)
+goto error;
 
 gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, );
 VBOX_UTF16_TO_UTF8(utf16, );
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 12/16] vboxDumpNetwork: Use a single utf16 variable

[Ján Tomko]

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 
---
 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, );
+gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, );
 
-VBOX_UTF16_TO_UTF8(hostIntUtf16, );
+VBOX_UTF16_TO_UTF8(utf16, );
 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, );
+gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, );
 
-VBOX_UTF16_TO_UTF8(intNetUtf16, );
+VBOX_UTF16_TO_UTF8(utf16, );
 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, );
+gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, );
 
-VBOX_UTF16_TO_UTF8(hostIntUtf16, );
+VBOX_UTF16_TO_UTF8(utf16, );
 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, );
-VBOX_UTF16_TO_UTF8(MACAddressUtf16, );
-VBOX_UTF16_FREE(MACAddressUtf16);
+gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, );
+VBOX_UTF16_TO_UTF8(utf16, );
+VBOX_UTF16_FREE(utf16);
 
 if (virMacAddrParseHex(MACAddress, >mac) < 0) {
 VBOX_UTF8_FREE(MACAddress);
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 13/16] vboxDumpNetwork: Use a single utf8 temp variable

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 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, );
 
-VBOX_UTF16_TO_UTF8(utf16, );
-ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt));
+VBOX_UTF16_TO_UTF8(utf16, );
+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, );
 
-VBOX_UTF16_TO_UTF8(utf16, );
-ignore_value(VIR_STRDUP(net->data.internal.name, intNet));
+VBOX_UTF16_TO_UTF8(utf16, );
+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, );
 
-VBOX_UTF16_TO_UTF8(utf16, );
-ignore_value(VIR_STRDUP(net->data.network.name, hostInt));
+VBOX_UTF16_TO_UTF8(utf16, );
+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, );
-VBOX_UTF16_TO_UTF8(utf16, );
+VBOX_UTF16_TO_UTF8(utf16, );
 VBOX_UTF16_FREE(utf16);
 
-if (virMacAddrParseHex(MACAddress, >mac) < 0) {
-VBOX_UTF8_FREE(MACAddress);
+if (virMacAddrParseHex(utf8, >mac) < 0) {
+VBOX_UTF8_FREE(utf8);
 goto error;
 }
 
-VBOX_UTF8_FREE(MACAddress);
+VBOX_UTF8_FREE(utf8);
 return net;
 
  error:
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/16] vboxDumpSharedFolders: return a value

[Ján Tomko]

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 
---
 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(, 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();
+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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 14/16] vboxDumpNetwork: use a switch for attachmentType

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 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, );
-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, );
@@ -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, );
@@ -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, );
@@ -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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 09/16] vboxDumpNetworks: delete pointless comment

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 07/16] vboxDumpNetworks: reduce indentation level

[Ján Tomko]

The 'enabled' bool is initialized to false, there is no need to nest the
conditions.

Signed-off-by: Ján Tomko 
---
 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, );
-if (adapter) {
-PRBool enabled = PR_FALSE;
-
+if (adapter)
 gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, );
-if (enabled) {
-vboxDumpNetwork(net, data, adapter);
 
-netAdpIncCnt++;
-}
+if (enabled) {
+vboxDumpNetwork(net, data, adapter);
 
-VBOX_RELEASE(adapter);
+netAdpIncCnt++;
 }
+
+VBOX_RELEASE(adapter);
 }
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 10/16] vboxDumpNetworks: do not allocate def->nets upfront

[Ján Tomko]

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 
---
 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, );
-if (adapter) {
-PRBool enabled = PR_FALSE;
-
-gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, );
-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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 11/16] vboxDumpNetwork: use virMacAddrParseHex

[Ján Tomko]

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 
---
 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, );
 VBOX_UTF16_TO_UTF8(MACAddressUtf16, );
-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, >mac));
+if (virMacAddrParseHex(MACAddress, >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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 01/16] vboxDumpSharedFolders: rename non-standard label

[Ján Tomko]

s/sharedFoldersCleanup/cleanup/

Signed-off-by: Ján Tomko 
---
 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();
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/16] vboxDumpNetwork: re-introduce this function

[Ján Tomko]

Split out per-adapter code from vboxDumpNetworks.

Signed-off-by: Ján Tomko 
---
 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, );
+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, );
+
+VBOX_UTF16_TO_UTF8(hostIntUtf16, );
+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, );
+
+VBOX_UTF16_TO_UTF8(intNetUtf16, );
+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, );
+
+VBOX_UTF16_TO_UTF8(hostIntUtf16, );
+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, );
+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 >= 351 &&
+   adapterType == NetworkAdapterType_Virtio) {
+/* Only vbox 3.1 and later support NetworkAdapterType_Virto */
+ignore_value(VIR_STRDUP(net->model, "virtio"));
+}
+
+gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, );
+VBOX_UTF16_TO_UTF8(MACAddressUtf16, );
+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, >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, );
 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, 
);
-if (attachmentType == NetworkAttachmentType_NAT) {
-
-net->type = VIR_DOMAIN_NET_TYPE_USER;
-
-} else if (attachmentType == NetworkAttachmentType_Bridged) {
- 

[libvirt] [PATCH 00/16] some more vboxDump cleanups

[Ján Tomko]

Inspired-by: Laine Stump 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH jenkins-ci] Fix workspace for python check/rpm jobs

[Daniel P . Berrangé]

The check/rpm jobs were using a different workspace to the build job, so
never saw the correct content.

Signed-off-by: Daniel P. Berrangé 
---

Pushed as a build fix for virt-manager CI

 jobs/python-distutils.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index 510769e..16eca1c 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -58,7 +58,7 @@
 name: '{name}-{branch}-py{pyver}-check'
 project-type: matrix
 description: '{title} Check (Python {pyver})'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}-py{pyver}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -97,7 +97,7 @@
 name: '{name}-{branch}-py{pyver}-rpm'
 project-type: matrix
 description: '{title} RPM (Python {pyver})'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}-py{pyver}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tools: avoid text spilling into variables

[Michal Privoznik]

On 01/16/2018 04:05 PM, Dariusz Gadomski wrote:
> From: Christian Ehrhardt 
> 
> While libvirt-guests.sh is running cases can let guest_is_on fail which
> causes check_guests_shutdown to print output.
> That output shall not spill into the users of function
> check_guests_shutdown which is therefore now returning values in a
> variable like guest_is_on already did.
> 
> Original-Author: Christian Ehrhardt 
> Modified-By: Jorge Niedbalski 
> ---
>  tools/libvirt-guests.sh.in | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index 8a158cca4..91a2f3283 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -329,12 +329,13 @@ guest_count()
>  # check_guests_shutdown URI GUESTS
>  # check if shutdown is complete on guests in "GUESTS" and returns only
>  # guests that are still shutting down
> +# Result is returned in "guests_shutting_down"
>  check_guests_shutdown()
>  {
>  uri=$1
>  guests=$2
>  
> -guests_up=
> +guests_shutting_down=
>  for guest in $guests; do
>  if ! guest_is_on "$uri" "$guest" >/dev/null 2>&1; then
>  eval_gettext "Failed to determine state of guest: \$guest. Not 
> tracking it anymore."
> @@ -342,10 +343,9 @@ check_guests_shutdown()
>  continue
>  fi
>  if "$guest_running"; then
> -guests_up="$guests_up $guest"
> +guests_shutting_down="$guests_shutting_down $guest"
>  fi
>  done
> -echo "$guests_up"
>  }
>  
>  # print_guests_shutdown URI BEFORE AFTER
> @@ -392,8 +392,10 @@ shutdown_guests_parallel()
>  guest=$1
>  shift
>  guests=$*
> -shutdown_guest_async "$uri" "$guest"
> -on_shutdown="$on_shutdown $guest"
> +if [ -z "$(echo $on_shutdown | grep $guest)" -a -n "$(guest_name 
> "$uri" "$guest")" ]; then

We prefer if test cond1 && test cond2 over -a.

I've fixed that, ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Libvirt-ci] Build failed in Jenkins: virt-manager-master-py3-check » libvirt-debian-9 #14

[Daniel P . Berrangé]

On Thu, Feb 22, 2018 at 05:02:15PM -0500, Cole Robinson wrote:
> I'm confused why this is still failing. I set up a debian9 vm,
> reproduced the issue which fails about 50% of the time, and verified
> that commit 059fb7d0ba1d414db3db485903972b48c1dda629 fixes it
> completely, or at least I can't reproduce. But this is still failing.

> 
> Weirdly though
> 
> On 02/22/2018 04:18 PM, c...@centos.org wrote:
> > See 
> > 
> > 
> > --
> > Started by upstream project "virt-manager-master-py3-check" build number 14
> > originally caused by:
> >  Started by upstream project "virt-manager-master-py3-build" build number 14
> >  originally caused by:
> >   Started by an SCM change
> > [EnvInject] - Loading node environment variables.
> > Building remotely on libvirt-debian-9 (libvirt) in workspace 
> > 
> > [virt-manager-master] $ /bin/sh -xe /tmp/jenkins3940620727838286920.sh
> > + MAKE=make
> > + uname
> > + unamestr=Linux
> > + [ Linux = FreeBSD ]
> > + export 
> > PATH=/home/jenkins/build/libvirt/bin:/home/jenkins/build/libvirt/bin:/home/jenkins/build/libvirt/bin:/home/jenkins/build/libvirt/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
> > + export 
> > PYTHONPATH=/home/jenkins/build/libvirt/lib/python3.4/site-packages:/home/jenkins/build/libvirt/lib64/python3.4/site-packages:/home/jenkins/build/libvirt/lib/python3.5/site-packages:/home/jenkins/build/libvirt/lib64/python3.5/site-packages:/home/jenkins/build/libvirt/lib/python3.6/site-packages:/home/jenkins/build/libvirt/lib64/python3.6/site-packages:/home/jenkins/build/libvirt/lib/python3.7/site-packages:/home/jenkins/build/libvirt/lib64/python3.7/site-packages
> > + python3 ./setup.py test
> > running test
> > ..F...s
> > ==
> > FAIL: testCLI0002virt_install_singleton_config_2 (tests.clitest.CLITests)
> > --
> > Traceback (most recent call last):
> >   File 
> > "
> >  line 1110, in 
> > return lambda s: cmdtemplate(s, cmd)
> >   File 
> > "
> >  line 1109, in cmdtemplate
> > _cmdobj.run(self)
> >   File 
> > "
> >  line 277, in run
> > tests.fail(err)
> 
> These line numbers don't match up with the line numbers in virt-manager
> git master. Is there some chance it's using old code? it certainly seems
> to be detecting new commits coming in which trigger builds, but I thing
> something weird is going on here

Yeah its very odd. I've blown away the git checkout and it succeeed on
rebuild, so lets see if it stays fixed...


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/4] travis: make all builds use VPATH

[Daniel P . Berrangé]

VPATH is not well tested by developers, so ensure travis exercises VPATH
in all scenarios.

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 4bdf034829..41a293451c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -103,7 +103,7 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && 
brew install rpcgen yajl; fi
 
 before_script:
-  - ./autogen.sh
+  - mkdir build && cd build && ../autogen.sh
 
 script:
   # Many unit tests still fail on macOS, and there are a bunch of issues with
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/4] travis: test upstart script handling on precise distro scenario

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 41a293451c..0328fcb8f1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,6 +6,8 @@ matrix:
   include:
 - compiler: gcc
   dist: precise
+  env:
+- CONFIGURE_ARGS=--with-init-script=upstart
 # Special scenario to run distcheck, so we don't waste time duplicating
 # work in all the other scenarios. Doesn't work on precise due to the
 # CVE-2012-3386 flaw being present on that Ubuntu version
@@ -103,7 +105,7 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && 
brew install rpcgen yajl; fi
 
 before_script:
-  - mkdir build && cd build && ../autogen.sh
+  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS
 
 script:
   # Many unit tests still fail on macOS, and there are a bunch of issues with
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/4] travis: add a scenario for running make distcheck

[Daniel P . Berrangé]

Running "make distcheck" ensures that we have CLEANFILES and uninstall
rules setup correctly, as well as validating VPATH builds succeeed.

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 3f26a1..4bdf034829 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,8 +6,13 @@ matrix:
   include:
 - compiler: gcc
   dist: precise
+# Special scenario to run distcheck, so we don't waste time duplicating
+# work in all the other scenarios. Doesn't work on precise due to the
+# CVE-2012-3386 flaw being present on that Ubuntu version
 - compiler: gcc
   dist: trusty
+  script:
+  - make -j3 distcheck
 - compiler: clang
   dist: precise
 - compiler: clang
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/4] travis: run 'make install' during build tests

[Daniel P . Berrangé]

Running 'make install' is important to catch some VPATH problems

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 0328fcb8f1..61f0e38d40 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -105,12 +105,12 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && 
brew install rpcgen yajl; fi
 
 before_script:
-  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS
+  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS 
--prefix=$(pwd)/../vroot
 
 script:
   # Many unit tests still fail on macOS, and there are a bunch of issues with
   # syntax-check as well, so skip those steps on that platform for now
-  - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check 
&& make -j3 check; fi
+  - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check 
&& make -j3 check; fi && make -j3 install
 
 after_failure:
   - echo 
''
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/4] Misc travis improvements

[Daniel P . Berrangé]

Various improvements to travis coverage. The first patch was posted
separately yesterday too.

Daniel P. Berrangé (4):
  travis: add a scenario for running make distcheck
  travis: make all builds use VPATH
  travis: test upstart script handling on precise distro scenario
  travis: run 'make install' during build tests

 .travis.yml | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] make: fix VPATH install of upstart files

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---

Pushed as a build fix

 src/Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index a7f03319c1..cb30c50f14 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -3181,7 +3181,8 @@ install-upstart: install-sysconfig
for f in $(UPSTART_FILES:%.upstart=%); \
do \
  tgt=`basename $$f` ; \
- $(INSTALL_SCRIPT) $$f.upstart $(DESTDIR)$(sysconfdir)/event.d/$$tgt ; 
\
+ $(INSTALL_SCRIPT) $(srcdir)/$$f.upstart \
+ $(DESTDIR)$(sysconfdir)/event.d/$$tgt ; \
done
 
 uninstall-upstart: uninstall-sysconfig
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v1 00/16] Start to modularize src/Makefile.am

[Michal Privoznik]

On 02/22/2018 01:56 PM, Daniel P. Berrangé wrote:
> The src/Makefile.am is quite large and has quite poor grouping of rules
> / variables / etc. This makes it increasingly hard to identify all the
> rules relating to a particular area of code.
> 
> Traditionally one might create Makefile.am's in each sub-directory and
> let make recurse into each. Recursive make is quite a bad idea in
> general though because it harms parallelization and means that make does
> not have a full view of dependencies.
> 
> This series thus takes a different approach to modularization which is
> to make use of the "include" statement to pull in makefile fragments
> from subdirectories.  automake fully expands all "include" statements
> when generating the Makefile.in, so we still end up with a single
> monolithic file for the eventual build. Aside from ensuring make still
> has a full view of dependencies, this also means that all variables are
> still in a global namespace.
> 
> In doing this split, I've taken the opportunity to santize the variable
> declarations into a consistent style. Any variable that is assigned more
> than one value now uses line continuations with exactly one value per
> line and a trailing $(NULL). It would be nice to enforce this style with
> a syntax-check rule but I've not figured this out yet.
> 
> This series only moves the virt drivers. So there's obviously a further
> series to follow behind this to finish the job for other drivers.
> 
> Daniel P. Berrangé (16):
>   make: split UML driver build rules into uml/Makefile.inc.am
>   make: split PHyp driver build rules into phyp/Makefile.inc.am
>   make: split test driver build rules into test/Makefile.inc.am
>   make: split ESX driver build rules into esx/Makefile.inc.am
>   make: split hyperv driver build rules into hyperv/Makefile.inc.am
>   make: split vmware driver build rules into vmware/Makefile.inc.am
>   make: split vbox driver build rules into vbox/Makefile.inc.am
>   make: split openvz driver build rules into openvz/Makefile.inc.am
>   make: split qemu driver build rules into qemu/Makefile.inc.am
>   make: split bhyve driver build rules into bhyve/Makefile.inc.am
>   make: split xenconfig driver build rules into
> xenconfig/Makefile.inc.am
>   make: split libxl driver build rules into libxl/Makefile.inc.am
>   make: split xen driver build rules into xen/Makefile.inc.am
>   make: split xenapi driver build rules into xenapi/Makefile.inc.am
>   make: split vz driver build rules into vz/Makefile.inc.am
>   make: split lxc driver build rules into lxc/Makefile.inc.am
> 
>  src/Makefile.am   | 872 
> +++---
>  src/bhyve/Makefile.inc.am |  73 
>  src/esx/Makefile.inc.am   |  90 +
>  src/hyperv/Makefile.inc.am|  59 +++
>  src/libxl/Makefile.inc.am | 104 +
>  src/lxc/Makefile.inc.am   | 207 ++
>  src/openvz/Makefile.inc.am|  28 ++
>  src/phyp/Makefile.inc.am  |  19 +
>  src/qemu/Makefile.inc.am  | 148 +++
>  src/test/Makefile.inc.am  |  26 ++
>  src/uml/Makefile.inc.am   |  46 +++
>  src/vbox/Makefile.inc.am  |  76 
>  src/vmware/Makefile.inc.am|  27 ++
>  src/vz/Makefile.inc.am|  38 ++
>  src/xen/Makefile.inc.am   |  67 
>  src/xenapi/Makefile.inc.am|  28 ++
>  src/xenconfig/Makefile.inc.am |  28 ++
>  17 files changed, 1122 insertions(+), 814 deletions(-)
>  create mode 100644 src/bhyve/Makefile.inc.am
>  create mode 100644 src/esx/Makefile.inc.am
>  create mode 100644 src/hyperv/Makefile.inc.am
>  create mode 100644 src/libxl/Makefile.inc.am
>  create mode 100644 src/lxc/Makefile.inc.am
>  create mode 100644 src/openvz/Makefile.inc.am
>  create mode 100644 src/phyp/Makefile.inc.am
>  create mode 100644 src/qemu/Makefile.inc.am
>  create mode 100644 src/test/Makefile.inc.am
>  create mode 100644 src/uml/Makefile.inc.am
>  create mode 100644 src/vbox/Makefile.inc.am
>  create mode 100644 src/vmware/Makefile.inc.am
>  create mode 100644 src/vz/Makefile.inc.am
>  create mode 100644 src/xen/Makefile.inc.am
>  create mode 100644 src/xenapi/Makefile.inc.am
>  create mode 100644 src/xenconfig/Makefile.inc.am
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V2] libxl: round memory values to next 1MiB increment

[Peter Krempa]

On Thu, Feb 22, 2018 at 10:31:48 -0700, Jim Fehlig wrote:
> libxl requires the memory sizes to be rounded to 1MiB increments.
> Attempting to start a domain that violates this requirement will
> fail with the marginally helpful error
> 
> 2018-02-22 01:55:32.921+: xc: panic: xc_dom_boot.c:141: 
> xc_dom_boot_mem_init: can't allocate low memory for domain: Out of memory
> 2018-02-22 01:55:32.921+: libxl: libxl_dom.c:671:libxl__build_dom: 
> xc_dom_boot_mem_init failed: No such file or directory
> 
> Round the maximum and current memory values to the next 1MiB
> increment when generating the libxl_domain_config object.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> V2:
> Set rounded memory values in virDomainDef object so they are correctly
> reflected in the live config.

ACK


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libvirt.spec.in: Don't distribute libvirtd.upstart

[Pavel Hrdina]

On Fri, Feb 23, 2018 at 09:50:38AM +0100, Michal Privoznik wrote:
> Firstly, for rpm we are building libvirt with
> --init-script=systemd or --init-script=redhat. So upstart is
> never enabled. And only due to a bug we installed
> libvirtd.upstart file.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  libvirt.spec.in | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] qemu: Fix updating device with boot order

[Michal Privoznik]

On 02/22/2018 03:21 PM, Jiri Denemark wrote:
> Commit v3.7.0-14-gc57f3fd2f8 prevented adding a 
> element to an inactive domain with global  element.
> However, as a result of that change updating any device with boot order
> would fail with 'boot order X is already used by another device', where
> "another device" is in fact the device which is being updated.
> 
> To fix this we have to ignore the device which we're about to update
> when checking for boot order conflicts.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1546971
> 
> Jiri Denemark (3):
>   lxc: Drop useless check in live device update
>   Pass oldDev to virDomainDefCompatibleDevice on device update
>   qemu: Fix updating device with boot order
> 
>  src/conf/domain_conf.c | 30 ++---
>  src/conf/domain_conf.h |  3 ++-
>  src/lxc/lxc_driver.c   | 18 +-
>  src/qemu/qemu_driver.c | 51 
> --
>  4 files changed, 75 insertions(+), 27 deletions(-)
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu_hotplug: Drop dead code in net update

[Michal Privoznik]

On 02/22/2018 03:00 PM, Jiri Denemark wrote:
> vm->def->nets[changeidx] can never be NULL for changeidx returned by
> virDomainNetFindIdx.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_hotplug.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] libvirt.spec.in: Don't distribute libvirtd.upstart

[Michal Privoznik]

Firstly, for rpm we are building libvirt with
--init-script=systemd or --init-script=redhat. So upstart is
never enabled. And only due to a bug we installed
libvirtd.upstart file.

Signed-off-by: Michal Privoznik 
---
 libvirt.spec.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 1eca4e39d..8f46f58b5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1825,7 +1825,6 @@ exit 0
 %{_sysconfdir}/rc.d/init.d/virtlogd
 %{_sysconfdir}/rc.d/init.d/virtlockd
 %endif
-%doc daemon/libvirtd.upstart
 %config(noreplace) %{_sysconfdir}/sysconfig/libvirtd
 %config(noreplace) %{_sysconfdir}/sysconfig/virtlogd
 %config(noreplace) %{_sysconfdir}/sysconfig/virtlockd
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list