Entering freeze for libvirt-10.6.0

2024-07-29 Thread Jiri Denemark
I have just tagged v10.6.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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


Re: [PATCH 0/5] virsysinfo: Be more forgiving when decoding OEM strings

2024-07-25 Thread Jiri Denemark
On Thu, Jul 18, 2024 at 16:08:02 +0200, Michal Privoznik wrote:
> See 4/5 for explanation.
> 
> Michal Prívozník (5):
>   vircommand: Initialize dryRunStatus to portable EXIT_SUCCESS instead
> of 0
>   virsysinfo: Trim newline when decoding OEM strings
>   tests: Add HPE Apollo test case to sysinfotest
>   virsysinfo: Be more forgiving when decoding OEM strings
>   virsysinfo: Calculate OEM string index better
> 
>  src/util/vircommand.c |   2 +-
>  src/util/virsysinfo.c |  36 +++-
>  .../aarch64-hpe-apollosysinfo.data| 162 ++
>  .../aarch64-hpe-apollosysinfo.expect  |  88 ++
>  tests/sysinfotest.c   |   9 +-
>  5 files changed, 285 insertions(+), 12 deletions(-)
>  create mode 100644 tests/sysinfodata/aarch64-hpe-apollosysinfo.data
>  create mode 100644 tests/sysinfodata/aarch64-hpe-apollosysinfo.expect

Reviewed-by: Jiri Denemark 


Re: [PATCH 4/5] virsysinfo: Be more forgiving when decoding OEM strings

2024-07-25 Thread Jiri Denemark
On Thu, Jul 18, 2024 at 16:08:06 +0200, Michal Privoznik wrote:
...
> diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
> index cdc2a7d87b..1fd8261dc1 100644
> --- a/src/util/virsysinfo.c
> +++ b/src/util/virsysinfo.c
> @@ -910,9 +910,20 @@ virSysinfoDMIDecodeOEMString(size_t i,
>  
>  /* Unfortunately, dmidecode returns 0 even if OEM String index is out
>   * of bounds, but it prints an error message in that case. Check stderr
> - * and return success/failure accordingly. */
> + * and return success/failure accordingly.
> + * To make matters worse, if there are two or more 'OEM String'
> + * sections then:
> + *
> + * a) we have no way of distinguishing them as dmidecode prints
> + *strings from all sections,
> + * b) if one section contains a valid string, but the other doesn't,
> + *then stdout contains the valid string and stderr contains the
> + *error "No OEM string number X*.
> + *
> + * Let's just hope there is not many systems like that.
> + */
>  
> -if (err && *err != '\0')
> +if (!(*str && **str != '\0') && err && *err != '\0')

I think

if ((!*str || **str == '\0') && err && *err != '\0')

might be a bit easier for humans to parse. At least for me it is easier
:-)

Jirka


Re: [PATCH v2] virt-host-validate: Allow longer list of CPU flags

2024-07-23 Thread Jiri Denemark
On Tue, Jul 23, 2024 at 13:54:33 +0200, Michal Privoznik wrote:
> On various occasions, virt-host-validate parses /proc/cpuinfo to
> learn about CPU flags (see virHostValidateGetCPUFlags()). It does
> so, by reading the file line by line until the line with CPU
> flags is reached. Then the line is split into individual flags
> (using space as a delimiter) and the list of flags is then
> iterated over.
> 
> This works, except for cases when the line with CPU flags is too
> long. Problem is - the line is capped at 1024 bytes and on newer
> CPUs (and newer kernels), the line can be significantly longer.
> I've seen a line that's ~1200 characters long (with 164 flags
> reported).
> 
> Switch to unbounded read from the file (getline()).
> 
> Resolves: https://issues.redhat.com/browse/RHEL-39969
> Signed-off-by: Michal Privoznik 
> ---
> 
> v2 of:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/LK74PN7JIDVKY5MZC4664RZTMOG7F5AR/
> 
> diff to v2:
> - Keep trimming of the optional newline
> 
>  tools/virt-host-validate-common.c | 18 +++---
>  1 file changed, 7 insertions(+), 11 deletions(-)

Reviewed-by: Jiri Denemark 


Re: [PATCH] virt-host-validate: Allow longer list of CPU flags

2024-07-23 Thread Jiri Denemark
On Tue, Jul 23, 2024 at 12:41:31 +0200, Michal Privoznik wrote:
> On various occasions, virt-host-validate parses /proc/cpuinfo to
> learn about CPU flags (see virHostValidateGetCPUFlags()). It does
> so, by reading the file line by line until the line with CPU
> flags is reached. Then the line is split into individual flags
> (using space as a delimiter) and the list of flags is then
> iterated over.
> 
> This works, except for cases when the line with CPU flags is too
> long. Problem is - the line is capped at 1024 bytes and on newer
> CPUs (and newer kernels), the line can be significantly longer.
> I've seen a line that's ~1200 characters long (with 164 flags
> reported).
> 
> Switch to unbounded read from the file (getline()).
> 
> Resolves: https://issues.redhat.com/browse/RHEL-39969
> Signed-off-by: Michal Privoznik 
> ---
>  tools/virt-host-validate-common.c | 16 
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/virt-host-validate-common.c 
> b/tools/virt-host-validate-common.c
> index 591143c24d..e5fa6606d2 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c
> @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void)
>  {
>  FILE *fp;
>  virBitmap *flags = NULL;
> +g_autofree char *line = NULL;
> +size_t linelen = 0;
>  
>  if (!(fp = fopen("/proc/cpuinfo", "r")))
>  return NULL;
>  
>  flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST);
>  
> -do {
> -char line[1024];
> +while (getline(, , fp) > 0) {
>  char *start;
>  g_auto(GStrv) tokens = NULL;
>  GStrv next;
>  
> -if (!fgets(line, sizeof(line), fp))
> -break;
> -
>  /* The line we're interested in is marked differently depending
>   * on the architecture, so check possible prefixes */
>  if (!STRPREFIX(line, "flags") &&
> @@ -129,12 +127,6 @@ virBitmap *virHostValidateGetCPUFlags(void)
>  !STRPREFIX(line, "facilities"))
>  continue;
>  
> -/* fgets() includes the trailing newline in the output buffer,
> - * so we need to clean that up ourselves. We can safely access
> - * line[strlen(line) - 1] because the checks above would cause
> - * us to skip empty strings */
> -line[strlen(line) - 1] = '\0';
> -

getline() also includes the trailing newline in the buffer (as long as
there is any newline) and thus we still need to drop it. We just don't
need to call strlen to compute the length. Although I would suggest
using a little bit more robust code instead of describing the existing
code is safe...

if (linelen > 1 && line[linelen - 1] == '\n')
line[linelen - 1] = '\0';

>  /* Skip to the separator */
>  if (!(start = strchr(line, ':')))
>  continue;
> @@ -153,7 +145,7 @@ virBitmap *virHostValidateGetCPUFlags(void)
>  if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0)
>  ignore_value(virBitmapSetBit(flags, value));
>  }
> -} while (1);
> +}
>  
>  VIR_FORCE_FCLOSE(fp);
>  

Jirka


Re: [PATCH v3] qemu: add a monitor to /proc/$pid when killing times out

2024-07-22 Thread Jiri Denemark
On Mon, Jul 22, 2024 at 10:55:05 +0200, Michal Prívozník wrote:
> On 7/19/24 17:44, Boris Fiuczynski wrote:
> > In cases when a QEMU process takes longer than the time sigterm and
> > sigkill are issued to kill the process do not simply fail and leave the
> > VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up
> > an fd on /proc/$pid and get notified when the QEMU process finally has
> > terminated to cleanup the VM state.
> > 
> > Resolves: https://issues.redhat.com/browse/RHEL-28819
> > Signed-off-by: Boris Fiuczynski 
> > ---
> >  src/qemu/qemu_domain.c  |   8 +++
> >  src/qemu/qemu_domain.h  |   2 +
> >  src/qemu/qemu_driver.c  |  18 ++
> >  src/qemu/qemu_process.c | 124 ++--
> >  src/qemu/qemu_process.h |   1 +
> >  5 files changed, 148 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 2134b11038..8147ff02fd 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data)
> >  
> >  virChrdevFree(priv->devs);
> >  
> > +if (priv->pidMonitored >= 0) {
> > +virEventRemoveHandle(priv->pidMonitored);
> > +priv->pidMonitored = -1;
> > +}
> > +
> >  /* This should never be non-NULL if we get here, but just in case... */
> >  if (priv->mon) {
> >  VIR_ERROR(_("Unexpected QEMU monitor still active during domain 
> > deletion"));
> > @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque)
> >  priv->blockjobs = virHashNew(virObjectUnref);
> >  priv->fds = virHashNew(g_object_unref);
> >  
> > +priv->pidMonitored = -1;
> > +
> >  /* agent commands block by default, user can choose different behavior 
> > */
> >  priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK;
> >  priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
> > @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
> >  case QEMU_PROCESS_EVENT_RESET:
> >  case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
> >  case QEMU_PROCESS_EVENT_MONITOR_EOF:
> > +case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
> >  case QEMU_PROCESS_EVENT_LAST:
> >  break;
> >  }
> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> > index d777559119..a5092dd7f0 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate {
> >  
> >  bool beingDestroyed;
> >  char *pidfile;
> > +int pidMonitored;
> >  
> >  virDomainPCIAddressSet *pciaddrs;
> >  virDomainUSBAddressSet *usbaddrs;
> > @@ -469,6 +470,7 @@ typedef enum {
> >  QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION,
> >  QEMU_PROCESS_EVENT_RESET,
> >  QEMU_PROCESS_EVENT_NBDKIT_EXITED,
> > +QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
> >  
> >  QEMU_PROCESS_EVENT_LAST
> >  } qemuProcessEventType;
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 9f3013e231..6b1e4084f6 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm,
> >  }
> >  
> >  
> > +static void
> > +processShutdownCompletedEvent(virQEMUDriver *driver,
> > +  virDomainObj *vm)
> > +{
> > +if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
> > +return;
> 
> Shouldn't this be:
> 
> if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0)
> return;
> 
> Otherwise looking good. No need to resend, I can fix that before pushing.

And followed by qemuProcessEndStopJob after calling qemuProcessStop.

Jirka


Plans for 10.6.0 release (freeze on Monday 29 Jul)

2024-07-22 Thread Jiri Denemark
We are getting close to 10.6.0 release of libvirt. Since I'll be away
from computers next week, I suggest moving the "last week in a month"
freeze a bit and aim for the release on Monday 05 Aug after entering the
freeze on Monday 29 Jul. There will be no RC2 this time.

I hope this works for everyone.

Jirka


[PATCH] qemu: Don't leave beingDestroyed=true on inactive domain

2024-07-11 Thread Jiri Denemark
Recent commit v10.4.0-87-gd9935a5c4f made a reasonable change to only
reset beingDestroyed back to false when vm->def->id is reset to make
sure other code can detect a domain is (about to become) inactive. It
even added a comment saying any caller of qemuProcessBeginStopJob is
supposed to call qemuProcessStop to clear beingDestroyed. But not every
caller really does so because they first call qemuProcessBeginStopJob
and then check whether a domain is still running. If not the
qemuProcessStop call is skipped leaving beingDestroyed=true. In case of
a persistent domain this may block incoming migrations of such domain as
the migration code would think the domain died unexpectedly (even though
it's still running).

The qemuProcessBeginStopJob function is a wrapper around
virDomainObjBeginJob, but virDomainObjEndJob was used directly for
cleanup. This patch introduces a new qemuProcessEndStopJob wrapper
around virDomainObjEndJob to properly undo everything
qemuProcessBeginStopJob did.

https://issues.redhat.com/browse/RHEL-43309

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c  |  4 ++--
 src/qemu/qemu_process.c | 20 
 src/qemu/qemu_process.h |  1 +
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f898d85667..9f3013e231 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2102,7 +2102,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
  endjob:
 if (ret == 0)
 qemuDomainRemoveInactive(driver, vm, 0, false);
-virDomainObjEndJob(vm);
+qemuProcessEndStopJob(vm);
 
  cleanup:
 virDomainObjEndAPI();
@@ -3888,7 +3888,7 @@ processMonitorEOFEvent(virQEMUDriver *driver,
 
  endjob:
 qemuDomainRemoveInactive(driver, vm, 0, false);
-virDomainObjEndJob(vm);
+qemuProcessEndStopJob(vm);
 }
 
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7cbe521a6e..25dfd04272 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8421,7 +8421,8 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags)
  * qemuProcessBeginStopJob:
  *
  * Stop all current jobs by killing the domain and start a new one for
- * qemuProcessStop.
+ * qemuProcessStop. The caller has to make sure qemuProcessEndStopJob is
+ * called to properly cleanup the job.
  */
 int
 qemuProcessBeginStopJob(virDomainObj *vm,
@@ -8448,8 +8449,9 @@ qemuProcessBeginStopJob(virDomainObj *vm,
 goto error;
 
 /* priv->beingDestroyed is deliberately left set to 'true' here. Caller
- * is supposed to call qemuProcessStop, which will reset it after
- * 'vm->def->id' is set to -1 */
+ * is supposed to call qemuProcessStop (which will reset it after
+ * 'vm->def->id' is set to -1) and/or qemuProcessEndStopJob to do proper
+ * cleanup. */
 return 0;
 
  error:
@@ -8458,6 +8460,16 @@ qemuProcessBeginStopJob(virDomainObj *vm,
 }
 
 
+void
+qemuProcessEndStopJob(virDomainObj *vm)
+{
+if (!virDomainObjIsActive(vm))
+QEMU_DOMAIN_PRIVATE(vm)->beingDestroyed = false;
+
+virDomainObjEndJob(vm);
+}
+
+
 void qemuProcessStop(virQEMUDriver *driver,
  virDomainObj *vm,
  virDomainShutoffReason reason,
@@ -8800,7 +8812,7 @@ qemuProcessAutoDestroy(virDomainObj *dom,
 
 qemuDomainRemoveInactive(driver, dom, 0, false);
 
-virDomainObjEndJob(dom);
+qemuProcessEndStopJob(dom);
 
 virObjectEventStateQueue(driver->domainEventState, event);
 }
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c1ea949215..cb67bfcd2d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -169,6 +169,7 @@ typedef enum {
 int qemuProcessBeginStopJob(virDomainObj *vm,
 virDomainJob job,
 bool forceKill);
+void qemuProcessEndStopJob(virDomainObj *vm);
 void qemuProcessStop(virQEMUDriver *driver,
  virDomainObj *vm,
  virDomainShutoffReason reason,
-- 
2.45.2


Re: [PATCH] vmx: Do not require all ID data for VMWare Distributed Switch

2024-07-08 Thread Jiri Denemark
On Mon, Jul 08, 2024 at 14:52:51 +0200, Martin Kletzander wrote:
> Similarly to commit 2482801608b8 we can safely ignore connectionId,
> portId and portgroupId in both XML and VMX as they are only a blind
> pass-through between XML and VMX and an ethernet without such parameters
> was spotted in the wild.  On top of that even our documentation says the
> whole VMWare Distrubuted Switch configuration is a best-effort.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-46099
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/conf/domain_conf.c| 11 -
>  src/conf/schemas/domaincommon.rng | 24 ---
>  src/vmx/vmx.c | 24 ---
>  ...-portid.vmx => ethernet-vds-no-params.vmx} |  2 --
>  ...-portid.xml => ethernet-vds-no-params.xml} |  2 +-
>  5 files changed, 37 insertions(+), 26 deletions(-)
>  rename tests/vmx2xmldata/{ethernet-vds-no-portid.vmx => 
> ethernet-vds-no-params.vmx} (76%)
>  rename tests/vmx2xmldata/{ethernet-vds-no-portid.xml => 
> ethernet-vds-no-params.xml} (82%)

Reviewed-by: Jiri Denemark 


Release of libvirt-10.5.0

2024-07-01 Thread Jiri Denemark
The 10.5.0 release of both libvirt and libvirt-python is tagged and
signed tarballs are available at

https://download.libvirt.org/
https://download.libvirt.org/python/

Thanks everybody who helped with this release by sending patches,
reviewing, testing, or providing feedback. Your work is greatly
appreciated.

* New features

  * Introduce SEV-SNP support

SEV-SNP is introduced as another type of . Its support
is reported in both domain capabilities and ``virt-host-validate``.

* Improvements

  * tools: virt-pki-validate has been rewritten in C

The ``virt-pki-validate`` shell script has been rewritten as a C program,
providing an output format that matches ``virt-host-validate``, removing
the dependency on ``certtool`` and providing more comprehensive checks
of the certificate properties.

  * qemu: implement iommu coldplug/unplug

The  device can be now cold plugged and/or cold unplugged.

  * Pass shutoff reason to release hook

Sometimes in release hook it is useful to know if the VM shutdown was
graceful or not. This is especially useful to do cleanup based on the VM
shutdown failure reason in release hook. Starting with this release the
last argument 'extra' is used to pass VM shutoff reason in the call to
release hook.

  * nodedev: improve DASD detection

In newer DASD driver versions the ID_TYPE tag is supported. This tag is
missing after a system reboot but when the ccw device is set offline and
online the tag is included. To fix this version independently we need to
check if a device detected as type disk is actually a DASD to maintain the
node object consistency and not end up with multiple node objects for
DASDs.

* Bug fixes

  * remote_daemon_dispatch: Unref sasl session when closing client connection

A memory leak was identified when a client started SASL but then suddenly
closed connection. This is now fixed.

  * qemu: Fix migration with disabled vmx-* CPU features

Migrating a domain with some vmx-* CPU features marked as disabled could
have failed as the destination would incorrectly expect those features to
be enabled after starting QEMU.

  * qemu: Fix ``libvirtd``/``virtqemud`` crash when VM shuts down during 
migration

The libvirt daemon could crash when a VM was shut down while being migrated
to another host.

Enjoy.

Jirka


Re: [PATCH] NEWS: Mention crash when VM shuts down during migration

2024-06-28 Thread Jiri Denemark
On Fri, Jun 28, 2024 at 15:00:29 +0200, Peter Krempa wrote:
> Signed-off-by: Peter Krempa 
> ---
>  NEWS.rst | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index 1ea98599db..5557018f33 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -65,6 +65,11 @@ v10.5.0 (unreleased)
>  have failed as the destination would incorrectly expect those features to
>  be enabled after starting QEMU.
> 
> +  * qemu: Fix ``libvirtd``/``virtqemud`` crash when VM shuts down during 
> migration
> +
> +The libvirt daemon could crash when a VM was shut down while being 
> migrated
> +to another host.
> +
> 
>  v10.4.0 (2024-06-03)
>  

Reviewed-by: Jiri Denemark 


[PATCH] NEWS: Mention migration fix with disabled vmx-* CPU features

2024-06-28 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 NEWS.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index b0b893b484..1ea98599db 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -59,6 +59,12 @@ v10.5.0 (unreleased)
 A memory leak was identified when a client started SASL but then suddenly
 closed connection. This is now fixed.
 
+  * qemu: Fix migration with disabled vmx-* CPU features
+
+Migrating a domain with some vmx-* CPU features marked as disabled could
+have failed as the destination would incorrectly expect those features to
+be enabled after starting QEMU.
+
 
 v10.4.0 (2024-06-03)
 
-- 
2.45.1


Re: [PATCH] NEWS: Document features/improvements/bug fixes I've participated in

2024-06-28 Thread Jiri Denemark
On Fri, Jun 28, 2024 at 10:54:14 +0200, Michal Privoznik wrote:
> There are some features/improvements/bug fixes I've either
> contributed or reviewed/merged. Document them for upcoming
> release.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  NEWS.rst | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index bc8acc62d4..d675c869a4 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -16,6 +16,10 @@ v10.5.0 (unreleased)
>  * **Removed features**
>  
>  * **New features**
> +  * Introduce SEV-SNP support
> +
> +SEV-SNP is introduced as another type of . Its 
> support
> +is reported in both domain capabilities and ``virt-host-validate``.
>  
>  * **Improvements**
>  
> @@ -26,8 +30,34 @@ v10.5.0 (unreleased)
>  the dependency on ``certtool`` and providing more comprehensive checks
>  of the certificate properties.
>  
> +  * qemu: implement iommu coldplug/unplug
> +
> +The  device can be now cold plugged and/or cold unplugged.
> +
> +  * Pass shutoff reason to release hook
> +
> +Sometimes in release hook it is useful to know if the VM shutdown was
> +graceful or not. This is especially useful to do cleanup based on the VM
> +shutdown failure reason in release hook. String with this release the 
> last

s/String/Starting/ ?

> +argument 'extra' is used to pass VM shutoff reason in the call to release
> +hook.
> +
> +  * nodedev: improve DASD detection
> +
> +In newer DASD driver versions the ID_TYPE tag is supported. This tag is
> +missing after a system reboot but when the ccw device is set offline and
> +online the tag is included. To fix this version independently we need to
> +check if devices detected as type disk is actually a DASD to maintain

s/if devices/if a device/

or alternatively

s/disk is/disk are/ and s/a DASD/DASDs/

> +the node object consistency and not end up with multiple node objects
> +for DASDs.
> +
>  * **Bug fixes**
>  
> +  * remote_daemon_dispatch: Unref sasl session when closing client connection
> +
> +A memory leak was identified when a client started SASL but then suddenly
> +closed connection. This is now fixed.
> +
>  
>  v10.4.0 (2024-06-03)
>  

Reviewed-by: Jiri Denemark 


Re: [PATCH] qemu: fix switchover-ack regression for old qemu

2024-06-28 Thread Jiri Denemark
On Thu, Jun 27, 2024 at 11:11:56 -0700, Jon Kohler wrote:
> When enabling switchover-ack on qemu from libvirt, the .party value
> was set to both source and target; however, qemuMigrationParamsCheck()
> only takes that into account to validate that the remote side of the
> migration supports the flag if it is marked optional or auto/always on.
> 
> In the case of switchover-ack, when enabled on only the dst and not
> the src, the migration will fail if the src qemu does not support
> switchover-ack, as the dst qemu will issue a switchover-ack msg:
> qemu/migration/savevm.c ->
>   loadvm_process_command ->
> migrate_send_rp_switchover_ack(mis) ->
>   migrate_send_rp_message(mis, MIG_RP_MSG_SWITCHOVER_ACK, 0, NULL)
> 
> Since the src qemu doesn't understand messages with header_type ==
> MIG_RP_MSG_SWITCHOVER_ACK, qemu will kill the migration with error:
>   qemu-kvm: RP: Received invalid message 0x0007 length 0x
>   qemu-kvm: Unable to write to socket: Bad file descriptor
> 
> Looking at the original commit [1] for optional migration capabilities,
> it seems that the spirit of optional handling was to enhance a given
> existing capability where possible. Given that switchover-ack
> exclusively depends on return-path, adding it as optional to that cap
> feels right.

Oops, sorry for not spotting this in my review and thanks for fixing it.

I guess we should add a possibility to check QEMU support for
capabilities in case they don't depend on another capability and they
are supposed to be enabled automatically. But that's a separate thing
which may possibly become useful in the future.

Reviewed-by: Jiri Denemark 


Re: [libvirt PATCH] vircgroup: fix g_variant_new_parsed format string causing abort

2024-06-28 Thread Jiri Denemark
On Thu, Jun 27, 2024 at 18:05:18 +0200, Pavel Hrdina wrote:
> The original code was incorrect and never tested because at the time of
> implementing it the cgroup file `io.weight` was not available.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-45185
> Introduced-by: 9c1693eff427661616ce1bd2795688f87288a412
> Signed-off-by: Pavel Hrdina 
> ---
>  src/util/vircgroupv1.c | 2 +-
>  src/util/vircgroupv2.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Jiri Denemark 


libvirt-10.5.0 release candidate 2

2024-06-28 Thread Jiri Denemark
I have just tagged v10.5.0-rc2 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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


Entering freeze for libvirt-10.5.0

2024-06-25 Thread Jiri Denemark
I have just tagged v10.5.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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


Re: [PATCH 0/8] Expose SEV-SNP in domcaps and virt-host-validate

2024-06-25 Thread Jiri Denemark
On Tue, Jun 25, 2024 at 11:48:45 +0200, Michal Privoznik wrote:
> This is a promised follow up to:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/7QQXVQXZATOIDYAJFOT45RPXRKX4GEWP/
> 
> Michal Prívozník (8):
>   libvirt_private.syms: Export virDomainLaunchSecurity enum handlers
>   qemuxmlconftest; Explicitly enable QEMU_CAPS_SEV_SNP_GUEST for
> "launch-security-sev-snp"
>   qemu_capabilities: Probe SEV capabilities even for
> QEMU_CAPS_SEV_SNP_GUEST
>   domcaps: Report launchSecurity
>   qemu: Fill launchSecurity in domaincaps
>   qemu_validate: Use domaincaps to validate supported launchSecurity
> type
>   virt-host-validate: Move AMD SEV into a separate func
>   virt-host-validate: Detect SEV-ES and SEV-SNP

Overall it looks OK (see replies to 3/8 and 5/8 for a few nits) and it
makes sense to me. But you should probably wait for a second look from
someone familiar with SEV to check the design makes sense.

Reviewed-by: Jiri Denemark 


Re: [PATCH 5/8] qemu: Fill launchSecurity in domaincaps

2024-06-25 Thread Jiri Denemark
On Tue, Jun 25, 2024 at 11:48:50 +0200, Michal Privoznik wrote:
> The inspiration for these rules comes from
> qemuValidateDomainDef().
> 
> Signed-off-by: Michal Privoznik 
> ---
...
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index adaf5f9c26..4f9895ba9c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -6514,6 +6514,24 @@ virQEMUCapsFillDomainDeviceCryptoCaps(virQEMUCaps 
> *qemuCaps,
>  }
>  
>  
> +void
> +virQEMUCapsFillDomainLaunchSecurity(virQEMUCaps *qemuCaps,
> +virDomainCapsLaunchSecurity 
> *launchSecurity)
> +{
> +launchSecurity->supported = VIR_TRISTATE_BOOL_YES;
> +launchSecurity->sectype.report = true;
> +
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST))
> +VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype, 
> VIR_DOMAIN_LAUNCH_SECURITY_SEV);
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_SNP_GUEST))
> +VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype, 
> VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP);
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST) &&
> +virQEMUCapsGet(qemuCaps, 
> QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT))
> +VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype, 
> VIR_DOMAIN_LAUNCH_SECURITY_PV);
> +}
> +
> +
> +

