- Devel - libvirt List Archives

[PATCH v5 00/16] Introduce virtio-mem <memory/> model
by Michal Privoznik 21 Sep '21

21 Sep '21

v4 of: https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html diff to v4: - Rebased onto current master - Worked in David's suggestions, e.g. rename from <actual/> to <current/>, implemented offline memory update, implemented --node argument to virsh update-memory-device, prealloc is OFF and reserve is ON for virtio-mem Some suggestions are left as future work. For instance: - Don't require memory slots because virtio-mem lives on PCI bus anyway - Allow path backed backend for virtio-mem - support .prealloc for virtio-mem object (not memory-backend-* !) I keep occasionally rebased version on my gitlab: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/ Michal Prívozník (16): virhostmem: Introduce virHostMemGetTHPSize() qemu_capabilities: Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI qemu_capabilities: Introduce QEMU_CAPS_MEMORY_BACKEND_RESERVE conf: Introduce virtio-mem <memory/> model qemu: Build command line for virtio-mem qemu: Wire up <memory/> live update qemu: Wire up <memory/> offline update Introduce <current/> property to virtio-mem conf: Introduce virDomainMemoryFindByDeviceAlias() qemu: Wire up MEMORY_DEVICE_SIZE_CHANGE event qemu: Refresh the current size of virtio-mem on monitor reconnect qemu: Account for both memballoon and virtio-mem qemuDomainSetMemoryFlags: Take virtio-mem into consideration virsh: Introduce update-memory-device command news: document recent virtio memory addition kbase: Document virtio-mem NEWS.rst | 7 + docs/formatdomain.rst | 45 ++- docs/kbase/index.rst | 4 + docs/kbase/memorydevices.rst | 150 ++++++++++ docs/kbase/meson.build | 1 + docs/manpages/virsh.rst | 30 ++ docs/schemas/domaincommon.rng | 16 ++ examples/c/misc/event-test.c | 17 ++ include/libvirt/libvirt-domain.h | 24 ++ src/conf/domain_conf.c | 126 ++++++++- src/conf/domain_conf.h | 16 ++ src/conf/domain_event.c | 84 ++++++ src/conf/domain_event.h | 10 + src/conf/domain_validate.c | 39 +++ src/libvirt_private.syms | 5 + src/qemu/qemu_alias.c | 10 +- src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_command.c | 25 +- src/qemu/qemu_domain.c | 33 ++- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 38 ++- src/qemu/qemu_driver.c | 259 +++++++++++++++++- src/qemu/qemu_hotplug.c | 18 ++ src/qemu/qemu_hotplug.h | 5 + src/qemu/qemu_monitor.c | 34 +++ src/qemu/qemu_monitor.h | 28 ++ src/qemu/qemu_monitor_json.c | 97 +++++-- src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_process.c | 72 +++++ src/qemu/qemu_validate.c | 8 + src/remote/remote_daemon_dispatch.c | 30 ++ src/remote/remote_driver.c | 32 +++ src/remote/remote_protocol.x | 15 +- src/remote_protocol-structs | 7 + src/security/security_apparmor.c | 1 + src/security/security_dac.c | 2 + src/security/security_selinux.c | 2 + src/util/virhostmem.c | 54 ++++ src/util/virhostmem.h | 3 + tests/domaincapsmock.c | 9 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 2 + ...mory-hotplug-virtio-mem.x86_64-latest.args | 41 +++ .../memory-hotplug-virtio-mem.xml | 67 +++++ tests/qemuxml2argvtest.c | 1 + ...emory-hotplug-virtio-mem.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + tools/virsh-domain.c | 181 ++++++++++++ 51 files changed, 1613 insertions(+), 56 deletions(-) create mode 100644 docs/kbase/memorydevices.rst create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml -- 2.32.0

4 29

question: about the bug: current master had lost the ability "Cancel disk mirrors after libvirtd restart"
by wangjie (P) 21 Sep '21

21 Sep '21

