August 2018 - Devel - libvirt List Archives

[libvirt] [RFC] cgroup settings and systemd daemon-reload conflict
by Nikolay Shirokovskiy 03 Oct '18

03 Oct '18

Hi, all. It turns out that systemd daemon-reload reset settings that are managable thru 'systemctl set-property' interface. > virsh schedinfo tst3 | grep global_quota global_quota : -1 > virsh schedinfo tst3 --set global_quota=50000 | grep global_quota global_quota : 50000 > systemctl daemon-reload > virsh schedinfo tst3 | grep global_quota global_quota : -1 This behaviour does not limited to cpu controller, same for blkio for example. I checked different versions of systemd (219 - Feb 15, and quite recent 236 - Dec 17) to make sure it is not kind of bug of old version. So systemd does not play well with direct writes to cgroup parameters that managable thru systemd. Looks like libvirtd needs to use systemd's dbus interface to change all such parameters. I only wonder how this can be unnoticed for such long time (creating cgroup for domain thru systemd - Jul 2013) as daemon-reload is called upon libvirtd package update. May be I miss something? Nikolay

2 4

[libvirt] [PATCH 00/10] libxl: PVHv2 support
by Marek Marczykowski-Górecki 24 Sep '18

24 Sep '18

This is a respin of my old PVHv1 patch[1], converted to PVHv2. Should the code use "PVH" name (as libxl does internally), or "PVHv2" as in many places in Xen documentation? I've chosen the former, but want to confirm it. Also, not sure about VIR_DOMAIN_OSTYPE_XENPVH (as discussed on PVHv1 patch) - while it will be messy in many cases, there is libxl_domain_build_info.u.{hvm,pv,pvh} which would be good to not mess up. Also, PVHv2 needs different kernel features than PV (CONFIG_XEN_PVH vs CONFIG_XEN_PV), so keeping the same VIR_DOMAIN_OSTYPE_XEN could be confusing. On the other hand, libxl_domain_build_info.u.pv is used in very few places (one section of libxlMakeDomBuildInfo), so guarding u.hvm access with VIR_DOMAIN_OSTYPE_HVM may be enough. For now I've reused VIR_DOMAIN_OSTYPE_XEN - in the driver itself, most of the code is the same as for PV. Since PVHv2 relies on features in newer Xen versions, I needed to convert also some older code. For example b_info->u.hvm.nested_hvm was deprecated in favor of b_info->nested_hvm. While the code do handle both old and new versions (obviously refusing PVHv2 if Xen is too old), this isn't the case for tests. How it should be handled, if at all? First few preparatory patches can be applied independently. [1] https://www.redhat.com/archives/libvir-list/2016-August/msg00376.html Marek Marczykowski-Górecki (10): docs: don't refer to deprecated 'linux' ostype in example docs: add documentation of arch element of capabilities.xml docs: update domain schema for machine attribute libxl: set shadow memory for any guest type, not only HVM libxl: prefer new location of nested_hvm in libxl_domain_build_info libxl: reorder libxlMakeDomBuildInfo for upcoming PVH support libxl: add support for PVH tests: add basic Xen PVH test xenconfig: add support for parsing type= xl config entry xenconfig: add support for type="pvh" docs/formatcaps.html.in | 20 +++- docs/formatdomain.html.in | 12 +- docs/schemas/domaincommon.rng | 3 +- src/libxl/libxl_capabilities.c | 23 +++- src/libxl/libxl_conf.c | 90 ++++++++++++----- src/libxl/libxl_driver.c | 6 +- src/xenconfig/xen_common.c | 27 ++++- src/xenconfig/xen_xl.c | 5 +- tests/libxlxml2domconfigdata/basic-pv.json | 1 +- tests/libxlxml2domconfigdata/basic-pvh.json | 49 +++++++++- tests/libxlxml2domconfigdata/basic-pvh.xml | 28 +++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 2 +- tests/libxlxml2domconfigdata/multiple-ip.json | 1 +- tests/libxlxml2domconfigdata/vnuma-hvm.json | 2 +- tests/libxlxml2domconfigtest.c | 1 +- tests/testutilsxen.c | 3 +- tests/xlconfigdata/test-fullvirt-type.cfg | 21 ++++- tests/xlconfigdata/test-fullvirt-type.xml | 27 +++++- tests/xlconfigdata/test-paravirt-type.cfg | 13 ++- tests/xlconfigdata/test-paravirt-type.xml | 25 +++++- tests/xlconfigdata/test-pvh-type.cfg | 13 ++- tests/xlconfigdata/test-pvh-type.xml | 25 +++++- tests/xlconfigtest.c | 3 +- 23 files changed, 358 insertions(+), 42 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.json create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.xml create mode 100644 tests/xlconfigdata/test-fullvirt-type.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-type.xml create mode 100644 tests/xlconfigdata/test-paravirt-type.cfg create mode 100644 tests/xlconfigdata/test-paravirt-type.xml create mode 100644 tests/xlconfigdata/test-pvh-type.cfg create mode 100644 tests/xlconfigdata/test-pvh-type.xml base-commit: 49a5fcfac7d13ac8082a80aff7d7156968afee0f -- git-series 0.9.1