Two empty lines would have been enough :-)

>  /**
>   * virQEMUCapsSupportsGICVersion:
>   * @qemuCaps: QEMU capabilities
> @@ -6678,6 +6696,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>  virDomainCapsDeviceChannel *channel = >channel;
>  virDomainCapsMemoryBacking *memoryBacking = >memoryBacking;
>  virDomainCapsDeviceCrypto *crypto = >crypto;
> +virDomainCapsLaunchSecurity *launchSecurity = >launchSecurity;
>  
>  virQEMUCapsFillDomainFeaturesFromQEMUCaps(qemuCaps, domCaps);
>  
> @@ -6717,6 +6736,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>  virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
>  virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
>  virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
> +virQEMUCapsFillDomainLaunchSecurity(qemuCaps, launchSecurity);
>  
>  return 0;
>  }
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index a98da8c2eb..ef71e8511e 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -867,6 +867,9 @@ void virQEMUCapsFillDomainDeviceChannelCaps(virQEMUCaps 
> *qemuCaps,
>  void virQEMUCapsFillDomainDeviceCryptoCaps(virQEMUCaps *qemuCaps,
> virDomainCapsDeviceCrypto 
> *crypto);
>  
> +void virQEMUCapsFillDomainLaunchSecurity(virQEMUCaps *qemuCaps,
> + virDomainCapsLaunchSecurity 
> *launchSecurity);
> +
>  bool virQEMUCapsGuestIsNative(virArch host,
>virArch guest);
>  
> diff --git a/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml 
> b/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml
> index c42a20763f..f9aacbfbf9 100644
> --- a/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml
> +++ b/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml
> @@ -319,5 +319,8 @@
>  
>  
>  
> +
> +  
> +

I think reporting launchSecurity as unsupported when no sectype is
available would make more sense.

>
>  

Jirka


Re: [PATCH 3/8] qemu_capabilities: Probe SEV capabilities even for QEMU_CAPS_SEV_SNP_GUEST

2024-06-25 Thread Jiri Denemark
On Tue, Jun 25, 2024 at 11:48:48 +0200, Michal Privoznik wrote:
> While it's very unlikely to have QEMU that supports SEV-SNP but
> doesn't support plain SEV, for completeness sake we ought to
> query SEV capabilities if QEMU supports either. And similarly to
> QEMU_CAPS_SEV_GUEST we need to clear the capability if talking to
> QEMU proves SEV is not really supported.
> 
> This in turn removes the 'sev-snp'guest' capability from on of

s/snp'/snp-/
s/ on / one /

> our test cases as Peter's machine he uses to refresh capabilities
> is not SEV capable. But that's okay. It's consistent with
> 'sev-guest' capability.

Jirka


Re: [PATCH v2] qemu: add support for qemu switchover-ack

2024-06-25 Thread Jiri Denemark
On Mon, Jun 24, 2024 at 10:38:51 -0700, Jon Kohler wrote:
> Add plumbing for QEMU's switchover-ack migration capability, which
> helps lower the downtime during VFIO migrations. This capability is
> enabled by default as long as both the source and destination support
> it.
> 
> Note: switchover-ack depends on the return path capability, so this may
> not be used when VIR_MIGRATE_TUNNELLED flag is set.
> 
> Extensive details about the qemu switchover-ack implementation are
> available in the qemu series v6 cover letter [1] where the highlight is
> the extreme reduction in guest visible downtime. In addition to the
> original test results below, I saw a roughly ~20% reduction in downtime
> for VFIO VGPU devices at minimum.
> 
>   === Test results ===
> 
>   The below table shows the downtime of two identical migrations. In the
>   first migration swithcover ack is disabled and in the second it is
>   enabled. The migrated VM is assigned with a mlx5 VFIO device which has
>   300MB of device data to be migrated.
> 
>   +--+---+--+
>   |Switchover ack| VFIO device data size | Downtime |
>   +--+---+--+
>   |   Disabled   | 300MB |  1900ms  |
>   |   Enabled| 300MB |  420ms   |
>   +--+---+--+
> 
>   Switchover ack gives a roughly 4.5 times improvement in downtime.
>   The 1480ms difference is time that is used for resource allocation for
>   the VFIO device in the destination. Without switchover ack, this time is
>   spent when the source VM is stopped and thus the downtime is much
>   higher. With switchover ack, the time is spent when the source VM is
>   still running.
> 
> [1] 
> https://patchwork.kernel.org/project/qemu-devel/cover/2023062201.29729-1-avih...@nvidia.com/
> 
> Signed-off-by: Jon Kohler 
> Cc: Alex Williamson 
> Cc: Avihai Horon 
> Cc: Markus Armbruster 
> Cc: Peter Xu 
> Cc: YangHang Liu 
> ---
> v1
>  - 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/2XCWPYAUE7HUIMSAYYWAUUYGGZ6WYR53/
> v1 -> v2:
>  - Addressed comments to simplify approach (Daniel, Jiri)
> ---
>  src/qemu/qemu_migration.h| 1 +
>  src/qemu/qemu_migration_params.c | 8 +++-
>  src/qemu/qemu_migration_params.h | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
> index ed62fd4a91..cd89e100e1 100644
> --- a/src/qemu/qemu_migration.h
> +++ b/src/qemu/qemu_migration.h
> @@ -62,6 +62,7 @@
>   VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES | \
>   VIR_MIGRATE_POSTCOPY_RESUME | \
>   VIR_MIGRATE_ZEROCOPY | \
> + VIR_MIGRATE_SWITCHOVER_ACK | \
>   0)
>  
>  /* All supported migration parameters and their types. */

This is a leftover from v1, it's no longer needed.

I removed this hunk and pushed the patch. Thanks.

Reviewed-by: Jiri Denemark 


[PATCH] qemu: Fix migration with disabled vmx-* CPU features

2024-06-24 Thread Jiri Denemark
When starting a domain on a host which lacks a vmx-* CPU feature which
is expected to be enabled by the CPU model specified in the domain XML,
libvirt properly marks such feature as disabled in the active domain
XML. But migrating the domain to a similar host which lacks the same
vmx-* feature will fail with libvirt reporting the feature as missing.
This is because of a bug in the hack ensuring backward compatibility
libvirt running on the destination thinks the missing feature is
expected to be enabled.

https://issues.redhat.com/browse/RHEL-40899

Fixes: v10.1.0-85-g5fbfa5ab8a
Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_x86.c | 44 +++-
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 7a70eed9e7..fcbce0ec46 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3021,10 +3021,7 @@ virCPUx86UpdateLive(virCPUDef *cpu,
 if (!(map = virCPUx86GetMap()))
 return -1;
 
-if (!(model = x86ModelFromCPU(cpu, map, -1)))
-return -1;
-
-if (hostPassthrough &&
+if (!(model = x86ModelFromCPU(cpu, map, -1)) ||
 !(modelDisabled = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_DISABLE)))
 return -1;
 
@@ -3037,12 +3034,19 @@ virCPUx86UpdateLive(virCPUDef *cpu,
 for (i = 0; i < map->nfeatures; i++) {
 virCPUx86Feature *feature = map->features[i];
 virCPUFeaturePolicy expected = VIR_CPU_FEATURE_LAST;
+bool explicit = false;
+bool ignore = false;
 
-if (x86DataIsSubset(>data, >data))
+if (x86DataIsSubset(>data, >data)) {
+explicit = true;
 expected = VIR_CPU_FEATURE_REQUIRE;
-else if (!hostPassthrough ||
- x86DataIsSubset(>data, >data))
+} else if (x86DataIsSubset(>data, >data)) {
+explicit = true;
+expected = VIR_CPU_FEATURE_DISABLE;
+} else if (!hostPassthrough) {
+/* implicitly disabled */
 expected = VIR_CPU_FEATURE_DISABLE;
+}
 
 if (x86DataIsSubset(, >data) &&
 x86DataIsSubset(, >data)) {
@@ -3052,30 +3056,36 @@ virCPUx86UpdateLive(virCPUDef *cpu,
 return -1;
 }
 
+/* Features enabled or disabled by the hypervisor are ignored by
+ * check='full' in case they were added to the model later and not
+ * explicitly mentioned in the CPU definition. This matches how libvirt
+ * behaved before the features were added.
+ */
+if (!explicit &&
+g_strv_contains((const char **) model->addedFeatures, 
feature->name))
+ignore = true;
+
 if (expected == VIR_CPU_FEATURE_DISABLE &&
 x86DataIsSubset(, >data)) {
 VIR_DEBUG("Feature '%s' enabled by the hypervisor", feature->name);
 
-/* Extra features enabled by the hypervisor are ignored by
- * check='full' in case they were added to the model later for
- * backward compatibility with the older definition of the model.
- */
-if (cpu->check == VIR_CPU_CHECK_FULL &&
-!g_strv_contains((const char **) model->addedFeatures, 
feature->name)) {
+if (cpu->check == VIR_CPU_CHECK_FULL && !ignore)
 virBufferAsprintf(, "%s,", feature->name);
-} else {
+else
 virCPUDefUpdateFeature(cpu, feature->name, 
VIR_CPU_FEATURE_REQUIRE);
-}
 }
 
 if (x86DataIsSubset(, >data) ||
 (expected == VIR_CPU_FEATURE_REQUIRE &&
  !x86DataIsSubset(, >data))) {
 VIR_DEBUG("Feature '%s' disabled by the hypervisor", 
feature->name);
-if (cpu->check == VIR_CPU_CHECK_FULL)
+
+if (cpu->check == VIR_CPU_CHECK_FULL && !ignore) {
 virBufferAsprintf(, "%s,", feature->name);
-else
+} else {
 virCPUDefUpdateFeature(cpu, feature->name, 
VIR_CPU_FEATURE_DISABLE);
+x86DataSubtract(, >data);
+}
 }
 }
 
-- 
2.45.1


Re: [PATCH] qemu: add support for qemu switchover-ack

2024-06-20 Thread Jiri Denemark
On Tue, Jun 18, 2024 at 16:14:29 +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 18, 2024 at 08:06:06AM -0700, Jon Kohler wrote:
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 2f5b01bbfe..9543629f30 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1100,6 +1100,17 @@ typedef enum {
> >   * Since: 8.5.0
> >   */
> >  VIR_MIGRATE_ZEROCOPY = (1 << 20),
> > +
> > +/* Use switchover ack migration capability to reduce downtime on VFIO
> > + * device migration. This prevents the source from stopping the VM and
> > + * completing the migration until an ACK is received from the 
> > destination
> > + * that it's OK to do so. Thus, a VFIO device can make sure that its
> > + * initial bytes were sent and loaded in the destination before the
> > + * source VM is stopped.
> > + *
> > + * Since: 10.5.0
> > + */
> > +VIR_MIGRATE_SWITCHOVER_ACK = (1 << 21),
> >  } virDomainMigrateFlags;
> 
> Do we really need a flag for this ?  Is there a credible scenario
> in which this flag works, and yet shouldn't be used by libvirt ?
> 
> IOW, can we just "do the right thing" and always enable this,
> except for TUNNELLED mode.

I discussed this capability some time ago with Peter (I think) and if
IIRC there was some downside when the capability is enabled for domains
that do not use VFIO. I don't remember exactly what it was about, but
perhaps introducing an extra delay in migration switchover? Peter, can
you add the details, please?

That said, I think we should be able to do the right thing anyway and
enable this capability only for domains that use VFIO.

Jirka


Plans for 10.5.0 release (freeze on Tuesday 25 Jun)

2024-06-18 Thread Jiri Denemark
We are getting close to 10.5.0 release of libvirt. To aim for the
release on Monday 01 Jul I suggest entering the freeze on Tuesday 25
Jun and tagging RC2 on Thursday 27 Jun.

I hope this works for everyone.

Jirka


Re: [RFC PATCH 0/1] support deprecated-props from query-cpu-model-expansion

2024-06-13 Thread Jiri Denemark
On Tue, Jun 04, 2024 at 11:42:25 -0400, Collin Walling wrote:
> The QEMU portion is designed for s390x such that there is a static list
> of hardcoded feature bits that are flagged for deprecation.  This list
> can be updated in follow-up patches as more features need to be flagged.

Good, a single static list should definitely be easier to handle.

> The query-cpu-model-expansion *response* has been modified to now
> include an /optional/ "deprecated props" string array (no additional
> parameter is required in the query JSON).  This array will be present if
> and only if there is data to be reported.  If an architecture does not
> support reporting deprecated features, or if there are no deprecated
> features to report, then array is simply omitted from the response.
> 
> The deprecated features are filtered against the full-expansion features
> for the particular CPU model.  In the example for a z14, we would expect
> all four currently deprecated features reported: bpb, csske, cte, and
> te, since those features are part of the z14 full expansion.  On an
> older model, say a z900, you'd only see bpb because the others are not
> present in the full-expansion.

In that case I think it would be nice if QEMU provided a way of getting
the list itself without any filtering. Mainly to make sure we're
providing a completely list for users which may decide to avoid using
such features in their non-host-model CPU definitions. And I can imagine
management layers might want to get a complete list too.

> >> Another conflict is setting this option to "on" would have no
> >> effect on the CPU model (I can't think of a reason why someone
> >> would want to explicitly enable these features).  This may not
> >> communicate well to the user.
> > 
> > I think have a separate configuration is better as it does not limit the
> > used to just a single CPU model. But a nested element with a text node
> > looks strange. An optional attribute for the  element would be
> > better.
> >> For host-model the expected behavior would be to either keep or
> > drop deprecated features from the CPU definition. When combined with
> > custom CPU mode where such feature may be already present I can imagine
> > three different options. Either keep deprecated features (that is do
> > nothing, just like we do now), drop such features silently, or fail to
> > start a domain in case the definition uses a deprecated feature.
> 
> I fear the last option may break some things?

The idea is the behavior would be controllable via a special value in
that attribute, which a used would have to explicitly specify to get
such behavior. In other words, a user would provide a custom CPU
definition and ask for deprecated features to be removed no matter what
they are currently. But I don't think we need to care about it now
because...

> > We could even create the attribute, but limit it only to host-model,
> > which would be mostly equivalent to your "host-recomended" mode, but
> > extensible to other modes in the future.
> 
> I think this is the better approach.

Agreed. Just do the minimal required thing while making it extensible.

> In tandem to your suggestion to add a new attribute to the cpu element,
> a quick mock-up of it could look like this:
> 
> 

Right. Although you may have fun coming with an attribute name, because
we are not very consistent with naming multi-word attributes. Some
people don't like underscores while others don't like camelCase :-)
Anyway, I think you should at least be consistent and use a single name
for the attribute in domain XML and the element in domcapabilities XML.

> >> To report these features, a  tag could be
> >> added to the domcapabilities output using the same format I use in
> >> my proposed patch for the qemu capabilities file:
> >>
> >>   
> >> 
> >>   ...
> >> 
> >> ...
> >> 
> >>   ...
> >> 
> >> 
> >>   
> >>   
> >>   
> >>   
> >> 
> >>   
> > 
> > We should stick with "features" rather than calling them "properties"
> > here to avoid confusion. Also this schema would mean the list of
> > deprecated features is indeed the same for all CPU models, which does
> > not seem to be the case here.
> 
> Noted to use "features" instead of "properties".
> 
> And you are correct that this makes the assumption that these features
> are available for *all* CPU models -- I did not think of this
> perspective.  This is populated from a query-cpu-model-expansion based
> on "host-model".
> 
> What would be a better way to report these features in the
> domcapabilities response?  Perhaps they should only be nested under the
> host-model?

Well, the list is static and common to all CPU models so keeping it as
another element under  as you suggested looks fine.

> ...another thought would be to implement a new feature policy
> "deprecated" that would allow these XML features to be applied to any
> CPU model.  This would flag the feature to either be disabled if it's
> 

Re: [RFC PATCH 0/1] support deprecated-props from query-cpu-model-expansion

2024-06-03 Thread Jiri Denemark
On Tue, May 07, 2024 at 18:24:20 -0400, Collin Walling wrote:
> QEMU will soon support reporting an optional array of deprecated
> features for an expanded CPU model via the query-cpu-model-expansion
> command.  The intended use of this data is to make it easier for a
> user to define a CPU model with features flagged as deprecated set to
> disabled, thus rendering the guest migratable to future hardware that
> will out-right drop support for said features.

So is the list of deprecated features a static list common to all CPU
models or does it depend on the CPU model? If it's static getting the
list via another QMP command (in addition to query-cpu-model-expansion)
would make sense. I would expect the query-cpu-model-expansion to limit
deprecated features to only those actually used by the expanded CPU
model, which would make constructing a complete list quite hard.

On the other hand, if the deprecated features depend on a CPU model,
we'd need a way to list all CPU models and their deprecated features to
be able to report such info in domain capabilities XML. Ideally without
having to call query-cpu-model-expansion on every single CPU model.

> Notes
> =
> 
>  - In my example below, I am running on a z14.2 machine.
> 
>  - The features that are flagged as deprecated for this model are: bpb, 
> csske, cte, te.

OK, so the deprecated features seem to depend on a specific CPU model.
Does query-cpu-model-expansion list them even if they are not specified
in the CPU model we want to expand? If not, we need another interface to
be able to report this info.

> Ideas
> =
> 
> New Host CPU Model
> --
> 
> Create a new CPU model that is a mirror of the host CPU model with deprecated 
> features turned off.  Let's call this model "host-recommended".  A user may 
> define this model in the guest XML as they would any other CPU model:
> 
>   
> 
> Just as how host-model works, anything defined nested in the  tag will 
> be ignored.
> 
> This model could potentially be listed in the domcapabilities output after 
> the host-model:
> 
>   
> 
>   ...
> 
> ...
> 
>   ...
> 
> 
>   ...
>   
>   
>   
>   
>   
>   
>   
>   
>   
>   
>   
>   
> 
> 
> New Nested Element Under 
> --
> 
> Create a new optional XML element nested under the  tag that may be used 
> to disable deprecated features.  This approach is more explicit compared to 
> creating a new CPU model, and allows the user to disable these features when 
> defining a specific model other than host-model.  Here is an example of what 
> the guest's defined XML for the CPU could look like:
> 
>   
> off
>   
> 
> However, a conflict arises with this approach: parameter priority.  It would 
> need to be discussed what the expected behavior should be if a user defines a 
> guest with both a mode to disable deprecated features and any deprecated 
> features listed with the 'require' policy, e.g.:
> 
>   
> z13.2-base
> 
> off
> 
>   
> 
> Another conflict is setting this option to "on" would have no effect on the 
> CPU model (I can't think of a reason why someone would want to explicitly 
> enable these features).  This may not communicate well to the user.

I think have a separate configuration is better as it does not limit the
used to just a single CPU model. But a nested element with a text node
looks strange. An optional attribute for the  element would be
better.

For host-model the expected behavior would be to either keep or
drop deprecated features from the CPU definition. When combined with
custom CPU mode where such feature may be already present I can imagine
three different options. Either keep deprecated features (that is do
nothing, just like we do now), drop such features silently, or fail to
start a domain in case the definition uses a deprecated feature.

We could even create the attribute, but limit it only to host-model,
which would be mostly equivalent to your "host-recomended" mode, but
extensible to other modes in the future.

> To report these features, a  tag could be added to the 
> domcapabilities output using the same format I use in my proposed patch for 
> the qemu capabilities file:
> 
>   
> 
>   ...
> 
> ...
> 
>   ...
> 
> 
>   
>   
>   
>   
> 
>   

We should stick with "features" rather than calling them "properties"
here to avoid confusion. Also this schema would mean the list of
deprecated features is indeed the same for all CPU models, which does
not seem to be the case here.

And one more interesting point: what to do with the baseline CPU
computation which expects to see host-model definitions from all hosts.
We'd need a way to provide the info about deprecated features to the
input data for baseline computation. In other words, to the host-model
definition itself. We could, for example, add a deprecated='yes|no'

Release of libvirt-10.4.0

2024-06-03 Thread Jiri Denemark
The 10.4.0 release of both libvirt and libvirt-python is tagged and
signed tarballs are available at

https://download.libvirt.org/
https://download.libvirt.org/python/

Thanks everybody who helped with this release by sending patches,
reviewing, testing, or providing feedback. Your work is greatly
appreciated.

* New features

  * qemu: Support for ras feature for virt machine type

It is now possible to set on/off ``ras`` feature in the domain XML for virt
(Arm) machine type as .

  * SSH proxy for VM

Libvirt now installs a binary helper that allows connecting to QEMU domains
via SSH using the following scheme: ``ssh user@qemu/virtualMachine``.

  * qemu: Support for ``virtio`` sound model

Sound devices can now be configured to use the virtio model with
. This model is available from QEMU 8.2.0
onwards.

  * network: use nftables to setup virtual network firewall rules

The network driver can now use nftables rules for the virtual
network firewalls, rather than iptables. With the standard build
options, nftables is preferred over iptables (with fallback to
iptables if nftables isn't installed), but this can be modified at
build time, or at runtime via the firewall_backend setting in
network.conf. (NB: the nwfilter driver still uses
ebtables/iptables).

* Improvements

  * qemu: add zstd to supported compression formats

Extend the list of supported formats of QEMU save image by adding zstd
compression.

  * qemu: Implement support for hotplugging evdev input devices

As of this release, hotplug and hotunplug of evdev  devices is
supported.

* Bug fixes

  * virsh/virt-admin: Fix ``--help`` option for all commands

A bug introduced in `v10.3.0 (2024-05-02)`_ caused that the attempt to print
help for any command by using the ``--help`` option in ``virsh`` and
``virt-admin`` would print::

  $ virsh list --help
  error: command 'list' doesn't support option --help

instead of the help output. A workaround for the affected version is to use
the help command::

  $ virsh help list

  * qemu: Fix ``virsh save`` and migration when storage in question is 
root_squashed NFS

Attempting to save a VM to a root_squash NFS mount or migrating with disks
hosted on such mount could, in some scenarios, result in error stating::

  'Unknown error 255'

The bug was introduced in `v10.1.0 (2024-03-01)`_.

  * qemu: Don't set affinity for isolcpus unless explicitly requested

When starting a domain, by default libvirt sets affinity of QEMU process to
all online CPUs. This also included isolated CPUs (``isolcpus=``) which is
wrong. As of this release, isolated CPUs are left untouched, unless
explicitly configured in domain XML.

  * qemu_hotplug: Properly assign USB address to hotplugged usb-net device

Previously, the network device hotplug logic would try to ensure only CCW
or PCI addresses. With recent support for the usb-net model, USB addresses
for usb-net network devices are assigned automatically.

  * qemu: Fix hotplug of ``virtiofs`` filesystem device with ``

libvirt-10.4.0 release candidate 2

2024-05-30 Thread Jiri Denemark
I have just tagged v10.4.0-rc2 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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


Re: [PATCH] log_cleaner: Detect rotated filenames properly

2024-05-30 Thread Jiri Denemark
On Wed, May 29, 2024 at 09:51:36 +0200, Michal Privoznik wrote:
> When removing rotated log files, their name is matched against a
> regex (@log_regex) and if they contain '.N' suffix the 'N' is
> then parsed into an integer. Well, due to a bug in
> virLogCleanerParseFilename() this is not how the code works. If
> the suffix isn't found then g_match_info_fetch() returns an empty
> string instead of NULL which then makes str2int parsing fail.
> Just check for this case before parsing the string.
> 
> Based on the original patch sent by David.
> 
> Reported-by: David Negreira 
> Signed-off-by: Michal Privoznik 
> ---
> 
> The original patch was posted here:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ENXJABODLWWXAR6RSKGMG6GAJKZMVKKM/
> 
> In the thread you can see me suggesting this alternative approach and
> David confirming it works. Therefore, I'd like to get this in before the
> release.
> 
>  src/logging/log_cleaner.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Jiri Denemark 


Re: [PATCH] NEWS: Document my contributions for upcoming release

2024-05-30 Thread Jiri Denemark
On Thu, May 30, 2024 at 15:01:05 +0200, Michal Privoznik wrote:
> These are either features/bugfixes I've worked on or
> participated in.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  NEWS.rst | 23 +++
>  1 file changed, 23 insertions(+)

Reviewed-by: Jiri Denemark 


Re: [PATCH] qemu_hotplug: Clear QoS if required in qemuDomainChangeNet()

2024-05-30 Thread Jiri Denemark
On Thu, May 30, 2024 at 11:59:36 +0200, Michal Privoznik wrote:
> In one of my recent commits, I've introduced
> virDomainInterfaceClearQoS() which is a helper that either calls
> virNetDevBandwidthClear() ('tc' implementation) or
> virNetDevOpenvswitchInterfaceClearQos() (for ovs ifaces). But I
> made a micro optimization which leads to a bug: the function
> checks whether passed iface has any QoS set and returns early if
> it has none. In majority of cases this is right thing to do, but
> when removing QoS on virDomainUpdateDeviceFlags() this is
> problematic. The new definition (passed as argument to
> virDomainInterfaceClearQoS()) contains no QoS (because user
> requested its removal) and thus instead of removing the old QoS
> setting nothing is done.
> 
> Fortunately, the fix is simple - pass olddev which contains the
> old QoS setting.
> 
> Fixes: 812a146dfe784315edece43d09f8d9e432f8230e
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_hotplug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4739beead8..c98b0b5d52 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -4071,7 +4071,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>  goto cleanup;
>  }
>  } else {
> -if (virDomainInterfaceClearQoS(vm->def, newdev) < 0)
> +if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)
>  goto cleanup;
>  }
>  

