March 2020 - Devel - libvirt List Archives

[PATCH] news: Mention regression in virDomainBlockCopy with shallow/reuse flags
by Peter Krempa 13 Mar '20

13 Mar '20

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- docs/news.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 7fd88f9998..92103ec308 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -48,6 +48,23 @@ <section title="Improvements"> </section> <section title="Bug fixes"> + <change> + <summary> + qemu: Open backing chain late for shallow block copy reusing external images + </summary> + <description> + With introduction of -blockdev specification of storage for qemu VMs + in libvirt-5.10 we've started opening the backing chain of the + destination/mirror of a virDomainBlockcopy started with + VIR_DOMAIN_BLOCK_COPY_REUSE_EXT | VIR_DOMAIN_BLOCK_COPY_SHALLOW flags + when starting the job rather than when virDomainBlockJobAbort with + VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT is issued. For users depending on + this undocumented quirk this caused a regression in behaviour as + the backing chain could not be modified while the copy of the top + image was progressing. Note that this fix also requires qemu-5.0 while + -blockdev is used starting from qemu-4.2. + </description> + </change> </section> </release> <release version="v6.1.0" date="2020-03-03"> -- 2.24.1

2 1

[PATCH 0/5] qemu: Allow late opening of backing chain on a shallow block-copy
by Peter Krempa 13 Mar '20

13 Mar '20

See patch 5/5 for explanation. As described in patch 1/5 this is based on patches which are not yet pushed in qemu. Peter Krempa (5): qemu: capabilities: Update qemu-5.0.0 capabilities for x86_64 qemuDomainBlockPivot: Move check prior to executing the pivot steps qemuDomainBlockCopyCommon: Record updated flags to block job qemu: capabilities: Introduce QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY qemu: blockcopy: Allow late opening of the backing chain of a shallow copy src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 75 +- .../caps_5.0.0.x86_64.replies | 2668 +++++++++-------- .../caps_5.0.0.x86_64.xml | 3 +- 5 files changed, 1435 insertions(+), 1314 deletions(-) -- 2.24.1

3 14

[libvirt PATCH] docs: Fix blockCopy's documentation on --pivot & --abort flags
by Kashyap Chamarthy 13 Mar '20

13 Mar '20

The --abort and --pivot flags for ``blockjob`` are mutually exclusive — you either abort and keep the destination copy as a point-in-time "snapshot"; or pivot to the new copy. But you cannot specify both flags at once. Rephrase blockCopy's doc fragment to that effect. Signed-off-by: Kashyap Chamarthy <kchamart(a)redhat.com> --- docs/manpages/virsh.rst | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index dc404ddfe8..e7d4933c93 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1254,18 +1254,19 @@ phases. Initially, the job must copy all data from the source, and during this phase, the job can only be canceled to revert back to the source disk, with no guarantees about the destination. After this phase completes, both the source and the destination remain mirrored until a -call to ``blockjob`` with the *--abort* and *--pivot* flags pivots over -to the copy, or a call without *--pivot* leaves the destination as a -faithful copy of that point in time. However, if *--wait* is specified, -then this command will block until the mirroring phase begins, or cancel -the operation if the optional *timeout* in seconds elapses or SIGINT is -sent (usually with ``Ctrl-C``). Using *--verbose* along with *--wait* -will produce periodic status updates. Using *--pivot* (similar to -``blockjob`` *--pivot*) or *--finish* (similar to ``blockjob`` *--abort*) -implies *--wait*, and will additionally end the job cleanly rather than -leaving things in the mirroring phase. If job cancellation is triggered -by timeout or by *--finish*, *--async* will return control to the user -as fast as possible, otherwise the command may continue to block a little +call is issued to ``blockjob`` with either the *--abort* flag, which +leaves the destination as a faithful copy of that point in time; or the +*--pivot* flag, which pivots over to the copy on the destination. +However, if *--wait* is specified, then this command will block until +the mirroring phase begins, or cancel the operation if the optional +*--timeout* in seconds elapses or SIGINT is sent (usually with +``Ctrl-C``). Using *--verbose* along with *--wait* will produce +periodic status updates. Using *--pivot* (similar to ``blockjob`` +*--pivot*) or *--finish* (similar to ``blockjob`` *--abort*) implies +*--wait*, and will additionally end the job cleanly rather than leaving +things in the mirroring phase. If job cancellation is triggered by +timeout or by *--finish*, *--async* will return control to the user as +fast as possible, otherwise the command may continue to block a little while longer until the job has actually cancelled. *path* specifies fully-qualified path of the disk. -- 2.21.0

1 0

[PATCH] apparmor: qemu load old shared objects
by Christian Ehrhardt 13 Mar '20

13 Mar '20

