- Devel - libvirt List Archives

[libvirt] [PATCH RESEND] qemu_driver: unlink new domain cfg file when rollback
by Chen Hanxiao 01 Nov '16

01 Nov '16

From: Chen Hanxiao <chenhanxiao(a)gmail.com> If we failed to unlink old dom cfg file, we goto rollback. But inside rollback, we fogot to unlink the new dom cfg file. This patch fixes this issue. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- src/qemu/qemu_driver.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e6f845d..3f4a2fb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19869,6 +19869,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, char *new_dom_name = NULL; char *old_dom_name = NULL; char *old_dom_cfg_file = NULL; + char *new_dom_cfg_file = NULL; virCheckFlags(0, ret); @@ -19882,6 +19883,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm, goto cleanup; } + if (!(new_dom_cfg_file = virDomainConfigFile(cfg->configDir, + new_dom_name))) { + goto cleanup; + } + event_old = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); @@ -19909,6 +19915,7 @@ qemuDomainRenameCallback(virDomainObjPtr vm, cleanup: VIR_FREE(old_dom_cfg_file); + VIR_FREE(new_dom_cfg_file); VIR_FREE(old_dom_name); VIR_FREE(new_dom_name); qemuDomainEventQueue(driver, event_old); @@ -19922,6 +19929,10 @@ qemuDomainRenameCallback(virDomainObjPtr vm, vm->def->name = old_dom_name; old_dom_name = NULL; } + + if (virFileExists(new_dom_cfg_file)) + unlink(new_dom_cfg_file); + goto cleanup; } -- 1.8.3.1

4 4

[libvirt] [PATCH v3 0/9] Implementation of QEMU vhost-scsi
by Eric Farman 31 Oct '16

31 Oct '16