Reviewed-by: Jiri Denemark 


Re: [libvirt PATCH] qemu_snapshot: fix memory leak when reverting external snapshot

2024-05-29 Thread Jiri Denemark
On Mon, May 27, 2024 at 20:02:39 +0200, Pavel Hrdina wrote:
> The code cleaning up virStorageSource doesn't free data allocated by
> virStorageSourceInit() so we need to call virStorageSourceDeinit()
> explicitly.
> 
> Fixes: 8e664737813378d2a1bdeacc2ca8e942327e2cab
> Resolves: https://issues.redhat.com/browse/RHEL-33044
> Signed-off-by: Pavel Hrdina 
> ---
>  src/qemu/qemu_snapshot.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 09ec959f10..f5260c4a22 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -2260,6 +2260,8 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
>   snapdisk->src->path);
>  }
>  
> +virStorageSourceDeinit(snapdisk->src);
> +
>  virDomainSnapshotDiskDefClear(snapdisk);
>  }
>  
> @@ -2277,6 +2279,8 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
>  VIR_WARN("Failed to remove snapshot image '%s'",
>   snapdisk->src->path);
>  }
> +
> +virStorageSourceDeinit(snapdisk->src);
>  }
>  }
>  

Reviewed-by: Jiri Denemark 


Re: [PATCH v2] run.in: Detect binaries in builddir properly