On qemu upgrades the old .so files usually are replaced. But on the other hand since a qemu process represents a guest instance it is usually kept around. That makes late addition of dynamic features e.g. 'hot-attach of a ceph disk' fail by trying to load a new version of e.f. block-rbd.so into an old still running qemu binary. Qemu adds a fallback to also load modules from a versioned directory in the temporary /var/run path. That way qemu is providing a way for packaging to store modules of an upgraded qemu package as needed until the next reboot. This change is allowing the qemu process access to these paths. Background: This is a continuation of a discussion at KVM Forum 2019 eventually becoming [1] and recently this change is queued to get into qemu properly [2]. [1]: https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg00005.html [2]: https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg03313.html Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/security/apparmor/libvirt-qemu | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 80986aec61..22415c9dfd 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -164,6 +164,11 @@ /usr/{lib,lib64}/qemu/*.so mr, /usr/lib/(a){multiarch}/qemu/*.so mr, + # let qemu load old shared objects after upgrades (LP: #1847361) + /{var/,}run/qemu/*/*.so mr, + # but explicitly deny with auditing writing to these files + audit deny /{var/,}run/qemu/*/*.so w, + # swtpm /{usr/,}bin/swtpm rmix, /usr/{lib,lib64}/libswtpm_libtpms.so mr, -- 2.25.1

2 1

[PATCH v2] conf: Don't generate machine names with a dot
by Michal Privoznik 13 Mar '20

13 Mar '20

According to the linked BZ, machined expects either valid hostname or valid FQDN (see systemd commit v239-3092-gd65652f1f2). While in case of multiple dots, a trailing one doesn't violate FQDN, it does violate the rule in case of something simple, like "domain.". But it's safe to remove it in both cases. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499 Fixes: 45464db8ba502764cf37ec9335770248bdb3d9a8 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://www.redhat.com/archives/libvir-list/2020-February/msg01138.html diff to v1: - Adjusted commit message as suggested by Jano - Fixed ".-" and "-." occurrences too, again suggested by Jano src/conf/domain_conf.c | 6 +++--- tests/virsystemdtest.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2d97daf80..246a78d39b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30933,7 +30933,7 @@ virDomainMachineNameAppendValid(virBufferPtr buf, if (strlen(virBufferCurrentContent(buf)) >= 64) break; - if (*name == '.') { + if (*name == '.' || *name == '-') { if (!skip_dot) virBufferAddChar(buf, *name); skip_dot = true; @@ -30948,8 +30948,8 @@ virDomainMachineNameAppendValid(virBufferPtr buf, virBufferAddChar(buf, *name); } - /* trailing dashes are not allowed */ - virBufferTrimChars(buf, "-"); + /* trailing dashes or dots are not allowed */ + virBufferTrimChars(buf, "-."); } #undef HOSTNAME_CHARS diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 1e36298189..f48a714cca 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -746,7 +746,8 @@ mymain(void) TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100, "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec"); TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10, - "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec"); + "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec-c"); + TEST_MACHINE("demo.-.test.", 11, "qemu-11-demo.test"); # define TESTS_PM_SUPPORT_HELPER(name, function) \ do { \ -- 2.24.1

2 1

[PATCH 0/2] qemu: fix crash in qemuDomainGetGuestInfo
by Peter Krempa 13 Mar '20

13 Mar '20

See patch 2/2 Peter Krempa (2): qemuAgentFSInfoFormatParams: Remove pointless returned value qemuDomainGetGuestInfo: Don't try to free a negative number of entries src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_driver.c | 43 ++++++++++++++++++++---------------------- 2 files changed, 21 insertions(+), 24 deletions(-) -- 2.24.1

3 6

[libvirt PATCH 0/4] Use g_strerror instead of virStrerror (glib chronicles)
by Ján Tomko 13 Mar '20

13 Mar '20

2 5

[libvirt PATCH] tests: fix double-lock of monitor in hotplug test
by Daniel P. Berrangé 13 Mar '20

13 Mar '20

The qemuMonitorTestNew() function returns with the monitor object locked, and expects it to still be locked when qemuMonitorTestFree is called. The qemuhotplug test, however, explicitly unlocks the monitor, but then forgets to lock it again. As a result the qemuMonitorTestFree function is unlocking a mutex it doesn't own. This bug has existed forever, but since we use normal POSIX mutexes and don't check the return value of pthread_mutex_lock/unlock we didn't see the error. It was harmless until the switch to the per monitor event loop which requires the thread synchronization to work reliably, whereupon it started crashing. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tests/qemuhotplugtest.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 8b411d63f0..d9244dca44 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -337,6 +337,8 @@ testQemuHotplug(const void *data) ret = testQemuHotplugUpdate(vm, dev); } + virObjectLock(priv->mon); + cleanup: VIR_FREE(domain_filename); VIR_FREE(device_filename); @@ -378,6 +380,7 @@ static void testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData *data) { qemuDomainObjPrivatePtr priv; + qemuMonitorPtr mon; if (!data) return; @@ -396,6 +399,8 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData *data) virObjectUnref(data->vm); } + mon = qemuMonitorTestGetMonitor(data->mon); + virObjectLock(mon); qemuMonitorTestFree(data->mon); VIR_FREE(data); } -- 2.24.1

2 1

[PATCH v2 00/19] incremental backup: Handle bitmaps during block-commit
by Peter Krempa 13 Mar '20

13 Mar '20

