- Devel - libvirt List Archives

[libvirt] [PATCH 00/55] qemu: Add support for -blockdev
by Peter Krempa 07 Aug '18

07 Aug '18

In this version everything except blockjobs should work properly. Stats were fixed so that they are properly extracted from the new output in qemu. Additionally I've also fixed the disk resize API which I forgot last time. The blockjobs support is still missing so they either will not work or will mess up internal state. That means that the last patch still should not be pushed leaving this feature dormant until we achieve full feature parity. Peter Krempa (55): tests: qemumonitorjson: Simplify debugging of 'blockInfo' test qemu: Improve errors in qemuDomainBlockResize qemu: monitor: Remove unsupported function check for 'block_resize' qemu: monitor: Remove useless 'locked' property from struct qemuDomainDiskInfo tests: qemucapabilities: Update capability data for qemu 3.0.0 Revert "qemu: monitor: Add the 'query-nodes' argument for query-blockstats" qemu: monitor: Reuse qemuMonitorJSONQueryBlock in qemuMonitorJSONBlockIoThrottleInfo qemu: monitor: Allow using 'id' instead of 'device' for 'block_set_io_throttle' qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk throttling qemu: monitor: Add 'nodename' argument for 'block_resize' tests: qemu: Drop disk from hostdev-mdev tests tests: qemuxml2argv: Fork CAPS_LATEST test cases for 'blockdev' tests: qemu: Add test data for backing chains and indexes qemu: hotplug: Don't generate alias when detaching disk util: virqemu: Simplify debugging if building QOM object with missing args qemu: caps: Add capability for using the blockdev infrastructure qemu: process: clear QEMU_CAPS_BLOCKDEV for VMs where we can't support it qemu: domain: Don't redetect backing chain when using -blockdev qemu: process: Don't detect nodenames when we support -blockdev conf: domain: Format out user provided backing chains in XML qemu: domain: Add infrastructure to generate block node names conf: Implement private data formatting and parsing for disks conf: Allow formatting and parsing of 'index' for disk source image qemu: Use proper backingIndex when reporting stats for backing chain qemu: Add field to store QDEV path of a disk in private data qemu: alias: Generate QDEV name of the block backend for disks qemu: domain: Add field for storing node name for copy-on-read qemu: proces: assign node names for user defined backing chains qemu: block: Add generator for the 'copy-on-read' blockdev driver qemu: domain: Prepare qemuDomainDiskGetBackendAlias for -blockdev qemu: command: format disk source commandline for -blockdev qemu: command: Add helper to check if disk throttling is enabled qemu: process: Setup disk io throttling for -blockdev qemu: driver: Use QOM backend name for disk IO throttling APIs qemu: driver: Prepare qemuDomainBlockResize for blockdev qemu: hotplug: Prepare for blockdev-add/blockdev-del with backing chains qemu: monitor: Add APIs for cdrom tray handling for -blockdev qemu: hotplug: Implement removable media change for -blockdev qemu: driver: Don't copy disk alias in qemuDomainBlocksStatsGather qemu: Explicitly find disks for stats totals qemu: monitor: Retrieve blockstats also by qdev and node-names qemu: monitor: Add APIs for refreshing disk capacity when using -blockdev qemu: driver: Don't pass 'virDomainDiskDefPtr' to qemuDomainGetStatsOneBlock qemu: monitor: Extract 'write-threshold' automatically for -blockdev qemu: Extract exporting of disk block statistics qemu: Extract exporting of the header for block stats qemu: Export stats relevant for the frontend separately qemu: Refactor control flow in qemuDomainGetStatsBlockExportDisk qemu: Report frontend stats only for the frontend entry qemu: Export stats relevant for the storage backend qemu: monitor: Report data also for 'qdev' entry in qemuMonitorJSONGetBlockInfo qemu: Use QOM path with query-block when using -blockdev qemu: driver: Allow using blockdev with qemuDomainBlocksStatsGather qemu: driver: Prepare qemuDomainGetStatsBlock (bulk disk stats) for -blockdev DO NOT APPLY: Enable QEMU_CAPS_BLOCKDEV if 'query-blockstats' works with -blockdev docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 19 + src/conf/domain_conf.c | 89 +++- src/conf/domain_conf.h | 7 + src/qemu/qemu_alias.c | 86 ++- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_block.c | 24 +- src/qemu/qemu_block.h | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 94 +++- src/qemu/qemu_command.h | 3 + src/qemu/qemu_domain.c | 173 +++++- src/qemu/qemu_domain.h | 10 +- src/qemu/qemu_driver.c | 419 +++++++++++---- src/qemu/qemu_hotplug.c | 175 +++++- src/qemu/qemu_monitor.c | 98 +++- src/qemu/qemu_monitor.h | 33 +- src/qemu/qemu_monitor_json.c | 395 +++++++++++--- src/qemu/qemu_monitor_json.h | 33 +- src/qemu/qemu_process.c | 82 ++- src/util/virqemu.c | 5 +- .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 591 +++------------------ tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 9 +- tests/qemumonitorjsontest.c | 34 +- tests/qemustatusxml2xmldata/modern-in.xml | 6 + tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args | 37 ++ tests/qemuxml2argvdata/disk-aio.x86_64-latest.args | 19 +- .../disk-backing-chains-index.x86_64-2.12.0.args | 1 + .../disk-backing-chains-index.x86_64-latest.args | 1 + .../qemuxml2argvdata/disk-backing-chains-index.xml | 145 +++++ .../disk-backing-chains-noindex.x86_64-2.12.0.args | 58 ++ .../disk-backing-chains-noindex.x86_64-latest.args | 163 ++++++ .../disk-backing-chains-noindex.xml | 145 +++++ .../qemuxml2argvdata/disk-cache.x86_64-2.12.0.args | 50 ++ .../qemuxml2argvdata/disk-cache.x86_64-latest.args | 50 +- .../disk-cdrom-network.x86_64-2.12.0.args | 41 ++ .../disk-cdrom-network.x86_64-latest.args | 32 +- .../disk-cdrom-tray.x86_64-2.12.0.args | 39 ++ .../disk-cdrom-tray.x86_64-latest.args | 24 +- .../qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 35 ++ .../qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 17 +- .../disk-copy_on_read.x86_64-2.12.0.args | 41 ++ .../disk-copy_on_read.x86_64-latest.args | 19 +- .../disk-detect-zeroes.x86_64-2.12.0.args | 37 ++ .../disk-detect-zeroes.x86_64-latest.args | 17 +- .../disk-error-policy.x86_64-2.12.0.args | 41 ++ .../disk-error-policy.x86_64-latest.args | 30 +- .../disk-floppy.x86_64-2.12.0.args | 35 ++ .../disk-network-gluster.x86_64-2.12.0.args | 44 ++ .../disk-network-gluster.x86_64-latest.args | 32 +- .../disk-network-iscsi.x86_64-2.12.0.args | 63 +++ .../disk-network-iscsi.x86_64-latest.args | 58 +- .../disk-network-nbd.x86_64-2.12.0.args | 46 ++ .../disk-network-nbd.x86_64-latest.args | 41 +- .../disk-network-rbd.x86_64-2.12.0.args | 61 +++ .../disk-network-rbd.x86_64-latest.args | 67 ++- .../disk-network-sheepdog.x86_64-2.12.0.args | 35 ++ .../disk-network-sheepdog.x86_64-latest.args | 16 +- .../disk-network-source-auth.x86_64-2.12.0.args | 47 ++ .../disk-network-source-auth.x86_64-latest.args | 30 +- .../disk-network-tlsx509.x86_64-2.12.0.args | 59 ++ .../disk-network-tlsx509.x86_64-latest.args | 61 ++- .../disk-readonly-disk.x86_64-2.12.0.args | 34 ++ .../disk-readonly-disk.x86_64-latest.args | 14 +- .../disk-shared.x86_64-2.12.0.args | 37 ++ .../disk-shared.x86_64-latest.args | 18 +- ...isk-virtio-scsi-reservations.x86_64-2.12.0.args | 43 ++ ...isk-virtio-scsi-reservations.x86_64-latest.args | 20 +- .../floppy-drive-fat.x86_64-2.12.0.args | 33 ++ .../hostdev-mdev-display-missing-graphics.xml | 6 - ...v-display-spice-egl-headless.x86_64-latest.args | 2 - .../hostdev-mdev-display-spice-egl-headless.xml | 6 - ...ev-mdev-display-spice-opengl.x86_64-latest.args | 2 - .../hostdev-mdev-display-spice-opengl.xml | 6 - ...dev-display-vnc-egl-headless.x86_64-latest.args | 2 - .../hostdev-mdev-display-vnc-egl-headless.xml | 6 - .../hostdev-mdev-display-vnc.x86_64-latest.args | 2 - .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 6 - tests/qemuxml2argvdata/hostdev-mdev-display.xml | 6 - .../hostdev-mdev-invalid-target-address.xml | 5 - .../qemuxml2argvdata/hostdev-mdev-precreated.args | 2 - tests/qemuxml2argvdata/hostdev-mdev-precreated.xml | 6 - .../hostdev-mdev-src-address-invalid.xml | 6 - tests/qemuxml2argvtest.c | 24 + .../disk-backing-chains-inactive.xml | 35 ++ .../disk-backing-chains-index-active.xml | 156 ++++++ .../disk-backing-chains-index-inactive.xml | 156 ++++++ .../disk-backing-chains-noindex-active.xml | 156 ++++++ .../disk-backing-chains-noindex-inactive.xml | 156 ++++++ tests/qemuxml2xmloutdata/disk-mirror-inactive.xml | 4 + .../disk-mirror-old-inactive.xml | 4 + tests/qemuxml2xmloutdata/hostdev-mdev-display.xml | 6 - .../qemuxml2xmloutdata/hostdev-mdev-precreated.xml | 6 - tests/qemuxml2xmltest.c | 2 + 95 files changed, 4017 insertions(+), 1087 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args create mode 120000 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args create mode 120000 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-index.xml create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.xml create mode 100644 tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-tray.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-copy_on_read.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-detect-zeroes.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-error-policy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-floppy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-gluster.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-nbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-readonly-disk.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-shared.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/floppy-drive-fat.x86_64-2.12.0.args create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-inactive.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-inactive.xml -- 2.16.2