2024-05-29 Thread Jiri Denemark
On Tue, May 28, 2024 at 10:49:34 +0200, Michal Privoznik wrote:
> When attempting to run:
> 
>   libvirt.git/_build # ./run --selinux ./src/libvirtd
> 
> the following error is thrown:
> 
>   Refusing to change selinux context of file './src/libvirtd' outside build 
> directory
> 
> which is obviously wrong. The problem is 'being inside of build
> directory' is detected by simple progpath.startswith(builddir).
> While builddir is an absolute path, progpath isn't necessarily.
> 
> And while looking into the code, I've noticed chcon() function
> accessing variable outside its scope when printing out the path
> it's working on.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> v2 of:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UZ6F7MWEJBUMUUBODXGAXQW4NY2UEEVF/
> 
> diff to v1:
> - error out if binary to run can't be identified (i.e. 'which' returns
>   None).
> 
>  run.in | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/run.in b/run.in
> index 5b89b3dcd5..cada74dfcd 100644
> --- a/run.in
> +++ b/run.in
> @@ -138,7 +138,7 @@ def change_unit(name, action):
>  
>  
>  def chcon(path, user, role, type):
> -print("Setting file context of {} to u={}, r={}, 
> t={}...".format(progpath,
> +print("Setting file context of {} to u={}, r={}, t={}...".format(path,
>   user,
>   role,
>   type))
> @@ -187,6 +187,10 @@ else:
>  try:
>  dorestorecon = False
>  progpath = shutil.which(prog)
> +if not progpath:
> +raise Exception("Can't find executable {}"
> +.format(prog))
> +progpath = os.path.abspath(progpath)
>  if len(try_stop_units):
>  print("Temporarily stopping systemd units...")

You can drop the second (now unreachable) check a few lines later:

if not progpath:
raise Exception("Can't find executable {} for selinux labeling"
.format(prog))


Reviewed-by: Jiri Denemark 


Entering freeze for libvirt-10.4.0

2024-05-28 Thread Jiri Denemark
I have just tagged v10.4.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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


Re: [PATCH] qemucapabilitiestest: Add test data for qemu-9.1 dev cycle

2024-05-28 Thread Jiri Denemark
On Mon, May 27, 2024 at 16:26:44 +0200, Peter Krempa wrote:
> Add test data based on qemu commit v9.0.0-995-g60b54b67c6 on x86_64
> 
> Comparison to previous release:
> 
> Feature additions:
>  - 9.1 machine type added
>  - 'SierraForest' cpu type added
>  - 'SapphireRapids-v3-x86_64-cpu' added
>  - 'VFIO_MIGRATION' event added (and corresponding 'migration-events'
>bool for the device
>  - 'exit-on-error' argument for 'migrate-incoming' added
>  - 'sev-guest' gained 'legacy-vm-type' boolean
>  - cpu topology added 'module' fields
>  - 'query-machines' gained 'compat-props' boolean and it returns various

The end of the sentence is missing here.

>  - 'deprecated-props' argument for 'query-cpu-model-expansion' added
> 
> Deprecated removals:
>  - legacy non-shared-storage migration fully removed (config/stats)
>  - legacy migration compression fully removed
>  - RDMA support removed
>  - dropped 'nios2' field type from 'query-cpus-fast' return data
>fields under 'compat-props'
> 
> Note that this dump was done on a newer kernel version which resulted in
> the 'pcommit' feature being removed from the few test cases which depend
> on the real CPU flag dump.
> 
> Signed-off-by: Peter Krempa 
> ---
>  .../domaincapsdata/qemu_9.1.0-q35.x86_64.xml  |   290 +
>  .../domaincapsdata/qemu_9.1.0-tcg.x86_64.xml  |   289 +
>  tests/domaincapsdata/qemu_9.1.0.x86_64.xml|   290 +
>  .../caps_9.1.0_x86_64.replies | 43370 
>  .../caps_9.1.0_x86_64.xml |  3946 ++
>  ...host-model-fallback-kvm.x86_64-latest.args | 2 +-
>  ...host-model-fallback-tcg.x86_64-latest.args | 2 +-
>  ...cpu-host-model-features.x86_64-latest.args | 2 +-
>  .../cpu-host-model-kvm.x86_64-latest.args | 2 +-
>  ...st-model-nofallback-kvm.x86_64-latest.args | 2 +-
>  ...st-model-nofallback-tcg.x86_64-latest.args | 2 +-
>  .../cpu-host-model-tcg.x86_64-latest.args | 2 +-
>  12 files changed, 48192 insertions(+), 7 deletions(-)
>  create mode 100644 tests/domaincapsdata/qemu_9.1.0-q35.x86_64.xml
>  create mode 100644 tests/domaincapsdata/qemu_9.1.0-tcg.x86_64.xml
>  create mode 100644 tests/domaincapsdata/qemu_9.1.0.x86_64.xml
>  create mode 100644 tests/qemucapabilitiesdata/caps_9.1.0_x86_64.replies
>  create mode 100644 tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml

Reviewed-by: Jiri Denemark 


Re: [PATCH v5 31/30] network: use iif/oif instead of iifname/oifname in nftables rules

2024-05-27 Thread Jiri Denemark
On Wed, May 22, 2024 at 23:13:33 -0400, Laine Stump wrote:
> 
> iifname/oifname need to lookup the string that contains the name of
> the interface each time a packet is checked, while iif/oif compare the
> ifindex of the interface, which is included directly in the
> packet. Conveniently, the rule is created using the *name* of the
> interface (which gets converted to ifindex as the rule is added), so
> no extra work is required other than changing the commandline option.
> 
> If it was the case that the interface could be deleted and re-added
> during the life of the rule, we would have to use Xifname (since
> deleting and re-adding the interface would result in ifindex
> changing), but for our uses this never happens, so Xif works for us,
> and undoubtedly improves performance by at least 0.001%.
> 
> Signed-off-by: Laine Stump 
> ---
>  src/network/network_nftables.c| 28 +--
>  .../nat-default-linux.nftables| 12 
>  .../nat-ipv6-linux.nftables   | 24 
>  .../nat-ipv6-masquerade-linux.nftables| 24 
>  .../nat-many-ips-linux.nftables   | 20 ++---
>  .../nat-no-dhcp-linux.nftables| 24 
>  .../nat-tftp-linux.nftables   | 12 
>  .../route-default-linux.nftables  | 12 ----
>  8 files changed, 78 insertions(+), 78 deletions(-)

Reviewed-by: Jiri Denemark 


Re: [PATCH] NEWS: document nftables support in network driver

2024-05-27 Thread Jiri Denemark
On Thu, May 23, 2024 at 22:57:21 -0400, Laine Stump wrote:
> Signed-off-by: Laine Stump 
> ---
>  NEWS.rst | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index 42b0f88128..14505116b1 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -33,6 +33,16 @@ v10.4.0 (unreleased)
>  . This model is available from QEMU 8.2.0
>  onwards.
>  
> +  * network: use nftables to setup virtual network firewall rules
> +
> +The network driver can now use nftables rules for the virtual
> +network firewalls, rather than iptables. With the standard build
> +options, nftables is preferred over iptables (with fallback to
> +iptables if nftables isn't installed), but this can be modified at
> +build time, or at runtime via the firewall_backend setting in
> +network.conf. (NB: the nwfilter driver still uses
> +ebtables/iptables).
> +
>  * **Improvements**
>  
>  * **Bug fixes**

Reviewed-by: Jiri Denemark 


Re: [PATCH 4/4] NEWS: Mention migration/save bug on root_squash NFS

2024-05-23 Thread Jiri Denemark
On Wed, May 22, 2024 at 18:00:19 +0200, Peter Krempa wrote:
> Signed-off-by: Peter Krempa 
> ---
>  NEWS.rst | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index b6985980ba..4a532bb673 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -51,7 +51,14 @@ v10.4.0 (unreleased)
> 
>$ virsh help list
> 
> +  * qemu: Fix ``virsh save`` and migration when storage in question is 
> root_squashed NFS
> 
> +Attempting to save a VM to a root_squash NFS mount or migrating with 
> disks
> +hosted on such mount could, in some scenarios, result in error stating::
> +
> +  'Unknown error 255'
> +
> +The bug was introduced in `v10.1.0 (2024-03-01)`_.
> 

One more empty line is needed here. Before this patch were three empty
lines (too many) and now only one :-)

>  v10.3.0 (2024-05-02)
>  

Reviewed-by: Jiri Denemark 


Re: [PATCH 3/4] virFileOpenForked: Fix handling of return value from virSocketSendFD()

2024-05-23 Thread Jiri Denemark
On Wed, May 22, 2024 at 18:00:18 +0200, Peter Krempa wrote:
> Commit 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e (v10.0.0-185-g91f4ebbac8)
> changed the return value of virSocketSendFD() from 0 to 1 on success.
> 
> Unfortunately in 'virFileOpenForked' the return value was used to report
> the error back to the main process from the fork'd child. As process
> return codes are positive only, the code negates the value of 'ret' and
> reports it. This resulted in the parent thinking the process exited with
> failure:
> 
>  # virsh save avocado-vt-vm1 /mnt/save
>  error: Failed to save domain 'avocado-vt-vm1' to /mnt/save
>  error: Error from child process creating '/mnt/save': Unknown error 255
> 
> This error reproduces on NFS mounts with 'root_squash' enabled. I've
> also observed it in one specific migration case when root_squash NFS is
> used with following error:
> 
>   Failed to open file '/var/lib/libvirt/images/alpine.qcow2': Unknown error 
> 255'
> 
> To fix the issue the code is refactored so that it doesn't actually
> touch the 'ret' variable needlessly and assigns to it only on failure
> cases, which prevents the '1' to be propagated to the parent process as
> '255' after negating and storing in the process return code.
> 
> Fixes: 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e
> Resolves: https://issues.redhat.com/browse/RHEL-36721
> Signed-off-by: Peter Krempa 
> ---
>  src/util/virfile.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Jiri Denemark 


Re: [PATCH 1/4] virfile: Modernize definition of virFileOpenForked/virFileOpenForceOwnerMode/virFileOpenAs

2024-05-23 Thread Jiri Denemark
On Wed, May 22, 2024 at 18:00:16 +0200, Peter Krempa wrote:
> Declare one argument per line and one variable per line and use boolean
> operators at the end of the line rather than at the beginning.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/util/virfile.c | 36 +---
>  1 file changed, 25 insertions(+), 11 deletions(-)

Reviewed-by: Jiri Denemark 


Re: [PATCH 2/4] virGetGroupList: Refactor and fix callers

2024-05-23 Thread Jiri Denemark
On Wed, May 22, 2024 at 18:00:17 +0200, Peter Krempa wrote:
> Use contemporary style for declarations and automatic memory clearing
> for a helper string.
> 
> Since the function can't fail any more, remove any mention of returning
> errno and remove error checks from all callers.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/lxc/lxc_container.c |  4 ++--
>  src/security/security_dac.c |  7 +--
>  src/util/vircommand.c   |  3 +--
>  src/util/virfile.c  |  8 
>  src/util/virutil.c  | 16 
>  tests/commandtest.c |  5 ++---
>  tools/virt-login-shell-helper.c |  3 +--
>  7 files changed, 15 insertions(+), 31 deletions(-)
> 
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 652697890f..7e460544fb 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -2059,8 +2059,8 @@ static int lxcContainerChild(void *data)
>  /* TODO is it safe to call it here or should this call be moved in
>   * front of the clone() as otherwise there might be a risk for a
>   * deadlock */
> -if ((ngroups = virGetGroupList(virCommandGetUID(cmd), 
> virCommandGetGID(cmd),
> -   )) < 0)
> +ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
> +  );
>      goto cleanup;

Looks like leftover goto here.

> 
>  ret = 0;
...

Reviewed-by: Jiri Denemark 


Re: [PATCH] run.in: Detect binaries in builddir properly

2024-05-23 Thread Jiri Denemark
On Wed, May 22, 2024 at 17:31:54 +0200, Michal Privoznik wrote:
> When attempting to run:
> 
>   libvirt.git/_build # ./run --selinux ./src/libvirtd
> 
> the following error is thrown:
> 
>   Refusing to change selinux context of file './src/libvirtd' outside build 
> directory
> 
> which is obviously wrong. The problem is 'being inside of build
> directory' is detected by simple progpath.startswith(builddir).
> While builddir is an absolute path, progpath isn't necessarily.
> 
> And while looking into the code, I've noticed chcon() function
> accessing variable outside its scope when printing out the path
> it's working on.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  run.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/run.in b/run.in
> index 5b89b3dcd5..87cf39a920 100644
> --- a/run.in
> +++ b/run.in
> @@ -138,7 +138,7 @@ def change_unit(name, action):
>  
>  
>  def chcon(path, user, role, type):
> -print("Setting file context of {} to u={}, r={}, 
> t={}...".format(progpath,
> +print("Setting file context of {} to u={}, r={}, t={}...".format(path,
>   user,
>   role,
>   type))
> @@ -186,7 +186,7 @@ else:
>  
>  try:
>  dorestorecon = False
> -progpath = shutil.which(prog)
> +progpath = os.path.abspath(shutil.which(prog) or prog)
>  if len(try_stop_units):
>  print("Temporarily stopping systemd units...")

If shutil.which(prog) returns None, progpath should be set to None
rather than an absolute path for prog. Because if prog does not exist
abspath would still construct a path for it. And later on we would fail
to report and error about nonexistent binary.

Jirka


Re: [PATCH] virt-host-validate: Improve translatability of messages printed by 'virHostMsgCheck()'

2024-05-21 Thread Jiri Denemark
On Mon, May 20, 2024 at 15:00:26 +0200, Peter Krempa wrote:
> Move the word 'Checking' into the appropriate formatting strings and
> mark all outstanding ones for translation.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/637
> Signed-off-by: Peter Krempa 
> ---
>  tools/virt-host-validate-bhyve.c  |  2 +-
>  tools/virt-host-validate-ch.c |  2 +-
>  tools/virt-host-validate-common.c | 18 +-
>  tools/virt-host-validate-qemu.c   |  4 ++--
>  4 files changed, 13 insertions(+), 13 deletions(-)
> 
...
> diff --git a/tools/virt-host-validate-common.c 
> b/tools/virt-host-validate-common.c
> index c8a23e2e99..58d8dbbaa1 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c
> @@ -66,7 +66,7 @@ void virHostMsgCheck(const char *prefix,
>  msg = g_strdup_vprintf(format, args);
>  va_end(args);
> 
> -fprintf(stdout, _("%1$6s: Checking %2$-60s: "), prefix, msg);
> +fprintf(stdout, "%1$6s: %2$-60s: ", prefix, msg);
>  }
> 
>  static bool virHostMsgWantEscape(void)

Shouldn't the formatting string change from %2$-60s to %2$-69s since the
string will now contain the extra "Checking " prefix?

Jirka


Plans for 10.4.0 release (freeze on Tuesday 28 May)

2024-05-20 Thread Jiri Denemark
We are getting close to 10.4.0 release of libvirt. To aim for the
release on Monday 03 Jun I suggest entering the freeze on Tuesday 28
May and tagging RC2 on Thursday 30 May.

I hope this works for everyone.

Jirka


[PATCH] network: Register dnsmasq with resolved only when really requested

2024-05-09 Thread Jiri Denemark
An incorrect check for domainRegister caused the DNS server for a
virtual domain to be registered with systemd-resolved even if
register='no' attribute was present. Only omitting the attribute
completely would disable the registration.

Reported-by: Daniel P. Berrangé 
Signed-off-by: Jiri Denemark 
---
 src/network/bridge_driver.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d89700c6ee..e5f9ecf9e8 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2016,7 +2016,9 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
 dnsmasqStarted = true;
 
-if (def->domain && def->domainRegister && dnsServer) {
+if (def->domain &&
+def->domainRegister == VIR_TRISTATE_BOOL_YES &&
+dnsServer) {
 unsigned int link;
 int rc;
 
-- 
2.45.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Release of libvirt-10.3.0

2024-05-02 Thread Jiri Denemark
Somehow I forgot to make rc2 on Tuesday, but the only commits after rc1
at that time were translations so I guess it's not a big deal.

The 10.3.0 release of both libvirt and libvirt-python is tagged and
signed tarballs are available at

https://download.libvirt.org/
https://download.libvirt.org/python/

Thanks everybody who helped with this release by sending patches,
reviewing, testing, or providing feedback. Your work is greatly
appreciated.

* New features

  * qemu: Proper support for USB network device

USB address is now automatically assigned to USB network devices thus they
can be used without manual configuration.

  * conf: Introduce memReserve attribute to 

Some PCI devices have large non-prefetchable memory. This can be a problem
in case when such device needs to be hotplugged as the firmware can't
foresee such situation. The user thus can override the value calculated at
start to accomodate for such devices.

* Improvements

  * Improve validation of USB devices

Certain USB device types ('sound', 'fs', 'chr', 'ccid' and 'net') were not
properly handled in the check whether the VM config supports USB and thus
would result in poor error messages.

  * virsh: Fix behaviour of ``--name`` and ``--parent`` used together when 
listing checkpoint and snapshots

The ``checkpoint-list`` and ``snapshot-list`` commands would ignore the
``--name`` option to print only the name when used with ``--parent``.

  * Extend libvirt-guests to shutdown only persistent VMs

Users can now choose to shutdown only persistent VMs when the host is being
shut down.

* Bug fixes

  * qemu: Fix migration with custom XML

Libvirt 10.2.0 would sometimes complain about incompatible CPU definition
when trying to migrate or save a domain and passing a custom XML even
though such XML was properly generated as migratable. Hitting this bug
depends on the guest CPU definition and the host on which a particular
domain was running.

  * qemu: Fix TLS hostname verification failure in certain non-shared storage 
migration scenarios

In certain scenarios (parallel migration, newly also post-copy migration)
libvirt would wrongly pass an empty hostname to QEMU to be used for TLS
certificate hostname validation, which would result into failure of the
non-shared storage migration step::

 error: internal error: unable to execute QEMU command 'blockdev-add': 
Certificate does not match the hostname

  * Create OVS ports as transient

Libvirt now creates OVS ports as transient which prevents them from
reappearing or going stale on sudden reboots.

  * Clear OVS QoS settings when domain shuts down

Libvirt now clears QoS settings on domain shutdown, so they no longer pile
up in OVS database.

Enjoy.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH RESEND] NEWS: Document my contributions for upcoming release

2024-05-02 Thread Jiri Denemark
On Thu, May 02, 2024 at 08:52:17 +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik 
> ---
> 
> Rebased version of the patch sent earlier, because the file was changed
> meanwhile.
> 
>  NEWS.rst | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index 7ced82e3da..554ca559a2 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -22,6 +22,13 @@ v10.3.0 (unreleased)
>  USB address is now automatically assigned to USB network devices thus 
> they
>  can be used without manual configuration.
>  
> +  * conf: Introduce memReserve to 
> +
> +Some PCI devices have large non-prefetchable memory. This is not a 
> problem
> +for coldplug because firmware sets up such devices properly. But it may 
> be
> +a problem for hotplug devices. To resolve this, new ``memReserve``
> +attribute is introduced which allows overriding value computed by 
> firmware.
> +
>  * **Improvements**
>  
>* Improve validation of USB devices
> @@ -35,6 +42,11 @@ v10.3.0 (unreleased)
>  The ``checkpoint-list`` and ``snapshot-list`` commands would ignore the
>  ``--name`` option to print only the name when used with ``--parent``.
>  
> +  * Extend libvirt-guests to shutdown only persistent VMs
> +
> +Users can now chose to shutdown only persistent VMs when the host is 
> being

s/chose/choose/

> +shut down.
> +
>  * **Bug fixes**
>  
>* qemu: Fix migration with custom XML
> @@ -54,6 +66,16 @@ v10.3.0 (unreleased)
>  
>   error: internal error: unable to execute QEMU command 'blockdev-add': 
> Certificate does not match the hostname
>  
> +  * Create OVS ports as transient
> +
> +Libvirt now creates OVS ports as transient which prevents them from
> +reappearing or going stale on sudden reboots.
> +
> +  * Clear OVS QoS settings when domain shuts down
> +
> +Libvirt now clears QoS settings on domain shutdown, so they no longer 
> pile
> +up in OVS database.
> +
>  
>  v10.2.0 (2024-04-02)
>  

Reviewed-by: Jiri Denemark 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH 2/2] NEWS: Document TLS migration bug, usb-net support and two fixes

2024-04-30 Thread Jiri Denemark
On Tue, Apr 30, 2024 at 10:19:45 +0200, Peter Krempa wrote:
> Signed-off-by: Peter Krempa 
> ---
>  NEWS.rst | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index da3fdb21ac..6b92270c9d 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -17,8 +17,24 @@ v10.3.0 (unreleased)
> 
>  * **New features**
> 
> +  * qemu: Proper support for USB network device
> +
> +USB network devices now get properly assigned a USB address and thus can

"get properly assigned a USB address" is weird

> +be used without manual configuration.
> +
>  * **Improvements**
> 
> +  * Improve validation of USB devices
> +
> +Certain USB device types ('sound', 'fs', 'chr', 'ccid' and 'net') were 
> not
> +properly handled in the check whether the VM config supports USB and thus
> +would result in poor error messages.
> +
> +  * virsh: Fix behaviour of ``--name`` and ``--parent`` used together when 
> listing checkpoint and snapshots
> +
> +The ``checkpoint-list`` and ``snapshot-list`` commands would ignore the
> +``--name`` option to print only the name when used with ``--parent``.
> +
>  * **Bug fixes**
> 
>* qemu: Fix migration with custom XML
> @@ -29,6 +45,15 @@ v10.3.0 (unreleased)
>  depends on the guest CPU definition and the host on which a particular
>  domain was running.
> 
> +  * qemu: Fix TLS hostname verification failure in certain non-shared 
> storage migration scenarios
> +
> +In certain scenarios (parallel migration, newly also post-copy migration)
> +libvirt would wrongly pass an empty hostname to QEMU to be used for TLS
> +certificate hostname validation, which would result into failure of the
> +non-shared storage migration step::
> +
> + error: internal error: unable to execute QEMU command 'blockdev-add': 
> Certificate does not match the hostname
> +

Reviewed-by: Jiri Denemark 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 4/4] cpu_map: Drop 'mpx' from x86 cpu models

2024-04-29 Thread Jiri Denemark
From: Tim Wiederhake 

The mpx feature was removed from the corresponding qemu cpu models.
With mpx in the libvirt cpu models, libvirt believes the feature
to be implicitly enabled when creating qemu VMs, while in fact it is
disabled.

This became an issue when commit 94eacd5a5f introduced new vmx-*
features, of which some are dependent on mpx (see "feature_dependencies"
table in qemu target/i386/cpu.c), e.g. vmx-exit-clear-bndcfgs and
vmx-entry-load-bndcfgs. These features cannot be enabled by qemu
without also mpx being enabled, leading to the error message

error: Failed to create domain from testdomain.xml
error: operation failed: guest CPU doesn't match
specification: missing features: mpx,vmx-exit-clear-bndcfgs,
vmx-entry-load-bndcfgs

when trying to create a VM with a "host-model" cpu on a host that
does support mpx and the mentioned vmx-* features:


  ...
  
  ...


Resolve the issue by removing mpx from libvirt's cpu models as well.

Signed-off-by: Tim Wiederhake 
---
 src/cpu_map/x86_Cascadelake-Server-noTSX.xml| 2 +-
 src/cpu_map/x86_Cascadelake-Server.xml  | 2 +-
 src/cpu_map/x86_Icelake-Server-noTSX.xml| 2 +-
 src/cpu_map/x86_Icelake-Server.xml  | 2 +-
 src/cpu_map/x86_Skylake-Client-IBRS.xml | 2 +-
 src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml   | 2 +-
 src/cpu_map/x86_Skylake-Client.xml  | 2 +-
 src/cpu_map/x86_Skylake-Server-IBRS.xml | 2 +-
 src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml   | 2 +-
 src/cpu_map/x86_Skylake-Server.xml  | 2 +-
 tests/cputestdata/x86_64-cpuid-Core-i5-6600-guest.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i5-6600-host.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Core-i5-6600-json.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7600U-guest.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7600U-host.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7700-guest.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7700-host.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-7700-json.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8550U-guest.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8550U-host.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8550U-json.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8700-guest.xml   | 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8700-host.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Core-i7-8700-json.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-json.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-guest.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-host.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-json.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1245-v5-guest.xml| 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1245-v5-host.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E3-1245-v5-json.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-5115-guest.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-5115-host.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-5115-json.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6130-guest.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6130-host.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6130-json.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6148-guest.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6148-host.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Gold-6148-json.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-9242-guest.xml | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-9242-host.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-Xeon-Platinum-9242-json.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-baseline-Cascadelake+Icelake.xml | 1 +
 .../x86_64-cpuid-baseline-Cascadelake+Skylake-IBRS.xml  | 1 +
 tests/cputestdata/x86_64-cpuid-baseline-Cascadelake+Skylake.xml | 1 +
 .../x86_64-cpuid-baseline-Cooperlake+Cascadelake.xml| 1 +
 tests/cputestdata/x86_64-cpuid-baseline-Cooperlake+Icelake.xml  | 1 +
 .../cputestdata/x86_64-cpuid-baseline-Skylake-Client+Server.xml | 1 +
 tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml

[PATCH 1/4] conf: Change return value of some CPU feature APIs

2024-04-29 Thread Jiri Denemark
The virCPUDefAddFeatureInternal helper function only fails if it is
called with VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE, which is only used in
virCPUDefAddFeature. The other callers (virCPUDefUpdateFeature and
virCPUDefAddFeatureIfMissing) will never get anything but 0 from
virCPUDefAddFeatureInternal and their return type can be changed to
void.

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c  | 12 
 src/conf/cpu_conf.h  |  4 +--
 src/cpu/cpu_s390.c   |  7 ++---
 src/cpu/cpu_x86.c| 58 +---
 src/qemu/qemu_capabilities.c |  5 ++--
 src/qemu/qemu_domain.c   |  3 +-
 6 files changed, 34 insertions(+), 55 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 4dca7e57ec..dcc164d165 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -938,13 +938,13 @@ virCPUDefAddFeatureInternal(virCPUDef *def,
 return 0;
 }
 
-int
+void
 virCPUDefUpdateFeature(virCPUDef *def,
const char *name,
int policy)
 {
-return virCPUDefAddFeatureInternal(def, name, policy,
-   VIR_CPU_ADD_FEATURE_MODE_UPDATE);
+virCPUDefAddFeatureInternal(def, name, policy,
+VIR_CPU_ADD_FEATURE_MODE_UPDATE);
 }
 
 int
@@ -957,13 +957,13 @@ virCPUDefAddFeature(virCPUDef *def,
 }
 
 
-int
+void
 virCPUDefAddFeatureIfMissing(virCPUDef *def,
  const char *name,
  int policy)
 {
-return virCPUDefAddFeatureInternal(def, name, policy,
-   VIR_CPU_ADD_FEATURE_MODE_NEW);
+virCPUDefAddFeatureInternal(def, name, policy,
+VIR_CPU_ADD_FEATURE_MODE_NEW);
 }
 
 
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index b10c23ee82..f71d942ce6 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -245,12 +245,12 @@ virCPUDefAddFeature(virCPUDef *cpu,
 const char *name,
 int policy);
 
-int
+void
 virCPUDefUpdateFeature(virCPUDef *cpu,
const char *name,
int policy);
 
-int
+void
 virCPUDefAddFeatureIfMissing(virCPUDef *def,
  const char *name,
  int policy);
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index ffb1a2cf7b..13695ee97a 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -72,10 +72,9 @@ virCPUs390Update(virCPUDef *guest,
 virCPUDefCopyModel(updated, host, true);
 
 for (i = 0; i < guest->nfeatures; i++) {
-   if (virCPUDefUpdateFeature(updated,
-  guest->features[i].name,
-  guest->features[i].policy) < 0)
-   return -1;
+   virCPUDefUpdateFeature(updated,
+  guest->features[i].name,
+  guest->features[i].policy);
 }
 
 virCPUDefStealModel(guest, updated, false);
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 0fad761809..8770f52d15 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -834,23 +834,19 @@ x86DataAddSignature(virCPUx86Data *data,
  * mentioned in @cpu to make sure these features will always be explicitly
  * listed in the CPU definition.
  */
-static int
+static void
 virCPUx86DisableRemovedFeatures(virCPUDef *cpu,
 virCPUx86Model *model)
 {
 char **feat = model->removedFeatures;
 
 if (!feat)
-return 0;
+return;
 
 while (*feat) {
-if (virCPUDefAddFeatureIfMissing(cpu, *feat, VIR_CPU_FEATURE_DISABLE) 
< 0)
-return -1;
-
+virCPUDefAddFeatureIfMissing(cpu, *feat, VIR_CPU_FEATURE_DISABLE);
 feat++;
 }
-
-return 0;
 }
 
 
@@ -901,10 +897,8 @@ x86DataToCPU(const virCPUx86Data *data,
 x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_DISABLE, , map))
 return NULL;
 
-if (cpuType == VIR_CPU_TYPE_GUEST) {
-if (virCPUx86DisableRemovedFeatures(cpu, model) < 0)
-return NULL;
-}
+if (cpuType == VIR_CPU_TYPE_GUEST)
+virCPUx86DisableRemovedFeatures(cpu, model);
 
 cpu->type = cpuType;
 
@@ -2914,7 +2908,7 @@ virCPUx86Baseline(virCPUDef **cpus,
 }
 
 
-static int
+static void
 x86UpdateHostModel(virCPUDef *guest,
const virCPUDef *host)
 {
@@ -2931,18 +2925,15 @@ x86UpdateHostModel(virCPUDef *guest,
 }
 
 for (i = 0; i < guest->nfeatures; i++) {
-if (virCPUDefUpdateFeature(updated,
-   guest->features[i].name,
-   guest->features[i].policy) < 0)
-return -1;
+virCPUDefUpdateFeature(updated,
+   guest->features[i].name,
+   guest->features[i].policy);
 }
 
 virCPUDefStealMode

[PATCH 0/4] Enable removing features from CPU models and remove mpx

2024-04-29 Thread Jiri Denemark
See 3/4 for details.

Jiri Denemark (3):
  conf: Change return value of some CPU feature APIs
  cpu: Add removedPolicy parameter to virCPUUpdate
  qemu: Enable removing features from CPU models

Tim Wiederhake (1):
  cpu_map: Drop 'mpx' from x86 cpu models

 src/conf/cpu_conf.c   | 12 +--
 src/conf/cpu_conf.h   |  4 +-
 src/cpu/cpu.c | 10 ++-
 src/cpu/cpu.h |  6 +-
 src/cpu/cpu_arm.c |  3 +-
 src/cpu/cpu_loongarch.c   |  3 +-
 src/cpu/cpu_ppc64.c   |  3 +-
 src/cpu/cpu_riscv64.c |  3 +-
 src/cpu/cpu_s390.c| 10 +--
 src/cpu/cpu_x86.c | 83 +--
 src/cpu_map/x86_Cascadelake-Server-noTSX.xml  |  2 +-
 src/cpu_map/x86_Cascadelake-Server.xml|  2 +-
 src/cpu_map/x86_Icelake-Server-noTSX.xml  |  2 +-
 src/cpu_map/x86_Icelake-Server.xml|  2 +-
 src/cpu_map/x86_Skylake-Client-IBRS.xml   |  2 +-
 src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml |  2 +-
 src/cpu_map/x86_Skylake-Client.xml|  2 +-
 src/cpu_map/x86_Skylake-Server-IBRS.xml   |  2 +-
 src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml |  2 +-
 src/cpu_map/x86_Skylake-Server.xml|  2 +-
 src/qemu/qemu_capabilities.c  |  5 +-
 src/qemu/qemu_domain.c|  6 +-
 src/qemu/qemu_process.c   | 36 +++-
 tests/cputest.c   |  4 +-
 .../x86_64-cpuid-Core-i5-6600-guest.xml   |  1 +
 .../x86_64-cpuid-Core-i5-6600-host.xml|  1 +
 .../x86_64-cpuid-Core-i5-6600-json.xml|  1 +
 .../x86_64-cpuid-Core-i7-7600U-guest.xml  |  1 +
 .../x86_64-cpuid-Core-i7-7600U-host.xml   |  1 +
 .../x86_64-cpuid-Core-i7-7600U-json.xml   |  1 +
 .../x86_64-cpuid-Core-i7-7700-guest.xml   |  1 +
 .../x86_64-cpuid-Core-i7-7700-host.xml|  1 +
 .../x86_64-cpuid-Core-i7-7700-json.xml|  1 +
 .../x86_64-cpuid-Core-i7-8550U-guest.xml  |  1 +
 .../x86_64-cpuid-Core-i7-8550U-host.xml   |  1 +
 .../x86_64-cpuid-Core-i7-8550U-json.xml   |  1 +
 .../x86_64-cpuid-Core-i7-8700-guest.xml   |  1 +
 .../x86_64-cpuid-Core-i7-8700-host.xml|  1 +
 .../x86_64-cpuid-Core-i7-8700-json.xml|  1 +
 .../x86_64-cpuid-Ice-Lake-Server-guest.xml|  1 +
 .../x86_64-cpuid-Ice-Lake-Server-host.xml |  1 +
 .../x86_64-cpuid-Ice-Lake-Server-json.xml |  1 +
 .../x86_64-cpuid-Xeon-E3-1225-v5-guest.xml|  1 +
 .../x86_64-cpuid-Xeon-E3-1225-v5-host.xml |  1 +
 .../x86_64-cpuid-Xeon-E3-1225-v5-json.xml |  1 +
 .../x86_64-cpuid-Xeon-E3-1245-v5-guest.xml|  1 +
 .../x86_64-cpuid-Xeon-E3-1245-v5-host.xml |  1 +
 .../x86_64-cpuid-Xeon-E3-1245-v5-json.xml |  1 +
 .../x86_64-cpuid-Xeon-Gold-5115-guest.xml |  1 +
 .../x86_64-cpuid-Xeon-Gold-5115-host.xml  |  1 +
 .../x86_64-cpuid-Xeon-Gold-5115-json.xml  |  1 +
 .../x86_64-cpuid-Xeon-Gold-6130-guest.xml |  1 +
 .../x86_64-cpuid-Xeon-Gold-6130-host.xml  |  1 +
 .../x86_64-cpuid-Xeon-Gold-6130-json.xml  |  1 +
 .../x86_64-cpuid-Xeon-Gold-6148-guest.xml |  1 +
 .../x86_64-cpuid-Xeon-Gold-6148-host.xml  |  1 +
 .../x86_64-cpuid-Xeon-Gold-6148-json.xml  |  1 +
 .../x86_64-cpuid-Xeon-Platinum-8268-guest.xml |  1 +
 .../x86_64-cpuid-Xeon-Platinum-8268-host.xml  |  1 +
 .../x86_64-cpuid-Xeon-Platinum-8268-json.xml  |  1 +
 .../x86_64-cpuid-Xeon-Platinum-9242-guest.xml |  1 +
 .../x86_64-cpuid-Xeon-Platinum-9242-host.xml  |  1 +
 .../x86_64-cpuid-Xeon-Platinum-9242-json.xml  |  1 +
 ..._64-cpuid-baseline-Cascadelake+Icelake.xml |  1 +
 ...puid-baseline-Cascadelake+Skylake-IBRS.xml |  1 +
 ..._64-cpuid-baseline-Cascadelake+Skylake.xml |  1 +
 ...-cpuid-baseline-Cooperlake+Cascadelake.xml |  1 +
 ...6_64-cpuid-baseline-Cooperlake+Icelake.xml |  1 +
 ...4-cpuid-baseline-Skylake-Client+Server.xml |  1 +
 .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml  |  1 +
 tests/domaincapsdata/qemu_4.2.0.x86_64.xml|  1 +
 .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml  |  1 +
 tests/domaincapsdata/qemu_5.0.0.x86_64.xml|  1 +
 ...-Icelake-Server-pconfig.x86_64-latest.args |  2 +-
 ...-host-model-fallback-kvm.x86_64-4.2.0.args |  2 +-
 ...-host-model-fallback-kvm.x86_64-5.0.0.args |  2 +-
 .../cpu-host-model-kvm.x86_64-4.2.0.args  |  2 +-
 .../cpu-host-model-kvm.x86_64-5.0.0.args  |  2 +-
 ...ost-model-nofallback-kvm.x86_64-4.2.0.args |  2 +-
 ...ost-model-nofallback-kvm.x86_64-5.0.0.args |  2 +-
 80 files changed, 174 insertions(+), 97 deletions(-)

-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 2/4] cpu: Add removedPolicy parameter to virCPUUpdate

2024-04-29 Thread Jiri Denemark
virCPUUpdate check the CPU definition for features that were marked as
removed in the specified CPU model and explicitly adds those that were
not mentioned in the definition. So far such features were added with
VIR_CPU_FEATURE_DISABLE policy, but the caller may want to use a
different policy in some situations, which is now possible via the
removedPolicy parameter.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c   | 10 +++---
 src/cpu/cpu.h   |  6 --
 src/cpu/cpu_arm.c   |  3 ++-
 src/cpu/cpu_loongarch.c |  3 ++-
 src/cpu/cpu_ppc64.c |  3 ++-
 src/cpu/cpu_riscv64.c   |  3 ++-
 src/cpu/cpu_s390.c  |  3 ++-
 src/cpu/cpu_x86.c   | 31 ---
 src/qemu/qemu_domain.c  |  3 ++-
 src/qemu/qemu_process.c |  6 --
 tests/cputest.c |  4 ++--
 11 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 33701811fb..a2518d7cc7 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -560,19 +560,23 @@ virCPUBaseline(virArch arch,
  * @arch: CPU architecture
  * @guest: guest CPU definition to be updated
  * @host: host CPU definition
+ * @removedPolicy: default policy for features removed from the CPU model
  *
  * Updates @guest CPU definition possibly taking @host CPU into account. This
  * is required for maintaining compatibility with older libvirt releases or to
  * support guest CPU definitions specified relatively to host CPU, such as CPUs
  * with VIR_CPU_MODE_CUSTOM and optional features or VIR_CPU_MATCH_MINIMUM, or
- * CPUs with VIR_CPU_MODE_HOST_MODEL.
+ * CPUs with VIR_CPU_MODE_HOST_MODEL. If @guest CPU uses a CPU model which
+ * specifies some features as removed, such features that were not already
+ * present in the @guest CPU definition will be added there with 
@removedPolicy.
  *
  * Returns 0 on success, -1 on error.
  */
 int
 virCPUUpdate(virArch arch,
  virCPUDef *guest,
- const virCPUDef *host)
+ const virCPUDef *host,
+ virCPUFeaturePolicy removedPolicy)
 {
 struct cpuArchDriver *driver;
 bool relative;
@@ -622,7 +626,7 @@ virCPUUpdate(virArch arch,
 return -1;
 }
 
-if (driver->update(guest, host, relative) < 0)
+if (driver->update(guest, host, relative, removedPolicy) < 0)
 return -1;
 
 VIR_DEBUG("model=%s", NULLSTR(guest->model));
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index a4cdb37f03..d092b4f3f0 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -81,7 +81,8 @@ typedef virCPUDef *
 typedef int
 (*virCPUArchUpdate)(virCPUDef *guest,
 const virCPUDef *host,
-bool relative);
+bool relative,
+virCPUFeaturePolicy removedPolicy);
 
 typedef int
 (*virCPUArchUpdateLive)(virCPUDef *cpu,
@@ -229,7 +230,8 @@ virCPUBaseline(virArch arch,
 int
 virCPUUpdate(virArch arch,
  virCPUDef *guest,
- const virCPUDef *host)
+ const virCPUDef *host,
+ virCPUFeaturePolicy removedPolicy)
 ATTRIBUTE_NONNULL(2);
 
 int
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 324c701e81..7a9c8163c8 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -448,7 +448,8 @@ virCPUarmGetMap(void)
 static int
 virCPUarmUpdate(virCPUDef *guest,
 const virCPUDef *host,
-bool relative)
+bool relative,
+virCPUFeaturePolicy removedPolicy G_GNUC_UNUSED)
 {
 g_autoptr(virCPUDef) updated = virCPUDefCopyWithoutModel(guest);
 
diff --git a/src/cpu/cpu_loongarch.c b/src/cpu/cpu_loongarch.c
index 78d9941320..0e2b0b5848 100644
--- a/src/cpu/cpu_loongarch.c
+++ b/src/cpu/cpu_loongarch.c
@@ -39,7 +39,8 @@ virCPULoongArchCompare(virCPUDef *host G_GNUC_UNUSED,
 static int
 virCPULoongArchUpdate(virCPUDef *guest G_GNUC_UNUSED,
   const virCPUDef *host G_GNUC_UNUSED,
-  bool relative G_GNUC_UNUSED)
+  bool relative G_GNUC_UNUSED,
+  virCPUFeaturePolicy removedPolicy G_GNUC_UNUSED)
 {
 return 0;
 }
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 448a0a7d85..13f5fc9c2c 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -654,7 +654,8 @@ virCPUppc64GetHost(virCPUDef *cpu,
 static int
 virCPUppc64Update(virCPUDef *guest,
   const virCPUDef *host G_GNUC_UNUSED,
-  bool relative G_GNUC_UNUSED)
+  bool relative G_GNUC_UNUSED,
+  virCPUFeaturePolicy removedPolicy G_GNUC_UNUSED)
 {
 /*
  * - host-passthrough doesn't even get here
diff --git a/src/cpu/cpu_riscv64.c b/src/cpu/cpu_riscv64.c
index 4cb795f849..276c80a401 100644
--- a/src/cpu/cpu_riscv64.c
+++ b/src/cpu/cpu_riscv64.c
@@ -49,7 +49,8 @@ virCPURiscv64ValidateFeatures(virCPUDef *cpu G_GNUC_UNUSED)
 static int
 virCPURiscv64Update(virCPUDef *guest,
 const virCPUDef *host,

Entering freeze for libvirt-10.3.0

2024-04-26 Thread Jiri Denemark
I have just tagged v10.3.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH] qemu: migration: Don't use empty string for 'tls-hostname' NBD blockdev

2024-04-24 Thread Jiri Denemark
On Fri, Apr 19, 2024 at 16:29:47 +0200, Peter Krempa wrote:
> While QEMU accepts and interprets an empty string in the tls-hostname
> field in migration parametes as if it's unset, the same does not apply
> for the 'tls-hostname' field when 'blockdev-add'-ing a NBD backend for
> non-shared storage migration.
> 
> When libvirt sets up migation with TLS in 'qemuMigrationParamsEnableTLS'
> the QEMU_MIGRATION_PARAM_TLS_HOSTNAME migration parameter will be set to
> empty string in case when the 'hostname' argument is passed as NULL.
> 
> Later on when setting up the NBD connections for non-shared storage
> migration 'qemuMigrationParamsGetTLSHostname', which fetches the value
> of the aforementioned TLS parameter.
> 
> This bug was mostly latent until recently as libvirt used
> MIGRATION_DEST_CONNECT_HOST mode in most cases which required the
> hostname to be passed, thus the parameter was set properly.
> 
> This changed with 8d693d79c40 for post-copy migration, where libvirt now
> instructs qemu to connect and thus passes NULL hostname to
> qemuMigrationParamsEnableTLS, which in turn causes libvirt to try to
> add NBD connection with empty string as tls-hostname resulting in:
> 
>   error: internal error: unable to execute QEMU command 'blockdev-add': 
> Certificate does not match the hostname
> 
> To address this modify 'qemuMigrationParamsGetTLSHostname' to undo the
> weird semantics the migration code uses to handle TLS hostname and make
> it return NULL if the hostname is an empty string.
> 
> Fixes: e8fa09d66bc
> Resolves: https://issues.redhat.com/browse/RHEL-32880
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_migration_params.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Jiri Denemark 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


RFC: Drop micro part of our release versioning scheme

2024-04-24 Thread Jiri Denemark
Hi,

Does anyone feel strongly against dropping the "micro" part from
libvirt(-python) versions? I think the original idea was to use this
number for maintenance releases in -maint branches, but we stopped doing
those a long time ago (v3.2.1 was the last and most likely even the only
release with micro > 0 since the change in numbering libvirt releases).
So the micro part looks quite useless, not to mention I am lazy to type
the .0 suffix all the time :-)

And if we decide to drop it, what would be the right time? This 10.3
release, the following 10.4 release or should we wait until 11.0?
Personally I'd do it just now, but someone might be relying on the
numbers and would prefer to know about such change in advance to prepare
for it. So perhaps 10.4 or the most conservative 11.0 would be better
options.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Plans for 10.3.0 release (freeze on Friday 26 Apr)

2024-04-19 Thread Jiri Denemark
We are getting close to 10.3.0 release of libvirt. To aim for the
release on Thursday 02 May I suggest entering the freeze on Friday
26 Apr and tagging RC2 on Tuesday 30 Apr.

I hope this works for everyone.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 4/4] qemu: Change return type of qemuDomainFixupCPUs to void

2024-04-17 Thread Jiri Denemark
The function never fails.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 2:
- virCPUDefCopy never returns NULL
- do not remove !*origCPU check in qemuDomainFixupCPUs
- make sure origCPU is set when reconnecting to running domains

 src/qemu/qemu_domain.c  | 12 
 src/qemu/qemu_domain.h  |  2 +-
 src/qemu/qemu_process.c |  8 +++-
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3dfabfda2f..3469f0d40c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11050,29 +11050,27 @@ qemuDomainUpdateCPU(virDomainObj *vm,
  *
  * This function can only be used on an active domain or when restoring a
  * domain which was running.
- *
- * Returns 0 on success, -1 on error.
  */
-int
+void
 qemuDomainFixupCPUs(virDomainObj *vm,
 virCPUDef **origCPU)
 {
 virArch arch = vm->def->os.arch;
 
 if (!ARCH_IS_X86(arch))
-return 0;
+return;
 
 if (!vm->def->cpu ||
 vm->def->cpu->mode != VIR_CPU_MODE_CUSTOM ||
 !vm->def->cpu->model)
-return 0;
+return;
 
 /* 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;
+return;
 
 if (virCPUDefFindFeature(vm->def->cpu, "cmt")) {
 g_autoptr(virCPUDef) fixedCPU = 
virCPUDefCopyWithoutModel(vm->def->cpu);
@@ -11093,8 +11091,6 @@ qemuDomainFixupCPUs(virDomainObj *vm,
 virCPUDefFree(*origCPU);
 *origCPU = g_steal_pointer();
 }
-
-return 0;
 }
 
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 264817eef9..a3089ea449 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -985,7 +985,7 @@ qemuDomainUpdateCPU(virDomainObj *vm,
 virCPUDef *cpu,
 virCPUDef **origCPU);
 
-int
+void
 qemuDomainFixupCPUs(virDomainObj *vm,
 virCPUDef **origCPU);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7fdb9ac23a..686f6ed6c1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8278,9 +8278,8 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
 /* No cookie means libvirt which saved the domain was too old to mess up
  * the CPU definitions.
  */
-if (cookie &&
-qemuDomainFixupCPUs(vm, >cpu) < 0)
-return -1;
+if (cookie)
+qemuDomainFixupCPUs(vm, >cpu);
 
 if (cookie && !cookie->slirpHelper)
 priv->disableSlirp = true;
@@ -8926,8 +8925,7 @@ qemuProcessRefreshCPU(virQEMUDriver *driver,
  * case the host-model is known to not contain features which QEMU
  * doesn't know about.
  */
-if (qemuDomainFixupCPUs(vm, >origCPU) < 0)
-return -1;
+qemuDomainFixupCPUs(vm, >origCPU);
 }
 
 return 0;
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 3/4] qemu: Change return type of qemuDomainUpdateCPU to void

2024-04-17 Thread Jiri Denemark
The function never fails.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_domain.c  | 12 
 src/qemu/qemu_domain.h  |  2 +-
 src/qemu/qemu_process.c |  8 +++-
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d91c7d478b..3dfabfda2f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11002,10 +11002,8 @@ virSaveCookieCallbacks virQEMUDriverDomainSaveCookie = 
{
  * 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.
  */
-int
+void
 qemuDomainUpdateCPU(virDomainObj *vm,
 virCPUDef *cpu,
 virCPUDef **origCPU)
@@ -11015,14 +11013,14 @@ qemuDomainUpdateCPU(virDomainObj *vm,
 *origCPU = NULL;
 
 if (!vm->def->cpu)
-return 0;
+return;
 
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
-return 0;
+return;
 
 /* 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;
+return;
 
 VIR_DEBUG("Replacing CPU definition");
 
@@ -11032,8 +11030,6 @@ qemuDomainUpdateCPU(virDomainObj *vm,
 vm->def->cpu = virCPUDefCopy(cpu);
 else
 vm->def->cpu = virCPUDefCopy(*origCPU);
-
-return 0;
 }
 
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index bd37cb245a..264817eef9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -980,7 +980,7 @@ virStorageSource *qemuDomainGetStorageSourceByDevstr(const 
char *devstr,
virDomainDef *def,
virDomainBackupDef 
*backupdef);
 
-int
+void
 qemuDomainUpdateCPU(virDomainObj *vm,
 virCPUDef *cpu,
 virCPUDef **origCPU);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f3e295ccf0..7fdb9ac23a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5674,8 +5674,7 @@ qemuProcessInit(virQEMUDriver *driver,
 if (qemuProcessPrepareQEMUCaps(vm, driver->qemuCapsCache) < 0)
 return -1;
 
-if (qemuDomainUpdateCPU(vm, updatedCPU, ) < 0)
-return -1;
+qemuDomainUpdateCPU(vm, updatedCPU, );
 
 if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, flags) < 0)
 return -1;
@@ -9138,9 +9137,8 @@ qemuProcessReconnect(void *opaque)
 qemuDomainVcpuPersistOrder(obj->def);
 
 /* Make sure priv->origCPU is always set. */
-if (!priv->origCPU &&
-qemuDomainUpdateCPU(obj, NULL, >origCPU) < 0)
-goto error;
+if (!priv->origCPU)
+qemuDomainUpdateCPU(obj, NULL, >origCPU);
 
 if (qemuProcessRefreshCPU(driver, obj) < 0)
 goto error;
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 2/4] NEWS: Mention migration bug with custom XML

2024-04-17 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
Reviewed-by: Peter Krempa 
---
 NEWS.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index c108e66f46..852dadf532 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -21,6 +21,14 @@ v10.3.0 (unreleased)
 
 * **Bug fixes**
 
+  * qemu: Fix migration with custom XML
+
+Libvirt 10.2.0 would sometimes complain about incompatible CPU definition
+when trying to migrate or save a domain and passing a custom XML even
+though such XML was properly generated as migratable. Hitting this bug
+depends on the guest CPU definition and the host on which a particular
+domain was running.
+
 
 v10.2.0 (2024-04-02)
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 1/4] qemu: Fix migration with custom XML

2024-04-17 Thread Jiri Denemark
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 
Tested-by: Han Han 
---

Notes:
Version 2:
- virCPUDefCopy never returns NULL
- do not remove !*origCPU check in qemuDomainFixupCPUs
- make sure origCPU is set when reconnecting to running domains

 src/qemu/qemu_domain.c  | 33 -
 src/qemu/qemu_process.c | 19 +++
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b869797a8..d91c7d478b 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,18 +11014,24 @@ 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)))
-return -1;
+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;
+
+VIR_DEBUG("Replacing CPU definition");
 
-VIR_DEBUG("Replacing CPU def with the updated one");
+*origCPU = g_steal_pointer(>def->cpu);
 
-*origCPU = vm->def->cpu;
-vm->def->cpu = cpu;
+if (cpu)
+vm->def->cpu = virCPUDefCopy(cpu);
+else
+vm->def->cpu = virCPUDefCopy(*origCPU);
 
 return 0;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e4bcb628cf..f3e295ccf0 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();
 
+if (rc == 0)
 def->cpu->check = VIR_CPU_CHECK_FULL;
-}
 
 return 0;
 }