4 29

[libvirt] [PATCH v2] virsh: Don't break loop of domblkinfo for disks
by Han Han 21 Sep '18

21 Sep '18

https://bugzilla.redhat.com/show_bug.cgi?id=1619625 --all option is added to cmdDomblkinfo since commit 62c39193 allowing to show all block devices info. Reset error when empty source in case error breaks the loop of domblkinfo for disks. Signed-off-by: Han Han <hhan(a)redhat.com> --- tools/virsh-domain-monitor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b9b4f9739b..576610f005 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -475,6 +475,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) int ndisks; size_t i; xmlNodePtr *disks = NULL; + char *source = NULL; char *target = NULL; char *protocol = NULL; @@ -505,16 +506,18 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) for (i = 0; i < ndisks; i++) { ctxt->node = disks[i]; + source = virXPathString("string(./source)", ctxt); protocol = virXPathString("string(./source/@protocol)", ctxt); target = virXPathString("string(./target/@dev)", ctxt); rc = virDomainGetBlockInfo(dom, target, &info, 0); if (rc < 0) { - /* If protocol is present that's an indication of a networked - * storage device which cannot provide statistics, so generate - * 0 based data and get the next disk. */ - if (protocol && !active && + /* For the case of empty cdrom, networked disk which cannot + * provide statistics, generate 0 based data and get the next + * disk. + */ + if (!source && protocol && !active && virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR && virGetLastErrorDomain() == VIR_FROM_STORAGE) { memset(&info, 0, sizeof(info)); @@ -526,6 +529,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) cmdDomblkinfoPrint(ctl, &info, target, human, false); + VIR_FREE(source); VIR_FREE(target); VIR_FREE(protocol); } @@ -540,6 +544,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) cleanup: virshDomainFree(dom); + VIR_FREE(source); VIR_FREE(target); VIR_FREE(protocol); VIR_FREE(disks); -- 2.18.0

3 2

[libvirt] [PATCH] virhostdev: Fix PCI devices are still attatched to stub driver bug
by Wu Zongyong 20 Sep '18

20 Sep '18