1 55

[libvirt] [PATCH] qemu: Check sev capability pointer before using it
by Han Han 07 Aug '18

07 Aug '18

https://bugzilla.redhat.com/show_bug.cgi?id=1612009 Check sev capability pointer in function qemuGetSEVInfoToParams to avoid null pointer dereferences. Signed-off-by: Han Han <hhan(a)redhat.com> --- src/qemu/qemu_driver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb0d4a8c7a..3daaef586f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21452,6 +21452,12 @@ qemuGetSEVInfoToParams(virQEMUCapsPtr qemuCaps, virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + if (!sev) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("SEV is not supported in this guest")); + return -1; + } + if (virTypedParamsAddString(&sevParams, &n, &maxpar, VIR_NODE_SEV_PDH, sev->pdh) < 0) return -1; -- 2.18.0

3 2

[libvirt] [PATCH] docs: formatdomain: fix spacing before parentheses
by Ján Tomko 06 Aug '18

06 Aug '18

Multiple cputune elements specified microseconds as the unit without putting a space before the parenthesis. There were also other occurrences. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- Pushed as trivial. docs/formatdomain.html.in | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 19b73125e1..b63467bd91 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -820,7 +820,7 @@ <dt><code>period</code></dt> <dd> The optional <code>period</code> element specifies the enforcement - interval(unit: microseconds). Within <code>period</code>, each vCPU of + interval (unit: microseconds). Within <code>period</code>, each vCPU of the domain will not be allowed to consume more than <code>quota</code> worth of runtime. The value should be in range [1000, 1000000]. A period with value 0 means no value. @@ -830,7 +830,7 @@ <dt><code>quota</code></dt> <dd> The optional <code>quota</code> element specifies the maximum allowed - bandwidth(unit: microseconds). A domain with <code>quota</code> as any + bandwidth (unit: microseconds). A domain with <code>quota</code> as any negative value indicates that the domain has infinite bandwidth for vCPU threads, which means that it is not bandwidth controlled. The value should be in range [1000, 18446744073709551] or less than 0. A quota @@ -863,8 +863,8 @@ <dt><code>emulator_period</code></dt> <dd> The optional <code>emulator_period</code> element specifies the enforcement - interval(unit: microseconds). Within <code>emulator_period</code>, emulator - threads(those excluding vCPUs) of the domain will not be allowed to consume + interval (unit: microseconds). Within <code>emulator_period</code>, emulator + threads (those excluding vCPUs) of the domain will not be allowed to consume more than <code>emulator_quota</code> worth of runtime. The value should be in range [1000, 1000000]. A period with value 0 means no value. Only QEMU driver support since 0.10.0 @@ -872,7 +872,7 @@ <dt><code>emulator_quota</code></dt> <dd> The optional <code>emulator_quota</code> element specifies the maximum - allowed bandwidth(unit: microseconds) for domain's emulator threads(those + allowed bandwidth (unit: microseconds) for domain's emulator threads (those excluding vCPUs). A domain with <code>emulator_quota</code> as any negative value indicates that the domain has infinite bandwidth for emulator threads (those excluding vCPUs), which means that it is not bandwidth controlled. @@ -884,7 +884,7 @@ <dt><code>iothread_period</code></dt> <dd> The optional <code>iothread_period</code> element specifies the - enforcement interval(unit: microseconds) for IOThreads. Within + enforcement interval (unit: microseconds) for IOThreads. Within <code>iothread_period</code>, each IOThread of the domain will not be allowed to consume more than <code>iothread_quota</code> worth of runtime. The value should be in range [1000, 1000000]. @@ -894,7 +894,7 @@ <dt><code>iothread_quota</code></dt> <dd> The optional <code>iothread_quota</code> element specifies the maximum - allowed bandwidth(unit: microseconds) for IOThreads. A domain with + allowed bandwidth (unit: microseconds) for IOThreads. A domain with <code>iothread_quota</code> as any negative value indicates that the domain IOThreads have infinite bandwidth, which means that it is not bandwidth controlled. The value should be in range @@ -4385,8 +4385,8 @@ USB, PCI and SCSI devices attached to the host can be passed through to the guest using the <code>hostdev</code> element. - since after 0.4.4 for USB, 0.6.0 for PCI(KVM only) - and 1.0.6 for SCSI(KVM only): + since after 0.4.4 for USB, 0.6.0 for PCI (KVM only) + and 1.0.6 for SCSI (KVM only): <pre> -- 2.13.6

