[libvirt] [PATCH v2] qemu: monitor: Fix a memory leak in qemuMonitorJSONAttachCharDevCommand
by Erik Skultety
With the current logic, we only free @tlsalias as part of the error
label and would have to free it explicitly earlier in the code. Convert
the error label to cleanup, so that we have only one sink, where we
handle all frees. Since since JSON object append operation consumes
pointers, make sure @backend is cleared before we hit the cleanup label.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f208dd05a..5ddc09ca6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6430,8 +6430,8 @@ static virJSONValuePtr
qemuMonitorJSONAttachCharDevCommand(const char *chrID,
const virDomainChrSourceDef *chr)
{
- virJSONValuePtr ret;
- virJSONValuePtr backend;
+ virJSONValuePtr ret = NULL;
+ virJSONValuePtr backend = NULL;
virJSONValuePtr data = NULL;
virJSONValuePtr addr = NULL;
const char *backend_type = NULL;
@@ -6440,7 +6440,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (!(backend = virJSONValueNewObject()) ||
!(data = virJSONValueNewObject())) {
- goto error;
+ goto cleanup;
}
switch ((virDomainChrType) chr->type) {
@@ -6456,14 +6456,14 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
case VIR_DOMAIN_CHR_TYPE_FILE:
backend_type = "file";
if (virJSONValueObjectAppendString(data, "out", chr->data.file.path) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
backend_type = STRPREFIX(chrID, "parallel") ? "parallel" : "serial";
if (virJSONValueObjectAppendString(data, "device",
chr->data.file.path) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_TCP:
@@ -6472,7 +6472,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
chr->data.tcp.service);
if (!addr ||
virJSONValueObjectAppend(data, "addr", addr) < 0)
- goto error;
+ goto cleanup;
addr = NULL;
telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
@@ -6480,13 +6480,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
virJSONValueObjectAppendBoolean(data, "telnet", telnet) < 0 ||
virJSONValueObjectAppendBoolean(data, "server", chr->data.tcp.listen) < 0)
- goto error;
+ goto cleanup;
if (chr->data.tcp.tlscreds) {
if (!(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID)))
- goto error;
+ goto cleanup;
if (virJSONValueObjectAppendString(data, "tls-creds", tlsalias) < 0)
- goto error;
+ goto cleanup;
}
break;
@@ -6496,14 +6496,14 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
chr->data.udp.connectService);
if (!addr ||
virJSONValueObjectAppend(data, "remote", addr) < 0)
- goto error;
+ goto cleanup;
if (chr->data.udp.bindHost) {
addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.bindHost,
chr->data.udp.bindService);
if (!addr ||
virJSONValueObjectAppend(data, "local", addr) < 0)
- goto error;
+ goto cleanup;
}
addr = NULL;
break;
@@ -6514,12 +6514,12 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (!addr ||
virJSONValueObjectAppend(data, "addr", addr) < 0)
- goto error;
+ goto cleanup;
addr = NULL;
if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
virJSONValueObjectAppendBoolean(data, "server", chr->data.nix.listen) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
@@ -6527,7 +6527,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (virJSONValueObjectAppendString(data, "type",
virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
@@ -6544,28 +6544,27 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
_("Hotplug unsupported for char device type '%d'"),
chr->type);
}
- goto error;
+ goto cleanup;
}
if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0 ||
virJSONValueObjectAppend(backend, "data", data) < 0)
- goto error;
+ goto cleanup;
data = NULL;
if (!(ret = qemuMonitorJSONMakeCommand("chardev-add",
"s:id", chrID,
"a:backend", backend,
NULL)))
- goto error;
+ goto cleanup;
+ backend = NULL;
- return ret;
-
- error:
+ cleanup:
VIR_FREE(tlsalias);
virJSONValueFree(addr);
virJSONValueFree(data);
virJSONValueFree(backend);
- return NULL;
+ return ret;
}
--
2.13.1
7 years, 5 months
[libvirt] [PATCH] qemu: monitor: Fix a memory leak in qemuMonitorJSONAttachCharDevCommand
by Erik Skultety
With the current logic, we only free @tlsalias as part of the error
label and would have to free it explicitly earlier in the code. Convert
the error label to cleanup, so that we have only one sink, where we
handle all frees. In order to do that we need to clear some JSON obj
pointers down the success road to avoid SIGSEGV, since JSON object
append operation consumes pointers.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 47 ++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f208dd05a..b8b73926f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6430,8 +6430,8 @@ static virJSONValuePtr
qemuMonitorJSONAttachCharDevCommand(const char *chrID,
const virDomainChrSourceDef *chr)
{
- virJSONValuePtr ret;
- virJSONValuePtr backend;
+ virJSONValuePtr ret = NULL;
+ virJSONValuePtr backend = NULL;
virJSONValuePtr data = NULL;
virJSONValuePtr addr = NULL;
const char *backend_type = NULL;
@@ -6440,7 +6440,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (!(backend = virJSONValueNewObject()) ||
!(data = virJSONValueNewObject())) {
- goto error;
+ goto cleanup;
}
switch ((virDomainChrType) chr->type) {
@@ -6456,14 +6456,14 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
case VIR_DOMAIN_CHR_TYPE_FILE:
backend_type = "file";
if (virJSONValueObjectAppendString(data, "out", chr->data.file.path) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
backend_type = STRPREFIX(chrID, "parallel") ? "parallel" : "serial";
if (virJSONValueObjectAppendString(data, "device",
chr->data.file.path) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_TCP:
@@ -6472,21 +6472,20 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
chr->data.tcp.service);
if (!addr ||
virJSONValueObjectAppend(data, "addr", addr) < 0)
- goto error;
- addr = NULL;
+ goto cleanup;
telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
virJSONValueObjectAppendBoolean(data, "telnet", telnet) < 0 ||
virJSONValueObjectAppendBoolean(data, "server", chr->data.tcp.listen) < 0)
- goto error;
+ goto cleanup;
if (chr->data.tcp.tlscreds) {
if (!(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID)))
- goto error;
+ goto cleanup;
if (virJSONValueObjectAppendString(data, "tls-creds", tlsalias) < 0)
- goto error;
+ goto cleanup;
}
break;
@@ -6496,16 +6495,15 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
chr->data.udp.connectService);
if (!addr ||
virJSONValueObjectAppend(data, "remote", addr) < 0)
- goto error;
+ goto cleanup;
if (chr->data.udp.bindHost) {
addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.bindHost,
chr->data.udp.bindService);
if (!addr ||
virJSONValueObjectAppend(data, "local", addr) < 0)
- goto error;
+ goto cleanup;
}
- addr = NULL;
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -6514,12 +6512,11 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (!addr ||
virJSONValueObjectAppend(data, "addr", addr) < 0)
- goto error;
- addr = NULL;
+ goto cleanup;
if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
virJSONValueObjectAppendBoolean(data, "server", chr->data.nix.listen) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
@@ -6527,7 +6524,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (virJSONValueObjectAppendString(data, "type",
virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0)
- goto error;
+ goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
@@ -6544,28 +6541,30 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
_("Hotplug unsupported for char device type '%d'"),
chr->type);
}
- goto error;
+ goto cleanup;
}
if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0 ||
virJSONValueObjectAppend(backend, "data", data) < 0)
- goto error;
- data = NULL;
+ goto cleanup;
if (!(ret = qemuMonitorJSONMakeCommand("chardev-add",
"s:id", chrID,
"a:backend", backend,
NULL)))
- goto error;
+ goto cleanup;
- return ret;
+ /* we must not free the following pointers as they've been collectively
+ * consumed by @ret, so clear them first
+ */
+ addr = data = backend = NULL;
- error:
+ cleanup:
VIR_FREE(tlsalias);
virJSONValueFree(addr);
virJSONValueFree(data);
virJSONValueFree(backend);
- return NULL;
+ return ret;
}
--
2.13.1
7 years, 5 months
[libvirt] [PATCH 00/26] Support multiple PHBs for pSeries guests
by Andrea Bolognani
Book 1: A Journey Begins
========================
These commits merely set the stage for what's coming next.
Adventure! Challenges! Friendship! Perhaps even love?
00-03: Trivial cleanups
04-05: Test suite changes that will make implementing
future test cases possible; get them out of the
way early in order to make patches more
straightforward when the time finally comes
06: Simple is beautiful
Book 2: From Great POWERs
=========================
Our hero faces and overcomes many hardships and by doing so
grows as an individual, but something is still amiss.
07-09: Get ready to remove some assumptions...
10: ... and then promptly remove them. Hashtag efficient
11-17: Introduce new characters-I mean, XML elements and
attributes and capabilities and all that jazz. Close
enough, I say.
18: By now, our hero has acquired amazing powers such as
being able to create pSeries guests with multiple
PHBs; unfortunately, he still has to rely on his
strenght and his strength alone when facing new
challenges. Wouldn't it be great if he could get
some help? Maybe that's exactly where the story is
heading, but we'll have to wait for the next books
to find out...
19: Prove to the world that the journey is more important
than the destination, yet you still gotta get there
at some point
Book 3: With a Little Help
==========================
Joined by a ragtag group of newfound friends, our hero can
push forward knowing that he will always be able to count
on their help, and that he'll never again be alone.
20: You can only know how far you've come if you marked
your starting point on the map
21-22: The Evil One can only recoil in horror as its
minions are banished from the land, one by one.
Onwards to the final battle!
Book 4: The Parting of the Ways
===============================
Every journey must come to an end. The Evil One is at last
ready to set in motion its plan to take over the entire land;
our hero and his companions are the only obstacles standing
in its way. Will they be able to stop impending doom? Maybe,
but chances are sure stacked against them. Will knowing this
stop them from fighting until their last breath? Of course
not. Is the title a Doctor Who reference? You better believe
it is!
23-25: More characters join the fray, as the narrative
gears up for an epic finale
26: The final battle takes place. Swords will clash,
spells will be cast, old friends will fall, and
when all is said and done... Well, you didn't
really expect me to give away the ending so
easily, did you? :)
Book 5: ???
===========
Whew, that was quite something, wasn't it? But wait, there's
more! The fifth book in this engrossing tetralogy is still
being drafted, but is meant to be published along with the
rest and provide insights and details that might have
escaped even the most attentive reader... Yes, I'm talking
about documentation here.
Hardcover and paperback editions available. Signed copies
can be purchased via mail order.
Andrea Bolognani (26):
conf: Remove obsolete comment
conf: Make virDomainPCIAddressSetGrow() private
conf: Tweak virDomainPCIAddressGetNextAddr() signature
tests: Update qemumemlock data
tests: Mock IOMMU groups
conf: Simplify slot allocation
qemu: Allow qemuBuildControllerDevStr() to return NULL
qemu: Tweak index number checking
conf: Move index number checking to drivers
qemu: Relax pci-root index requirement for pSeries guests
schema: Allow <target index='...'/>
conf: Parse and format <target index='...'/>
schema: Allow 'spapr-pci-host-bridge' controller model
conf: Add 'spapr-pci-host-bridge' controller model
qemu: Automatically pick index and model for pci-root controllers
qemu: Introduce QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE
qemu: Deal with PHB naming convention
qemu: Use multiple PHBs for pSeries guests
tests: Add tests for pSeries guests with multiple PHBs
tests: Add baseline tests for automatic PHB usage
qemu: Use PHBs to fill holes in PCI bus numbering
qemu: Use PHBs when extending the guest PCI topology
conf: Introduce isolation groups
schema: Allow <isolationGroup/>
conf: Parse and format <isolationGroup/>
qemu: Isolate hostdevs on pSeries guests
docs/schemas/domaincommon.rng | 12 ++
src/bhyve/bhyve_device.c | 4 +-
src/bhyve/bhyve_domain.c | 15 ++
src/conf/device_conf.h | 8 +-
src/conf/domain_addr.c | 138 +++++++--------
src/conf/domain_addr.h | 28 +--
src/conf/domain_conf.c | 50 +++++-
src/conf/domain_conf.h | 6 +
src/libvirt_private.syms | 1 -
src/libxl/libxl_domain.c | 14 ++
src/lxc/lxc_domain.c | 14 ++
src/openvz/openvz_driver.c | 14 ++
src/qemu/qemu_capabilities.c | 3 +
src/qemu/qemu_capabilities.h | 3 +
src/qemu/qemu_command.c | 146 +++++++++++++---
src/qemu/qemu_command.h | 9 +-
src/qemu/qemu_domain.c | 14 ++
src/qemu/qemu_domain_address.c | 190 ++++++++++++++++++---
src/qemu/qemu_hotplug.c | 5 +-
src/uml/uml_driver.c | 14 ++
src/vz/vz_driver.c | 14 ++
src/xen/xen_driver.c | 14 ++
.../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 5 +-
.../qemuargv2xml-pseries-nvram.xml | 5 +-
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
.../qemumemlock-pc-hardlimit+hostdev.xml | 2 +-
.../qemumemlock-pc-hardlimit+locked+hostdev.xml | 2 +-
tests/qemumemlockdata/qemumemlock-pc-hostdev.xml | 2 +-
.../qemumemlock-pc-locked+hostdev.xml | 2 +-
.../qemumemlock-pseries-hardlimit+hostdev.xml | 2 +-
...emumemlock-pseries-hardlimit+locked+hostdev.xml | 2 +-
.../qemumemlock-pseries-hostdev.xml | 2 +-
.../qemumemlock-pseries-locked+hostdev.xml | 2 +-
tests/qemumemlocktest.c | 21 ++-
.../qemuxml2argv-pseries-hostdev.args | 25 +++
.../qemuxml2argv-pseries-hostdev.xml | 33 ++++
.../qemuxml2argv-pseries-many-buses-1.args | 22 +++
.../qemuxml2argv-pseries-many-buses-1.xml | 20 +++
.../qemuxml2argv-pseries-many-buses-2.args | 22 +++
.../qemuxml2argv-pseries-many-buses-2.xml | 18 ++
.../qemuxml2argv-pseries-many-devices.args | 53 ++++++
.../qemuxml2argv-pseries-many-devices.xml | 49 ++++++
.../qemuxml2argv-pseries-phb-default-missing.args | 22 +++
.../qemuxml2argv-pseries-phb-default-missing.xml | 16 ++
.../qemuxml2argv-pseries-phb-simple.args | 22 +++
.../qemuxml2argv-pseries-phb-simple.xml | 17 ++
tests/qemuxml2argvtest.c | 51 +++++-
.../qemuxml2xmlout-panic-pseries.xml | 5 +-
.../qemuxml2xmlout-ppc64-usb-controller-legacy.xml | 5 +-
.../qemuxml2xmlout-ppc64-usb-controller.xml | 5 +-
.../qemuxml2xmlout-pseries-hostdev.xml | 56 ++++++
...xml => qemuxml2xmlout-pseries-many-buses-1.xml} | 19 ++-
...xml => qemuxml2xmlout-pseries-many-buses-2.xml} | 20 ++-
.../qemuxml2xmlout-pseries-many-devices.xml | 125 ++++++++++++++
.../qemuxml2xmlout-pseries-nvram.xml | 5 +-
.../qemuxml2xmlout-pseries-panic-missing.xml | 5 +-
.../qemuxml2xmlout-pseries-panic-no-address.xml | 5 +-
...qemuxml2xmlout-pseries-phb-default-missing.xml} | 18 +-
...m.xml => qemuxml2xmlout-pseries-phb-simple.xml} | 18 +-
tests/qemuxml2xmltest.c | 47 ++++-
tests/virpcimock.c | 43 ++++-
61 files changed, 1306 insertions(+), 204 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-hostdev.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-hostdev.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-1.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-1.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-devices.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-many-devices.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-default-missing.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-default-missing.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-hostdev.xml
copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-nvram.xml => qemuxml2xmlout-pseries-many-buses-1.xml} (55%)
copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-nvram.xml => qemuxml2xmlout-pseries-many-buses-2.xml} (54%)
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-many-devices.xml
copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-nvram.xml => qemuxml2xmlout-pseries-phb-default-missing.xml} (56%)
copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-nvram.xml => qemuxml2xmlout-pseries-phb-simple.xml} (56%)
--
2.7.5
7 years, 5 months
[libvirt] [PATCH 0/3] Yet another round of hugepage fixes
by Michal Privoznik
This time with some tests.
You may recognize 2/3 which I've posted here:
https://www.redhat.com/archives/libvir-list/2017-June/msg00258.html
Michal Privoznik (3):
qemuxml2xmltest: Test hugepage enabled domains
qemu: Allow memAccess for hugepages again
qemu: Prefer hugepages over mem source='file'
src/qemu/qemu_command.c | 24 +++------
.../qemuxml2argv-fd-memory-numa-topology2.args | 2 +-
.../qemuxml2argv-hugepages-memaccess.args | 42 +++++++++++++++
.../qemuxml2argv-hugepages-memaccess.xml | 62 ++++++++++++++++++++++
.../qemuxml2argv-hugepages-memaccess2.args | 39 ++++++++++++++
.../qemuxml2argv-hugepages-memaccess2.xml | 62 ++++++++++++++++++++++
.../qemuxml2argv-hugepages-pages4.xml | 14 +++--
.../qemuxml2argv-hugepages-pages5.xml | 14 +++--
.../qemuxml2argv-hugepages-pages6.xml | 14 +++--
tests/qemuxml2argvtest.c | 6 +++
.../qemuxml2xmlout-hugepages-memaccess.xml | 1 +
.../qemuxml2xmlout-hugepages-memaccess2.xml | 1 +
.../qemuxml2xmlout-hugepages-pages4.xml | 1 +
.../qemuxml2xmlout-hugepages-pages5.xml | 1 +
.../qemuxml2xmlout-hugepages-pages6.xml | 1 +
tests/qemuxml2xmltest.c | 5 ++
16 files changed, 262 insertions(+), 27 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-memaccess.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-memaccess2.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages4.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages5.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages6.xml
--
2.13.0
7 years, 5 months
[libvirt] [PATCH v3 0/6] Fix error reporting in streams
by Michal Privoznik
diff to v3:
-In the last patch, instead of reporting errors from callbacks, report system
error based on errno set by the callbacks.
Michal Privoznik (6):
virfdstream: Check for thread error more frequently
fdstream: Report error from the I/O thread
virStream*All: Call virStreamAbort() more frequently
virStream*All: Preserve reported error
virFileInData: preserve errno in cleanup path
virStream*All: Report error if a callback fails
daemon/stream.c | 18 ++++++---
include/libvirt/libvirt-stream.h | 17 +++++++-
src/libvirt-stream.c | 83 +++++++++++++++++++++++++++++++---------
src/util/virfdstream.c | 22 ++++++++---
src/util/virfile.c | 5 ++-
5 files changed, 113 insertions(+), 32 deletions(-)
--
2.13.0
7 years, 5 months
[libvirt] [PATCH v2 0/2] Two simple hugepage fixes
by Michal Privoznik
v2 of:
https://www.redhat.com/archives/libvir-list/2017-June/msg00395.html
diff to v1:
- Moved the decision logic to a separate static function as requested in the review.
- In 2/2 the function header moved to a different location as requested.
Michal Privoznik (2):
qemuProcessBuildDestroyHugepagesPath: create path more frequently
qemuDomainAttachMemory: Crate hugepage dir if needed
src/qemu/qemu_hotplug.c | 3 +++
src/qemu/qemu_process.c | 52 ++++++++++++++++++++++++++++++++++++++++++++-----
src/qemu/qemu_process.h | 5 +++++
3 files changed, 55 insertions(+), 5 deletions(-)
--
2.13.0
7 years, 5 months
[libvirt] [PATCH] daemonUnixSocketPaths: Unify exit paths
by Michal Privoznik
Right now, there is a lot of exit points from the function.
Depending on their position they need to copy the same free
calls. This goes against our style where we usually have just one
exit point from the function which also does the necessary free.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
daemon/libvirtd.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index d17a694c9..db239f0d4 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -262,50 +262,47 @@ daemonUnixSocketPaths(struct daemonConfig *config,
char **rosockfile,
char **admsockfile)
{
+ int ret = -1;
+ char *rundir = NULL;
+
if (config->unix_sock_dir) {
if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0)
- goto error;
+ goto cleanup;
if (privileged) {
- if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0)
- goto error;
- if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0)
- goto error;
+ if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0 ||
+ virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0)
+ goto cleanup;
}
} else {
if (privileged) {
if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 ||
VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 ||
VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0)
- goto error;
+ goto cleanup;
} else {
- char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ goto cleanup;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
- VIR_FREE(rundir);
- goto error;
+ goto cleanup;
}
umask(old_umask);
if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 ||
- virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) {
- VIR_FREE(rundir);
- goto error;
- }
-
- VIR_FREE(rundir);
+ virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0)
+ goto cleanup;
}
}
- return 0;
- error:
- return -1;
+ ret = 0;
+ cleanup:
+ VIR_FREE(rundir);
+ return ret;
}
--
2.13.0
7 years, 5 months
[libvirt] [PATCH] fix memory leak in daemonUnixSocketPaths
by Yi Wang
The @rundir is allocated in virGetUserRuntimeDirectory, may lost
when virFileMakePath failed.
Signed-off-by: Xi Xu <xu.xi8(a)zte.com.cn>
---
daemon/libvirtd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index bac4bc1..d17a694 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -288,6 +288,7 @@ daemonUnixSocketPaths(struct daemonConfig *config,
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
+ VIR_FREE(rundir);
goto error;
}
umask(old_umask);
--
1.8.3.1
7 years, 5 months
[libvirt] [PATCH 0/2] Two simple hugepage fixes
by Michal Privoznik
Ideally, we would have the security driver relabelling the paths qemu is going
to touch. And this indeed is how I approached this problem firstly. But there
are couple of problems:
a) we generate the paths, add them onto the cmd line and forget them. You won't
find them in virDomainDef. The secdriver cannot reconstruct them either as they
may depend on qemu.conf (admin can whitelist just a few hugetlbfs mount points).
b) Storing the paths in virDomainDef (which is all the the secriver sees) turns
out to be not trivial too: where would you store them? In function that
constructs the paths (qemuBuildMemoryBackendStr) we don't know what 'device'
are we working with. All the callers either pass memory dev directly (for
<memory/> cells), or construct a dummy one (for <memoryBacking/> and <numa/>
cells) and pass it. This we wouldn't know where to store the constructed path
anyway.
This solution presented here turned out to be the least painful (yet not very
clear from design POV, I give you that).
Michal Privoznik (2):
qemuProcessBuildDestroyHugepagesPath: create path more frequently
qemuDomainAttachMemory: Crate hugepage dir if needed
src/qemu/qemu_hotplug.c | 3 +++
src/qemu/qemu_process.c | 44 +++++++++++++++++++++++++++++++++++++++-----
src/qemu/qemu_process.h | 5 +++++
3 files changed, 47 insertions(+), 5 deletions(-)
--
2.13.0
7 years, 5 months
[libvirt] [PATCH] qemu: Fix starting a domain with corrupted managed save file
by Jiri Denemark
Commit v3.4.0-44-gac793bd71 fixed a memory leak, but failed to return
the special -3 value. Thus an attempt to start a domain with corrupted
managed save file would removed the corrupted file and report
"An error occurred, but the cause is unknown" instead of starting the
domain from scratch.
https://bugzilla.redhat.com/show_bug.cgi?id=1460962
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_driver.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6654fd3e2..ba1dba5b7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6450,17 +6450,15 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
*ret_def = def;
*ret_data = data;
+ cleanup:
virObjectUnref(caps);
-
return fd;
error:
virDomainDefFree(def);
virQEMUSaveDataFree(data);
VIR_FORCE_CLOSE(fd);
- virObjectUnref(caps);
-
- return -1;
+ goto cleanup;
}
static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
--
2.13.1
7 years, 5 months