@@ -9147,6 +9137,11 @@ qemuProcessReconnect(void *opaque)
 
 qemuDomainVcpuPersistOrder(obj->def);
 
+/* Make sure priv->origCPU is always set. */
+if (!priv->origCPU &&
+qemuDomainUpdateCPU(obj, NULL, >origCPU) < 0)
+goto error;
+
 if (qemuProcess

[PATCH v2 0/4] qemu: Fix migration with custom XML

2024-04-17 Thread Jiri Denemark
Version 2:
- see patch 1/4 for more details
- added two cleanups

Jiri Denemark (4):
  qemu: Fix migration with custom XML
  NEWS: Mention migration bug with custom XML
  qemu: Change return type of qemuDomainUpdateCPU to void
  qemu: Change return type of qemuDomainFixupCPUs to void

 NEWS.rst|  8 +++
 src/qemu/qemu_domain.c  | 51 -
 src/qemu/qemu_domain.h  |  4 ++--
 src/qemu/qemu_process.c | 29 ---
 4 files changed, 45 insertions(+), 47 deletions(-)

-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH 1/2] qemu: Fix migration with custom XML

2024-04-17 Thread Jiri Denemark
On Wed, Apr 17, 2024 at 09:15:39 +0200, Peter Krempa wrote:
> On Tue, Apr 16, 2024 at 19:53:25 +0200, Jiri Denemark 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 
> > ---
> >  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
> > @@ -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;
> 
> 
> First two reads were very confusing as this is overwriting the argument
> 
> 
> >  
> > -VIR_DEBUG("Replacing CPU def with the updated one");
> > +VIR_DEBUG("Replacing CPU definition");
> >  
> >  *origCPU = vm->def->cpu;
> >  vm->def->cpu = cpu;
> 
> ... just to store it here. At this point it's no longer necessary. You
> can first move the old def into 'origCPU' and then directly assign into
> the variable to avoid the overwrite for readability.

Oh right, virCPUDefCopy can never return NULL.

> 
> > @@ -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;
> 
> This function is called from two places:
> 
> src/qemu/qemu_process.c=qemuProcessStartWithMemoryState(virConnectPtr conn,
> src/qemu/qemu_process.c:qemuDomainFixupCPUs(vm, >cpu) < 0)

Oops, nice catch. Can I pretend I added this hunk to check if reviewers
pay enough attention? :-) The call is guarded by cookie != NULL, but no
check for cookie->cpu is done. I'll drop this hunk.

