On Wed, Apr 17, 2024 at 1:54 AM Jiri Denemark <jdene...@redhat.com> wrote:
> Ages ago origCPU in domain private data was introduced to provide > backward compatibility when migrating to an old libvirt, which did not > support fetching updated CPU definition from QEMU. Thus origCPU will > contain the original CPU definition before such update. But only if the > update actually changed anything. Let's always fill origCPU with the > original definition when starting a domain so that we can rely on it > being always set, even if it matches the updated definition. > > This fixes migration or save operations with custom domain XML after > commit v10.1.0-88-g14d3517410, which expected origCPU to be always set > to the CPU definition from inactive XML to check features explicitly > requested by a user. > > https://issues.redhat.com/browse/RHEL-30622 > > Signed-off-by: Jiri Denemark <jdene...@redhat.com> > --- > src/qemu/qemu_domain.c | 38 +++++++++++++++++++++----------------- > src/qemu/qemu_process.c | 14 ++------------ > 2 files changed, 23 insertions(+), 29 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 6b869797a8..ca50d435d2 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -10995,12 +10995,13 @@ virSaveCookieCallbacks > virQEMUDriverDomainSaveCookie = { > * @vm: domain which is being started > * @cpu: CPU updated when the domain was running previously (before > migration, > * snapshot, or save) > - * @origCPU: where to store the original CPU from vm->def in case @cpu was > - * used instead > + * @origCPU: where to store the original CPU from vm->def > * > - * Replace the CPU definition with the updated one when QEMU is new > enough to > - * allow us to check extra features it is about to enable or disable when > - * starting a domain. The original CPU is stored in @origCPU. > + * Save the original CPU definition from inactive XML in @origCPU so that > we > + * can safely update it and still be able to check what exactly a user > asked > + * for. The domain definition will either contain a copy of the original > CPU > + * definition or a copy of @cpu in case the domain was already running and > + * we're just restoring a saved state or preparing for incoming migration. > * > * Returns 0 on success, -1 on error. > */ > @@ -11013,15 +11014,25 @@ qemuDomainUpdateCPU(virDomainObj *vm, > > *origCPU = NULL; > > - if (!cpu || !vm->def->cpu || > - !virQEMUCapsGet(priv->qemuCaps, > QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) || > - virCPUDefIsEqual(vm->def->cpu, cpu, false)) > + if (!vm->def->cpu) > return 0; > > - if (!(cpu = virCPUDefCopy(cpu))) > + if (!virQEMUCapsGet(priv->qemuCaps, > QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) > + return 0; > + > + /* nothing to do if only topology part of CPU def is used */ > + if (vm->def->cpu->mode == VIR_CPU_MODE_CUSTOM && !vm->def->cpu->model) > + return 0; > + > + if (cpu) > + cpu = virCPUDefCopy(cpu); > + else > + cpu = virCPUDefCopy(vm->def->cpu); > + > + if (!cpu) > return -1; > > - VIR_DEBUG("Replacing CPU def with the updated one"); > + VIR_DEBUG("Replacing CPU definition"); > > *origCPU = vm->def->cpu; > vm->def->cpu = cpu; > @@ -11064,13 +11075,6 @@ qemuDomainFixupCPUs(virDomainObj *vm, > !vm->def->cpu->model) > return 0; > > - /* Missing origCPU means QEMU created exactly the same virtual CPU > which > - * we asked for or libvirt was too old to mess up the translation from > - * host-model. > - */ > - if (!*origCPU) > - return 0; > - > if (virCPUDefFindFeature(vm->def->cpu, "cmt")) { > g_autoptr(virCPUDef) fixedCPU = > virCPUDefCopyWithoutModel(vm->def->cpu); > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index e4bcb628cf..8165c28dbc 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -4437,8 +4437,6 @@ qemuProcessUpdateLiveGuestCPU(virDomainObj *vm, > virCPUData *disabled) > { > virDomainDef *def = vm->def; > - qemuDomainObjPrivate *priv = vm->privateData; > - g_autoptr(virCPUDef) orig = NULL; > int rc; > > if (!enabled) > @@ -4449,19 +4447,11 @@ qemuProcessUpdateLiveGuestCPU(virDomainObj *vm, > !def->cpu->model)) > return 0; > > - orig = virCPUDefCopy(def->cpu); > - > - if ((rc = virCPUUpdateLive(def->os.arch, def->cpu, enabled, > disabled)) < 0) { > + if ((rc = virCPUUpdateLive(def->os.arch, def->cpu, enabled, > disabled)) < 0) > return -1; > - } else if (rc == 0) { > - /* Store the original CPU in priv if QEMU changed it and we didn't > - * get the original CPU via migration, restore, or snapshot > revert. > - */ > - if (!priv->origCPU && !virCPUDefIsEqual(def->cpu, orig, false)) > - priv->origCPU = g_steal_pointer(&orig); > > + if (rc == 0) > def->cpu->check = VIR_CPU_CHECK_FULL; > - } > > return 0; > } > -- > 2.44.0 > _______________________________________________ > Devel mailing list -- devel@lists.libvirt.org > To unsubscribe send an email to devel-le...@lists.libvirt.org > Test pass with this patched to v10.2.0-96-gded74b3369 -- Tested-by: Han Han <h...@redhat.com>
_______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