bug reproduce process: 1、perform migrateToURI3. 2、kill libvirtd when enter memory migration phase，and restart libvirtd. 3、perform migrateToURI3 again and again，migrateToURI3 will fail forever with err-msg "Requested operation is not valid: domain has active block job" I found the reasion which trigger the bug as follow: 1、the qemuBlockJobData is not persistent when libvirtd restart，so the job which return from qemuBlockJobDiskGetJob while always NULL, so qemuMigrationSrcNBDCopyCancel will not be taken. 2、calltrace: qemuProcessReconnect ->qemuProcessRecoverJob ->qemuProcessRecoverMigrationOut ->qemuMigrationSrcCancel 3、code as follow: qemuMigrationSrcCancel(virQEMUDriver *driver, virDomainObj *vm) { ... ... for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDef *disk = vm->def->disks[i]; qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuBlockJobData *job; if (!(job = qemuBlockJobDiskGetJob(disk)) || //the job is always NULL !!! !qemuBlockJobIsRunning(job)) diskPriv->migrating = false; if (diskPriv->migrating) { qemuBlockJobSyncBegin(job); storage = true; } virObjectUnref(job); } ... ... if (storage && qemuMigrationSrcNBDCopyCancel(driver, vm, true, QEMU_ASYNC_JOB_NONE, NULL) < 0) return -1; ... ... } 4、I think current master had lost the ability of the followed patch: http://10.175.124.40/cgit/cgit.cgi/code.huawei.com/libvirt.git/commit/?id=e… can you give some suggestions to fix it?

2 2

RFC: revert to external snapshot API
by Nikolay Shirokovskiy 21 Sep '21

21 Sep '21

Hi, all. I want to implement reverting to external snapshot functionality. I've checked the mailing list history and found some previous attempts (not sure if this is a complete list of them). [1] was done in 2012 by the Redhat team itself. [2] and [3] was done in 2018. Looks like it was not clear how the API should look like. For example we have disk.qcow2 <- disk.snap1 chain <- disk.snap2 chain (after having snap1 and snap2 snapshots). Now we want to revert to snap1 snapshot. The snapshot state is held by disk.qcow2 image. We can run reverted domain on disk.qcow2 itself but then we lose snap1 (held by disk.qcow2) and snap2 (held by disk.snap1). So we definitely should run on some overlay over disk.qcow2. But then what name should be given to overlay? We should have an option for mgmt to specify this name like in case of snapshots itself. The [1] has some discussion on adding extra options to reverting functionality. Like for example if we revert back to snap2 then having the ability to run from state in disk.snap2 rather than disk.snap1. My opinion is there is no need to as if one wants to revert to the state of disk2.snap2 it can take a snapshot (some snap3). At the same time one needs to be aware that revert operation loses current state and later one can revert only to the states of snapshots. This is the way internal snapshots work and the way one expects external snapshots to work too. The [2] takes an approach of reusing current top image as overlay on revert so that in the above example disk.snap2 will be overlay over disk.qcow2 on reverting to snap1 snapshot. IMHO this is a confusing naming scheme. The [3] suggests a different scheme for naming images. For example after taking snapshot snap1 the chain should be disk.snap1 <- disk.qcow2 which looks very appealing to me. With this naming using the [2] approach is quite natural. Implementing this does not look hard even for a running domain but this is a big change to API and all mgmt need to be aware of (well it could be done safely using a new flag). Anyway we can go on with current image names. In order to specify overlay image name let's introduce new API: int virDomainRevertToSnapshotXML(virDomainSnapshotPtr snapshot, char *xmlDesc, unsigned int flags) with XML like: <domainsnapshotrevert> <disks> <disk name='vda'> <source file='/path/to/revert/overlay'/> </disk> </disks> </domainsnapshotrevert> Having an XML looks like a bit overkill right now but I could not find a better solution. If overlay name is omitted the generated name will be like disk.snap1-1 in the above example to be in alignment with creating a snapshot case. So that disk.snap1* images hold states derived from snap1 state. We can also support reverting to external snapshot thru existing virDomainRevertToSnapshot for those who rely on generated names using this naming scheme. [1] Re: [PATCHv4 4/4] snapshot: qemu: Implement reverting of external snapshots https://listman.redhat.com/archives/libvir-list/2012-November/msg00932.html [2] Re: [PATCH 1/5] snapshot: Implement reverting for external disk snapshots https://listman.redhat.com/archives/libvir-list/2018-November/msg00338.html [3] External disk snapshot paths and the revert operation https://listman.redhat.com/archives/libvir-list/2018-October/msg01110.html

3 6

[PATCH 0/2] selinux: Don't ignore ENOENT in Permissive mode
by Michal Privoznik 21 Sep '21

21 Sep '21

See 2/2 for explanation. Michal Prívozník (2): selinux: Swap two blocks handling setfilecon_raw() failure selinux: Don't ignore ENOENT in Permissive mode src/security/security_selinux.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) -- 2.32.0

3 4

[PATCH 0/4] virsh: Follow up improvements
by Michal Privoznik 20 Sep '21

20 Sep '21

These are the fixes I spotted when reviewing Peter's patchset earlier. Michal Prívozník (4): virsh: Provide local path completer for screenshot --file virsh: Provide local path completer for vol-download --file vsh: Extend checks for aliased commands vsh: Ensure that bool --options don't have completer tools/virsh-domain.c | 1 + tools/virsh-volume.c | 6 +----- tools/vsh.c | 30 +++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) -- 2.32.0

2 5

[PATCH 0/4] Various cleanups
by Peter Krempa 20 Sep '21

20 Sep '21

Few patches from old branches. Peter Krempa (4): util: virdevmapper: Sanitize use of macros for buffer size virDevMapperGetTargets: Use a linked list as return type util: virstring: Remove unused 'virStringListMerge' qemuMonitorJSONGetStatus: Refactor cleanup src/libvirt_private.syms | 1 - src/qemu/qemu_cgroup.c | 11 ++++---- src/qemu/qemu_monitor_json.c | 18 +++++-------- src/qemu/qemu_namespace.c | 9 +++---- src/util/virdevmapper.c | 49 +++++++++++++++++------------------- src/util/virdevmapper.h | 2 +- src/util/virstring.c | 35 -------------------------- src/util/virstring.h | 3 --- tests/qemuhotplugmock.c | 10 +++----- 9 files changed, 43 insertions(+), 95 deletions(-) -- 2.31.1

2 5

[PATCH 00/14] virsh: Completion improvements and checking tool
by Peter Krempa 20 Sep '21

20 Sep '21

When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers. After this patchset the following completers are still missing: $ virsh self-test --completers-missing attach-disk: source, targetbus, driver, subdriver, cache, io, type, mode, sourcetype, source-protocol, source-host-transport, source-host-socket attach-interface: type, source, script, model blockcopy: dest change-media: source detach-interface: type domdisplay: type domxml-from-native: format domxml-to-native: format dump: file get-user-sshkeys: user metadata: uri, key numatune: mode, nodeset qemu-monitor-event: event restore: file save: file save-image-define: file save-image-dumpxml: file save-image-edit: file screenshot: file set-user-sshkeys: user set-user-password: user cpu-models: arch domcapabilities: virttype, emulatorbin, arch, machine hypervisor-cpu-baseline: virttype, emulator, arch, machine hypervisor-cpu-compare: virttype, emulator, arch, machine maxvcpus: type iface-bridge: bridge iface-unbridge: bridge net-update: command, section nodedev-detach: driver snapshot-create-as: memspec find-storage-pool-sources-as: type find-storage-pool-sources: type pool-create-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver pool-define-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver vol-create-as: format, backing-vol, backing-vol-format vol-download: file vol-wipe: algorithm cd: dir The list is not so long any more. Mostly missing completers are for remote file paths (passed to libvirt or the VM itself), few enums, guest-agent-based user list and a few which need to be assesed. Peter Krempa (14): virshCheckpointNameCompleter: Sanitize forward declaration use virsh-completer*.h: Use modern header style virsh: Remove hack using 'VSH_CMD_FLAG_ALIAS' to hide virsh commands vshCmddefCheckInternals: Sanitize command alias validation vsh: Introduce '--completers-missing' for 'self-test' command virsh-snapshot: Use 'virshSnapshotNameCompleter' for '--from' of 'snapshot-list' virsh: Use 'virshStoragePoolNameCompleter' for two options vsh: Add completer for '--command' of 'help' command virsh: Provide completers for options taking comma separated list of disk targets virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh virsh: completer: Introduce dummy completer for local files virsh: Use 'virshCompletePathLocalExisting' for options reading local files virsh: Introduce virshCompleteEmpty and use it for places where we can't suggest anything virsh-completer: Provide completer for '--top' and '--base' for blockjobs tools/virsh-backup.c | 2 + tools/virsh-checkpoint.c | 3 + tools/virsh-completer-checkpoint.h | 7 +- tools/virsh-completer-domain.c | 136 +++++++++++++++++++++++ tools/virsh-completer-domain.h | 170 ++++++++++++++++++----------- tools/virsh-completer-host.h | 28 +++-- tools/virsh-completer-interface.h | 14 ++- tools/virsh-completer-network.h | 35 +++--- tools/virsh-completer-nodedev.h | 21 ++-- tools/virsh-completer-nwfilter.h | 14 ++- tools/virsh-completer-pool.h | 21 ++-- tools/virsh-completer-secret.h | 14 ++- tools/virsh-completer-snapshot.h | 7 +- tools/virsh-completer-volume.h | 14 ++- tools/virsh-completer.c | 35 ++++++ tools/virsh-completer.h | 18 ++- tools/virsh-domain.c | 84 +++++++++++++- tools/virsh-network.c | 1 + tools/virsh-pool.c | 9 ++ tools/virsh-secret.c | 2 + tools/virsh-snapshot.c | 4 + tools/virsh-volume.c | 12 +- tools/virsh.c | 4 +- tools/virsh.h | 2 + tools/virt-admin.c | 3 +- tools/vsh.c | 66 +++++++++-- tools/vsh.h | 10 +- 27 files changed, 568 insertions(+), 168 deletions(-) -- 2.31.1

4 24

[PATCH 0/2] virsh: Fix fallback code path for vcpuinfo
by Peter Krempa 20 Sep '21

20 Sep '21

Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 2.31.1

5 6

[libvirt PATCH] gitlab: remove obsolete job rules for TEMPORARILY_DISABLED variable
by Daniel P. Berrangé 20 Sep '21

20 Sep '21

We previously had a 'rules:' entry that caused a job to be skipped if the variable "TEMPORARILY_DISABLED" was set. This is no longer needed since we can set a similar flag in ci/manifest.yml and re-generate to temporarily skip a job. Unfortunately the 'rules:' entry had an unexpected side-effect on the pipelines that was never previously noticed. Instead of only running pipelines on push, the mere existance of the 'rules:' entry caused triggering of pipelines on merge requests too. The newly auto-generated ci/gitlab.yml file does not have a 'rules:' for the container job template, and thus only runs on git push. The result is that build jobs try to run on merge requests and the container jobs they depend on don't exist. This breaks the entire pipeline with a message that the config is invalid due to broken job dependencies. This fixes a regression introduced in commit ccc7a44adbea003d6a0dc2f156adb2856c28bd4c Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Sep 9 14:49:01 2021 +0100 ci: re-generate containers/gitlab config from manifest Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .gitlab-ci.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b396a1511d..d486faca58 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -18,10 +18,6 @@ include: '/ci/gitlab.yml' .native_build_job: extends: .gitlab_native_build_job - rules: - - if: "$TEMPORARILY_DISABLED" - allow_failure: true - - when: on_success cache: paths: - ccache/ @@ -45,10 +41,6 @@ include: '/ci/gitlab.yml' paths: - ccache/ key: "$CI_JOB_NAME" - rules: - - if: "$TEMPORARILY_DISABLED" - allow_failure: true - - when: on_success before_script: - *script_variables script: -- 2.31.1

2 1

[PATCH] tools: virsh-snapshot: refactor small functions
by Kristina Hanicova 20 Sep '21

20 Sep '21

This patch includes: * removal of dead code * simplifying nested if conditions * removal of unnecessary variables * usage of "direct" boolean return Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com> --- tools/virsh-snapshot.c | 43 +++++++++++++++--------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 2659b4340d..3bdad03df0 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -771,7 +771,6 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, g_autofree char *xml = NULL; g_autoptr(xmlDoc) xmldoc = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - int ret = -1; g_autofree char *state = NULL; if (!snapshot) @@ -796,20 +795,17 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, return -1; } if (STREQ(state, "disk-snapshot")) { - ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) == - (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); - } else { - if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) - ret = 0; - else if (STREQ(state, "shutoff")) - ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE); - else - ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE); + return ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) == + (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); } - return ret; + if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) + return 0; + if (STREQ(state, "shutoff")) + return !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE); + return !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE); } /* @@ -869,14 +865,8 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) * attempt a fallback. */ current = virDomainSnapshotIsCurrent(snapshot, 0); if (current < 0) { - g_autoptr(virshDomainSnapshot) other = NULL; - vshResetLibvirtError(); current = 0; - if (other) { - if (STREQ(name, virDomainSnapshotGetName(other))) - current = 1; - } } vshPrint(ctl, "%-15s %s\n", _("Current:"), current > 0 ? _("yes") : _("no")); @@ -1776,10 +1766,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) vshResetLibvirtError(); result = virDomainRevertToSnapshot(snapshot, flags); } - if (result < 0) - return false; - return true; + return result >= 0; } /* @@ -1844,16 +1832,15 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on * older servers that reject the flag, by manually computing the * list of descendants. But that's a lot of code to maintain. */ - if (virDomainSnapshotDelete(snapshot, flags) == 0) { - if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) - vshPrintExtra(ctl, _("Domain snapshot %s children deleted\n"), name); - else - vshPrintExtra(ctl, _("Domain snapshot %s deleted\n"), name); - } else { + if (virDomainSnapshotDelete(snapshot, flags) < 0) { vshError(ctl, _("Failed to delete snapshot %s"), name); return false; } + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) + vshPrintExtra(ctl, _("Domain snapshot %s children deleted\n"), name); + else + vshPrintExtra(ctl, _("Domain snapshot %s deleted\n"), name); return true; } -- 2.31.1

3 2