February 2021 - Devel - libvirt List Archives

[PATCH 0/2] backup: Don't try to update stats if VM isn't alive
by Peter Krempa 01 Mar '21

01 Mar '21

Doing so results in a crash. Peter Krempa (2): qemuBackupJobTerminate: Move cleanup of temp files earlier qemuBackupJobTerminate: Don't calculate backup job stats if VM isn't active src/qemu/qemu_backup.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) -- 2.29.2

2 3

[PATCH 0/4] Storage volume (crash) fixes
by Peter Krempa 01 Mar '21

01 Mar '21

Patch 1/4 should go into this release, the rest can wait. Peter Krempa (4): storageBackendProbeTarget: Check return value of virStorageSourceNewFromBacking storageBackendProbeTarget: Don't fail if backing store can't be parsed virStorageVolDefFormat: Extract formatting of source extents virStorageVolDefFormat: Don't format empty <source> src/conf/storage_conf.c | 59 +++++++++++-------- src/storage/storage_util.c | 5 +- .../storagevolxml2xmlout/vol-file-backing.xml | 2 - tests/storagevolxml2xmlout/vol-file-iso.xml | 2 - .../storagevolxml2xmlout/vol-file-naming.xml | 2 - tests/storagevolxml2xmlout/vol-file.xml | 2 - .../vol-gluster-dir-neg-uid.xml | 2 - .../storagevolxml2xmlout/vol-gluster-dir.xml | 2 - .../vol-logical-backing.xml | 2 - tests/storagevolxml2xmlout/vol-logical.xml | 2 - .../storagevolxml2xmlout/vol-luks-cipher.xml | 2 - tests/storagevolxml2xmlout/vol-luks.xml | 2 - tests/storagevolxml2xmlout/vol-partition.xml | 2 - .../vol-qcow2-0.10-lazy.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 - .../vol-qcow2-encryption.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-luks.xml | 2 - .../vol-qcow2-nobacking.xml | 2 - .../vol-qcow2-nocapacity.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2.xml | 2 - tests/storagevolxml2xmlout/vol-sheepdog.xml | 2 - 22 files changed, 35 insertions(+), 69 deletions(-) -- 2.29.2

2 6

[PATCH] build-aux: increase tests timeout
by Roman Bogorodskiy 01 Mar '21

01 Mar '21

Meson default timeout for test() is 30 seconds. This may be not enough for some tests like sc_prohibit_nonreentrant or sc_libvirt_unmarked_diagnostics, so set it to 60 seconds. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- On my system these two tests always timeout with the default value. That's what I have after increasing timeout: 157/288 libvirt:syntax-check / sc_prohibit_nonreentrant OK 52.18s 183/288 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK 31.48s build-aux/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/build-aux/meson.build b/build-aux/meson.build index c506feefd2..4acd5e16c1 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -44,6 +44,7 @@ if git potfiles_dep, ], suite: 'syntax-check', + timeout: 60, ) endforeach endif -- 2.30.0

3 5

Entering freeze for libvirt-7.1.0
by Jiri Denemark 25 Feb '21

25 Feb '21

I have just tagged v7.1.0-rc1 in the repository and pushed signed tarballs and source RPMs to https://libvirt.org/sources/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka

1 1

[libvirt PATCH] qemu_domainjob: Make copy of owner API
by Jiri Denemark 25 Feb '21

25 Feb '21