> src/qemu/qemu_process.c=qemuProcessRefreshCPU(virQEMUDriver *driver,
> src/qemu/qemu_process.c:if (qemuDomainFixupCPUs(vm, >origCPU) < > 0
> 
> 'priv->origCPU' which is used in the latter is AFAIU filled only in
> 'qemuDomainUpdateCPU' called when starting up a qemu process and in
> 'qemuDomainObjPrivateXMLParse' when loading a status XML.
> 
> In both cases there are code paths which may leave 'priv->origCPU' to be
> NULL.
> 
> The following code will dereference it unconditionally:
> 
>   if (virCPUDefFindFeature(*origCPU, "cmt")) { 

This specific issue would not exist if I didn't forget to update one
more place... see below.

> > 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
> > @@ -4449

Re: [PATCH 2/2] NEWS: Mention migration bug with custom XML

2024-04-17 Thread Jiri Denemark
On Wed, Apr 17, 2024 at 09:17:31 +0200, Peter Krempa wrote:
> On Tue, Apr 16, 2024 at 19:53:26 +0200, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark 
> > ---
> >  NEWS.rst | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/NEWS.rst b/NEWS.rst
> > index c108e66f46..852dadf532 100644
> > --- a/NEWS.rst
> > +++ b/NEWS.rst
> > @@ -21,6 +21,14 @@ v10.3.0 (unreleased)
> >  
> >  * **Bug fixes**
> >  
> > +  * qemu: Fix migration with custom XML
> 
> custom migration XML perhaps ? or custom definition for migration? Each
> VM xml is custom ;)

Yeah, I didn't like too many "migration" words on a single line :-)

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 2/2] NEWS: Mention migration bug with custom XML

2024-04-16 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 NEWS.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index c108e66f46..852dadf532 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -21,6 +21,14 @@ v10.3.0 (unreleased)
 
 * **Bug fixes**
 
+  * qemu: Fix migration with custom XML
+
+Libvirt 10.2.0 would sometimes complain about incompatible CPU definition
+when trying to migrate or save a domain and passing a custom XML even
+though such XML was properly generated as migratable. Hitting this bug
+depends on the guest CPU definition and the host on which a particular
+domain was running.
+
 
 v10.2.0 (2024-04-02)
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 1/2] qemu: Fix migration with custom XML

2024-04-16 Thread Jiri Denemark
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 
---
 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();
 
+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


[PATCH 0/2] qemu: Fix migration with custom XML

2024-04-16 Thread Jiri Denemark
Jiri Denemark (2):
  qemu: Fix migration with custom XML
  NEWS: Mention migration bug with custom XML

 NEWS.rst|  8 
 src/qemu/qemu_domain.c  | 38 +-
 src/qemu/qemu_process.c | 14 ++
 3 files changed, 31 insertions(+), 29 deletions(-)

-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH] qemu: Use g_autoptr in qemuProcessInit

2024-04-16 Thread Jiri Denemark
The only thing we need to free in the cleanup code is virCPUDef and for
that we already have g_autoptr handler.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8145205fa8..e4bcb628cf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5657,8 +5657,7 @@ qemuProcessInit(virQEMUDriver *driver,
 {
 qemuDomainObjPrivate *priv = vm->privateData;
 int stopFlags;
-virCPUDef *origCPU = NULL;
-int ret = -1;
+g_autoptr(virCPUDef) origCPU = NULL;
 
 VIR_DEBUG("vm=%p name=%s id=%d migration=%d",
   vm, vm->def->name, vm->def->id, migration);
@@ -5668,7 +5667,7 @@ qemuProcessInit(virQEMUDriver *driver,
 if (virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("VM is already active"));
-goto cleanup;
+return -1;
 }
 
 /* in case when the post parse callback failed we need to re-run it on the
@@ -5678,18 +5677,18 @@ qemuProcessInit(virQEMUDriver *driver,
 
 /* we don't have the private copy of qemuCaps at this point */
 if (virDomainDefPostParse(vm->def, 0, driver->xmlopt, NULL) < 0)
-goto cleanup;
+return -1;
 }
 
 VIR_DEBUG("Determining emulator version");
 if (qemuProcessPrepareQEMUCaps(vm, driver->qemuCapsCache) < 0)
-goto cleanup;
+return -1;
 
 if (qemuDomainUpdateCPU(vm, updatedCPU, ) < 0)
-goto cleanup;
+return -1;
 
 if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, flags) < 0)
-goto cleanup;
+return -1;
 
 /* Do this upfront, so any part of the startup process can add
  * runtime state to vm->def that won't be persisted. This let's us
@@ -5697,12 +5696,12 @@ qemuProcessInit(virQEMUDriver *driver,
  */
 VIR_DEBUG("Setting current domain def as transient");
 if (virDomainObjSetDefTransient(driver->xmlopt, vm, priv->qemuCaps) < 0)
-goto cleanup;
+return -1;
 
 if (flags & VIR_QEMU_PROCESS_START_PRETEND) {
 if (qemuDomainSetPrivatePaths(driver, vm) < 0) {
 virDomainObjRemoveTransientDef(vm);
-goto cleanup;
+return -1;
 }
 } else {
 vm->def->id = qemuDriverAllocateID(driver);
@@ -5724,18 +5723,14 @@ qemuProcessInit(virQEMUDriver *driver,
 priv->origCPU = g_steal_pointer();
 }
 
-ret = 0;
-
- cleanup:
-virCPUDefFree(origCPU);
-return ret;
+return 0;
 
  stop:
 stopFlags = VIR_QEMU_PROCESS_STOP_NO_RELABEL;
 if (migration)
 stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
 qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, 
stopFlags);
-goto cleanup;
+return -1;
 }
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH] qemusecuritytest: Call real virFileExists in mock

2024-04-09 Thread Jiri Denemark
On Tue, Apr 09, 2024 at 10:41:37 +0200, Michal Privoznik wrote:
> When I suggested to Jim to call real virFileExists() I forgot to
> also suggest calling init_syms(). Without it, real_virFileExists
> pointer might be left unset. And indeed, that's what we were
> seeing on FreeBSD.
> 
> This effectively reverts commit 4b5cc57ed35dc24d11673dd3f04bfb8073c0340d.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Green pipeline:
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1245457856
> 
> Okay, it doesn't test FreeBSD, but it tests x86_64-ubuntu-2204-clang
> which was also experiencing the failure:
> 
> https://gitlab.com/libvirt/libvirt/-/jobs/6574951734
> 
>  tests/qemusecuritymock.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Jiri Denemark 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Release of libvirt-10.2.0

2024-04-02 Thread Jiri Denemark
The 10.2.0 release of both libvirt and libvirt-python is tagged and
signed tarballs are available at

https://download.libvirt.org/
https://download.libvirt.org/python/

Thanks everybody who helped with this release by sending patches,
reviewing, testing, or providing feedback. Your work is greatly
appreciated.

* New features

  * ch: Basic save and restore support for ch driver

The ch driver now supports basic save and restore operations. This is
functional on domains without any network, host device config defined.
The `path` parameter for save and restore should be a directory.

  * qemu: Support for driver type ``mtp`` in  devices

The ``mtp`` driver type exposes the ``usb-mtp`` device in QEMU. The
guest can access files on this driver through the Media Transfer
Protocol (MTP).

  * qemu: Added support for the loongarch64 architecture

It is now possible for libvirt to run loongarch64 guests, including on
other architectures via TCG. For the best results, it is recommended to
use the upcoming QEMU 9.0.0 release together with the development version
of edk2.

  * qemu: Introduce virDomainGraphicsReload API

Reloading the graphics display is now supported for QEMU guests using
VNC. This is useful to make QEMU reload the TLS certificates without
restarting the guest. Available via the ``virDomainGraphicsReload`` API
and the ``domdisplay-reload`` virsh command.

* Bug fixes

  * qemu: Fix migration from libvirt older than 9.10.0 when vmx is enabled

A domain with vmx feature enabled (which may be even done automatically
with ``mode='host-model'``) started by libvirt 9.9.0 or older cannot be
migrated to libvirt 9.10.0, 10.0.0, and 10.1.0 as the target host would
complain about a lot of extra ``vmx-*`` features. Migration of similar
domains started by the affected releases to libvirt 9.9.0 and older
does not work either. Since libvirt 10.2.0 migration works again with
libvirt 9.9.0 and older in both directions. Migration from the affected
releases to 10.2.0 works as well, but the other direction remains broken
unless the fix is backported.

  * node_device: Don't report spurious errors from PCI VPD parsing

In last release the PCI Vital Product Data parser was enhanced to report
errors but that effort failed as some kernels have the file but don't allow
reading it causing logs to be spammed with::

  libvirtd[21055]: operation failed: failed to read the PCI VPD data

Since the data is used only in the node device XML and errors are ignored if
the parsing failed, this release removes all the error reporting.

  * qemu: set correct SELinux label for unprivileged virtiofsd

It is now possible to use virtiofsd-based  shares even
if the guest is confined using SELinux.

  * qemu: fix a crash on unprivileged virtiofsd hotplug

Hotplugging virtiofsd-based filesystems works now.

  * virt-admin: Fix segfault when libvirtd dies

``virt-admin`` no longer crashes when ``libvirtd`` unexpectedly closes
the connection.

Enjoy.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


libvirt-10.2.0 release candidate 2

2024-03-28 Thread Jiri Denemark
I have just tagged v10.2.0-rc2 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Entering freeze for libvirt-10.2.0

2024-03-25 Thread Jiri Denemark
I have just tagged v10.2.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/

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
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Plans for 10.2.0 release (freeze on Monday 25 Mar)

2024-03-19 Thread Jiri Denemark
We are getting close to 10.2.0 release of libvirt. To aim for the
release on Tuesday 02 Apr I suggest entering the freeze on Monday 25
Mar and tagging RC2 on Thursday 28 Mar.

I hope this works for everyone.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH] virt-admin: Fix segfault when libvirtd dies

2024-03-19 Thread Jiri Denemark
On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
> On 3/19/24 12:02, Adam Julis wrote:
> > vshAdmCatchDisconnect requires non-NULL structure vshControl for
> > getting connection name (stored at opaque), but
> > virAdmConnectRegisterCloseCallback at vshAdmConnect called it
> > with NULL.
> > 
> > Signed-off-by: Adam Julis 
> > ---
> >  tools/virt-admin.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> > index 37bc6fe4f0..0766032e4a 100644
> > --- a/tools/virt-admin.c
> > +++ b/tools/virt-admin.c
> > @@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> >  return -1;
> >  } else {
> >  if (virAdmConnectRegisterCloseCallback(priv->conn, 
> > vshAdmCatchDisconnect,
> > -   NULL, NULL) < 0)
> > +   ctl, NULL) < 0)
> >  vshError(ctl, "%s", _("Unable to register disconnect 
> > callback"));
> >  
> >  if (priv->wantReconnect)
> 
> Hi,
> 
> if that is the case I would then expect that 
> virAdmConnectRegisterCloseCallback() needs to also be updated with:
> 
> +virCheckNonNullArgGoto(opaque, error);
> 
> or something like that. Is there a reason why it isn't? We better catch the 
> error early if the API is used wrongly.

Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but other
callbacks registered with virAdmConnectRegisterCloseCallback may not
need any opaque data.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [libvirt PATCH 00/12] Add vmx-* features from qemu

2024-03-15 Thread Jiri Denemark
On Thu, Mar 14, 2024 at 16:51:31 -0400, Sergio Durigan Junior wrote:
> On Wednesday, March 13 2024, I wrote:
> 
> > On Tuesday, March 12 2024, Jiri Denemark wrote:
> >
> >> On Wed, Jan 24, 2024 at 22:19:56 -0500, Sergio Durigan Junior wrote:
> >>> On Monday, January 22 2024, I wrote:
> >>> 
> >>> > On Wednesday, November 22 2023, Jiri Denemark wrote:
> >>> >
> >>> >> On Thu, Nov 09, 2023 at 15:30:46 +0100, Tim Wiederhake wrote:
> >>> >>> For rationale, see patch 3 (cpu_map: No longer ignore vmx- features in
> >>> >>> sync_qemu_features_i386.py).
> >>> >>> 
> >>> >>> Adding features in bunches (one patch per msr index), as this series
> >>> >>> is adding ~100 features.
> >>> >>> 
> >>> >>> This also adds cpu features "gds-no" and "amx-complex" and brings
> >>> >>> libvirt in sync with qemu commit ad6ef0a42e.
> >>> >>> 
> >>> >>> Tim Wiederhake (12):
> >>> >>>   cpu_map: Add missing feature "gds-no"
> >>> >>>   cpu_map: Add missing feature "amx-complex"
> >>> >>>   cpu_map: No longer ignore vmx- features in 
> >>> >>> sync_qemu_features_i386.py
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x480
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x485
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x48B
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x48C
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x48D
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x48E
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x48F
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x490
> >>> >>>   cpu_map: Add missing vmx features from MSR 0x491
> >>> >>
> >>> >> I haven't really checked the values of each feature you're adding, but
> >>> >> all changes except for those in patch 3 are generated so I expect they
> >>> >> are correct :-)
> >>> >
> >>> > Hi there,
> >>> >
> >>> > Apologies if this is a PEBCAK, but I'm noticing a strange behaviour when
> >>> > testing live migrations from libvirt 9.6 to 9.10+ (including 10.0).
> >>> > "virsh migrate --live ..." says:
> >>> >
> >>> > error: operation failed: guest CPU doesn't match specification:
> >>> > extra features:
> >>> > vmx-ins-outs,vmx-true-ctls,vmx-store-lma,vmx-activity-hlt,vmx-vmwrite-vmexit-fields,vmx-apicv-xapic,vmx-ept,vmx-desc-exit,vmx-rdtscp-exit,vmx-apicv-x2apic,vmx-vpid,vmx-wbinvd-exit,vmx-unrestricted-guest,vmx-apicv-register,vmx-apicv-vid,vmx-rdrand-exit,vmx-invpcid-exit,vmx-vmfunc,vmx-shadow-vmcs,vmx-invvpid,vmx-invvpid-single-addr,vmx-invvpid-all-context,vmx-ept-execonly,vmx-page-walk-4,vmx-ept-2mb,vmx-ept-1gb,vmx-invept,vmx-eptad,vmx-invept-single-context,vmx-invept-all-context,vmx-intr-exit,vmx-nmi-exit,vmx-vnmi,vmx-preemption-timer,vmx-posted-intr,vmx-vintr-pending,vmx-tsc-offset,vmx-hlt-exit,vmx-invlpg-exit,vmx-mwait-exit,vmx-rdpmc-exit,vmx-rdtsc-exit,vmx-cr3-load-noexit,vmx-cr3-store-noexit,vmx-cr8-load-exit,vmx-cr8-store-exit,vmx-flexpriority,vmx-vnmi-pending,vmx-movdr-exit,vmx-io-exit,vmx-io-bitmap,vmx-mtf,vmx-msr-bitmap,vmx-monitor-exit,vmx-pause-exit,vmx-secondary-ctls,vmx-exit-nosave-debugctl,vmx-exit-load-perf-global-ctrl,vmx-exit-ack-intr,vmx-exit-save-pat,vmx-exit-load-pat,vmx-exit-save-efer,vmx-exit-load-efer,vmx-exit-save-preemption-timer,vmx-entry-noload-debugctl,vmx-entry-ia32e-mode,vmx-entry-load-perf-global-ctrl,vmx-entry-load-pat,vmx-entry-load-efer,vmx-eptp-switching
> >>> >
> >>> > Judging by the missing features it's complaining about, it seems to be
> >>> > something related to what was discussed in this thread.
> >>> >
> >>> > I could provide more details about the setup, although it's pretty
> >>> > straightforward (create a VM under libvirt < 9.10 and try to live
> >>> > migrate it to a system running libvirt >= 9.10).
> >>> 
> >>> Hi again,
> >>> 
> >>> Any news on this one?  If it is indeed a regression, it seems like an
> >>> important one.  While looking through the open issues on gitlab I
> >>> stumbled upon this one:
> >>> 
> 

Re: [PATCH 04/22] qemu: domain: Drop added features from migratable CPU

2024-03-14 Thread Jiri Denemark
On Wed, Mar 13, 2024 at 09:58:47 +0100, Peter Krempa wrote:
> On Tue, Mar 12, 2024 at 18:06:44 +0100, Jiri Denemark wrote:
> > Features marked with added='yes' in CPU model definitions have to be
> > removed before migration, otherwise older libvirt would complain about
> > unknown CPU features. We only do this for features that were enabled for
> > a given CPU model even with older libvirt, which just ignored the
> > features. And only for features we added ourselves when updating CPU
> > definition during domain startup, that is we do not remove features
> > which were explicitly mentioned by a user.
> > 
> > That said, this is not the safest thing we could do, but it's
> > effectively the same thing we did before the affected features were
> > added: we ignored them completely on both sides of migration.
> 
> IIUC if the feature is part of the model it'll never be expanded into
> the definition and thus will never be checked during migration, right?

All features known to libvirt are checked during migration, even those
that are part of a model in our CPU map. If a feature is only part of a
model in QEMU and not libvirt, it will be explicitly added to the
definition once domain starts so that it can be checked during
migration. But, if such feature is unknown to libvirt, it will just be
ignored completely.

> Thus effectively the full checking could be done only for migration
> between new versions if the destination somehow signals that it's new
> enough for the source to not drop the features.

Theoretically yes, but I could not come up with a reasonable way to do
this. This algorithm to ignore features is quite generic and can be used
in the future for other features (but hopefully we won't have to do it).
I was thinking about sending a list of all supported CPU features in a
migration cookie, but that sounds like a little bit too much as we
currently support 332 features on x86.

> > @@ -6664,6 +6691,25 @@ qemuDomainMakeCPUMigratable(virArch arch,
> >  return -1;
> >  }
> >  
> > +if (virCPUx86GetAddedFeatures(cpu->model, ) < 0)
> > +return -1;
> > +
> > +/* Drop features marked as added in a cpu model, but only
> > + * when they are not mentioned in origCPU, i.e., when they were not
> > + * explicitly mentioned by the user.
> > + */
> > +if (data.added) {
> > +g_auto(GStrv) keep = NULL;
> > +
> > +if (origCPU) {
> > +keep = virCPUDefListFeatures(origCPU);
> > +data.keep = keep;
> > +}
> > +
> > +if (virCPUDefFilterFeatures(cpu, qemuDomainDropAddedCPUFeatures, 
> > ) < 0)
> > +return -1;
> 
> Mmm, O(n^2) ...

Well, there are at most 77 features marked as added, so it's at most
n*77, which makes it O(n) :-) And even n is limited to 332...

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [libvirt PATCH 00/12] Add vmx-* features from qemu

2024-03-12 Thread Jiri Denemark
On Wed, Jan 24, 2024 at 22:19:56 -0500, Sergio Durigan Junior wrote:
> On Monday, January 22 2024, I wrote:
> 
> > On Wednesday, November 22 2023, Jiri Denemark wrote:
> >
> >> On Thu, Nov 09, 2023 at 15:30:46 +0100, Tim Wiederhake wrote:
> >>> For rationale, see patch 3 (cpu_map: No longer ignore vmx- features in
> >>> sync_qemu_features_i386.py).
> >>> 
> >>> Adding features in bunches (one patch per msr index), as this series
> >>> is adding ~100 features.
> >>> 
> >>> This also adds cpu features "gds-no" and "amx-complex" and brings
> >>> libvirt in sync with qemu commit ad6ef0a42e.
> >>> 
> >>> Tim Wiederhake (12):
> >>>   cpu_map: Add missing feature "gds-no"
> >>>   cpu_map: Add missing feature "amx-complex"
> >>>   cpu_map: No longer ignore vmx- features in sync_qemu_features_i386.py
> >>>   cpu_map: Add missing vmx features from MSR 0x480
> >>>   cpu_map: Add missing vmx features from MSR 0x485
> >>>   cpu_map: Add missing vmx features from MSR 0x48B
> >>>   cpu_map: Add missing vmx features from MSR 0x48C
> >>>   cpu_map: Add missing vmx features from MSR 0x48D
> >>>   cpu_map: Add missing vmx features from MSR 0x48E
> >>>   cpu_map: Add missing vmx features from MSR 0x48F
> >>>   cpu_map: Add missing vmx features from MSR 0x490
> >>>   cpu_map: Add missing vmx features from MSR 0x491
> >>
> >> I haven't really checked the values of each feature you're adding, but
> >> all changes except for those in patch 3 are generated so I expect they
> >> are correct :-)
> >
> > Hi there,
> >
> > Apologies if this is a PEBCAK, but I'm noticing a strange behaviour when
> > testing live migrations from libvirt 9.6 to 9.10+ (including 10.0).
> > "virsh migrate --live ..." says:
> >
> > error: operation failed: guest CPU doesn't match specification: extra 
> > features: 
> > vmx-ins-outs,vmx-true-ctls,vmx-store-lma,vmx-activity-hlt,vmx-vmwrite-vmexit-fields,vmx-apicv-xapic,vmx-ept,vmx-desc-exit,vmx-rdtscp-exit,vmx-apicv-x2apic,vmx-vpid,vmx-wbinvd-exit,vmx-unrestricted-guest,vmx-apicv-register,vmx-apicv-vid,vmx-rdrand-exit,vmx-invpcid-exit,vmx-vmfunc,vmx-shadow-vmcs,vmx-invvpid,vmx-invvpid-single-addr,vmx-invvpid-all-context,vmx-ept-execonly,vmx-page-walk-4,vmx-ept-2mb,vmx-ept-1gb,vmx-invept,vmx-eptad,vmx-invept-single-context,vmx-invept-all-context,vmx-intr-exit,vmx-nmi-exit,vmx-vnmi,vmx-preemption-timer,vmx-posted-intr,vmx-vintr-pending,vmx-tsc-offset,vmx-hlt-exit,vmx-invlpg-exit,vmx-mwait-exit,vmx-rdpmc-exit,vmx-rdtsc-exit,vmx-cr3-load-noexit,vmx-cr3-store-noexit,vmx-cr8-load-exit,vmx-cr8-store-exit,vmx-flexpriority,vmx-vnmi-pending,vmx-movdr-exit,vmx-io-exit,vmx-io-bitmap,vmx-mtf,vmx-msr-bitmap,vmx-monitor-exit,vmx-pause-exit,vmx-secondary-ctls,vmx-exit-nosave-debugctl,vmx-exit-load-perf-global-ctrl,vmx-exit-ack-intr,vmx-exit-save-pat,vmx-exit-load-pat,vmx-exit-save-efer,vmx-exit-load-efer,vmx-exit-save-preemption-timer,vmx-entry-noload-debugctl,vmx-entry-ia32e-mode,vmx-entry-load-perf-global-ctrl,vmx-entry-load-pat,vmx-entry-load-efer,vmx-eptp-switching
> >
> > Judging by the missing features it's complaining about, it seems to be
> > something related to what was discussed in this thread.
> >
> > I could provide more details about the setup, although it's pretty
> > straightforward (create a VM under libvirt < 9.10 and try to live
> > migrate it to a system running libvirt >= 9.10).
> 
> Hi again,
> 
> Any news on this one?  If it is indeed a regression, it seems like an
> important one.  While looking through the open issues on gitlab I
> stumbled upon this one:
> 
> https://gitlab.com/libvirt/libvirt/-/issues/568
> 
> ... which seems to describe the same problem.

Yes, it's the same issue. I've just sent patches that should fix this
regression for review: 
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ROAHSG43CLIZPPJV4ZL76VQYUAFXOXXE/

Sorry for the delay.

Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 06/22] Add vmx-* features to Broadwell*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Broadwell-IBRS.xml   | 75 
 src/cpu_map/x86_Broadwell-noTSX-IBRS.xml | 75 
 src/cpu_map/x86_Broadwell-noTSX.xml  | 75 
 src/cpu_map/x86_Broadwell.xml| 75 
 4 files changed, 300 insertions(+)