[Author note: Apologies for an extra release or two in between versions, I was sidetracked by another project. This is probably too close to the impending freeze for 2.4, so I just updated doc to 2.5 in anticipation. In rearranging the patches, I've inserted a cleanup patch at the head that was mentioned in the v2 review, and which could go separately.] This patch series provides a libvirt implementation of the vhost-scsi interface in QEMU. As near as I can see, this was discussed upstream in July 2014[1], and ended in a desire to replace a vhost-scsi controller in favor of a hostdev element instead[2]. Host Filesystem Example: # ls /sys/kernel/config/target/vhost/ discovery_auth naa.5001405df3e54061 version # ls /sys/kernel/config/target/vhost/naa.5001405df3e54061/tpgt_1/lun/ lun_0 QEMU Example (snippet): -device vhost-scsi-ccw,wwpn=naa.5001405df3e54061,devno=fe.0.1000 Libvirt Example (snippet): <hostdev mode='subsystem' type='scsi_host'> <source protocol='vhost' wwpn='naa.5001405df3e54061'/> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1000'/> </hostdev> Guest Viewpoint: # lsscsi [1:0:1:0] disk LIO-ORG disk0 4.0 /dev/sda # dmesg | grep 1: [ 6.065735] scsi host1: Virtio SCSI HBA [ 6.093892] scsi 1:0:1:0: Direct-Access LIO-ORG disk0 4.0 PQ: 0 ANSI: 5 [ 6.313615] sd 1:0:1:0: Attached scsi generic sg0 type 0 [ 6.314981] sd 1:0:1:0: [sda] 29360128 512-byte logical blocks: (15.0 GB/14.0 GiB) [ 6.317290] sd 1:0:1:0: [sda] Write Protect is off [ 6.317566] sd 1:0:1:0: [sda] Mode Sense: 43 00 10 08 [ 6.317853] sd 1:0:1:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA [ 6.352722] sd 1:0:1:0: [sda] Attached SCSI disk Changelog: v2.1->v3: - Rebase - Rebased to current master (26 October) - Comments - Added an early patch to do some additional typecasting in the switch statements of hostdev.subsys.type - Did some reordering of patches, to hopefully flow better - Implemented an activeHostHostdevs list, which is used by the cgroup and security codepaths - doc changes -- s/2.2/2.5/ and s/HBI/HBA/ - Added a "none" protocol type for scsi_host hostdevs (which is invalid) - Restored the apparmor and selinux codepaths that got lost from v1 - Added a proper check for a valid scsi_host protocol, and saving that value within the HostdevDef struct - Fixed a compiler warning with call to virDomainPCIAddressEnsureAddr - Removed the rest of vhostfdSize, since multiple fd's are not allowed by QEMU - Fixed cleanup of vhostfd in error from building -device string - Moved the "conf" chunk from "hotplug" patch to "introduce" patch - Added xml2xml test - Added a proper calculation of "address" in virDomainAuditHostdev - Added a virFileExists check before open(/dev/vhost-scsi) - Addressed a number of lines >80 characters - Things *NOT* done (later?) - Investigation/tie-in with virsh nodedev-list stuff - Implementation of 'num_queues', 'max_sectors', and 'cmd_per_lun' (Need to research these in the virtio space, before figuring out how to apply to vhost-scsi) - Dropping the "naa." prefix of wwn - Split the "tests" patch into earlier patches - Other v2.1: https://www.redhat.com/archives/libvir-list/2016-September/msg00148.html v2: https://www.redhat.com/archives/libvir-list/2016-August/msg01028.html v1: https://www.redhat.com/archives/libvir-list/2016-July/msg01004.html [1] http://www.redhat.com/archives/libvir-list/2014-July/msg01235.html [2] http://www.redhat.com/archives/libvir-list/2014-July/msg01390.html Eric Farman (9): qemu: Introduce vhost-scsi capability Introduce a "scsi_host" hostdev type util: Management routines for scsi_host devices qemu: Add vhost-scsi string for -device parameter qemu: Allow hotplug of vhost-scsi device conf: Wire up the vhost-scsi connection from/to XML security: Include vhost-scsi in security labels tests: Introduce basic vhost-scsi test docs: Add vhost-scsi docs/formatdomain.html.in | 24 ++ docs/schemas/domaincommon.rng | 23 ++ src/Makefile.am | 1 + src/conf/domain_audit.c | 7 + src/conf/domain_conf.c | 91 ++++++- src/conf/domain_conf.h | 18 ++ src/libvirt_private.syms | 19 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 39 +++ src/qemu/qemu_command.c | 79 ++++++ src/qemu/qemu_command.h | 5 + src/qemu/qemu_domain_address.c | 10 + src/qemu/qemu_hostdev.c | 41 +++ src/qemu/qemu_hostdev.h | 8 + src/qemu/qemu_hotplug.c | 160 +++++++++++ src/security/security_apparmor.c | 20 ++ src/security/security_dac.c | 46 ++++ src/security/security_selinux.c | 43 +++ src/util/virhost.c | 301 +++++++++++++++++++++ src/util/virhost.h | 72 +++++ src/util/virhostdev.c | 155 +++++++++++ src/util/virhostdev.h | 16 ++ tests/domaincapsschemadata/full.xml | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 41 +++ tests/qemuxml2argvmock.c | 9 + tests/qemuxml2argvtest.c | 3 + .../qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml | 1 + tests/qemuxml2xmltest.c | 3 + 41 files changed, 1272 insertions(+), 2 deletions(-) create mode 100644 src/util/virhost.c create mode 100644 src/util/virhost.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml -- 1.9.1

2 11

[libvirt] [PATCH] vsh: Pass correct values for command line completion
by John Ferlan 31 Oct '16

31 Oct '16

Commit id 'dcfdf341' passes 'opts_need_arg' and 'opts_seen' to vshCmddefGetData, but that seems to be incorrect as those values are not initialized properly (something at least one compiler found). Instead the static 'const_opts_need_arg' and 'const_opts_seen' values should be passed. By passing unitialized values leads to not finding possible options for simpler commands (domfsfreeze for example), where if you're in a virsh shell using command line completion - you'll get a list of files in your current directory instead of two options --domain and --mountpoint (as would happen with this patch applied. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- tools/vsh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index 9558dad..17199ae 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2788,7 +2788,8 @@ vshReadlineParse(const char *text, int state) /* No -- option provided and some other token given * Try to find the default option. */ - if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, &opts_seen)) + if (!(opt = vshCmddefGetData(cmd, &const_opts_need_arg, + &const_opts_seen)) || opt->type == VSH_OT_BOOL) goto error; opt_exists = true; -- 2.7.4

2 1

[libvirt] [PATCH v3] Fixes virsh save-restore/migration when memory detach not in LIFO
by Nitesh Konkar 31 Oct '16

31 Oct '16