Currently, PCI devices will not be rebound to host drivers but attached to the stub driver when: 1) use libvirt to start a virtual machine with PCI devices assigned, then stop libvirtd process and shutdown the virtual machine. Finally, PCI devices are still bound to the stub driver instead of host drivers after libvirt start again. 2) use libvirt to shutdown a virtual machine wtih PCI devices assigned, then stop libvirtd process before libvirt try to rebind PCI devices to host drivers. Finally, PCI devices are still bound to the stub driver after libvirt start again. Notice that the comment on the top of virPCIDeviceDetach as follows: activeDevs should be a list of all PCI devices currently in use by a domain.inactiveDevs is a list of all PCI devices that libvirt has detached from the host driver + attached to the stub driver, but hasn't yet assigned to a domain. It's not reasonable that libvirt filter out devices that are either not active or not used by the current domain and driver. For devices belong to domains that has been shutdown before libvirt start, we should put them into inactiveDevs if then meet the condition that PCI devices have been detached from the host driver + attached to the stub driver. Moreover, we set orignal states of PCI devices when build PCI devices list in virHostdevGetPCIHostDeviceList if states has been set in struct virDomainHostdevDefPtr. Signed-off-by: Wu Zongyong <cordius.wu(a)huawei.com> --- src/util/virhostdev.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index ca79c37..ecf95e3 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -235,6 +235,7 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; + virDomainHostdevOrigStatesPtr origstates = &hostdev->origstates; virPCIDevicePtr pci; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -262,6 +263,10 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN); else virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM); + + virPCIDeviceSetUnbindFromStub(pci, origstates->states.pci.unbind_from_stub); + virPCIDeviceSetRemoveSlot(pci, origstates->states.pci.remove_slot); + virPCIDeviceSetReprobe(pci, origstates->states.pci.reprobe); } return pcidevs; @@ -1008,8 +1013,19 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, continue; } } else { - virPCIDeviceListDel(pcidevs, pci); - continue; + int stub = virPCIDeviceGetStubDriver(pci); + if (stub > VIR_PCI_STUB_DRIVER_NONE && + stub < VIR_PCI_STUB_DRIVER_LAST) { + /* The device is bound to a known stub driver: add a copy + * to the inactive list */ + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + if (virPCIDeviceListAddCopy(mgr->inactivePCIHostdevs, pci) < 0) { + VIR_ERROR(_("Failed to add PCI device %s to the inactive list"), + virGetLastErrorMessage()); + virResetLastError(); + } + } } i++; -- 1.9.1

3 3

[libvirt] [PATCH v3] qemu: Ignore nwfilter binding instantiation issues during reconnect
by John Ferlan 20 Sep '18

20 Sep '18

https://bugzilla.redhat.com/show_bug.cgi?id=1607202 It's essentially stated in the nwfilterBindingDelete that we will allow the admin to shoot themselves in the foot by deleting the nwfilter binding which then allows them to undefine the nwfilter that is in use for the running guest... However, by allowing this we cause a problem for libvirtd restart reconnect processing which would then try to recreate the missing binding attempting to use the deleted filter resulting in an error and thus shutting the guest down. So rather than keep adding virDomainConfNWFilterInstantiate flags to "ignore" specific error conditions, modify the logic to ignore, but VIR_WARN errors other than ignoreExists. This will at least allow the guest to not shutdown for only nwfilter binding errors that we can now perhaps recover from since we have the binding create/delete capability. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html Differences to v2. Leave the ignoreExists bool, but just allow and VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue processing all filters from error point too. src/qemu/qemu_process.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ab749389ee..61a277f468 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def) } } -static int -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists) +/* Attempt to instantiate the filters. Ignore failures because it's + * possible that someone deleted a filter binding and the associated + * filter while the guest was running and we don't want that action + * to cause failure to keep the guest running during the reconnection + * processing. Nor do we necessarily want other failures to do the + * same. We'll just log the error conditions other than of course + * ignoreExists possibility (e.g. the true flag) */ +static void +qemuProcessFiltersInstantiate(virDomainDefPtr def) { size_t i; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; if ((net->filter) && (net->ifname)) { - if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0) - return 1; + if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, + true) < 0) { + VIR_WARN("filter '%s' instantiation for '%s' failed '%s'", + net->filter, net->ifname, virGetLastErrorMessage()); + virResetLastError(); + } } } - - return 0; } static int @@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque) qemuProcessNotifyNets(obj->def); - if (qemuProcessFiltersInstantiate(obj->def, true)) - goto error; + qemuProcessFiltersInstantiate(obj->def); if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; -- 2.17.1

2 4