diff --git a/src/cpu_map/x86_Broadwell-IBRS.xml 
b/src/cpu_map/x86_Broadwell-IBRS.xml
index 9033d5fcd5..1484903298 100644
--- a/src/cpu_map/x86_Broadwell-IBRS.xml
+++ b/src/cpu_map/x86_Broadwell-IBRS.xml
@@ -59,6 +59,81 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Broadwell-noTSX-IBRS.xml 
b/src/cpu_map/x86_Broadwell-noTSX-IBRS.xml
index c044b60e36..13f08435b7 100644
--- a/src/cpu_map/x86_Broadwell-noTSX-IBRS.xml
+++ b/src/cpu_map/x86_Broadwell-noTSX-IBRS.xml
@@ -57,6 +57,81 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Broadwell-noTSX.xml 
b/src/cpu_map/x86_Broadwell-noTSX.xml
index 637f29ba1c..4293b3aeee 100644
--- a/src/cpu_map/x86_Broadwell-noTSX.xml
+++ b/src/cpu_map/x86_Broadwell-noTSX.xml
@@ -56,6 +56,81 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Broadwell.xml b/src/cpu_map/x86_Broadwell.xml
index 82939a4509..37dd1dabcf 100644
--- a/src/cpu_map/x86_Broadwell.xml
+++ b/src/cpu_map/x86_Broadwell.xml
@@ -58,6 +58,81 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 17/22] Add vmx-* features to SandyBridge*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_SandyBridge-IBRS.xml | 63 
 src/cpu_map/x86_SandyBridge.xml  | 63 
 2 files changed, 126 insertions(+)

diff --git a/src/cpu_map/x86_SandyBridge-IBRS.xml 
b/src/cpu_map/x86_SandyBridge-IBRS.xml
index fbdb4f2bf6..297eea8e88 100644
--- a/src/cpu_map/x86_SandyBridge-IBRS.xml
+++ b/src/cpu_map/x86_SandyBridge-IBRS.xml
@@ -41,6 +41,69 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_SandyBridge.xml b/src/cpu_map/x86_SandyBridge.xml
index 7c85ed42df..20ea378c47 100644
--- a/src/cpu_map/x86_SandyBridge.xml
+++ b/src/cpu_map/x86_SandyBridge.xml
@@ -40,6 +40,69 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 22/22] NEWS: Document the fix for migration or vmx enabled domains

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 NEWS.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index 44b775b546..489201d3fc 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -21,6 +21,18 @@ v10.2.0 (unreleased)
 
 * **Bug fixes**
 
+  * qemu: Fix migration from libvirt older than 9.10.0 when vmx is enabled
+
+A domain with vmx feature enabled (which may be even done automatically
+with ``mode='host-model'``) started by libvirt 9.9.0 or older cannot be
+migrated to libvirt 9.10.0, 10.0.0, and 10.1.0 as the target host would
+complain about a lot of extra ``vmx-*`` features. Migration of similar
+domains started by the affected releases to libvirt 9.9.0 and older
+does not work either. Since libvirt 10.2.0 migration works again with
+libvirt 9.9.0 and older in both directions. Migration from the affected
+releases to 10.2.0 works as well, but the other direction remains broken
+unless the fix is backported.
+
 
 v10.1.0 (2024-03-01)
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 19/22] Add vmx-* features to Skylake*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Skylake-Client-IBRS.xml   | 71 +
 src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml | 72 ++
 src/cpu_map/x86_Skylake-Client.xml| 71 +
 src/cpu_map/x86_Skylake-Server-IBRS.xml   | 74 ++
 src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml | 76 +++
 src/cpu_map/x86_Skylake-Server.xml| 74 ++
 6 files changed, 438 insertions(+)

diff --git a/src/cpu_map/x86_Skylake-Client-IBRS.xml 
b/src/cpu_map/x86_Skylake-Client-IBRS.xml
index 5709e7c2f9..1c77f9595b 100644
--- a/src/cpu_map/x86_Skylake-Client-IBRS.xml
+++ b/src/cpu_map/x86_Skylake-Client-IBRS.xml
@@ -67,6 +67,77 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml 
b/src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml
index ffba34502a..dca117028c 100644
--- a/src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml
+++ b/src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml
@@ -65,6 +65,78 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Skylake-Client.xml 
b/src/cpu_map/x86_Skylake-Client.xml
index 14cd57e176..68e5d2d038 100644
--- a/src/cpu_map/x86_Skylake-Client.xml
+++ b/src/cpu_map/x86_Skylake-Client.xml
@@ -66,6 +66,77 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Skylake-Server-IBRS.xml 
b/src/cpu_map/x86_Skylake-Server-IBRS.xml
index 9fb3488809..e467b76242 100644
--- a/src/cpu_map/x86_Skylake-Server-IBRS.xml
+++ b/src/cpu_map/x86_Skylake-Server-IBRS.xml
@@ -69,6 +69,80 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml 
b/src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml
index c162c0acc3..b8601da0cb 100644
--- a/src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml
+++ b/src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml
@@ -67,6 +67,82 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Skylake-Server.xml 
b/src/cpu_map/x86_Skylake-Server.xml
index e022d94c84..cc1e9ddd7f 100644
--- a/src/cpu_map/x86_Skylake-Server.xml
+++ b/src/cpu_map/x86_Skylake-Server.xml
@@ -68,6 +68,80 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 21/22] Add vmx-* features to Westmere*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Westmere-IBRS.xml | 63 +++
 src/cpu_map/x86_Westmere.xml  | 63 +++
 2 files changed, 126 insertions(+)

diff --git a/src/cpu_map/x86_Westmere-IBRS.xml 
b/src/cpu_map/x86_Westmere-IBRS.xml
index c7898f0c22..a5abe8a1e1 100644
--- a/src/cpu_map/x86_Westmere-IBRS.xml
+++ b/src/cpu_map/x86_Westmere-IBRS.xml
@@ -36,5 +36,68 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
diff --git a/src/cpu_map/x86_Westmere.xml b/src/cpu_map/x86_Westmere.xml
index 16e4ad6c30..161f1a078e 100644
--- a/src/cpu_map/x86_Westmere.xml
+++ b/src/cpu_map/x86_Westmere.xml
@@ -37,5 +37,68 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 15/22] Add vmx-* features to Nehalem*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Nehalem-IBRS.xml | 61 
 src/cpu_map/x86_Nehalem.xml  | 61 
 2 files changed, 122 insertions(+)

diff --git a/src/cpu_map/x86_Nehalem-IBRS.xml b/src/cpu_map/x86_Nehalem-IBRS.xml
index 00d0d2fe51..0cfee14c0f 100644
--- a/src/cpu_map/x86_Nehalem-IBRS.xml
+++ b/src/cpu_map/x86_Nehalem-IBRS.xml
@@ -38,5 +38,66 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
diff --git a/src/cpu_map/x86_Nehalem.xml b/src/cpu_map/x86_Nehalem.xml
index 9968001fe7..74ee64ce1c 100644
--- a/src/cpu_map/x86_Nehalem.xml
+++ b/src/cpu_map/x86_Nehalem.xml
@@ -37,5 +37,66 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 20/22] Add vmx-* features to Snowridge

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Snowridge.xml | 76 +++
 1 file changed, 76 insertions(+)

diff --git a/src/cpu_map/x86_Snowridge.xml b/src/cpu_map/x86_Snowridge.xml
index 383a24d367..bc410bd8f8 100644
--- a/src/cpu_map/x86_Snowridge.xml
+++ b/src/cpu_map/x86_Snowridge.xml
@@ -62,6 +62,82 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 18/22] Add vmx-* features to SapphireRapids

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_SapphireRapids.xml | 77 ++
 1 file changed, 77 insertions(+)

diff --git a/src/cpu_map/x86_SapphireRapids.xml 
b/src/cpu_map/x86_SapphireRapids.xml
index 2297feeeca..40164a47e2 100644
--- a/src/cpu_map/x86_SapphireRapids.xml
+++ b/src/cpu_map/x86_SapphireRapids.xml
@@ -103,6 +103,83 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 16/22] Add vmx-* features to Penryn

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Penryn.xml | 29 +
 1 file changed, 29 insertions(+)

diff --git a/src/cpu_map/x86_Penryn.xml b/src/cpu_map/x86_Penryn.xml
index 29d4cd635b..b31f96fa43 100644
--- a/src/cpu_map/x86_Penryn.xml
+++ b/src/cpu_map/x86_Penryn.xml
@@ -33,5 +33,34 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 13/22] Add vmx-* features to IvyBridge*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_IvyBridge-IBRS.xml | 67 ++
 src/cpu_map/x86_IvyBridge.xml  | 67 ++
 2 files changed, 134 insertions(+)

diff --git a/src/cpu_map/x86_IvyBridge-IBRS.xml 
b/src/cpu_map/x86_IvyBridge-IBRS.xml
index 430bc3232d..27d85d86c4 100644
--- a/src/cpu_map/x86_IvyBridge-IBRS.xml
+++ b/src/cpu_map/x86_IvyBridge-IBRS.xml
@@ -47,6 +47,73 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_IvyBridge.xml b/src/cpu_map/x86_IvyBridge.xml
index eaf5d02e82..72031cfdc6 100644
--- a/src/cpu_map/x86_IvyBridge.xml
+++ b/src/cpu_map/x86_IvyBridge.xml
@@ -46,6 +46,73 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 14/22] Add vmx-* features to kvm*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_kvm32.xml | 18 ++
 src/cpu_map/x86_kvm64.xml | 20 
 2 files changed, 38 insertions(+)

diff --git a/src/cpu_map/x86_kvm32.xml b/src/cpu_map/x86_kvm32.xml
index 9dd96d5b56..ac28c53bd0 100644
--- a/src/cpu_map/x86_kvm32.xml
+++ b/src/cpu_map/x86_kvm32.xml
@@ -23,5 +23,23 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
diff --git a/src/cpu_map/x86_kvm64.xml b/src/cpu_map/x86_kvm64.xml
index 185af06f78..970a8e73d5 100644
--- a/src/cpu_map/x86_kvm64.xml
+++ b/src/cpu_map/x86_kvm64.xml
@@ -27,5 +27,25 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 11/22] Add vmx-* features to Haswell*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Haswell-IBRS.xml   | 73 ++
 src/cpu_map/x86_Haswell-noTSX-IBRS.xml | 73 ++
 src/cpu_map/x86_Haswell-noTSX.xml  | 73 ++
 src/cpu_map/x86_Haswell.xml| 73 ++
 4 files changed, 292 insertions(+)

diff --git a/src/cpu_map/x86_Haswell-IBRS.xml b/src/cpu_map/x86_Haswell-IBRS.xml
index 0ffe2bae0d..57b980d14f 100644
--- a/src/cpu_map/x86_Haswell-IBRS.xml
+++ b/src/cpu_map/x86_Haswell-IBRS.xml
@@ -55,6 +55,79 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Haswell-noTSX-IBRS.xml 
b/src/cpu_map/x86_Haswell-noTSX-IBRS.xml
index 75d709c009..fcae023ffb 100644
--- a/src/cpu_map/x86_Haswell-noTSX-IBRS.xml
+++ b/src/cpu_map/x86_Haswell-noTSX-IBRS.xml
@@ -53,6 +53,79 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Haswell-noTSX.xml 
b/src/cpu_map/x86_Haswell-noTSX.xml
index b0a0faa856..7404052065 100644
--- a/src/cpu_map/x86_Haswell-noTSX.xml
+++ b/src/cpu_map/x86_Haswell-noTSX.xml
@@ -52,6 +52,79 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
diff --git a/src/cpu_map/x86_Haswell.xml b/src/cpu_map/x86_Haswell.xml
index ee16b30f19..99986c5c45 100644
--- a/src/cpu_map/x86_Haswell.xml
+++ b/src/cpu_map/x86_Haswell.xml
@@ -54,6 +54,79 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
   
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 12/22] Add vmx-* features to Icelake*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Icelake-Server-noTSX.xml | 77 
 src/cpu_map/x86_Icelake-Server.xml   | 72 ++
 2 files changed, 149 insertions(+)

diff --git a/src/cpu_map/x86_Icelake-Server-noTSX.xml 
b/src/cpu_map/x86_Icelake-Server-noTSX.xml
index 7c9c32c977..072f8145c4 100644
--- a/src/cpu_map/x86_Icelake-Server-noTSX.xml
+++ b/src/cpu_map/x86_Icelake-Server-noTSX.xml
@@ -80,6 +80,83 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Icelake-Server.xml 
b/src/cpu_map/x86_Icelake-Server.xml
index b4685bead0..3a35145d7f 100644
--- a/src/cpu_map/x86_Icelake-Server.xml
+++ b/src/cpu_map/x86_Icelake-Server.xml
@@ -82,6 +82,78 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 10/22] Add vmx-* features to core{,2}duo

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_core2duo.xml | 26 ++
 src/cpu_map/x86_coreduo.xml  | 18 ++
 2 files changed, 44 insertions(+)

diff --git a/src/cpu_map/x86_core2duo.xml b/src/cpu_map/x86_core2duo.xml
index 412039fe55..ea23a6c662 100644
--- a/src/cpu_map/x86_core2duo.xml
+++ b/src/cpu_map/x86_core2duo.xml
@@ -30,5 +30,31 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
diff --git a/src/cpu_map/x86_coreduo.xml b/src/cpu_map/x86_coreduo.xml
index e2fda9a1d4..24900e637f 100644
--- a/src/cpu_map/x86_coreduo.xml
+++ b/src/cpu_map/x86_coreduo.xml
@@ -26,5 +26,23 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 08/22] Add vmx-* features to Conroe

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Conroe.xml | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/src/cpu_map/x86_Conroe.xml b/src/cpu_map/x86_Conroe.xml
index 4cacee6142..955297ffc3 100644
--- a/src/cpu_map/x86_Conroe.xml
+++ b/src/cpu_map/x86_Conroe.xml
@@ -31,5 +31,31 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 09/22] Add vmx-* features to Cooperlake

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Cooperlake.xml | 76 ++
 1 file changed, 76 insertions(+)

diff --git a/src/cpu_map/x86_Cooperlake.xml b/src/cpu_map/x86_Cooperlake.xml
index ceca687334..af428f2781 100644
--- a/src/cpu_map/x86_Cooperlake.xml
+++ b/src/cpu_map/x86_Cooperlake.xml
@@ -81,6 +81,82 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 07/22] Add vmx-* features to Cascadelake*

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/x86_Cascadelake-Server-noTSX.xml | 76 
 src/cpu_map/x86_Cascadelake-Server.xml   | 74 +++
 2 files changed, 150 insertions(+)

diff --git a/src/cpu_map/x86_Cascadelake-Server-noTSX.xml 
b/src/cpu_map/x86_Cascadelake-Server-noTSX.xml
index bfd4629836..056f43d088 100644
--- a/src/cpu_map/x86_Cascadelake-Server-noTSX.xml
+++ b/src/cpu_map/x86_Cascadelake-Server-noTSX.xml
@@ -70,6 +70,82 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
diff --git a/src/cpu_map/x86_Cascadelake-Server.xml 
b/src/cpu_map/x86_Cascadelake-Server.xml
index 335e9cb584..88e51c2067 100644
--- a/src/cpu_map/x86_Cascadelake-Server.xml
+++ b/src/cpu_map/x86_Cascadelake-Server.xml
@@ -72,6 +72,80 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 05/22] cpu_map: Do not ignore VMX features in sync_qemu_models script

2024-03-12 Thread Jiri Denemark
Signed-off-by: Jiri Denemark 
---
 src/cpu_map/sync_qemu_models_i386.py | 114 ++-
 1 file changed, 111 insertions(+), 3 deletions(-)

diff --git a/src/cpu_map/sync_qemu_models_i386.py 
b/src/cpu_map/sync_qemu_models_i386.py
index 1c6a2d4d27..9e96b4244e 100755
--- a/src/cpu_map/sync_qemu_models_i386.py
+++ b/src/cpu_map/sync_qemu_models_i386.py
@@ -184,12 +184,120 @@ def translate_feature(name):
 "MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY": "skip-l1dfl-vmentry",
 "MSR_ARCH_CAP_TAA_NO": "taa-no",
 "MSR_CORE_CAP_SPLIT_LOCK_DETECT": "split-lock-detect",
+
+# FEAT_VMX_PROCBASED_CTLS
+"VMX_CPU_BASED_VIRTUAL_INTR_PENDING": "vmx-vintr-pending",
+"VMX_CPU_BASED_USE_TSC_OFFSETING": "vmx-tsc-offset",
+"VMX_CPU_BASED_HLT_EXITING": "vmx-hlt-exit",
+"VMX_CPU_BASED_INVLPG_EXITING": "vmx-invlpg-exit",
+"VMX_CPU_BASED_MWAIT_EXITING": "vmx-mwait-exit",
+"VMX_CPU_BASED_RDPMC_EXITING": "vmx-rdpmc-exit",
+"VMX_CPU_BASED_RDTSC_EXITING": "vmx-rdtsc-exit",
+"VMX_CPU_BASED_CR3_LOAD_EXITING": "vmx-cr3-load-noexit",
+"VMX_CPU_BASED_CR3_STORE_EXITING": "vmx-cr3-store-noexit",
+"VMX_CPU_BASED_CR8_LOAD_EXITING": "vmx-cr8-load-exit",
+"VMX_CPU_BASED_CR8_STORE_EXITING": "vmx-cr8-store-exit",
+"VMX_CPU_BASED_TPR_SHADOW": "vmx-flexpriority",
+"VMX_CPU_BASED_VIRTUAL_NMI_PENDING": "vmx-vnmi-pending",
+"VMX_CPU_BASED_MOV_DR_EXITING": "vmx-movdr-exit",
+"VMX_CPU_BASED_UNCOND_IO_EXITING": "vmx-io-exit",
+"VMX_CPU_BASED_USE_IO_BITMAPS": "vmx-io-bitmap",
+"VMX_CPU_BASED_MONITOR_TRAP_FLAG": "vmx-mtf",
+"VMX_CPU_BASED_USE_MSR_BITMAPS": "vmx-msr-bitmap",
+"VMX_CPU_BASED_MONITOR_EXITING": "vmx-monitor-exit",
+"VMX_CPU_BASED_PAUSE_EXITING": "vmx-pause-exit",
+"VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS": "vmx-secondary-ctls",
+
+# FEAT_VMX_SECONDARY_CTLS
+"VMX_SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES": "vmx-apicv-xapic",
+"VMX_SECONDARY_EXEC_ENABLE_EPT": "vmx-ept",
+"VMX_SECONDARY_EXEC_DESC": "vmx-desc-exit",
+"VMX_SECONDARY_EXEC_RDTSCP": "vmx-rdtscp-exit",
+"VMX_SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE": "vmx-apicv-x2apic",
+"VMX_SECONDARY_EXEC_ENABLE_VPID": "vmx-vpid",
+"VMX_SECONDARY_EXEC_WBINVD_EXITING": "vmx-wbinvd-exit",
+"VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST": "vmx-unrestricted-guest",
+"VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT": "vmx-apicv-register",
+"VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY": "vmx-apicv-vid",
+"VMX_SECONDARY_EXEC_PAUSE_LOOP_EXITING": "vmx-ple",
+"VMX_SECONDARY_EXEC_RDRAND_EXITING": "vmx-rdrand-exit",
+"VMX_SECONDARY_EXEC_ENABLE_INVPCID": "vmx-invpcid-exit",
+"VMX_SECONDARY_EXEC_ENABLE_VMFUNC": "vmx-vmfunc",
+"VMX_SECONDARY_EXEC_SHADOW_VMCS": "vmx-shadow-vmcs",
+"VMX_SECONDARY_EXEC_ENCLS_EXITING": "vmx-encls-exit",
+"VMX_SECONDARY_EXEC_RDSEED_EXITING": "vmx-rdseed-exit",
+"VMX_SECONDARY_EXEC_ENABLE_PML": "vmx-pml",
+"VMX_SECONDARY_EXEC_XSAVES": "vmx-xsaves",
+"VMX_SECONDARY_EXEC_TSC_SCALING": "vmx-tsc-scaling",
+"VMX_SECONDARY_EXEC_ENABLE_USER_WAIT_PAUSE": 
"vmx-enable-user-wait-pause",
+
+# FEAT_VMX_PINBASED_CTLS
+"VMX_PIN_BASED_EXT_INTR_MASK": "vmx-intr-exit",
+"VMX_PIN_BASED_NMI_EXITING": "vmx-nmi-exit",
+"VMX_PIN_BASED_VIRTUAL_NMIS": "vmx-vnmi",
+"VMX_PIN_BASED_VMX_PREEMPTION_TIMER": "vmx-preemption-timer",
+"VMX_PIN_BASED_POSTED_INTR": "vmx-posted-intr",
+
+# FEAT_VMX_EXIT_CTLS
+"VMX_VM_EXIT_SAVE_DEBUG_CONTROLS": "vmx-exit-nosave-debugctl",
+"VMX_VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL": 
"vmx-exit-load-perf-global-ctrl",
+"VMX_VM_EXIT_ACK_INTR_ON_EXIT": "vmx-exit-ack-intr",
+"VMX_VM_EXIT_SAVE_IA3