Currently the migration stream references the memory blocks by name (which is supplied by libvirt) rather than by there order. With the current code that is assigning aliases for memory backend objects this won't happen and since qemu is treating the memory object links differently migration does not work in such case. This patch ensures slot number alocation for the memory modules beforehand and assign alias accordingly. This keeps slot numbers consistent with the aliases always. Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com> --- src/conf/domain_conf.h | 1 + src/qemu/qemu_alias.c | 36 +++++++++++++++++++++++++----------- src/qemu/qemu_alias.h | 6 ++++-- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_hotplug.c | 5 ++++- 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fd3ae8e..22b5fe1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2145,6 +2145,7 @@ struct _virDomainDef { virDomainBlkiotune blkio; virDomainMemtune mem; + virBitmapPtr memslotsptr; virDomainVcpuDefPtr *vcpus; size_t maxvcpus; diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index cc83fec..8deb054 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -332,21 +332,34 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def, } -int -qemuAssignDeviceMemoryAlias(virDomainDefPtr def, - virDomainMemoryDefPtr mem) +void +qemuAssignDeviceMemorySlot(virDomainDefPtr def, + virDomainMemoryDefPtr mem) { size_t i; - int maxidx = 0; - int idx; + int minidx = 0; - for (i = 0; i < def->nmems; i++) { - if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm")) >= maxidx) - maxidx = idx + 1; + if (mem->info.addr.dimm.base) { + minidx = mem->info.addr.dimm.slot; + } else { + for (i = 0; i < def->mem.memory_slots; i++) { + if (!virBitmapIsBitSet(def->memslotsptr, i)) { + minidx = i; + break; + } + } } - if (virAsprintf(&mem->info.alias, "dimm%d", maxidx) < 0) - return -1; + ignore_value(virBitmapSetBit(def->memslotsptr, minidx)); + mem->info.addr.dimm.slot = minidx; +} + + +int +qemuAssignDeviceMemoryAlias(virDomainMemoryDefPtr mem) +{ + if (virAsprintf(&mem->info.alias, "dimm%d", mem->info.addr.dimm.slot) < 0) + return -1; return 0; } @@ -475,7 +488,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nmems; i++) { - if (virAsprintf(&def->mems[i]->info.alias, "dimm%zu", i) < 0) + qemuAssignDeviceMemorySlot(def, def->mems[i]); + if (virAsprintf(&def->mems[i]->info.alias, "dimm%d", def->mems[i]->info.addr.dimm.slot) < 0) return -1; } diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 11d9fde..c6cb568 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -57,8 +57,10 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, int qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng); -int qemuAssignDeviceMemoryAlias(virDomainDefPtr def, - virDomainMemoryDefPtr mems); +void qemuAssignDeviceMemorySlot(virDomainDefPtr def, + virDomainMemoryDefPtr); + +int qemuAssignDeviceMemoryAlias(virDomainMemoryDefPtr mems); int qemuAssignDeviceShmemAlias(virDomainDefPtr def, virDomainShmemDefPtr shmem, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9b1a32e..263e78f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2386,6 +2386,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefVcpusPostParse(def) < 0) goto cleanup; + if (def->mem.memory_slots) + def->memslotsptr = virBitmapNew(def->mem.memory_slots); + ret = 0; cleanup: virObjectUnref(qemuCaps); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 72dd93b..5a3af10 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1898,7 +1898,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) < 0) goto cleanup; - if (qemuAssignDeviceMemoryAlias(vm->def, mem) < 0) + qemuAssignDeviceMemorySlot(vm->def, mem); + + if (qemuAssignDeviceMemoryAlias(mem) < 0) goto cleanup; if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) @@ -4427,6 +4429,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, } mem = vm->def->mems[idx]; + ignore_value(virBitmapClearBit(vm->def->memslotsptr, memdef->info.addr.dimm.slot)); if (!mem->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 1.9.3

3 2

[libvirt] Entering freeze for libvirt-2.4.0
by Daniel Veillard 31 Oct '16

31 Oct '16