1 0

[libvirt] [PATCH] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
by Katerina Koukiou 06 Aug '18

06 Aug '18

We need to first perform the checks for changed/missing elements and then we can overwrite the missing ones. Otherwise we might falsely report successfull update, because some elements got overwritten before the validity checks. https://bugzilla.redhat.com/show_bug.cgi?id=1599513 Signed-off-by: Katerina Koukiou <kkoukiou(a)redhat.com> --- src/qemu/qemu_hotplug.c | 43 ++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1488f0a7c2..8d98c149e2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3445,23 +3445,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, goto cleanup; } - /* info: if newdev->info is empty, fill it in from olddev, - * otherwise verify that it matches - nothing is allowed to - * change. (There is no helper function to do this, so - * individually check the few feidls of virDomainDeviceInfo that - * are relevant in this case). + /* info: Nothing is allowed to change. First check for changes and + * then fill the missing newdev->info from olddev. */ - if (!virDomainDeviceAddressIsValid(&newdev->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && - virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) { - goto cleanup; - } - if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci, - &newdev->info.addr.pci)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot modify network device guest PCI address")); - goto cleanup; - } /* grab alias from olddev if not set in newdev */ if (!newdev->info.alias && VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0) @@ -3469,26 +3455,51 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* device alias is checked already in virDomainDefCompatibleDevice */ + if (newdev->info.rombar == VIR_TRISTATE_BOOL_ABSENT) + newdev->info.rombar = olddev->info.rombar; if (olddev->info.rombar != newdev->info.rombar) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device rom bar setting")); goto cleanup; } + + if (!newdev->info.romfile && + VIR_STRDUP(newdev->info.romfile, olddev->info.romfile) < 0) + goto cleanup; if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network rom file")); goto cleanup; } + + if (newdev->info.bootIndex == 0) + newdev->info.bootIndex = olddev->info.bootIndex; if (olddev->info.bootIndex != newdev->info.bootIndex) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device boot index setting")); goto cleanup; } + + if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT) + newdev->info.romenabled = olddev->info.romenabled; if (olddev->info.romenabled != newdev->info.romenabled) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device rom enabled setting")); goto cleanup; } + + /* if pci addr is missing or is invalid we overwrite all info struct */ + if (!virDomainDeviceAddressIsValid(&newdev->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) { + goto cleanup; + } + if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci, + &newdev->info.addr.pci)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot modify network device guest PCI address")); + goto cleanup; + } /* (end of device info checks) */ if (STRNEQ_NULLABLE(olddev->filter, newdev->filter) || -- 2.17.1