[PATCH 04/22] qemu: domain: Drop added features from migratable CPU

2024-03-12 Thread Jiri Denemark
Features marked with added='yes' in CPU model definitions have to be
removed before migration, otherwise older libvirt would complain about
unknown CPU features. We only do this for features that were enabled for
a given CPU model even with older libvirt, which just ignored the
features. And only for features we added ourselves when updating CPU
definition during domain startup, that is we do not remove features
which were explicitly mentioned by a user.

That said, this is not the safest thing we could do, but it's
effectively the same thing we did before the affected features were
added: we ignored them completely on both sides of migration.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_domain.c   | 50 ++--
 src/qemu/qemu_domain.h   |  3 +-
 src/qemu/qemu_migration_cookie.c |  5 +++-
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 31a6509166..535612000d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -66,6 +66,7 @@
 #include "backup_conf.h"
 #include "virutil.h"
 #include "virsecureerase.h"
+#include "cpu/cpu_x86.h"
 
 #include 
 #include 
@@ -6643,10 +6644,36 @@ qemuDomainDefCopy(virQEMUDriver *driver,
 }
 
 
+typedef struct {
+const char * const *added;
+GStrv keep;
+} qemuDomainDropAddedCPUFeaturesData;
+
+
+static bool
+qemuDomainDropAddedCPUFeatures(const char *name,
+   virCPUFeaturePolicy policy G_GNUC_UNUSED,
+   void *opaque)
+{
+qemuDomainDropAddedCPUFeaturesData *data = opaque;
+
+if (!g_strv_contains(data->added, name))
+return true;
+
+if (data->keep && g_strv_contains((const char **) data->keep, name))
+return true;
+
+return false;
+}
+
+
 int
 qemuDomainMakeCPUMigratable(virArch arch,
-virCPUDef *cpu)
+virCPUDef *cpu,
+virCPUDef *origCPU)
 {
+qemuDomainDropAddedCPUFeaturesData data = { 0 };
+
 if (cpu->mode != VIR_CPU_MODE_CUSTOM ||
 !cpu->model ||
 !ARCH_IS_X86(arch))
@@ -6664,6 +6691,25 @@ qemuDomainMakeCPUMigratable(virArch arch,
 return -1;
 }
 
+if (virCPUx86GetAddedFeatures(cpu->model, ) < 0)
+return -1;
+
+/* Drop features marked as added in a cpu model, but only
+ * when they are not mentioned in origCPU, i.e., when they were not
+ * explicitly mentioned by the user.
+ */
+if (data.added) {
+g_auto(GStrv) keep = NULL;
+
+if (origCPU) {
+keep = virCPUDefListFeatures(origCPU);
+data.keep = keep;
+}
+
+if (virCPUDefFilterFeatures(cpu, qemuDomainDropAddedCPUFeatures, 
) < 0)
+return -1;
+}
+
 return 0;
 }
 
@@ -6843,7 +6889,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriver *driver,
 }
 
 if (def->cpu &&
-qemuDomainMakeCPUMigratable(def->os.arch, def->cpu) < 0)
+qemuDomainMakeCPUMigratable(def->os.arch, def->cpu, origCPU) < 0)
 return -1;
 
 /* Old libvirt doesn't understand  elements so
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 07b0318258..626df6338f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1049,7 +1049,8 @@ qemuDomainSupportsCheckpointsBlockjobs(virDomainObj *vm)
 
 int
 qemuDomainMakeCPUMigratable(virArch arch,
-virCPUDef *cpu);
+virCPUDef *cpu,
+virCPUDef *origCPU);
 
 int
 qemuDomainInitializePflashStorageSource(virDomainObj *vm,
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index c53d42a018..90cc079c1a 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -540,12 +540,15 @@ static int
 qemuMigrationCookieAddCPU(qemuMigrationCookie *mig,
   virDomainObj *vm)
 {
+qemuDomainObjPrivate *priv = vm->privateData;
+
 if (mig->cpu)
 return 0;
 
 mig->cpu = virCPUDefCopy(vm->def->cpu);
 
-if (qemuDomainMakeCPUMigratable(vm->def->os.arch, mig->cpu) < 0)
+if (qemuDomainMakeCPUMigratable(vm->def->os.arch, mig->cpu,
+priv->origCPU) < 0)
 return -1;
 
 mig->flags |= QEMU_MIGRATION_COOKIE_CPU;
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 03/22] conf: cpu: Introduce virCPUDefListFeatures

2024-03-12 Thread Jiri Denemark
The function returns a list of explicitly mentioned features in the CPU
definition.

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c  | 24 
 src/conf/cpu_conf.h  |  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 28 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 6e6e1b9a89..08f57a4df8 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -982,6 +982,30 @@ virCPUDefFindFeature(const virCPUDef *def,
 }
 
 
+/**
+ * virCPUDefListFeatures:
+ * @def: CPU definition
+ *
+ * Provides a list of feature names explicitly mentioned in the CPU definition
+ * regardless on the policy. The caller is responsible for freeing the list.
+ *
+ * Returns a NULL-terminated list of feature names.
+ */
+char **
+virCPUDefListFeatures(const virCPUDef *def)
+{
+char **list;
+size_t i;
+
+list = g_new0(char *, def->nfeatures + 1);
+
+for (i = 0; i < def->nfeatures; i++)
+list[i] = g_strdup(def->features[i].name);
+
+return list;
+}
+
+
 int
 virCPUDefFilterFeatures(virCPUDef *cpu,
 virCPUDefFeatureFilter filter,
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 2694022fed..a24ae4a29f 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -270,6 +270,9 @@ virCPUDefCheckFeatures(virCPUDef *cpu,
void *opaque,
char ***features);
 
+char **
+virCPUDefListFeatures(const virCPUDef *def);
+
 virCPUDef **
 virCPUDefListParse(const char **xmlCPUs,
unsigned int ncpus,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 98a925933e..d5c0c634f5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -111,6 +111,7 @@ virCPUDefFree;
 virCPUDefFreeFeatures;
 virCPUDefFreeModel;
 virCPUDefIsEqual;
+virCPUDefListFeatures;
 virCPUDefListFree;
 virCPUDefListParse;
 virCPUDefNew;
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 02/22] qemu: domain: Check arch in qemuDomainMakeCPUMigratable

2024-03-12 Thread Jiri Denemark
The content is arch specific and checking for Icelake-Server CPU model
on non-x86 architectures does not make sense.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_domain.c   | 14 ++
 src/qemu/qemu_domain.h   |  3 ++-
 src/qemu/qemu_migration_cookie.c |  2 +-
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8c85446c3d..31a6509166 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6644,10 +6644,15 @@ qemuDomainDefCopy(virQEMUDriver *driver,
 
 
 int
-qemuDomainMakeCPUMigratable(virCPUDef *cpu)
+qemuDomainMakeCPUMigratable(virArch arch,
+virCPUDef *cpu)
 {
-if (cpu->mode == VIR_CPU_MODE_CUSTOM &&
-STREQ_NULLABLE(cpu->model, "Icelake-Server")) {
+if (cpu->mode != VIR_CPU_MODE_CUSTOM ||
+!cpu->model ||
+!ARCH_IS_X86(arch))
+return 0;
+
+if (STREQ(cpu->model, "Icelake-Server")) {
 /* Originally Icelake-Server CPU model contained pconfig CPU feature.
  * It was never actually enabled and thus it was removed. To enable
  * migration to QEMU 3.1.0 (with both new and old libvirt), we
@@ -6837,7 +6842,8 @@ qemuDomainDefFormatBufInternal(virQEMUDriver *driver,
 return -1;
 }
 
-if (def->cpu && qemuDomainMakeCPUMigratable(def->cpu) < 0)
+if (def->cpu &&
+qemuDomainMakeCPUMigratable(def->os.arch, def->cpu) < 0)
 return -1;
 
 /* Old libvirt doesn't understand  elements so
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 98c188cc8f..07b0318258 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1048,7 +1048,8 @@ qemuDomainSupportsCheckpointsBlockjobs(virDomainObj *vm)
 G_GNUC_WARN_UNUSED_RESULT;
 
 int
-qemuDomainMakeCPUMigratable(virCPUDef *cpu);
+qemuDomainMakeCPUMigratable(virArch arch,
+virCPUDef *cpu);
 
 int
 qemuDomainInitializePflashStorageSource(virDomainObj *vm,
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 4361949cca..c53d42a018 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -545,7 +545,7 @@ qemuMigrationCookieAddCPU(qemuMigrationCookie *mig,
 
 mig->cpu = virCPUDefCopy(vm->def->cpu);
 
-if (qemuDomainMakeCPUMigratable(mig->cpu) < 0)
+if (qemuDomainMakeCPUMigratable(vm->def->os.arch, mig->cpu) < 0)
 return -1;
 
 mig->flags |= QEMU_MIGRATION_COOKIE_CPU;
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 01/22] cpu: x86: Add support for adding features to existing CPU models

2024-03-12 Thread Jiri Denemark
This is not a good idea in general, but we can (and have to) do it in
specific cases when a feature has always been part of a CPU model in
hypervisor's definition, but we ignored it and did not include the
feature in our definition.

Blindly adding the features to the CPU map and not adding them to
existing CPU models breaks migration between old and new libvirt in both
directions. New libvirt would complain the features got unexpectedly
enabled (as they were not mentioned in the incoming domain XML) even
though they were also enabled on the source and the old libvirt just
didn't know about them. On the other hand, old libvirt would refuse to
accept incoming migration of a domain started by new libvirt because the
domain XML would contain CPU features unknown to the old libvirt.

This is exactly what happened when several vmx-* features were added a
few releases back. Migration between libvirt releases before and after
the addition is now broken.

This patch adds support for added these features to existing CPU models
by marking them with added='yes'. The features will not be considered
part of the CPU model and will be described explicitly via additional
 elements, but the compatibility check will not complain if
they are enabled by the hypervisor even though they were not explicitly
mentioned in the CPU definition and incoming migration from old libvirt
will succeed.

To fix outgoing migration to old libvirt, we also need to drop all those
features from domain XML unless they were explicitly requested by the
user. This will be handled by a later patch.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_x86.c| 69 ++--
 src/cpu/cpu_x86.h|  3 ++
 src/libvirt_private.syms |  1 +
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index e8409ce616..b6193d84a7 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -148,6 +148,17 @@ struct _virCPUx86Model {
 virCPUx86Signatures *signatures;
 virCPUx86Data data;
 GStrv removedFeatures;
+
+/* Features added to the CPU model after its original version was released.
+ * Such features are not really considered part of the model, but the
+ * compatibility check will not complain if they are enabled by the
+ * hypervisor even though they were not explicitly mentioned in the CPU
+ * definition. This should only be used for features which were always
+ * included in the CPU model by the hypervisor, but libvirt didn't support
+ * them when introducing the CPU model. In other words, they were enabled,
+ * but we ignored them.
+ */
+GStrv addedFeatures;
 };
 
 typedef struct _virCPUx86Map virCPUx86Map;
@@ -1276,6 +1287,7 @@ x86ModelFree(virCPUx86Model *model)
 virCPUx86SignaturesFree(model->signatures);
 virCPUx86DataClear(>data);
 g_strfreev(model->removedFeatures);
+g_strfreev(model->addedFeatures);
 g_free(model);
 }
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUx86Model, x86ModelFree);
@@ -1291,6 +1303,7 @@ x86ModelCopy(virCPUx86Model *model)
 copy->signatures = virCPUx86SignaturesCopy(model->signatures);
 x86DataCopy(>data, >data);
 copy->removedFeatures = g_strdupv(model->removedFeatures);
+copy->addedFeatures = g_strdupv(model->addedFeatures);
 copy->vendor = model->vendor;
 
 return g_steal_pointer();
@@ -1596,17 +1609,20 @@ x86ModelParseFeatures(virCPUx86Model *model,
 g_autofree xmlNodePtr *nodes = NULL;
 size_t i;
 size_t nremoved = 0;
+size_t nadded = 0;
 int n;
 
 if ((n = virXPathNodeSet("./feature", ctxt, )) <= 0)
 return n;
 
 model->removedFeatures = g_new0(char *, n + 1);
+model->addedFeatures = g_new0(char *, n + 1);
 
 for (i = 0; i < n; i++) {
 g_autofree char *ftname = NULL;
 virCPUx86Feature *feature;
 virTristateBool rem;
+virTristateBool added;
 
 if (!(ftname = virXMLPropString(nodes[i], "name"))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1632,10 +1648,21 @@ x86ModelParseFeatures(virCPUx86Model *model,
 continue;
 }
 
+if (virXMLPropTristateBool(nodes[i], "added",
+   VIR_XML_PROP_NONE,
+   ) < 0)
+return -1;
+
+if (added == VIR_TRISTATE_BOOL_YES) {
+model->addedFeatures[nadded++] = g_strdup(ftname);
+continue;
+}
+
 x86DataAdd(>data, >data);
 }
 
 model->removedFeatures = g_renew(char *, model->removedFeatures, nremoved 
+ 1);
+model->addedFeatures = g_renew(char *, model->addedFeatures, nadded + 1);
 
 return 0;
 }
@@ -3030,11 +3057,13 @@ virCPUx86UpdateLive(virCPUDef *cpu,
 if (expected == VIR_CPU_FEATURE_DISABLE &&
 x86DataIsSubset(, >data)) {
   

[PATCH 00/22] qemu: Fix migration from libvirt older than 9.10.0 when vmx is enabled

2024-03-12 Thread Jiri Denemark
This series fixes a migration regression introduced in 9.10.0 release
which added support for a bunch of vmx-* CPU features. Domains with vmx
feature enabled started by libvirt 9.9.0 or older cannot be migrated to
current releases 9.10.0 and newer.

After this series migration works again with libvirt 9.9.0 and older in
both directions. Migration from the affected releases to the fixed
version works as well, but the other direction remains broken unless the
fix is backported.

See patches 1/22 and 4/22 for more details about the issue and the fix.

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/568

Jiri Denemark (22):
  cpu: x86: Add support for adding features to existing CPU models
  qemu: domain: Check arch in qemuDomainMakeCPUMigratable
  conf: cpu: Introduce virCPUDefListFeatures
  qemu: domain: Drop added features from migratable CPU
  cpu_map: Do not ignore VMX features in sync_qemu_models script
  Add vmx-* features to Broadwell*
  Add vmx-* features to Cascadelake*
  Add vmx-* features to Conroe
  Add vmx-* features to Cooperlake
  Add vmx-* features to core{,2}duo
  Add vmx-* features to Haswell*
  Add vmx-* features to Icelake*
  Add vmx-* features to IvyBridge*
  Add vmx-* features to kvm*
  Add vmx-* features to Nehalem*
  Add vmx-* features to Penryn
  Add vmx-* features to SandyBridge*
  Add vmx-* features to SapphireRapids
  Add vmx-* features to Skylake*
  Add vmx-* features to Snowridge
  Add vmx-* features to Westmere*
  NEWS: Document the fix for migration or vmx enabled domains

 NEWS.rst  |  12 ++
 src/conf/cpu_conf.c   |  24 
 src/conf/cpu_conf.h   |   3 +
 src/cpu/cpu_x86.c |  69 ++-
 src/cpu/cpu_x86.h |   3 +
 src/cpu_map/sync_qemu_models_i386.py  | 114 +-
 src/cpu_map/x86_Broadwell-IBRS.xml|  75 
 src/cpu_map/x86_Broadwell-noTSX-IBRS.xml  |  75 
 src/cpu_map/x86_Broadwell-noTSX.xml   |  75 
 src/cpu_map/x86_Broadwell.xml |  75 
 src/cpu_map/x86_Cascadelake-Server-noTSX.xml  |  76 
 src/cpu_map/x86_Cascadelake-Server.xml|  74 
 src/cpu_map/x86_Conroe.xml|  26 
 src/cpu_map/x86_Cooperlake.xml|  76 
 src/cpu_map/x86_Haswell-IBRS.xml  |  73 +++
 src/cpu_map/x86_Haswell-noTSX-IBRS.xml|  73 +++
 src/cpu_map/x86_Haswell-noTSX.xml |  73 +++
 src/cpu_map/x86_Haswell.xml   |  73 +++
 src/cpu_map/x86_Icelake-Server-noTSX.xml  |  77 
 src/cpu_map/x86_Icelake-Server.xml|  72 +++
 src/cpu_map/x86_IvyBridge-IBRS.xml|  67 ++
 src/cpu_map/x86_IvyBridge.xml |  67 ++
 src/cpu_map/x86_Nehalem-IBRS.xml  |  61 ++
 src/cpu_map/x86_Nehalem.xml   |  61 ++
 src/cpu_map/x86_Penryn.xml|  29 +
 src/cpu_map/x86_SandyBridge-IBRS.xml  |  63 ++
 src/cpu_map/x86_SandyBridge.xml   |  63 ++
 src/cpu_map/x86_SapphireRapids.xml|  77 
 src/cpu_map/x86_Skylake-Client-IBRS.xml   |  71 +++
 src/cpu_map/x86_Skylake-Client-noTSX-IBRS.xml |  72 +++
 src/cpu_map/x86_Skylake-Client.xml|  71 +++
 src/cpu_map/x86_Skylake-Server-IBRS.xml   |  74 
 src/cpu_map/x86_Skylake-Server-noTSX-IBRS.xml |  76 
 src/cpu_map/x86_Skylake-Server.xml|  74 
 src/cpu_map/x86_Snowridge.xml |  76 
 src/cpu_map/x86_Westmere-IBRS.xml |  63 ++
 src/cpu_map/x86_Westmere.xml  |  63 ++
 src/cpu_map/x86_core2duo.xml  |  26 
 src/cpu_map/x86_coreduo.xml   |  18 +++
 src/cpu_map/x86_kvm32.xml |  18 +++
 src/cpu_map/x86_kvm64.xml |  20 +++
 src/libvirt_private.syms  |   2 +
 src/qemu/qemu_domain.c|  60 -
 src/qemu/qemu_domain.h|   4 +-
 src/qemu/qemu_migration_cookie.c  |   5 +-
 45 files changed, 2487 insertions(+), 12 deletions(-)

-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH] docs: Update documentation of CPU models in domain caps

2024-03-05 Thread Jiri Denemark
Using check='none' when starting a domain with a CPU model marked as
usable is no longer needed as libvirt will do the right thing even with
check='partial'.

Signed-off-by: Jiri Denemark 
---
 docs/formatdomaincaps.rst | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
index ef752a0f3a..1999112944 100644
--- a/docs/formatdomaincaps.rst
+++ b/docs/formatdomaincaps.rst
@@ -237,10 +237,12 @@ more details about it:
using the CPU model and no additional feature elements. Models marked as
usable (``usable='yes'``) can be safely used in domain XMLs with
``check='none'`` as the hypervisor guarantees the model can be used on the
-   current host and additional checks done by libvirt are redundant. In fact,
-   disabling libvirt checks via ``check='none'`` for such models is recommended
-   to avoid needless issues with starting domains when libvirt's definition of
-   a particular model differs from hypervisor's definition. The
+   current host and additional checks done by libvirt are redundant.
+   :since:`Since 10.2.0` libvirt automatically detects this situation and
+   avoids the redundant checks, with older releases disabling libvirt checks
+   via ``check='none'`` for such models is recommended to avoid needless
+   issues with starting domains when libvirt's definition of a particular model
+   differs from hypervisor's definition. The
``deprecated`` attribute reflects the hypervisor's policy on usage of this
model :since:`(since 7.1.0)`. The ``vendor`` attribute :since:`(since 
8.9.0)`
contains the vendor of the CPU model for users who want to use CPU models
-- 
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


  1   2   >