As planned I just tagged the release in git and pushed signed tarball and rpms to the usual place: ftp://libvirt.org/libvirt/ Not in my normal environment this week so I hadn't had a chance to test the build before pushing, sorry about this but things went smoothly, I noticed a warning in virsh however: CC virsh-virsh-host.o vsh.c: In function 'vshReadlineParse': vsh.c:516:9: warning: 'opts_need_arg' may be used uninitialized in this function [-Wmaybe-uninitialized] i = ffsl(*opts_need_arg) - 1; ^~~~~~~~~~~~~~~~~~~~ vsh.c:2658:14: note: 'opts_need_arg' was declared here uint64_t opts_need_arg, opts_seen; ^~~~~~~~~~~~~ that probably ought to be fixed for release. I hope to be able to push RC2 during the week-end and possibly the final 2.4.0 on the 1st Nov if all goes well. please give it some testing, thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

3 5

[libvirt] [PATCH v2] vsh: Using VSH_REQUIRE_OPTION rather than virReportError
by Kothapally Madhu Pavan 29 Oct '16

29 Oct '16

Correcting the error reporting method by using VSH_REQUIRE_OPTION instead of virReportError Signed-off-by: Kothapally Madhu Pavan <kmp(a)linux.vnet.ibm.com> --- tools/virsh-domain.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 050e7fb..e1cb2ac 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10443,6 +10443,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("live", "offline"); VSH_EXCLUSIVE_OPTIONS("timeout-suspend", "timeout-postcopy"); + VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy"); if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -10474,12 +10475,6 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) } if (vshCommandOptBool(cmd, "postcopy-after-precopy")) { - if (!vshCommandOptBool(cmd, "postcopy")) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("--postcopy-after-precopy can only be used with " - "--postcopy")); - goto cleanup; - } iterEvent = virConnectDomainEventRegisterAny( priv->conn, dom, VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION,

2 1

[libvirt] [PATCH 00/17] Avoid races with udev
by Michal Privoznik 29 Oct '16

29 Oct '16

I've came across interesting bug recently. The problem was that user tried to start a domain, but qemu was denied access to some device. Even though we relabelled it initially. By debugging I found the root cause: while we were starting qemu, udev came and restored original security labels. Sigh. We have two options here: a) write out series of udev rules so that whenever it tries to relabel something our rule will stop it from doing so b) write a small helper binary that will udev call in order to: 1) detect whether device is in use by libvirt 2) get seclabel that was set by libvirt These patches implement the latter approach. While these patches make life easier for us, there is still a race when udev might restore the device's seclabel before we had chance to flush internal database of seclabels for the helper binary. This is something I'm currently focusing on. But before I get there, here are patches that makes the problem much more bearable. In case you want to try these patches, here are some scratch builds: https://mprivozn.fedorapeople.org/udev/ Also, you can find them on my branch: https://github.com/zippy2/libvirt/commits/udev_labels2 This beast is turned off by default, to turn it on you'll need to add: write_udev=1 to qemu.conf. Michal Privoznik (17): virseclabel.h: Include stdbool.h virseclabel: Introduce virSecurityDeviceLabelDefNewLabel security_dac: Pass manager to virSecurityDACSetImageLabel security_dac: Pass manager to virSecurityDACRestoreFileLabelInternal virudev: Introduce basic skeleton virudev: Implement virUdevMgrAddLabel and virUdevMgrRemoveAllLabels virudev: Introduce virUdevMgrDump tests: Introduce virudevtest virudev: Parse virUdevMgr from JSON virudev: Introduce virUdevMgrLookupLabels util: Introduce libvirt_udevhelper security: Wire up virUdevMgr qemu.conf: Introduce write_udev qemu: Wire up virUdevMgr qemu: Reload virUdevMgr on start virudevtest: Introduce device filtering qemu: Filter uninteresting paths for virUdevMgr libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + po/POTFILES.in | 2 + src/Makefile.am | 21 ++ src/libvirt_private.syms | 15 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 + src/qemu/qemu_conf.c | 3 + src/qemu/qemu_conf.h | 5 + src/qemu/qemu_domain.c | 12 +- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 40 +- src/qemu/qemu_hotplug.c | 35 +- src/qemu/qemu_process.c | 47 ++- src/qemu/qemu_process.h | 3 + src/qemu/test_libvirtd_qemu.aug.in | 1 + src/security/security_dac.c | 103 ++++-- src/security/security_manager.c | 16 + src/security/security_manager.h | 5 + src/security/security_selinux.c | 47 ++- src/util/udevhelper.c | 137 +++++++ src/util/virseclabel.c | 14 + src/util/virseclabel.h | 6 + src/util/virudev.c | 588 ++++++++++++++++++++++++++++++ src/util/virudev.h | 63 ++++ tests/Makefile.am | 12 + tests/virudevmock.c | 29 ++ tests/virudevtest.c | 312 ++++++++++++++++ tests/virudevtestdata/complex.json | 30 ++ tests/virudevtestdata/empty.json | 5 + tests/virudevtestdata/simple-dac.json | 13 + tests/virudevtestdata/simple-selinux.json | 13 + 32 files changed, 1535 insertions(+), 54 deletions(-) create mode 100644 src/util/udevhelper.c create mode 100644 src/util/virudev.c create mode 100644 src/util/virudev.h create mode 100644 tests/virudevmock.c create mode 100644 tests/virudevtest.c create mode 100644 tests/virudevtestdata/complex.json create mode 100644 tests/virudevtestdata/empty.json create mode 100644 tests/virudevtestdata/simple-dac.json create mode 100644 tests/virudevtestdata/simple-selinux.json -- 2.8.4