[libvirt] [PATCH 00/10] Introduce x86 Cache Monitoring Technology (CMT)
by Wang Huaqiang 15 Sep '18

15 Sep '18

This series of patches introduced the x86 Cache Monitoring Technology (CMT) to libvirt by interacting with kernel resource control (resctrl) interface. CMT is one of the Intel(R) x86 CPU feature which belongs to the Resource Director Technology (RDT). CMT reports the occupancy of the last level cache, which is shared by all CPU cores. We have serval discussion about the enabling of CMT, please refer to following links for the RFCs. RFCv3 https://www.redhat.com/archives/libvir-list/2018-August/msg01213.html RFCv2 https://www.redhat.com/archives/libvir-list/2018-July/msg00409.html https://www.redhat.com/archives/libvir-list/2018-July/msg01241.html RFCv1 https://www.redhat.com/archives/libvir-list/2018-June/msg00674.html 1. About reason why CMT is necessary in libvirt? The perf events of 'CMT, MBML, MBMT' have been phased out since Linux kernel commit c39a0e2c8850f08249383f2425dbd8dbe4baad69, in libvirt the perf based cmt,mbm will not work with the latest linux kernel. These patches add CMT feature to libvirt through kernel resctrlfs interface. 2. Interfaces for CMT from the high level. 2.1 Query the host capability of CMT. The element 'monitor' represents the host capabilities of CMT. The explanations of involved CMT attributes: - 'maxAllocs' denotes the maximum monitoring groups could be created, which is limited by the number of hardware 'RMID'. - 'threshold' denotes the upper bound of cache occupancy for current group, in bytes, to determine if an RMID can be reused. - element 'feature' denotes the monitoring feature supported. - 'llc_occupancy' is the feature for reporting the last level cache occupancy information. # virsh capabilities ... <cache> <bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'> <control granularity='768' unit='KiB' type='code' maxAllocs='8'/> <control granularity='768' unit='KiB' type='data' maxAllocs='8'/> + <monitor threshold='540672' unit='B' maxAllocs='176'/> + <feature name=llc_occupancy/> + </monitor> </bank> <bank id='1' level='3' type='both' size='15' unit='MiB' cpus='6-11'> <control granularity='768' unit='KiB' type='code' maxAllocs='8'/> <control granularity='768' unit='KiB' type='data' maxAllocs='8'/> + <monitor threshold='540672' unit='B' maxAllocs='176'/> + <feature name=llc_occupancy/> + </monitor> </bank> </cache> ... 2.2 Create cache monitoring group (cache monitor). The main interface for creating monitoring group is through XML file. The proposed configuration is like: <cputune> <cachetune vcpus='1'> <cache id='0' level='3' type='code' size='7680' unit='KiB'/> <cache id='1' level='3' type='data' size='3840' unit='KiB'/> + <monitor vcpus='1'/> </cachetune> <cachetune vcpus='4-7'> + <monitor vcpus='4-6'/> </cachetune> </cputune> In above XML, created 2 cache resctrl allocation groups and 2 resctrl monitoring groups. The changes of cache monitor will be effective in next booting of VM. 2.3 Show CMT result through command 'domstats' Adding the interface in qemu to report this information for resource monitor group through command 'virsh domstats --cpu-total'. Below is a typical output: # virsh domstats 1 --cpu-total Domain: 'ubuntu16.04-base' ... cpu.cache.monitor.count=2 cpu.cache.0.name=vcpus_1 cpu.cache.0.vcpus=1 cpu.cache.0.bank.count=2 cpu.cache.0.bank.0.id=0 cpu.cache.0.bank.0.bytes=4505600 cpu.cache.0.bank.1.id=1 cpu.cache.0.bank.1.bytes=5586944 cpu.cache.1.name=vcpus_4-6 cpu.cache.1.vcpus=4,5,6 cpu.cache.1.bank.count=2 cpu.cache.1.bank.0.id=0 cpu.cache.1.bank.0.bytes=17571840 cpu.cache.1.bank.1.id=1 cpu.cache.1.bank.1.bytes=29106176 **Changes Since RFCv3** In the output of 'domstats', added 'cpu.cache.<cmt_group_index>.bank.<bank_index>.id' to tell the OS assigned cache bank id of current cache. Changes is prefixed with a '+': # virsh domstats 1 --cpu-total Domain: 'ubuntu16.04-base' ... cpu.cache.monitor.count=2 cpu.cache.0.name=vcpus_1 cpu.cache.0.vcpus=1 cpu.cache.0.bank.count=2 + cpu.cache.0.bank.0.id=0 cpu.cache.0.bank.0.bytes=4505600 + cpu.cache.0.bank.1.id=1 cpu.cache.0.bank.1.bytes=5586944 cpu.cache.1.name=vcpus_4-6 cpu.cache.1.vcpus=4,5,6 cpu.cache.1.bank.count=2 + cpu.cache.1.bank.0.id=0 cpu.cache.1.bank.0.bytes=17571840 + cpu.cache.1.bank.1.id=1 cpu.cache.1.bank.1.bytes=29106176 Wang Huaqiang (10): conf: Renamed 'controlBuf' to 'childrenBuf' util: add interface retrieving CMT capability conf: Add CMT capability to host test: add test case for resctrl monitor util: resctrl: refactoring some functions util: Introduce resctrl monitor for CMT conf: refactor virDomainResctrlAppend conf: introduce resctrl monitor group in domain qemu: Introduce resctrl monitoring group qemu: Report cache occupancy (CMT) with domstats .gnulib | 1 - docs/formatdomain.html.in | 14 +- docs/schemas/capability.rng | 28 + docs/schemas/domaincommon.rng | 11 +- src/conf/capabilities.c | 51 +- src/conf/capabilities.h | 1 + src/conf/domain_conf.c | 159 +++++- src/conf/domain_conf.h | 20 + src/libvirt-domain.c | 9 + src/libvirt_private.syms | 6 + src/qemu/qemu_driver.c | 265 ++++++++- src/qemu/qemu_process.c | 40 +- src/util/virresctrl.c | 597 +++++++++++++++++++-- src/util/virresctrl.h | 48 +- tests/genericxml2xmlindata/cachetune-cdp.xml | 2 + .../cachetune-colliding-monitors.xml | 36 ++ tests/genericxml2xmlindata/cachetune-small.xml | 1 + tests/genericxml2xmlindata/cachetune.xml | 3 + tests/genericxml2xmltest.c | 4 + .../resctrl/info/L3_MON/max_threshold_occupancy | 1 + .../linux-resctrl/resctrl/info/L3_MON/mon_features | 3 + .../linux-resctrl/resctrl/info/L3_MON/num_rmids | 1 + tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 6 + 23 files changed, 1208 insertions(+), 99 deletions(-) delete mode 160000 .gnulib create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-monitors.xml create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_threshold_occupancy create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_features create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmids -- 2.7.4

