November 2017 - Devel - libvirt List Archives

[libvirt] [PATCH 0/2] vbox: Add support for 5.2.x
by Dawid Zamirski 10 Nov '17

10 Nov '17

With each minor VBOX release a new SDK header needs to be added and wired up to the driver. This is basically the same as last time [1]. The first patch will be sent as a compressed attachment because it's a large header file taken from the SDK that would not pass the mailing list size constraints. [1] https://www.redhat.com/archives/libvir-list/2016-November/msg00355.html Dawid Zamirski (2): vbox: Add vbox 5.2 CAPI header file. vbox: Add support for 5.2.x src/Makefile.am | 1 + src/vbox/vbox_CAPI_v5_2.h | 26870 ++++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_V5_2.c | 13 + src/vbox/vbox_common.h | 2 + src/vbox/vbox_storage.c | 2 + src/vbox/vbox_tmpl.c | 2 + src/vbox/vbox_uniformed_api.h | 1 + 7 files changed, 26891 insertions(+) create mode 100644 src/vbox/vbox_CAPI_v5_2.h create mode 100644 src/vbox/vbox_V5_2.c -- 2.14.2

2 4

[libvirt] [PATCH 0/4] qemu: fix usage of disk and storage source private data
by Peter Krempa 10 Nov '17

10 Nov '17

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

2 6

[libvirt] [PATCH] conf: Properly parse <backingStore/>
by Peter Krempa 10 Nov '17

10 Nov '17

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

2 1

[libvirt] [PATCH 0/2] qemu: block: Fix issues pointed out by coverity
by Peter Krempa 10 Nov '17

10 Nov '17

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

2 3

[libvirt] KVM Forum BoF session notes: QEMU configuration/command-line/QMP
by Eduardo Habkost 10 Nov '17

10 Nov '17

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_Int… 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

1 0

[libvirt] [PATCH] apparmor: add network netlink raw rule
by Cédric Bosdonnat 10 Nov '17

10 Nov '17

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

3 2

[libvirt] [PATCH v3 00/11] qemu: replace nested job with interruptible async job state
by Nikolay Shirokovskiy 09 Nov '17

09 Nov '17

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

1 11

[libvirt] [PATCH] virconf: properly set the end of content
by Jim Fehlig 09 Nov '17

09 Nov '17

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

2 2

[libvirt] [PATCH] virconf: properly set the end of content
by Jim Fehlig 09 Nov '17

09 Nov '17

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

2 2

[libvirt] [PATCH v3 0/5] nwfilter common object adjustments
by John Ferlan 09 Nov '17

09 Nov '17

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

1 6