5 35

[libvirt] [PATCH] network: fix endless loop when starting network with multiple IPs and no dhcp
by Laine Stump 29 Oct '16

29 Oct '16

(From the "How the Hell did I not see this?" files) commit 9065cfaa added the ability to disable DNS services for a libvirt virtual network. If neither DNS nor DHCP is needed for a network, then we don't need to start dnsmasq, so code was added to check for this. Unfortunately, it was written with a great lack of attention to detail (I can say that, because I was the author), and the loop that checked if DHCP is needed for the network would never end if the network had multiple IP addresses, and none of them had a <dhcp> section (which would have contained a <range> or <host> element). This patch rewrites the check to be more compact and (more importantly) finite. This bug was present in release 2.2.0 and 2.3.0, so will need to be backported to any relevant maintainence branches. Reported here: https://www.redhat.com/archives/libvirt-users/2016-October/msg00032.html https://www.redhat.com/archives/libvirt-users/2016-October/msg00045.html --- src/network/bridge_driver.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9d7fc31..a3ee3f3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1410,20 +1410,22 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver, int ret = -1; dnsmasqContext *dctx = NULL; - if (!(ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, 0))) { - /* no IP addresses, so we don't need to run */ - ret = 0; - goto cleanup; - } - /* see if there are any IP addresses that need a dhcp server */ - for (i = 0; ipdef && !needDnsmasq; - ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, i + 1)) { + i = 0; + while ((ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, i))) { + i++; if (ipdef->nranges || ipdef->nhosts) needDnsmasq = true; } + if (i == 0) { + /* no IP addresses at all, so we don't need to run */ + ret = 0; + goto cleanup; + } + if (!needDnsmasq && network->def->dns.enable == VIR_TRISTATE_BOOL_NO) { + /* no DHCP services needed, and user disabled DNS service */ ret = 0; goto cleanup; } -- 2.7.4

2 1

[libvirt] [PATCH 00/10] qemu: agent: bugfix and improvments
by Nikolay Shirokovskiy 29 Oct '16

29 Oct '16

Patch 1 fixes the bug I dealt with. Patches 2-3 is a somewhat protection from similar bugs. Patches 5-6 drop error flag from the agent altogether, so now there are less places for bugs to lurk. Rest patches cleanup loop function. I think 1-3 can be dropped if 6 is approved. It is just 6 can be considered controversial. Nikolay Shirokovskiy (10): qemu: agent: unset error flag in EOF handler qemu: agent: dont set error after agent cleanup qemu: agent: clear error flag upon init qemu: agent: handle agent connection errors in one place qemu: agent: straigten up failed agent start case qemu: agent: drop agent error flag (6) qemu: agent: simplify io loop qemu: agent: exit early if error is already set qemu: agent: fix loop error messages qemu: agent: drop redundant referencing in loop src/qemu/qemu_agent.c | 183 ++++++++++++++++++------------------------- src/qemu/qemu_agent.h | 2 - src/qemu/qemu_domain.c | 41 +++++----- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_driver.c | 9 +-- src/qemu/qemu_migration.c | 13 +-- src/qemu/qemu_process.c | 90 ++++----------------- tests/qemumonitortestutils.c | 1 - 8 files changed, 119 insertions(+), 221 deletions(-) -- 1.8.3.1

2 31

[libvirt] [RFC PATCH v2 1/1] qemu: host NUMA hugepage policy without guest NUMA
by Sam Bobroff 28 Oct '16