5 41

[libvirt] [PATCH 0/5] qemu: misc graphics fixes
by Nikolay Shirokovskiy 13 Sep '18

13 Sep '18

Nikolay Shirokovskiy (5): qemu: fix typo in vnc port releasing qemu: simplify graphics port releasing qemu: vnc: mark websocket as used on reconnect qemu: mark graphics ports as used on migration qemu: keep websocketGenerated on libvirtd restarts src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_migration.c | 6 ++++++ src/qemu/qemu_process.c | 51 ++++++++++++++++++++++------------------------- src/qemu/qemu_process.h | 3 +++ 5 files changed, 43 insertions(+), 27 deletions(-) -- 1.8.3.1

2 13

[libvirt] [PATCH 0/3] Allow inputvol when creating vol from inputvol to be encrypted
by John Ferlan 12 Sep '18

12 Sep '18

https://bugzilla.redhat.com/show_bug.cgi?id=1613737 Details in the patches (and even more in the bz). John Ferlan (3): storage: Remove secretPath from _virStorageBackendQemuImgInfo storage: Allow for inputvol to have any format for encryption storage: Allow inputvol to be encrypted src/storage/storage_util.c | 79 ++++++++++++++++--- src/storage/storage_util.h | 1 + .../luks-convert-encrypt.argv | 11 +++ .../luks-convert-encrypt2fileqcow2.argv | 7 ++ .../luks-convert-encrypt2fileraw.argv | 7 ++ .../luks-convert-qcow2.argv | 9 +++ tests/storagevolxml2argvtest.c | 19 ++++- tests/storagevolxml2xmlin/vol-encrypt1.xml | 21 +++++ tests/storagevolxml2xmlin/vol-encrypt2.xml | 21 +++++ tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 +++++ 10 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt.argv create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv create mode 100644 tests/storagevolxml2argvdata/luks-convert-qcow2.argv create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml -- 2.17.1