This is now based on top of a different implementation of the fix for late opening of backing chain during block-copy: https://www.redhat.com/archives/libvir-list/2020-March/msg00317.html Changes: - fixes suggested by Pavel - changes to capability use - rebased on master This series uses blockdev-reopen which was not yet stabilized in qemu, thus is experimental for now, but since incremental backup is experimental there's no problem with that. You can fetch the full branch along with patches for enabling the bits at: git fetch https://gitlab.com/pipo.sk/libvirt.git backup-commit You also need to use patched qemu which you can fetch from: git fetch https://gitlab.com/pipo.sk/qemu.git reopen-fixes Peter Krempa (19): qemu: capabilities: Add QEMU_CAPS_BLOCKDEV_REOPEN qemu: monitor: Add handler for blockdev-reopen qemu: block: implement helpers for blockdev-reopen qemuCheckpointDiscardBitmaps: Reopen images for bitmap modifications qemuCheckpointDiscardBitmaps: Use correct field for checkpoint bitmap name qemuDomainBlockCommit: Move checks depending on capabilities after liveness check qemu: domain: Extract formatting of 'commit' blockjob data into a function qemu: domain: Extract parsing of 'commit' blockjob data into a function qemu: blockjob: Store list of bitmaps disabled prior to commit qemublocktest: Fix and optimize fake image chain qemu: block: Implement helpers for dealing with bitmaps during block commit qemublocktest: Add tests for handling of bitmaps during block-commit qemublocktest: Add more tests for block-commit bitmap handling with snapshots qemublocktest: Add tests of broken bitmap chain handling during block-commit qemuBlockJobDiskNewCommit: Propagate 'disabledBitmapsBase' qemuDomainBlockCommit: Handle bitmaps on start of commit qemuDomainBlockPivot: Handle merging of bitmaps when pivoting an active block-commit qemu: blockjob: Handle bitmaps after finish of normal block-commit qemu: blockjob: Re-enable bitmaps after failed block-copy src/qemu/qemu_block.c | 320 ++++++++++++++++++ src/qemu/qemu_block.h | 23 ++ src/qemu/qemu_blockjob.c | 97 +++++- src/qemu/qemu_blockjob.h | 3 + src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_checkpoint.c | 9 +- src/qemu/qemu_domain.c | 110 ++++-- src/qemu/qemu_driver.c | 78 ++++- src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 21 ++ src/qemu/qemu_monitor_json.h | 4 + tests/qemublocktest.c | 126 ++++++- .../bitmapblockcommit/basic-1-2 | 119 +++++++ .../bitmapblockcommit/basic-1-3 | 119 +++++++ .../bitmapblockcommit/basic-2-3 | 2 + .../bitmapblockcommit/snapshots-1-2 | 49 +++ .../bitmapblockcommit/snapshots-1-3 | 76 +++++ .../bitmapblockcommit/snapshots-1-4 | 126 +++++++ .../bitmapblockcommit/snapshots-1-5 | 130 +++++++ .../bitmapblockcommit/snapshots-2-3 | 49 +++ .../bitmapblockcommit/snapshots-2-4 | 99 ++++++ .../bitmapblockcommit/snapshots-2-5 | 103 ++++++ .../bitmapblockcommit/snapshots-3-4 | 72 ++++ .../bitmapblockcommit/snapshots-3-5 | 76 +++++ .../bitmapblockcommit/snapshots-4-4 | 11 + .../bitmapblockcommit/snapshots-4-5 | 33 ++ .../snapshots-synthetic-broken-1-2 | 27 ++ .../snapshots-synthetic-broken-1-3 | 66 ++++ .../snapshots-synthetic-broken-1-4 | 73 ++++ .../snapshots-synthetic-broken-1-5 | 73 ++++ .../snapshots-synthetic-broken-2-3 | 43 +++ .../snapshots-synthetic-broken-2-4 | 50 +++ .../snapshots-synthetic-broken-2-5 | 50 +++ .../snapshots-synthetic-broken-3-4 | 27 ++ .../snapshots-synthetic-broken-3-5 | 27 ++ .../snapshots-synthetic-broken-4-5 | 20 ++ .../blockjob-blockdev-in.xml | 4 + 39 files changed, 2290 insertions(+), 43 deletions(-) create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-2 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-2-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-3 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-3-4 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-3-5 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-4-5 -- 2.24.1

2 39

[libvirt PATCH] util: ensure min/maj are initialized in virGetDeviceID
by Daniel P. Berrangé 13 Mar '20

13 Mar '20

The stub impl of virGetDeviceID just returns ENOSYS and does not initialize the min/maj output parameters. This lead to a false positive warning on mingw about possible use of uninitialized variables. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/util/virutil.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index f77ee05dbf..ddd8805101 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1407,9 +1407,10 @@ virGetDeviceID(const char *path, int *maj, int *min) #else int virGetDeviceID(const char *path G_GNUC_UNUSED, - int *maj G_GNUC_UNUSED, - int *min G_GNUC_UNUSED) + int *maj, + int *min) { + *maj = *min = 0; return -ENOSYS; } #endif -- 2.24.1

2 1