RE: join running core dump job
> -Original Message- > From: Thanos Makatos > Sent: Monday, March 4, 2024 5:24 PM > To: devel@lists.libvirt.org > Subject: join running core dump job > > Is there a way to programmatically wait for a previously initiated > virDomainCoreDumpWithFormat() where the process that started it died? I'm > looking at the API and don't seem to find anything relevant. I suppose I > could > poll via virDomainGetJobStats(), but, ideally, I'd like a function that would > join > the dump job and return when the dump job finishes. > ___ > Devel mailing list -- devel@lists.libvirt.org > To unsubscribe send an email to devel-le...@lists.libvirt.org I see there's qemuDumpWaitForCompletion(), looks promising. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 1/5] qemu.conf changes to support multiple memory backend directories
Hi Martin, Answers inline. Thanks for helping with the review and all the tips! On 3/1/24 04:00, Martin Kletzander wrote: On Mon, Jan 29, 2024 at 04:43:53PM -0500, mgal...@akamai.com wrote: From: Michael Galaxy In our case, we almost always have two NUMA nodes, so in that example, we have two PMEM regions which are created on the Linux kernel command line that get mounted into those two locations for libvirt to use. There are PMEM devices which you then expose as filesystems to use for libvirt as a backing for VM's PMEMs. Do I understand that correctly? If yes, how are these different? Can't they be passed through? So, these are very different. QEMU currently already supports passing through PMEM for guest internal use (The guest puts its own filesystem onto the passed-through PMEM device). In our case, we are using the PMEM area only in the host to place the QEMU memory backing for all guests into a single PMEM area. To support NUMA correctly, QEMU needs to support mutiple host-level PMEM areas which have been pre-configured to be NUMA aware. This is strictly for the purposes of doing live updates, not as a mechanism for guests to internally take advantage of persistent memory... that's a completely different use case (which in and of itself is very interesting, but not what we are using it for). That's how it works. Does that make sense? (I'll work on those other requests, thank you) - Michael ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 2/2] storage: fix testcases on changing the default qcow2 compat to 1.1
After changing the default qcow2 image to 1.1 from 0.10. Some of the testcases in `storagevolxml2` here need fixing. This patch changes the expected compat version in each of these files that call qemu-img. As per qemu's Qcow docs the qemu-img command gives a 1.1 compatible version image. These testcases are written for 0.10, and should be upgraded. Also there is a testcase `qcow2-1.1.argv`. Which is aimed at testing 1.1 version specifically, I think a new testcase should be made to replace it called `qcow2-0.10.argv`. This requires renaming test files and changes at many places, not sure so didn't include in the patch. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/602 Signed-off-by: Abhiram Tilak --- .../storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv| 2 +- .../qcow2-luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks.argv| 2 +- .../qcow2-nobacking-convert-prealloc-compat.argv| 2 +- .../storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv | 2 +- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocow-compat.argv| 2 +- tests/storagevolxml2argvdata/qcow2-zerocapacity.argv| 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv index 4b9ccfe8dc..705604b162 100644 --- a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /var/lib/libvirt/images/sparse-qcow2.img \ 1073741824K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-compat.argv b/tests/storagevolxml2argvdata/qcow2-compat.argv index bf3a50a7f3..40fbe065e0 100644 --- a/tests/storagevolxml2argvdata/qcow2-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-compat.argv @@ -2,6 +2,6 @@ qemu-img \ create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,compat=0.10 \ +-o backing_fmt=raw,compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv index dbc7deb16a..b68da425d9 100644 --- a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv @@ -2,6 +2,6 @@ qemu-img \ convert \ -f raw \ -O qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /dev/HostVG/Swap \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv index d89622d4a6..3068b4b38d 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv @@ -2,7 +2,7 @@ qemu-img \ create \ -f qcow2 \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ --o encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=0.10 \ +-o encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=1.1 \ /var/lib/libvirt/images/OtherDemoLuks.img \ 5242880K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv index 4d910552d0..948e9ac66d 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /var/lib/libvirt/images/sparse-qcow2.img \ 1073741824K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks.argv b/tests/storagevolxml2argvdata/qcow2-luks.argv index 308316c90c..a3be41a406 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks.argv @@ -3,6 +3,6 @@ create \ -f qcow2 \ -b /dev/null \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ --o backing_fmt=raw,encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=0.10 \ +-o backing_fmt=raw,encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=1.1 \ /var/lib/libvirt/images/OtherDemoLuks.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv index
[PATCH 1/2] storage: change assigning qcow2 compat to 1.1 from 0.10 automatically
In the file `storage/storage_util.c` currently `compat` varible is begin assigned to 0.10 by default. This patch changes this default value to 1.1. This is done in efforts to upgrade the default qcow2 image version to 1.1. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/602 Signed-off-by: Abhiram Tilak --- src/storage/storage_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 7bf815d978..28d5fce4f0 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -765,7 +765,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDef *encinfo, if (info->compat) virBufferAsprintf(, "compat=%s,", info->compat); else if (info->format == VIR_STORAGE_FILE_QCOW2) -virBufferAddLit(, "compat=0.10,"); +virBufferAddLit(, "compat=1.1,"); if (info->clusterSize > 0) virBufferAsprintf(, "cluster_size=%llu,", info->clusterSize); -- 2.42.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 0/2] storage: Upgrade default qcow2 verion to 1.1
Right now the default qcow2 version if not specified explicitly, defaults to 0.10. Certain features like live snapshots, cluster size specification require 1.1 to work. This patch series aims at upgrading the qcow2 default image version, and use compatibility version 1.1 unless specified. This also requires correcting multiple testcases related to this compat version. If any other locations require changing, they will be added as a part of this series. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/602 Abhiram Tilak (2): storage: change assigning qcow2 compat to 1.1 from 0.10 automatically storage: fix testcases on changing the default qcow2 compat to 1.1 src/storage/storage_util.c | 2 +- .../storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv| 2 +- .../qcow2-luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks.argv| 2 +- .../qcow2-nobacking-convert-prealloc-compat.argv| 2 +- .../storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv | 2 +- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocow-compat.argv| 2 +- tests/storagevolxml2argvdata/qcow2-zerocapacity.argv| 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) -- 2.42.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations
Hello Zhao, On Mon, 4 Mar 2024 at 12:19, Zhao Liu wrote: > > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; > > This indicates the default maxcpus is initialized as 0 if user doesn't > specifies it. * 'has_maxcpus' should be set only if maxcpus > 0. If maxcpus == 0, then setting 'has_maxcpus=1' seems convoluted. > However, we could initialize maxcpus as other default value, e.g., > > maxcpus = config->has_maxcpus ? config->maxcpus : 1. === hw/core/machine.c machine_initfn /* default to mc->default_cpus */ ms->smp.cpus = mc->default_cpus; ms->smp.max_cpus = mc->default_cpus; static void machine_class_base_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); mc->max_cpus = mc->max_cpus ?: 1; mc->min_cpus = mc->min_cpus ?: 1; mc->default_cpus = mc->default_cpus ?: 1; } === * Looking at the above bits, it seems smp.cpus & smp.max_cpus are initialised to 1 via default_cpus in MachineClass object. >> if (config->has_maxcpus && config->maxcpus == 0) > This check only wants to identify the case that user sets the 0. > If the default maxcpus is initialized as 0, then (maxcpus == 0) will > fail if user doesn't set maxcpus. > > But it is still necessary to distinguish whether maxcpus is user-set or > auto-initialized. * If it is set to zero(0) either by user or by auto-initialise, it is still invalid, right? > If it is user-set, -smp should fail is there's invalid maxcpus/invalid > topology. > > Otherwise, if it is auto-initialized, its value should be adjusted based > on other topology components as the above calculation in (*). * Why have such diverging ways? * Could we simplify it as - If cpus/maxcpus==0, it is invalid, show an error and exit. - If cpus/maxcpus > 0, but incorrect for topology, then re-calculate the correct value based on topology parameters. If the re-calculated value is still incorrect or unsatisfactory, then show an error and exit. * Saying that user setting cpu/maxcpus=0 is invalid and auto-initialising it to zero(0) is valid, is not consistent. ...wdyt? --- - Prasad ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] ci: refresh with lcitool manifest
On Mon, Mar 04, 2024 at 04:34:35PM +, Daniel P. Berrangé wrote: > Picks up the switch from FreeBSD 13.2 to 13.3 > > Signed-off-by: Daniel P. Berrangé > --- > ci/gitlab/builds.yml | 2 +- > ci/lcitool/targets/freebsd-13.yml | 2 -- > 2 files changed, 1 insertion(+), 3 deletions(-) > delete mode 100644 ci/lcitool/targets/freebsd-13.yml Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
join running core dump job
Is there a way to programmatically wait for a previously initiated virDomainCoreDumpWithFormat() where the process that started it died? I'm looking at the API and don't seem to find anything relevant. I suppose I could poll via virDomainGetJobStats(), but, ideally, I'd like a function that would join the dump job and return when the dump job finishes. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v3 01/12] cpu_map: update script to handle versioned CPUs
On 3/1/24 10:13, Daniel P. Berrangé wrote: On Fri, Mar 01, 2024 at 10:36:12AM -0600, Jonathon Jongsma wrote: On 3/1/24 10:13 AM, Daniel P. Berrangé wrote: On Tue, Feb 20, 2024 at 05:08:02PM -0700, Jim Fehlig wrote: On 12/15/23 15:11, Jonathon Jongsma wrote: Previously, the script only generated the parent CPU and any versions that had a defined alias. The script now generates all CPU versions. Any version that had a defined alias will continue to use that alias, but those without aliases will use the generated name $BASECPUNAME-vN. The reason for this change is two-fold. First, we need to add new models that support new features (such as SEV-SNP). To deal with this, the script now generates model definitions for all versions. But we also need to ensure that our CPU definitions are migration-safe. To deal with this issue we need to make sure we're always using the canonical versioned names for CPUs. Related to migration safety, do we need to be concerned with the expansion of 'host-model' CPU? E.g. is it possible 'host-model' expands to EPYC before introducing the new models, and EPYC-v4 afterwards? If so, what are the ramifications of that? Yes, I see that happening on my laptop in domcapabilities: Currently libvirt reports: Snowridge Intel ...snip... and after this series it reports: Snowridge-v4 Intel ...snip... That's not wrong per-se, becasue Snowrigde-v4 has a smaller delta against my host CPU. The problem is that libvirt updates the *live* XML for the guest with this expansion. IIUC, if we now attempt to live migrate to a compatible machine running older libvirt the migrate will fail as old libvirt doesn't know the -v4 CPU. Downstream, we (SUSE) don't really support migrating from new -> old. Is this something we aim to support upstream? I'm not sure how to address this ? But don't we have this issue any time we add a new CPU model to libvirt? Anytime there's a new model, it has the potential to be a closer match to the host CPU than an existing model definition was. As I mentioned in my previous reply, when e.g. the -noTSX CPU variants were added, didn't the same sort of thing (potentially) happen? Or am I doing something meaningfully different in this patch set than what happens in those scenarios? I think it probably /did/ happen, but that doesn't make it acceptable. The noTSX stuff was the cause of massive amounts of compatibility pain for mgmt apps, so the incompatibility in libvirt might have been glossed over. We're adding alot of new versions here, so the possibly increasing the visibility/impact of this libvirt change. It can happen when we introduce an entirely new CPU model too. E.g. on a Genoa machine, prior to commit bfe53e9145c, host model expanded to EPYC-Milan AMD After commit bfe53e9145c EPYC-Genoa AMD Regards, Jim ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] qemu: Optimize CPU check='partial' for usable CPUs
Ideally check='partial' would check exactly the features QEMU would want to enable when asked for a specific CPU model (and features). But there is no way we could ask QEMU how a specific CPU would look like. So we use our definition from CPU map, which may slightly differ as QEMU adds or removes features from CPU models, and thus we may end up checking features which QEMU would not enable while missing some required ones. We can do better in specific cases, though. If a CPU definition uses only a model and disabled features (or none at all), we already know whether QEMU can enable all features required by the CPU model as that's what we use to set usable='yes' attribute in the list of available CPU models in domain capbilities XML. So when a usable CPU model is requested without asking for additional features (disabling features is fine) we can avoid our possible inaccurate check using our CPU map. For backward compatibility we only consider usable models. If a specified model is not usable, we still check it the old way and even let QEMU start it (and disable some features) in case our definition lacks some features compared to QEMU. Signed-off-by: Jiri Denemark --- src/qemu/qemu_capabilities.c | 44 src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_process.c | 1 + 3 files changed, 48 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e383d85920..cb9846dfcf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2433,6 +2433,50 @@ virQEMUCapsIsCPUDeprecated(virQEMUCaps *qemuCaps, } +/** + * virQEMUCapsIsCPUUsable: + * @qemuCaps: QEMU capabilities + * @type: virtualization type (kvm vs tcg) + * @cpu: CPU definition to check, including explicitly configured features + * + * Checks whether @cpu is considered usable by QEMU, i.e., all features + * required by the CPU model are supported and can be enabled. If so, we can + * avoid checking the CPU according to its definition in the CPU map when + * starting a domain with check='partial'. Our checks could be inaccurate + * anyway because QEMU might have changed the definition of the CPU model + * since we added it into the CPU map. + * + * Returns true iff @cpu is usable. + */ +bool +virQEMUCapsIsCPUUsable(virQEMUCaps *qemuCaps, + virDomainVirtType type, + virCPUDef *cpu) +{ +qemuMonitorCPUDefs *defs; +size_t i; + +if (!cpu->model || +!(defs = virQEMUCapsGetAccel(qemuCaps, type)->cpuModels)) +return false; + +/* CPU model usability is valid only when CPU def does not contain any + * features or all features are disabled. + */ +for (i = 0; i < cpu->nfeatures; i++) { +if (cpu->features[i].policy != VIR_CPU_FEATURE_DISABLE) +return false; +} + +for (i = 0; i < defs->ncpus; i++) { +if (STREQ(defs->cpus[i].name, cpu->model)) +return defs->cpus[i].usable == VIR_DOMCAPS_CPU_USABLE_YES; +} + +return false; +} + + bool virQEMUCapsIsMachineDeprecated(virQEMUCaps *qemuCaps, virDomainVirtType type, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 486a4a6f63..ef1ad2c01c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -770,6 +770,9 @@ const char *virQEMUCapsGetMachineDefaultCPU(virQEMUCaps *qemuCaps, bool virQEMUCapsIsCPUDeprecated(virQEMUCaps *qemuCaps, virDomainVirtType type, const char *model); +bool virQEMUCapsIsCPUUsable(virQEMUCaps *qemuCaps, +virDomainVirtType type, +virCPUDef *cpu); bool virQEMUCapsIsMachineDeprecated(virQEMUCaps *qemuCaps, virDomainVirtType type, const char *machine); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6e51d6586b..8e35d0a73f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6288,6 +6288,7 @@ qemuProcessUpdateGuestCPU(virDomainDef *def, g_autoptr(virDomainCapsCPUModels) cpuModels = NULL; if (def->cpu->check == VIR_CPU_CHECK_PARTIAL && +!virQEMUCapsIsCPUUsable(qemuCaps, def->virtType, def->cpu) && virCPUCompare(hostarch, virQEMUCapsGetHostModel(qemuCaps, def->virtType, VIR_QEMU_CAPS_HOST_CPU_FULL), -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] cpu: x86: Check for invalid CPU data from hypervisor
Recently a kernel bug caused QEMU to report a CPU feature as enabled while listing it in the "unavailable-features" list of features that were requested, but could not be enabled. The feature was actually enabled, but we marked it as disabled when starting a domain. Later when the domain is migrated, the destination requests the feature to be disabled, which breaks the guest ABI or if we are lucky QEMU just fails to load the migration stream. Let's make similar bugs more visible in the future by refusing to even start the domain. Signed-off-by: Jiri Denemark --- src/cpu/cpu_x86.c | 8 1 file changed, 8 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 6b2531b360..e8409ce616 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -3019,6 +3019,14 @@ virCPUx86UpdateLive(virCPUDef *cpu, x86DataIsSubset(>data, >data)) expected = VIR_CPU_FEATURE_DISABLE; +if (x86DataIsSubset(, >data) && +x86DataIsSubset(, >data)) { +virReportError(VIR_ERR_OPERATION_FAILED, + _("hypervisor provided conflicting CPU data: feature '%1$s' is both enabled and disabled at the same time"), + feature->name); +return -1; +} + if (expected == VIR_CPU_FEATURE_DISABLE && x86DataIsSubset(, >data)) { VIR_DEBUG("Feature '%s' enabled by the hypervisor", feature->name); -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] ci: refresh with lcitool manifest
Picks up the switch from FreeBSD 13.2 to 13.3 Signed-off-by: Daniel P. Berrangé --- ci/gitlab/builds.yml | 2 +- ci/lcitool/targets/freebsd-13.yml | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 ci/lcitool/targets/freebsd-13.yml diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index 49a0b27029..b23a3989b9 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -619,7 +619,7 @@ x86_64-freebsd-13: needs: [] allow_failure: false variables: -CIRRUS_VM_IMAGE_NAME: freebsd-13-2 +CIRRUS_VM_IMAGE_NAME: freebsd-13-3 CIRRUS_VM_IMAGE_SELECTOR: image_family CIRRUS_VM_INSTANCE_TYPE: freebsd_instance INSTALL_COMMAND: pkg install -y diff --git a/ci/lcitool/targets/freebsd-13.yml b/ci/lcitool/targets/freebsd-13.yml deleted file mode 100644 index 0337926479..00 --- a/ci/lcitool/targets/freebsd-13.yml +++ /dev/null @@ -1,2 +0,0 @@ -cirrus: - image_name: freebsd-13-2 -- 2.43.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations
Hi Prasad, On Mon, Mar 04, 2024 at 11:23:58AM +0530, Prasad Pandit wrote: > Date: Mon, 4 Mar 2024 11:23:58 +0530 > From: Prasad Pandit > Subject: Re: [PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" > SMP configurations > > On Mon, 4 Mar 2024 at 10:02, Zhao Liu wrote: > > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c > > index 25019c91ee36..96533886b14e 100644 > > --- a/hw/core/machine-smp.c > > +++ b/hw/core/machine-smp.c > > @@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms, > > (config->has_cores && config->cores == 0) || > > (config->has_threads && config->threads == 0) || > > (config->has_maxcpus && config->maxcpus == 0)) { > > -warn_report("Deprecated CPU topology (considered invalid): " > > -"CPU topology parameters must be greater than zero"); > > +error_setg(errp, "Invalid CPU topology: " > > + "CPU topology parameters must be greater than zero"); > > +return; > > } > > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; This indicates the default maxcpus is initialized as 0 if user doesn't specifies it. For this case - no user configuration - maxcpus will be re-calculated as: maxcpus = maxcpus > 0 ? maxcpus : drawers * books * sockets * dies * clusters * cores * threads; (*) > ... > if (config->has_maxcpus && config->maxcpus == 0) This check only wants to identify the case that user sets the 0. > > * The check (has_maxcpus && maxcpus == 0) seems to be repeating above, > maybe we could check if (maxcpus == 0) error_setg(). If the default maxcpus is initialized as 0, then (maxcpus == 0) will fail if user doesn't set maxcpus. However, we could initialize maxcpus as other default value, e.g., maxcpus = config->has_maxcpus ? config->maxcpus : 1. But it is still necessary to distinguish whether maxcpus is user-set or auto-initialized. If it is user-set, -smp should fail is there's invalid maxcpus/invalid topology. Otherwise, if it is auto-initialized, its value should be adjusted based on other topology components as the above calculation in (*). > And same for > other topology parameters? Other parameters also have the similar needs to distinguish if they're set by user. So the check needs to also cover has_* fields. > * Also a check to ensure cpus <= maxcpus is required I think. > Yes, the valid topology needs this. This code block already covers this case ;-): if (maxcpus < cpus) { g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); error_setg(errp, "Invalid CPU topology: " "maxcpus must be equal to or greater than smp: " "%s == maxcpus (%u) < smp_cpus (%u)", topo_msg, maxcpus, cpus); return; } Thanks, Zhao ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations
On Mon, 4 Mar 2024 at 10:02, Zhao Liu wrote: > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c > index 25019c91ee36..96533886b14e 100644 > --- a/hw/core/machine-smp.c > +++ b/hw/core/machine-smp.c > @@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms, > (config->has_cores && config->cores == 0) || > (config->has_threads && config->threads == 0) || > (config->has_maxcpus && config->maxcpus == 0)) { > -warn_report("Deprecated CPU topology (considered invalid): " > -"CPU topology parameters must be greater than zero"); > +error_setg(errp, "Invalid CPU topology: " > + "CPU topology parameters must be greater than zero"); > +return; > } unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; ... if (config->has_maxcpus && config->maxcpus == 0) * The check (has_maxcpus && maxcpus == 0) seems to be repeating above, maybe we could check if (maxcpus == 0) error_setg(). And same for other topology parameters? * Also a check to ensure cpus <= maxcpus is required I think. Thank you. --- - Prasad ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations
From: Zhao Liu The "parameter=0" SMP configurations have been marked as deprecated since v6.2. For these cases, -smp currently returns the warning and adjusts the zeroed parameters to 1 by default. Remove the above compatibility logic in v9.0, and return error directly if any -smp parameter is set as 0. Signed-off-by: Zhao Liu --- docs/about/deprecated.rst | 16 docs/about/removed-features.rst | 15 +++ hw/core/machine-smp.c | 5 +++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 36bd3e15ef06..872974640252 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -36,22 +36,6 @@ and will cause a warning. The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on`` rather than ``delay=off``. -``-smp`` ("parameter=0" SMP configurations) (since 6.2) -''' - -Specified CPU topology parameters must be greater than zero. - -In the SMP configuration, users should either provide a CPU topology -parameter with a reasonable value (greater than zero) or just omit it -and QEMU will compute the missing value. - -However, historically it was implicitly allowed for users to provide -a parameter with zero value, which is meaningless and could also possibly -cause unexpected results in the -smp parsing. So support for this kind of -configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will -be removed in the near future, users have to ensure that all the topology -members described with -smp are greater than zero. - Plugin argument passing through ``arg=`` (since 6.1) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 417a0e4fa1d9..f9cf874f7b1f 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -489,6 +489,21 @@ The ``-singlestep`` option has been turned into an accelerator property, and given a name that better reflects what it actually does. Use ``-accel tcg,one-insn-per-tb=on`` instead. +``-smp`` ("parameter=0" SMP configurations) (removed in 9.0) + + +Specified CPU topology parameters must be greater than zero. + +In the SMP configuration, users should either provide a CPU topology +parameter with a reasonable value (greater than zero) or just omit it +and QEMU will compute the missing value. + +However, historically it was implicitly allowed for users to provide +a parameter with zero value, which is meaningless and could also possibly +cause unexpected results in the -smp parsing. So support for this kind of +configurations (e.g. -smp 8,sockets=0) is removed since 9.0, users have +to ensure that all the topology members described with -smp are greater +than zero. User-mode emulator command line arguments - diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 25019c91ee36..96533886b14e 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms, (config->has_cores && config->cores == 0) || (config->has_threads && config->threads == 0) || (config->has_maxcpus && config->maxcpus == 0)) { -warn_report("Deprecated CPU topology (considered invalid): " -"CPU topology parameters must be greater than zero"); +error_setg(errp, "Invalid CPU topology: " + "CPU topology parameters must be greater than zero"); +return; } /* -- 2.34.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org