2 10

[libvirt] [PATCH v4] qemu: Introduce state_lock_timeout to qemu.conf
by Yi Wang 06 Sep '18

06 Sep '18

When doing some job holding state lock for a long time, we may come across error: "Timed out during operation: cannot acquire state change lock" Well, sometimes it's not a problem and users wanner continue to wait, and this patch allow users decide how long time they can wait the state lock. Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn> Reviewed-by: Xi Xu <xu.xi8(a)zte.com.cn> --- changes in v4: - fox syntax-check error changes in v3: - add user-friendly description and nb of state lock - check validity of stateLockTimeout changes in v2: - change default value to 30 in qemu.conf - set the default value in virQEMUDriverConfigNew() v4 Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 10 ++++++++++ src/qemu/qemu_conf.c | 14 ++++++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 5 +---- src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index ddc4bbf..f7287ae 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -93,6 +93,7 @@ module Libvirtd_qemu = | limits_entry "max_core" | bool_entry "dump_guest_core" | str_entry "stdio_handler" + | int_entry "state_lock_timeout" let device_entry = bool_entry "mac_filter" | bool_entry "relaxed_acs_check" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index cd57b3c..8920a1a 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -667,6 +667,16 @@ # #max_queued = 0 + +# When two or more threads want to work with the same domain they use a +# job lock to mutually exclude each other. However, waiting for the lock +# is limited up to state_lock_timeout seconds. +# NB, strong recommendation to set the timeout longer than 30 seconds. +# +# Default is 30 +# +#state_lock_timeout = 60 + ################################################################### # Keepalive protocol: # This allows qemu driver to detect broken connections to remote diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a4f545e..c761cae 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -129,6 +129,9 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def) #endif +/* Give up waiting for mutex after 30 seconds */ +#define QEMU_JOB_WAIT_TIME (1000ull * 30) + virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) { virQEMUDriverConfigPtr cfg; @@ -346,6 +349,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->glusterDebugLevel = 4; cfg->stdioLogD = true; + cfg->stateLockTimeout = QEMU_JOB_WAIT_TIME; + if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST))) goto error; @@ -863,6 +868,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0) goto cleanup; + if (virConfGetValueInt(conf, "state_lock_timeout", &cfg->stateLockTimeout) < 0) + goto cleanup; + if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0) goto cleanup; @@ -1055,6 +1063,12 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg) return -1; } + if (cfg->stateLockTimeout <= 0) { + virReportError(VIR_ERR_CONF_SYNTAX, "%s", + _("state_lock_timeout should larger than zero")); + return -1; + } + return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a8d84ef..97cf2e1 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -190,6 +190,8 @@ struct _virQEMUDriverConfig { int keepAliveInterval; unsigned int keepAliveCount; + int stateLockTimeout; + int seccompSandbox; char *migrateHost; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 886e3fb..5a2ca52 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv, priv->job.agentActive == QEMU_AGENT_JOB_NONE)); } -/* Give up waiting for mutex after 30 seconds */ -#define QEMU_JOB_WAIT_TIME (1000ull * 30) - /** * qemuDomainObjBeginJobInternal: * @driver: qemu driver @@ -6714,7 +6711,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, } priv->jobs_queued++; - then = now + QEMU_JOB_WAIT_TIME; + then = now + cfg->stateLockTimeout; retry: if ((!async && job != QEMU_JOB_DESTROY) && diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index f1e8806..dc5de96 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -105,3 +105,4 @@ module Test_libvirtd_qemu = { "pr_helper" = "/usr/bin/qemu-pr-helper" } { "swtpm_user" = "tss" } { "swtpm_group" = "tss" } +{ "state_lock_timeout" = "60" } -- 1.8.3.1

