[libvirt] [PATCH 0/4] qemu: fix usage of disk and storage source private data
by Peter Krempa
There were a few cases where we'd use private data for a storage source
which was not allocated. Fix it by checking whether it is allocated.
Qemu command line parser allocated disk definition via VIR_ALLOC rather
than the proper function. This resulted into the disk private data
strucutre not being allocated. That one is required.
Peter Krempa (4):
qemu: parse: Allocate disk definition with private data
qemu: Tolerate storage source private data being NULL
qemu: domain: Don't allocate storage source private data if not needed
qemu: process: Setup disk secrets when preparing disks
src/qemu/qemu_command.c | 18 ++++++++++++++----
src/qemu/qemu_domain.c | 14 ++++++++------
src/qemu/qemu_hotplug.c | 17 ++++++++++++-----
src/qemu/qemu_parse_command.c | 4 +---
src/qemu/qemu_process.c | 14 +++++++++-----
5 files changed, 44 insertions(+), 23 deletions(-)
--
2.14.3
7 years, 1 month
[libvirt] [PATCH] conf: Properly parse <backingStore/>
by Peter Krempa
The terminator would not be parsed properly since the XPath selector was
looking for an populated element, and also the code did not bother
assigning the terminating virStorageSourcePtr to the backingStore
property of the parent.
Some tests would catch it if there wasn't bigger fallout from the change
to backing store termination in a693fdba0111. Fix them properly now.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1509110
---
src/conf/domain_conf.c | 14 ++++++--------
.../qemuxml2xmlout-disk-active-commit.xml | 1 +
.../qemuxml2xmlout-disk-backing-chains-active.xml | 3 +++
.../qemuxml2xmlout-disk-mirror-active.xml | 4 ++++
.../qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml | 4 ++++
.../qemuxml2xmlout-seclabel-static-labelskip.xml | 1 +
6 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7dfd7b54e..a1ca307c4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8545,23 +8545,21 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
char *idx = NULL;
int ret = -1;
- if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
+ if (!(ctxt->node = virXPathNode("./backingStore", ctxt))) {
ret = 0;
goto cleanup;
}
- if (!(type = virXMLPropString(ctxt->node, "type"))) {
- /* terminator does not have a type */
- if (VIR_ALLOC(backingStore) < 0)
- goto cleanup;
+ if (VIR_ALLOC(backingStore) < 0)
+ goto cleanup;
+ /* terminator does not have a type */
+ if (!(type = virXMLPropString(ctxt->node, "type"))) {
+ VIR_STEAL_PTR(src->backingStore, backingStore);
ret = 0;
goto cleanup;
}
- if (VIR_ALLOC(backingStore) < 0)
- goto cleanup;
-
if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
(idx = virXMLPropString(ctxt->node, "index")) &&
virStrToLong_uip(idx, NULL, 10, &backingStore->id) < 0) {
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml
index 5766e4aea..cc26af109 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml
@@ -20,6 +20,7 @@
<backingStore type='block' index='1'>
<format type='raw'/>
<source dev='/dev/HostVG/QEMUGuest1'/>
+ <backingStore/>
</backingStore>
<mirror type='block' job='active-commit'>
<format type='raw'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml
index 828defcc2..d1fd2442c 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml
@@ -49,6 +49,7 @@
<backingStore type='file' index='6'>
<format type='raw'/>
<source file='/tmp/Fedora-17-x86_64-Live-KDE.iso'/>
+ <backingStore/>
</backingStore>
</backingStore>
</backingStore>
@@ -63,6 +64,7 @@
<source protocol='gluster' name='Volume1/Image'>
<host name='example.org' port='6000'/>
</source>
+ <backingStore/>
<target dev='vdc' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</disk>
@@ -79,6 +81,7 @@
<backingStore type='file' index='1'>
<format type='qcow2'/>
<source file='/tmp/image.qcow'/>
+ <backingStore/>
</backingStore>
<target dev='vdd' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml
index 252bde338..c1e8a33ec 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml
@@ -16,6 +16,7 @@
<emulator>/usr/bin/qemu-system-i686</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
+ <backingStore/>
<mirror type='block' job='copy' ready='yes'>
<source dev='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
@@ -24,12 +25,14 @@
</disk>
<disk type='block' device='cdrom'>
<source dev='/dev/HostVG/QEMUGuest2'/>
+ <backingStore/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
+ <backingStore/>
<mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
@@ -39,6 +42,7 @@
</disk>
<disk type='file' device='disk'>
<source file='/tmp/logs.img'/>
+ <backingStore/>
<mirror type='file' file='/tmp/logcopy.img' format='qcow2' job='copy' ready='abort'>
<format type='qcow2'/>
<source file='/tmp/logcopy.img'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
index f4bd39a58..e390bc02f 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
@@ -16,6 +16,7 @@
<emulator>/usr/bin/qemu-system-i686</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
+ <backingStore/>
<mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'>
<source file='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
@@ -24,12 +25,14 @@
</disk>
<disk type='block' device='cdrom'>
<source dev='/dev/HostVG/QEMUGuest2'/>
+ <backingStore/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
+ <backingStore/>
<mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
@@ -39,6 +42,7 @@
</disk>
<disk type='file' device='disk'>
<source file='/tmp/logs.img'/>
+ <backingStore/>
<target dev='vdb' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
</disk>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml
index 91f573db7..d37b950cb 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml
@@ -18,6 +18,7 @@
<source dev='/dev/HostVG/QEMUGuest1'>
<seclabel model='selinux' labelskip='yes'/>
</source>
+ <backingStore/>
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
--
2.14.3
7 years, 1 month
[libvirt] [PATCH 0/2] qemu: block: Fix issues pointed out by coverity
by Peter Krempa
Few unlikely memleaks and a more likely NULL deref.
Peter Krempa (2):
qemu: block: Break out early on invalid storage sources
qemu: block: Don't leak server JSON object from protocol generators
src/qemu/qemu_block.c | 76 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 28 deletions(-)
--
2.14.3
7 years, 1 month
[libvirt] KVM Forum BoF session notes: QEMU configuration/command-line/QMP
by Eduardo Habkost
Hi,
Below are the notes I collected during the BoF session about QEMU
configuration, command-line and QMP. Sorry for taking so long to
send them.
All inaccuracies and mistakes below are my own fault. (I learned
the hard way that trying to take notes while participating in the
discussion at the same time is not a good idea.)
--------------------------------
KVM Forum 2017 BoF session: configuration, command-line and QMP
2017-10-26
People: Jiri Denemark, Laine Stump, Peter Krempa,
Markus Armbruster, Eduardo Habkost, Igor Mammedov,
Kevin Wolf, Eduardo Otubo, Alistair Francis,
Kashyap Chamarthy, Max Reitz
----------------
First topic: early QMP and -paused series by Igor
ehabkost tried to explain the original problem and Igor's proposal[1].
Markus: startup time is important, some things can make it slow. Personally,
not so much concerned about querying stuff. Just cache it!
Markus, about starting a monitor earlier: "it just makes sense". One problem
is: command-line processing ordering mess in QEMU. It's tempting but very
scary. very easy to screw up. About command-line ordering, “I'd just go
left-to-right”.
ehabkost: is gradual change with NUMA stuff the right path? (Markus: "what's the
right path?"; ehabkost: "I don't know"). If you are using CPU hotplug you still
might have some stuff not appearing in the command-line, because it uses
query-hotpluggable-cpus. pkrempa: if you're using cpu hotplug you don't care
about startup time. NUMA is not different.
Markus's objection to current -paused series: the need for another option/state.
We basically have 3 states/phases:
1: between exec and monitor available
2: between monitor and machines start to runs (in that phase a device_add is/can be a cold plug)
3: then the machine actually runs, and all device_add is hotplug
ehabkost: that's how I understand it, too.
ehabkost: question for libvirt developers: is there an "expansion" operation
where libvirt could fill-up missing data on the XML based on info queried from
QEMU? (Answer seems to be yes)
----------------
Some comments from Igor and Peter about query-hotpluggable-cpus and the need to
choose the right device type (sometimes TYPE_CPU is not appropriate for
device_add, but only CPU cores, like on Power).
ehabkost: that's probably another use case for query-device-slots: knowing that
power machines can't device_add threads, but just cores. Tricky part: providing
slot information at qemu-probing time (with -machine none), before any machine
was built.
----------------
Some discussion about QEMU capability caching, why we need it, what we need to
get rid of it (answer: early QMP?).
Markus: would --query-FOO for some FOOs help? libvirt developers seem to prefer
early QMP.
----------------
Now that libvirt queries QEMU for host CPUID capabilities, there are more
interesting ways the cache may need to be invalidated.
Markus: “there's a it of a combinatorial explosion, i.e. query results can
depend on so much more than just the QEMU version and machine type. Makes
invalidating the cache tricky, and possibly even finding something useful in it
insufficiently likely to be worth the trouble.”
----------------
Some discussion about libvirt/qemu versions compatibility. Markus proposes that
we could choose to make newer QEMU require a more recent libvirt version.
OpenStack Nova re-evaluates every 6 months what next minimum libvirt and QEMU
versions to use (via constants: NEXT_MIN_LIBVIRT_VERSION, NEXT_MIN_QEMU_VERSION)
Markus: “The version decoupling we have is really nice, but I think an
occasional dependency would be tolerable. Consider it when it saves lots of
work.”
Markus: “Also: drop support for old versions of QEMU. Anything before 1.5?”
----------------
References and notes:
[1] References for “Early QMP”:
* Igor’s proposal at:
Subject: [Qemu-devel] [RFC 0/6] enable numa configuration before machine_init() from HMP/QMP
https://www.mail-archive.com/qemu-devel@nongnu.org/msg488601.html
* And what Eduardo has suggested at:
http://www.linux-kvm.org/images/4/46/03x06A-Eduardo_HabkostMachine-type_I...
Comments from Markus:
> Related: "Why /two/ config interfaces?" in my KVM Forum talk. The two
> being CLI and QMP.
>
> We've long held the idea to support starting QEMU with a minimal command
> line, then do all configuration via QMP. Not possible today, because
> lots of things can only be done via CLI. In part because QMP becomes
> available only after quite a few config boats have sailed.
>
> Making QMP available earlier during startup ("early QMP") would be a
> stepping stone towards the "only QMP" goal.
>
> Related: rebasing CLI onto QAPI would make replicating CLI-only
> interfaces in QMP easier.
--
Eduardo
7 years, 1 month
[libvirt] [PATCH] apparmor: add network netlink raw rule
by Cédric Bosdonnat
The rule 'network netlink raw' fixes these denials on libvirtd start:
apparmor="DENIED" operation="create" profile="/usr/sbin/libvirtd" pid=12969
comm="libvirtd" family="netlink" sock_type="raw" protocol=0
requested_mask="create" denied_mask="create"
---
examples/apparmor/usr.sbin.libvirtd | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd
index 819068ffc..8ac5233cc 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -36,6 +36,7 @@
network inet6 dgram,
network packet dgram,
network packet raw,
+ network netlink raw,
ptrace (trace) peer=unconfined,
ptrace (trace) peer=/usr/sbin/libvirtd,
--
2.14.3
7 years, 1 month
[libvirt] [PATCH v3 00/11] qemu: replace nested job with interruptible async job state
by Nikolay Shirokovskiy
Hello, everyone.
This is successor to [1]
"[PATCH v2 RFC 0/4] qemu: replace nested job with interruptible async job state".
I tried to expand on series rationale, to make better splitting and to
add comments.
ABSTRACT
Let's use interruptible async job state instead of nested jobs
to control concurrent execution of async and regular jobs.
RATIONALE
I'm not quite happy with current implementation of concurrent
execution async and regular jobs because it is not straigthforward.
What is current implementation of jobs concurrency? While async job is running
any compatible regular job is allowed to run concurrently. Practically it means
that as soon as async job drops the domain lock another rpc thread can run in,
find domain in the domain list and start a regular job. This is what happens
when for example migration drops the lock before waiting for migration
completion event or before sending request to destination side. But if we are
going to send command to qemu monitor and accordingly drop domain lock this
concurrency is not desired because qemu monitor can not handle simultaneus
commands now. (This is probably not the only reason. I guess we don't want to
think about what can happen if regular job will run concurrenly for example to
migration at any place. Ability to run concurrently during disk mirror or state
transfer which take the most time of migration looks enough). Thus nested jobs
were introduced. Now if async job wants to send command to monitor it start
special nested jobs beforehand thus effectively block concurrent regular jobs.
To me it sounds like hacky way to implement the fact that async job allows
concurrency only in limited places. Let's call them interruptible. So
I suggest introduce interruptible state for async job. Before we are going
to wait for migration completion for example we enter this state allowing
regular jobs run concurrently. Later when we meet migration completion
condition we wait for concurrent regular job to finish if any before
proceed. This final step is required in this approach by definition -
when async job is not interruptible no regular jobs can run concurrenly.
On practice this protects us from sending qemu monitor command from async
job while concurrent regular job is not finished its qemu monitor communication
yet.
Sounds like this final wait can have consequenses on migration. Like what if
finishing concurrent job take too much time. In this case we get increased
migration downtime. Well we will get this increase with current appoach too.
Because right after migration completion condition is met we fetch migration
stats and wait in enter monitor function anyway until concurrent job is
finished. This also demontrates that current appoach isolates async and
concurrent regular job loosely. It is possible that regular job starts then
async job resumes and continues its execution until it requires qemu monitor and
only then regular job finishes its execution. I guess it will be easy to reason
about concurrency if when concurrent regular is started async job can not
continue until regular job is finished. Check patch 4 for a demonstration
that job isolation is good.
TESTS
To test concurrency I ran about 100 times a migration of non-empty
domain with shared disk in a run with concurrent domstats for this
domain in a tight loop. Looks good.
MISSING PARTS
- Enter/exit interruptible state at the end/begin of migration phases.
Until this point is done we have a bit of degradation because domain
won't response for example to queries between migration phases.
- Update THREADS.txt
I'm going to implement missing parts as soon as RFC has overall approvement.
TODO
1 Drop driver argument from calls to qemuDomainObjEnterMonitor
2 Replace qemuDomainObjEnterMonitorAsync with qemuDomainObjEnterMonitor
3 Make qemuDomainObjExitMonitor void and drop its driver argument.
[1] https://www.redhat.com/archives/libvir-list/2017-May/msg00018.html
Nikolay Shirokovskiy (11):
qemu: introduce routines for interruptible state
qemu: check interruptible state instead of taking nested job
qemu: enter interruptlible state where appropriate
qemu: remove nested job usage from qemuProcessStop
qemu: remove unused qemuDomainObjBeginNestedJob
qemu: remove QEMU_JOB_ASYNC_NESTED
qemu: remove liveness check from qemuDomainObjExitMonitor
qemu: drop qemuDomainObjEnterMonitorInternal
qemu: drop qemuDomainObjExitMonitorInternal
conf: cleanup: drop virDomainObjWait
qemu: rename qemuDomainNestedJobAllowed
src/conf/domain_conf.c | 19 ----
src/conf/domain_conf.h | 1 -
src/libvirt_private.syms | 1 -
src/qemu/qemu_domain.c | 265 ++++++++++++++++++++++++++++++----------------
src/qemu/qemu_domain.h | 17 ++-
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_migration.c | 18 ++--
src/qemu/qemu_process.c | 19 +---
8 files changed, 195 insertions(+), 147 deletions(-)
--
1.8.3.1
7 years, 1 month
[libvirt] [PATCH] virconf: properly set the end of content
by Jim Fehlig
There was a recent report of the xen-xl converter not handling
config files missing an ending newline
https://www.redhat.com/archives/libvir-list/2017-October/msg01353.html
Commit 3cc2a9e0 fixed a similar problem when parsing content of a
file but missed parsing in-memory content. But AFAICT, the better
fix is to properly set the end of the content when initializing the
virConfParserCtxt in virConfParse().
This commit reverts the part of 3cc2a9e0 that appends a newline to
files missing it, and fixes setting the end of content when
initializing virConfParserCtxt. A test is also added to check
parsing in-memory content missing an ending newline.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/util/virconf.c | 13 ++--------
tests/virconftest.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 39c2bd917..a82a509ca 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -705,7 +705,7 @@ virConfParse(const char *filename, const char *content, int len,
ctxt.filename = filename;
ctxt.base = ctxt.cur = content;
- ctxt.end = content + len - 1;
+ ctxt.end = content + len;
ctxt.line = 1;
ctxt.conf = virConfCreate(filename, flags);
@@ -745,7 +745,7 @@ virConfReadFile(const char *filename, unsigned int flags)
{
char *content;
int len;
- virConfPtr conf = NULL;
+ virConfPtr conf;
VIR_DEBUG("filename=%s", NULLSTR(filename));
@@ -757,17 +757,8 @@ virConfReadFile(const char *filename, unsigned int flags)
if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0)
return NULL;
- if (len && len < MAX_CONFIG_FILE_SIZE && content[len - 1] != '\n') {
- VIR_DEBUG("appending newline to busted config file %s", filename);
- if (VIR_REALLOC_N(content, len + 2) < 0)
- goto cleanup;
- content[len++] = '\n';
- content[len] = '\0';
- }
-
conf = virConfParse(filename, content, len, flags);
- cleanup:
VIR_FREE(content);
return conf;
diff --git a/tests/virconftest.c b/tests/virconftest.c
index a8b18bae0..3cf0df3ac 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -77,6 +77,72 @@ static int testConfRoundTrip(const void *opaque)
}
+static int testConfMemoryNoNewline(const void *opaque ATTRIBUTE_UNUSED)
+{
+ const char *srcdata = \
+ "ullong = '123456789'\n" \
+ "string = 'foo'\n" \
+ "uint = 12345";
+
+ virConfPtr conf = virConfReadString(srcdata, 0);
+ int ret = -1;
+ virConfValuePtr val;
+ unsigned long long llvalue;
+ char *str = NULL;
+ int uintvalue;
+
+ if (!conf)
+ return -1;
+
+ if (!(val = virConfGetValue(conf, "ullong")))
+ goto cleanup;
+
+ if (val->type != VIR_CONF_STRING)
+ goto cleanup;
+
+ if (virStrToLong_ull(val->str, NULL, 10, &llvalue) < 0)
+ goto cleanup;
+
+ if (llvalue != 123456789) {
+ fprintf(stderr, "Expected '123' got '%llu'\n", llvalue);
+ goto cleanup;
+ }
+
+ if (virConfGetValueType(conf, "string") !=
+ VIR_CONF_STRING) {
+ fprintf(stderr, "expected a string for 'string'\n");
+ goto cleanup;
+ }
+
+ if (virConfGetValueString(conf, "string", &str) < 0)
+ goto cleanup;
+
+ if (STRNEQ_NULLABLE(str, "foo")) {
+ fprintf(stderr, "Expected 'foo' got '%s'\n", str);
+ goto cleanup;
+ }
+
+ if (virConfGetValueType(conf, "uint") != VIR_CONF_ULLONG) {
+ fprintf(stderr, "expected an unsigned long for 'uint'\n");
+ goto cleanup;
+ }
+
+ if (virConfGetValueInt(conf, "uint", &uintvalue) < 0)
+ goto cleanup;
+
+ if (uintvalue != 12345) {
+ fprintf(stderr, "Expected 12345 got %ud\n", uintvalue);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(str);
+ virConfFree(conf);
+ return ret;
+}
+
+
static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED)
{
const char *srcdata = \
@@ -414,6 +480,9 @@ mymain(void)
if (virTestRun("no-newline", testConfRoundTrip, "no-newline") < 0)
ret = -1;
+ if (virTestRun("memory-no-newline", testConfMemoryNoNewline, NULL) < 0)
+ ret = -1;
+
if (virTestRun("int", testConfParseInt, NULL) < 0)
ret = -1;
--
2.14.2
7 years, 1 month
[libvirt] [PATCH] virconf: properly set the end of content
by Jim Fehlig
There was a recent report of the xen-xl converter not handling
config files missing an ending newline
https://www.redhat.com/archives/libvir-list/2017-October/msg01353.html
Commit 3cc2a9e0 fixed a similar problem when parsing content of a
file but missed parsing in-memory content. But AFAICT, the better
fix is to properly set the end of the content when initializing the
virConfParserCtxt in virConfParse().
This commit reverts the part of 3cc2a9e0 that appends a newline to
files missing it, and fixes setting the end of content when
initializing virConfParserCtxt.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/util/virconf.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 39c2bd917..a82a509ca 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -705,7 +705,7 @@ virConfParse(const char *filename, const char *content, int len,
ctxt.filename = filename;
ctxt.base = ctxt.cur = content;
- ctxt.end = content + len - 1;
+ ctxt.end = content + len;
ctxt.line = 1;
ctxt.conf = virConfCreate(filename, flags);
@@ -745,7 +745,7 @@ virConfReadFile(const char *filename, unsigned int flags)
{
char *content;
int len;
- virConfPtr conf = NULL;
+ virConfPtr conf;
VIR_DEBUG("filename=%s", NULLSTR(filename));
@@ -757,17 +757,8 @@ virConfReadFile(const char *filename, unsigned int flags)
if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0)
return NULL;
- if (len && len < MAX_CONFIG_FILE_SIZE && content[len - 1] != '\n') {
- VIR_DEBUG("appending newline to busted config file %s", filename);
- if (VIR_REALLOC_N(content, len + 2) < 0)
- goto cleanup;
- content[len++] = '\n';
- content[len] = '\0';
- }
-
conf = virConfParse(filename, content, len, flags);
- cleanup:
VIR_FREE(content);
return conf;
--
2.14.2
7 years, 1 month
[libvirt] [PATCH v3 0/5] nwfilter common object adjustments
by John Ferlan
v2: https://www.redhat.com/archives/libvir-list/2017-July/msg00673.html
(and a few pings along the way)
Don't think much survived from v2 - this is a fresh start anyway.
Perhaps old patch 2 the same, but beyond that a different approach
to remove recursive read/write locks and replace with using rwlock
read/write where the write's are in very tight confines.
I've run the changes through avocado with success. There were some
really strange deadlocks along the way - even causing libvirtd to
go defunct. There's a lot of strange ways to use/access the nwfilters.
John Ferlan (5):
nwfilter: Add update locking to Initialization
nwfilter: Remove unnecessary UUID comparison bypass
nwfilter: Convert _virNWFilterObj to use virObjectRWLockable
nwfilter: Convert _virNWFilterObjList to use virObjectRWLockable
nwfilter: Remove need for nwfilterDriverLock in some API's
src/conf/virnwfilterobj.c | 555 +++++++++++++++++++++++----------
src/conf/virnwfilterobj.h | 11 +-
src/libvirt_private.syms | 3 +-
src/nwfilter/nwfilter_driver.c | 77 +++--
src/nwfilter/nwfilter_gentech_driver.c | 11 +-
5 files changed, 433 insertions(+), 224 deletions(-)
--
2.13.6
7 years, 1 month
[libvirt] [PATCH 0/4] Couple more hotplug cleanups
by John Ferlan
Patches speak for themselves - they also resolve a couple of new
Coverity whines.
John Ferlan (4):
conf,qemu: Check for NULL addrs in virDomainUSBAddressRelease
conf,qemu: Check for NULL addrs in virDomainUSBAddressEnsure
qemu: Remove unnecessary virtio disk detach info.alias check
qemu: Remove unnecessary controller detach info.alias check
src/conf/domain_addr.c | 5 ++++-
src/conf/domain_addr.h | 4 ++--
src/qemu/qemu_domain_address.c | 7 ++-----
src/qemu/qemu_hotplug.c | 43 ++++++++++++++----------------------------
4 files changed, 22 insertions(+), 37 deletions(-)
--
2.13.6
7 years, 1 month