28 Oct '16

At the moment, guests that are backed by hugepages in the host are only able to use policy to control the placement of those hugepages on a per-(guest-)CPU basis. Policy applied globally is ignored. Such guests would use <memoryBacking><hugepages/></memoryBacking> and a <numatune> block with <memory mode=... nodeset=.../> but no <memnode .../> elements. This patch corrects this by, in this specific case, changing the QEMU command line from "-mem-prealloc -mem-path=..." (which cannot specify NUMA policy) to "-object memory-backend-file ..." (which can). Note: This is not visible to the guest and does not appear to create a migration incompatibility. Signed-off-by: Sam Bobroff <sam.bobroff(a)au1.ibm.com> --- Hello libvirt community, This patch is a RFC of an attempt to understand and fix a problem with NUMA memory placement policy being ignored in a specific situation. Previous discussion on this issue: https://www.redhat.com/archives/libvir-list/2016-October/msg00033.html https://www.redhat.com/archives/libvir-list/2016-October/msg00514.html (Sorry, this is a long description because the situation is complicated and there are several ways of approaching it and I need to explain my reasoning for choosing this specific method.) The issue I'm trying to fix: The problem is apparent when using this combination of features: * The guest is backed by hugepages in the host. * The backing memory is constrained by NUMA memory placement policy. * The guest either does not use NUMA nodes at all, or does but does not specify a per-node policy. The guest XML would therefore contain blocks similar to the following: <memoryBacking><hugepages/></memoryBacking> <numatune><memory mode='strict' nodeset='0'/></numatune> (And does not use <numatune><memnode ..>... .) (Policy works correctly without hugepages, or when memnode is used.) What happens is that the guest runs successfully but the NUMA memory placement policy is ignored, allowing memory to be allocated on any (host) node. Attempted solutions: The most obvious approach to me is a trivial patch to qemuBuildMemoryBackendStr(). The test that checks to see if a memory-backend is necessary (around line 3332 in qemu_command.c) is this: /* If none of the following is requested... */ if (!needHugepage && !userNodeset && !memAccess && !nodeSpecified && !force) { /* report back that using the new backend is not necessary * to achieve the desired configuration */ ret = 1; } else { This test does not consider the nodemask, so when no other reason exists to use a memory-backend (which is the case in point) the policy is lost. Adding nodemask to this test appears to easily fixe the issue. However: * It only works when the guest uses NUMA nodes (because otherwise the whole function is skipped at a higher level). * When the guest does use NUMA nodes, it causes a memory-backend object to be added to each one (when they did not previously have one) and this causes migration incompatability. So it seems worthwhile to look for other options. Other ways of controlling NUMA memory placement are numactl (libnuma) or cgroups but in practice neither can work in this situation due to the interaction of several features: * numactl and cgroups can only apply policy to all memory allocated by that thread. * When a guest is backed by hugepages, memory needs to be preallcated. * QEMU performs memory pre-allocation from it's main thread, not the VCPU threads. This seems to mean that policy applied to the QEMU VCPU threads does not apply to pre-allocated memory and policy applied to the QEMU main thread incorrectly applies to all of QEMU's allocations, not just the VCPU ones. My testing seems to confirm this. If that is true, then neither of those approaches can work (at least, not without changes to QEMU). At this point it appeared that the only ways to proceed would all require a migration-breaking change, so I started to look at QEMU to see if that could be worked around somehow. ... another option: While investigating QEMU I discovered that it is possible to use a QEMU memory backend object without attaching it to any guest NUMA node and when it is used that way it does not break migration. This is (to me at least) a new way of using a memory-backend object, but it appears to directly replace -mem-path and -mem-prealloc and additionally allow NUMA memory placement policy to be specified. (The ID field is still required even though it is not used.) For example, this... -m 4G -mem-path /dev/hugepages -mem-prealloc ... seems to be equavalent to this: -m 4G -object memory-backend-file,id=xxx,prealloc=yes,path=/dev/hugepages,size=4G But this is now possible as well: -m 4G -object memory-backend-file,id=xxx,prealloc=yes,path=/dev/hugepages,host-nodes=0-1,size=4G ... so it seems to be a way of solving this issue without breaking migration. Additionally, there seems to be code specifically in QEMU to prevent the migration data from changing in this case, which leads me to believe that this is an intended use of the option but I haven't directly consulted with the QEMU community. Implementation: My patch is based on this last case. It is intended to work by switching the "-mem-path" style argument to an un-attached memory-backend-file, which is why the change is done within qemuBuildMemPathStr(). In the case where QEMU doesn't support the needed option, I've opted to raise a warning rather than an error, because existing guests may have this configuration and although their policy is being ignored they do at least run and I assume it's important to maintain this. I realize that this policy can't be changed at run-time by libvirt, unlike policy set by cgroups or libnuma, but memory-backend objects are already in use in more common situations, and already have this limitation so I don't think this is adding significantly to that problem. Questions: So does this line of reasoning make sense? Is this a good way to fix it? Are there better solutions? Are there concerns with using memory-backend in this way: do I need to consult with the QEMU community before a libvirt patch that uses it could be accepted? Should the warning be an error? It seemed safest to me to only use the backend object when necessary, but there doesn't seem to be a reason to avoid using it whenever it is available. Is there one? Should I have done that instead? Thanks for reading! Sam. v2: Incorporated review feedback from Peter Krempa <pkrempa(a)redhat.com> and Martin Kletzander <mkletzan(a)redhat.com>. src/qemu/qemu_command.c | 56 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 405e118..0dea9fd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7105,12 +7105,18 @@ qemuBuildSmpCommandLine(virCommandPtr cmd, static int qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, - const virDomainDef *def, + virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virCommandPtr cmd) + virCommandPtr cmd, + virBitmapPtr auto_nodeset) { const long system_page_size = virGetSystemPageSizeKB(); char *mem_path = NULL; + virBitmapPtr nodemask = NULL; + const char *backendType = NULL; + char *backendStr = NULL; + virJSONValuePtr props = NULL; + int rv = -1; /* * No-op if hugepages were not requested. @@ -7135,18 +7141,50 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, if (qemuGetHupageMemPath(cfg, def->mem.hugepages[0].size, &mem_path) < 0) return -1; - virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path", mem_path, NULL); + if (virDomainNumatuneMaybeGetNodeset(def->numa, auto_nodeset, + &nodemask, -1) < 0) + return -1; + if (nodemask && virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + if (qemuBuildMemoryBackendStr(virDomainDefGetMemoryInitial(def), + 0, -1, NULL, auto_nodeset, + def, qemuCaps, cfg, &backendType, + &props, false) < 0) + goto cleanup; + /* The memory-backend object created here is not going to be + * attached anywhere and there is only one so the ID can be hard coded. + * It acts like -mem-prealloc -mem-path ... but allows policy to be + * set. */ + if (!(backendStr = virQEMUBuildObjectCommandlineFromJSON(backendType, + "mem", + props))) + goto cleanup; + virCommandAddArgList(cmd, "-object", backendStr, NULL); + rv = 0; + cleanup: + virJSONValueFree(props); + VIR_FREE(backendStr); + } else { + if (nodemask) + /* Ideally this would be an error, but if it was it would + * break existing guests already using this configuration. */ + VIR_WARN("Memory file backend objects are " + "unsupported by QEMU binary. Global NUMA " + "hugepage policy will be ignored."); + virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path", mem_path, NULL); + rv = 0; + } VIR_FREE(mem_path); - return 0; + return rv; } static int qemuBuildMemCommandLine(virCommandPtr cmd, virQEMUDriverConfigPtr cfg, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virBitmapPtr auto_nodeset) { if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) return -1; @@ -7170,7 +7208,7 @@ qemuBuildMemCommandLine(virCommandPtr cmd, * there is no numa node specified. */ if (!virDomainNumaGetNodeCount(def->numa) && - qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) + qemuBuildMemPathStr(cfg, def, qemuCaps, cmd, auto_nodeset) < 0) return -1; if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_REALTIME_MLOCK)) { @@ -7307,7 +7345,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } if (!needBackend && - qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) + qemuBuildMemPathStr(cfg, def, qemuCaps, cmd, auto_nodeset) < 0) goto cleanup; for (i = 0; i < ncells; i++) { @@ -9385,7 +9423,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0) goto error; - if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps) < 0) + if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps, nodeset) < 0) goto error; if (qemuBuildSmpCommandLine(cmd, def) < 0) -- 2.10.0.297.gf6727b0

1 0