3 4

Re: [libvirt] [PATCH] qga: ignore non present cpus when handling qmp_guest_get_vcpus()
by Igor Mammedov 06 Sep '18

06 Sep '18

On Thu, 30 Aug 2018 17:51:13 +0200 Laszlo Ersek <lersek(a)redhat.com> wrote: > +Drew > > On 08/30/18 14:08, Igor Mammedov wrote: > > If VM has VCPUs plugged sparselly (for example a VM started with > > 3 VCPUs (cpu0, cpu1 and cpu2) and then cpu1 was hotunplugged so > > only cpu0 and cpu2 are present), QGA will rise a error > > error: internal error: unable to execute QEMU agent command 'guest-get-vcpus': > > open("/sys/devices/system/cpu/cpu1/"): No such file or directory > > when > > virsh vcpucount FOO --guest > > is executed. > > Fix it by ignoring non present CPUs when fetching CPUs status from sysfs. > > > > Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> > > --- > > qga/commands-posix.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index 37e8a2d..2929872 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > @@ -2044,7 +2044,9 @@ static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu, > > vcpu->logical_id); > > dirfd = open(dirpath, O_RDONLY | O_DIRECTORY); > > if (dirfd == -1) { > > - error_setg_errno(errp, errno, "open(\"%s\")", dirpath); > > + if (!(sys2vcpu && errno == ENOENT)) { > > + error_setg_errno(errp, errno, "open(\"%s\")", dirpath); > > + } > > } else { > > static const char fn[] = "online"; > > int fd; > > > > Originally these guest agent commands (both getting and setting) were > meant to be used in the absence of real VCPU hot[un]plug, as a fallback > / place-holder. > > If the latter (= real VCPU hot(un)plug) works, then these guest agent > commands shouldn't be used at all. Technically there isn't reasons for "get" not to work in sparse usecase hence the patch. > Drew, do I remember correctly? ... The related RHBZ is > <https://bugzilla.redhat.com/show_bug.cgi?id=924684>. (It's a private > one, and I'm not at liberty to open it up, so my apologies to non-RH folks.) > > Anyway, given that "set" should be a subset of the "get" return value > (as documented in the command schema), if we fix up "get" to work with > sparse topologies, then "set" should work at once. > > However... as far as I understand, this change will allow > qmp_guest_get_vcpus() to produce a GuestLogicalProcessor object for the > missing (hot-unplugged) VCPU, with the following contents: > - @logical-id: populated by the loop, > - @online: false (from g_malloc0()), > - @can-offline: present (from the loop), and false (from g_malloc0()). > > The smaller problem with this might be that "online==false && > can-offline==false" is nonsensical and has never been returned before. I > don't know how higher level apps will react. > > The larger problem might be that a higher level app could simply copy > this output structure into the next "set" call unchanged, and then that > "set" call will fail. Libvirt it seems that survives such outrageous output > I wonder if, instead of this patch, we should rework > qmp_guest_get_vcpus(), to silently skip processors for which this > dirpath ENOENT condition arises (i.e., return a shorter list of > GuestLogicalProcessor objects). > But, again, I wouldn't mix this guest agent command with real VCPU > hot(un)plug in the first place. The latter is much-much better, so if > it's available, use that exclusively? Agreed, Maybe we can block invalid usecase on libvirt side with a more clear error message as libvirt sort of knows that sparse cpus are supported. > > Thanks, > Laszlo

3 2