Using the job owner API name directly works fine as long as it is a static string or the owner's thread is still running. However, this is not always the case. For example, when the owner API name is filled in a job when we're reconnecting to existing domains after daemon restart, the dynamically allocated owner name will disappear with the reconnecting thread. Any follow up usage of the pointer will read random memory. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_domainjob.c | 12 ++++++------ src/qemu/qemu_process.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 3c2c6b9179..b58d6837ad 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -190,7 +190,7 @@ qemuDomainObjResetJob(qemuDomainJobObjPtr job) { job->active = QEMU_JOB_NONE; job->owner = 0; - job->ownerAPI = NULL; + g_clear_pointer(&job->ownerAPI, g_free); job->started = 0; } @@ -200,7 +200,7 @@ qemuDomainObjResetAgentJob(qemuDomainJobObjPtr job) { job->agentActive = QEMU_AGENT_JOB_NONE; job->agentOwner = 0; - job->agentOwnerAPI = NULL; + g_clear_pointer(&job->agentOwnerAPI, g_free); job->agentStarted = 0; } @@ -210,7 +210,7 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObjPtr job) { job->asyncJob = QEMU_ASYNC_JOB_NONE; job->asyncOwner = 0; - job->asyncOwnerAPI = NULL; + g_clear_pointer(&job->asyncOwnerAPI, g_free); job->asyncStarted = 0; job->phase = 0; job->mask = QEMU_JOB_DEFAULT_MASK; @@ -890,7 +890,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, obj, obj->def->name); priv->job.active = job; priv->job.owner = virThreadSelfID(); - priv->job.ownerAPI = virThreadJobGet(); + priv->job.ownerAPI = g_strdup(virThreadJobGet()); priv->job.started = now; } else { VIR_DEBUG("Started async job: %s (vm=%p name=%s)", @@ -901,7 +901,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; priv->job.asyncJob = asyncJob; priv->job.asyncOwner = virThreadSelfID(); - priv->job.asyncOwnerAPI = virThreadJobGet(); + priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet()); priv->job.asyncStarted = now; priv->job.current->started = now; } @@ -917,7 +917,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); priv->job.agentActive = agentJob; priv->job.agentOwner = virThreadSelfID(); - priv->job.agentOwnerAPI = virThreadJobGet(); + priv->job.agentOwnerAPI = g_strdup(virThreadJobGet()); priv->job.agentStarted = now; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..398f63282e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3732,7 +3732,7 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, /* Restore the config of the async job which is not persisted */ priv->jobs_queued++; priv->job.asyncJob = QEMU_ASYNC_JOB_BACKUP; - priv->job.asyncOwnerAPI = virThreadJobGet(); + priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet()); priv->job.asyncStarted = now; qemuDomainObjSetAsyncJobMask(vm, (QEMU_JOB_DEFAULT_MASK | -- 2.30.0

2 1

[libvirt PATCH] qemuProcessUpdateGuestCPU: Check cpu if check=full is set
by Tim Wiederhake 25 Feb '21

25 Feb '21

libvirt performs cpu checking if "check" is set to "partial", but skips checking the cpu if "check" is set to "full". See https://bugzilla.redhat.com/show_bug.cgi?id=1840770 Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..5b8c1397ef 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6149,6 +6149,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUConvertLegacy(hostarch, def->cpu) < 0) return -1; + if (def->cpu->check == VIR_CPU_CHECK_FULL) { + virCPUDefPtr host = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_FULL); + + if (virCPUCompare(hostarch, host, def->cpu, true) < 0) + return -1; + } + /* nothing to update for host-passthrough / maximum */ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) { -- 2.26.2

2 1

[PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF
by Peng Liang 25 Feb '21

25 Feb '21

qemuMonitorUnregister will be called in multiple threads (e.g. threads in rpc worker pool and the vm event thread). In some cases, it isn't protected by the monitor lock, which may lead to call g_source_unref more than one time and a use-after-free problem eventually. Add the missing lock in qemuProcessHandleMonitorEOF (which is the only position missing lock of monitor I found). Suggested-by: Michal Privoznik <mprivozn(a)redhat.com> Signed-off-by: Peng Liang <liangpeng10(a)huawei.com> --- This patch is v2 of https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html. v1 -> v2: Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic function in qemuMonitorUnregister. src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d25c26343a7f..14e6b1fe9626 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void); void qemuMonitorRegister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); +/* Must be called with monitor locked. */ void qemuMonitorUnregister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); void qemuMonitorClose(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d930ff9a74f6..bfa742577f32 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -318,7 +318,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon, /* We don't want this EOF handler to be called over and over while the * thread is waiting for a job. */ + virObjectLock(mon); qemuMonitorUnregister(mon); + virObjectUnlock(mon); /* We don't want any cleanup from EOF handler (or any other * thread) to enter qemu namespace. */ -- 2.29.2

2 3

[PATCH 0/3] Use g_autoptr and g_autofree
by Kristina Hanicova 25 Feb '21

25 Feb '21

This uses g_autofree (if possible) in file networkcommon_conf and g_autoptr instead of virNetDevIPRouteFree where possible. Kristina Hanicova (3): networkcommon_conf: Use g_autofree where possible Use g_autoptr instead of virNetDevIPRouteFree if possible Remove redundant variables/labels src/conf/domain_conf.c | 3 +- src/conf/networkcommon_conf.c | 74 +++++++++++++++-------------------- src/lxc/lxc_native.c | 10 ++--- src/vz/vz_sdk.c | 3 +- 4 files changed, 36 insertions(+), 54 deletions(-) -- 2.29.2

3 6

[PATCH] qemu_monitor: Document qemuMonitorUnregister()
by Michal Privoznik 24 Feb '21

24 Feb '21

The most important bit is that the caller is expected to pass locked monitor. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_monitor.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ed35da17e1..73f337a6be 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -850,6 +850,13 @@ qemuMonitorRegister(qemuMonitorPtr mon) } +/** + * qemuMonitorUnregister: + * @mon: monitor object + * + * Unregister monitor from the event loop. The monitor object + * must be locked before calling this function. + */ void qemuMonitorUnregister(qemuMonitorPtr mon) { -- 2.26.2

2 1

[PATCH v2 4/4] utils: Deprecate inexact fractional suffix sizes
by Eric Blake 24 Feb '21

24 Feb '21

The value '1.1k' is inexact; 1126.4 bytes is not possible, so we happen to truncate it to 1126. Our use of fractional sizes is intended for convenience, but when a user specifies a fraction that is not a clean translation to binary, truncating/rounding behind their backs can cause confusion. Better is to deprecate inexact values, which still leaves '1.5k' as valid, but alerts the user to spell out their values as a precise byte number in cases where they are currently being rounded. Note that values like '0.1G' in the testsuite need adjustment as a result. Since qemu_strtosz() does not have an Err** parameter, and plumbing that in would be a much larger task, we instead go with just directly emitting the deprecation warning to stderr. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm not a fan of this patch, but am proposing it for discussion purposes. --- docs/system/deprecated.rst | 9 +++++++++ tests/test-cutils.c | 6 +++--- tests/test-keyval.c | 4 ++-- tests/test-qemu-opts.c | 4 ++-- util/cutils.c | 9 +++++++-- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 113c2e933f1b..2c9cb849eec5 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -154,6 +154,15 @@ Input parameters that take a size value should only use a size suffix the value is hexadecimal. That is, '0x20M' is deprecated, and should be written either as '32M' or as '0x2000000'. +inexact sizes via scaled fractions (since 6.0) +'''''''''''''''''''''''''''''''''''''''''''''' + +Input parameters that take a size value should only use a fractional +size (such as '1.5M') that will result in an exact byte value. The +use of inexact values (such as '1.1M') that require truncation or +rounding is deprecated, and you should instead consider writing your +unusual size in bytes (here, '1153433' or '1153434' as desired). + QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/tests/test-cutils.c b/tests/test-cutils.c index bad3a6099389..c6c33866277b 100644 --- a/tests/test-cutils.c +++ b/tests/test-cutils.c @@ -2124,11 +2124,11 @@ static void test_qemu_strtosz_float(void) g_assert_cmpint(res, ==, 1024); g_assert(endptr == str + 3); - /* For convenience, we permit values that are not byte-exact */ - str = "12.345M"; + /* Fractional values should still be byte-exact */ + str = "12.125M"; err = qemu_strtosz(str, &endptr, &res); g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB)); + g_assert_cmpint(res, ==, (uint64_t) (12.125 * MiB)); g_assert(endptr == str + 7); } diff --git a/tests/test-keyval.c b/tests/test-keyval.c index e20c07cf3ea5..7a45c22942e6 100644 --- a/tests/test-keyval.c +++ b/tests/test-keyval.c @@ -525,7 +525,7 @@ static void test_keyval_visit_size(void) visit_free(v); /* Suffixes */ - qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.1G,sz5=16777215T", + qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.125G,sz5=16777215T", NULL, NULL, &error_abort); v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); qobject_unref(qdict); @@ -537,7 +537,7 @@ static void test_keyval_visit_size(void) visit_type_size(v, "sz3", &sz, &error_abort); g_assert_cmphex(sz, ==, 2 * MiB); visit_type_size(v, "sz4", &sz, &error_abort); - g_assert_cmphex(sz, ==, GiB / 10); + g_assert_cmphex(sz, ==, GiB / 8); visit_type_size(v, "sz5", &sz, &error_abort); g_assert_cmphex(sz, ==, 16777215ULL * TiB); visit_check_struct(v, &error_abort); diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 6568e31a7229..549e994938fe 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -720,10 +720,10 @@ static void test_opts_parse_size(void) g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, 8); g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 1536); g_assert_cmphex(qemu_opt_get_size(opts, "size3", 0), ==, 2 * MiB); - opts = qemu_opts_parse(&opts_list_02, "size1=0.1G,size2=16777215T", + opts = qemu_opts_parse(&opts_list_02, "size1=0.125G,size2=16777215T", false, &error_abort); g_assert_cmpuint(opts_count(opts), ==, 2); - g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 10); + g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 8); g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215ULL * TiB); /* Beyond limit with suffix */ diff --git a/util/cutils.c b/util/cutils.c index 6a8a175e0d71..1154b9de131a 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -246,12 +246,13 @@ static int64_t suffix_mul(char suffix, int64_t unit) * The size parsing supports the following syntaxes * - 12345 - decimal, scale determined by @default_suffix and @unit * - 12345{bBkKmMgGtTpPeE} - decimal, scale determined by suffix and @unit - * - 12345.678{kKmMgGtTpPeE} - decimal, scale determined by suffix, and - * fractional portion is truncated to byte + * - 12345.678{kKmMgGtTpPeE} - decimal, scale determined by suffix, if + * fractional portion is exact * - 0x7fEE - hexadecimal, unit determined by @default_suffix * * The following cause a deprecation warning, and may be removed in the future * - 0xabc{kKmMgGtTpP} - hex with scaling suffix + * - 12345.678{kKmMgGtTpPeE} - decimal with inexact fraction that caused truncation * * The following are intentionally not supported * - octal, such as 08 @@ -342,6 +343,10 @@ static int do_strtosz(const char *nptr, const char **end, retval = -ERANGE; goto out; } + if (mul_required && fraction * mul != (uint64_t) (fraction * mul)) { + warn_report("Using a fractional size that is not an exact byte " + "multiple is deprecated: %s", nptr); + } *result = val * mul + (uint64_t) (fraction * mul); retval = 0; -- 2.30.1

2 2