2 1

[libvirt] [PATCH] qemu: emit error when trying to update blkiotune group_name in qemuDomainChangeDiskLive
by Katerina Koukiou 06 Aug '18

06 Aug '18

All rest of blkiotune parameters are not updatable through UpdateDeviceFlags API. https://bugzilla.redhat.com/show_bug.cgi?id=1601677 Signed-off-by: Katerina Koukiou <kkoukiou(a)redhat.com> --- The BZ was requesting to add support for updating the group_name for blkdeviotune here, though, all the rest of blkdeviotune elements are not updatable though UpdateDeviceFlags API. I think emmiting error and keeping all blkiodevtune parameters updatable in a consistent way is the right way to go. src/qemu/qemu_domain.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index de056272e8..0aa440e0b5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8791,6 +8791,9 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(blkdeviotune.size_iops_sec, "blkdeviotune size_iops_sec", true); + CHECK_EQ(blkdeviotune.group_name, + "blkdeviotune group_name", + true); if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, -- 2.17.1

2 1

[libvirt] [RFC v2 1/2] process: wait longer on kill per assigned Hostdev
by Christian Ehrhardt 06 Aug '18

06 Aug '18

It was found that in cases with host devices virProcessKillPainfully might be able to send signal zero to the target PID for quite a while with the process already being gone from /proc/<PID>. That is due to cleanup and reset of devices which might include a secondary bus reset that on top of the actions taken has a 1s delay to let the bus settle. Due to that guests with plenty of Host devices could easily exceed the default timeouts. To solve that, this adds an extra delay of 2s per hostdev that is associated to a VM. Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/qemu/qemu_process.c | 5 +++-- src/util/virprocess.c | 18 +++++++++++++++--- src/util/virprocess.h | 1 + 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c4e33723d1..c20cc11e51 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6814,8 +6814,9 @@ qemuProcessKill(virDomainObjPtr vm, unsigned int flags) return 0; } - ret = virProcessKillPainfully(vm->pid, - !!(flags & VIR_QEMU_PROCESS_KILL_FORCE)); + ret = virProcessKillPainfullyDelay(vm->pid, + !!(flags & VIR_QEMU_PROCESS_KILL_FORCE) + vm->def->nhostdevs * 2); return ret; } diff --git a/src/util/virprocess.c b/src/util/virprocess.c index f92b0dce37..aefe655771 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -344,15 +344,19 @@ int virProcessKill(pid_t pid, int sig) * Returns 0 if it was killed gracefully, 1 if it * was killed forcibly, -1 if it is still alive, * or another error occurred. + * + * Callers can proide an extra delay to wait longer + * than the default. */ int -virProcessKillPainfully(pid_t pid, bool force) +virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay) { size_t i; int ret = -1; + unsigned int delay = 75 + (extradelay*5); const char *signame = "TERM"; - VIR_DEBUG("vpid=%lld force=%d", (long long)pid, force); + VIR_DEBUG("vpid=%lld force=%d delay=%u", (long long)pid, force, pid); /* This loop sends SIGTERM, then waits a few iterations (10 seconds) * to see if it dies. If the process still hasn't exited, and @@ -360,9 +364,12 @@ virProcessKillPainfully(pid_t pid, bool force) * wait up to 5 seconds more for the process to exit before * returning. * + * An extra delay can be specified for cases that are expected to clean + * up slower than usual. + * * Note that setting @force could result in dataloss for the process. */ - for (i = 0; i < 75; i++) { + for (i = 0; i < delay; i++) { int signum; if (i == 0) { signum = SIGTERM; /* kindly suggest it should exit */ @@ -405,6 +412,11 @@ virProcessKillPainfully(pid_t pid, bool force) } +int virProcessKillPainfully(pid_t pid, bool force) +{ + return virProcessKillPainfullyDelay(pid, force, 0); +} + #if HAVE_SCHED_GETAFFINITY int virProcessSetAffinity(pid_t pid, virBitmapPtr map) diff --git a/src/util/virprocess.h b/src/util/virprocess.h index 3c5a882772..b72603ca8e 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -55,6 +55,7 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) int virProcessKill(pid_t pid, int sig); int virProcessKillPainfully(pid_t pid, bool force); +int virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int); int virProcessSetAffinity(pid_t pid, virBitmapPtr map); -- 2.17.1

1 1

[libvirt] [RFC 0/2] Fix detection of slow guest shutdown
by Christian Ehrhardt 06 Aug '18

06 Aug '18

Hi, I was recently looking into a case which essentially looked like this: 1. virsh shutdown guest 2. after <1 second the qemu process was gone from /proc/ 3. but libvirt spun in virProcessKillPainfully because the process was still reachable via signals 4. virProcessKillPainfully eventually fails after 15 seconds and the guest stays in "in shutdown" state forever This is not one of the common cases I've found for virProcessKillPainfully to break: - bad I/O e.g. NFS gets qemu stuck - CPU overload stalls things to death - qemu not being reaped (by init) All of the above would have the process still available in /proc/<pid> as Zombie or in uninterruptible sleep, but that is not true in my case. It turned out that the case was dependent on the amount of hostdev resources passed to the guest. Debugging showed that with 8 and more likely 16 GPUs passed it took ~18 seconds from SIGTERM to "no more be reachable with signal 0". I haven't conducted much more tests but stayed on the 16 GPU case, but I'm rather sure more devices might make it take even longer. Discussion with a few kernel folks revealed that the kill(2) man page on signal 0 has to be taken very literal "check for the existence of a process ID" - you can read this as "the PID exists, but the Process is no more". I'm unsure why the kernel would take that much time to clean up as I thought taking away /proc/<PID> would be almost the last step in the cleanup of a task. patch 2: I happened to find that there seems to be no much better way than signal-0 to check, but finding that this isn't reliable if the kernel can still accept for quite some time even with the pid gone from all other interfaces that I could find - so I wanted to suggest a fallback in virProcessKillPainfully that considers the absence of /proc/<pid> as a valid "the process is gone" as well on top of the ESRCH of signal-0. We could also use the open FDs we have e.g. to the qemu monitor to see if the remote end is dead, but that didn't seem more readable/reliable to me and would have to cross quite some code to know about the FDs. But maybe someone else here has the insight what exactly would take the time in the kernel that I see and that might bring us to totally different solutions (therefore RFC). patch 1: Finally after working through this code for a while I got the feeling that if we are in a bad/non-responsive case after 10 seconds upgrading to SIGKILL we should give it some more time to take effect. We reach this in stressful cases only anyway and only if force is set, so then waiting a bit more helps to resolve some of the other cases that I found on the mailing list about virProcessKillPainfully being stuck. If one has a personal interest in the 15 seconds we had before lets add a VIR_WARN on 15 seconds if that would be better, but overall wait a bit more. P.S. Afer a short discussion with Daniel on IRC I'm also adding Alex explicitly for passthrough experience. P.P.S.: For now this really is only meant as an RFC to kick off the discussion. I got taken away the system that I could trigger this case easily on before I could complete a final verification. But the case is interesting enough to start the discussion now. Christian Ehrhardt (2): process: wait longer 5->30s on hard shutdown process: accept the lack of /proc/<pid> as valid process removal src/util/virprocess.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) -- 2.17.1

4 7

[libvirt] [PATCH v3 0/4] iscsi-direct: first part
by clem＠lse.epita.fr 06 Aug '18

06 Aug '18

From: Clementine Hayat <clem(a)lse.epita.fr> Hello, This is the implementation of the iscsi-direct backend storage pool version 3. v1: https://www.redhat.com/archives/libvir-list/2018-July/msg00918.html v2: https://www.redhat.com/archives/libvir-list/2018-July/msg01528.html Best Regards, -- Clementine Hayat Clementine Hayat (4): configure: Introduce libiscsi in build system storage: Introduce iscsi_direct pool type storage: Implement iscsi_direct pool backend news: add storage pool iscsi-direct configure.ac | 9 +- docs/news.xml | 9 + docs/schemas/storagepool.rng | 35 ++ docs/storage.html.in | 30 ++ m4/virt-libiscsi.m4 | 30 ++ m4/virt-storage-iscsi-direct.m4 | 44 ++ src/conf/domain_conf.c | 1 + src/conf/storage_conf.c | 22 +- src/conf/storage_conf.h | 1 + src/conf/virstorageobj.c | 2 + src/storage/Makefile.inc.am | 24 + src/storage/storage_backend.c | 6 + src/storage/storage_backend_iscsi_direct.c | 452 ++++++++++++++++++ src/storage/storage_backend_iscsi_direct.h | 6 + src/storage/storage_driver.c | 1 + .../pool-iscsi-direct-auth.xml | 14 + .../pool-iscsi-direct.xml | 12 + .../pool-iscsi-direct-auth.xml | 17 + .../pool-iscsi-direct.xml | 14 + tests/storagepoolxml2xmltest.c | 2 + tools/virsh-pool.c | 3 + 21 files changed, 728 insertions(+), 6 deletions(-) create mode 100644 m4/virt-libiscsi.m4 create mode 100644 m4/virt-storage-iscsi-direct.m4 create mode 100644 src/storage/storage_backend_iscsi_direct.c create mode 100644 src/storage/storage_backend_iscsi_direct.h create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-direct-auth.xml create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-direct.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-direct-auth.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-direct.xml -- 2.18.0

4 8

[libvirt] [RFC] secdrivers remembering original labels
by Michal Privoznik 06 Aug '18

06 Aug '18

Dear list, we have this very old bug [1] that I just keep pushing in front of me. I made several attempts to fix it. However, none of them made into the tree. I guess it's time to have discussion what to do about it. IIRC, the algorithm that I implemented last was to keep original label in XATTRs (among with some ref counter) and the last one to restore the label will look there and find the original label. There was a problem with two libvirtds fighting over a file on shared FS. So I guess my question is can we come up with a design that would work? Or at least work to the extent that we're satisfied with? Personally, I like the XATTRs approach. And to resolve the NFS race we can invent yet another lockspace to guard labelling - I also have a bug for that [2] (although, I'm not that familiar with lockspaces). I guess doing disk metadata locking is not going to be trivial, is it? Michal 1: https://bugzilla.redhat.com/show_bug.cgi?id=547546 2: https://bugzilla.redhat.com/show_bug.cgi?id=1524792

3 7

[libvirt] Release of libvirt-4.6.0
by Daniel Veillard 06 Aug '18

06 Aug '18

Okay it took a bit longer than usual but the relrase is out, it's tagged in git and signed tarball and rpms have been pushed to the usual place, but there was quite a few bug fixes going in after RC2 was cut. ftp://libvirt.org/libvirt/ I also made libvirt-python 4.6.0 bindings release in a similar fashion available at : ftp://libvirt.org/libvirt/python/ So though the release note don't list any bug fixes we know there is a batch of those in, otherwise this is mainly features and improvement targetting QEmu and virsh users: New features: - qemu: Implement the HTM pSeries feature Users can now decide whether HTM (Hardware Transactional Memory) support should be available to the guest. - qemu: Enable VNC console for mediated devices Host devices now support a new atribute 'display' which can be used to turn on frame buffer rendering on a vgpu mediated device instead of on an emulated GPU, like QXL. Improvements: - qemu: Introduce a new video model of type 'none' Introduce a new video model type that disables the automatic addition of a video device to domains with 'graphics' specified in their XML. This can be useful with GPU mediated devices which can serve as the only rendering devices within the guest. - virsh: Add --alias to attach-disk and attach-interface commands Add option --alias to set customized device alias name when using attach-disk or attach-interface commands. - virsh: Support usb and sata address to attach-disk Usb or sata address could be used when attach-disk with --address. For example, use usb address as usb:<bus>.<port>, use sata address as <controller>.<bus>.<unit>. Thanks everybody for your help with this release, hopefully the next one beginning of Sept will be more timely ! Enjoy, Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0