[libvirt PATCH 00/16] expose tainting and deprecations in public API

Daniel P. Berrangé posted 16 patches 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210122171836.3523210-1-berrange@redhat.com
docs/formatdomaincaps.html.in                 |  10 +-
docs/schemas/capability.rng                   |   8 ++
docs/schemas/domaincaps.rng                   |   8 ++
include/libvirt/libvirt-domain.h              |   7 ++
src/conf/capabilities.c                       |   2 +
src/conf/capabilities.h                       |   1 +
src/conf/domain_capabilities.c                |  14 ++-
src/conf/domain_capabilities.h                |   4 +-
src/conf/domain_conf.c                        |  29 ++++-
src/conf/domain_conf.h                        |   5 +
src/driver-hypervisor.h                       |  11 ++
src/libvirt-domain.c                          |  95 +++++++++++++++
src/libvirt_private.syms                      |   1 +
src/libvirt_public.syms                       |   6 +
src/qemu/qemu_capabilities.c                  |  60 +++++++++-
src/qemu/qemu_capabilities.h                  |   6 +
src/qemu/qemu_capspriv.h                      |   3 +-
src/qemu/qemu_domain.c                        | 111 ++++++++++++++++--
src/qemu/qemu_domain.h                        |   7 ++
src/qemu/qemu_driver.c                        |  66 +++++++++++
src/qemu/qemu_monitor.c                       |   1 +
src/qemu/qemu_monitor.h                       |   2 +
src/qemu/qemu_monitor_json.c                  |   8 ++
src/qemu/qemu_process.c                       |   5 +
src/remote/remote_daemon_dispatch.c           |  91 ++++++++++++++
src/remote/remote_driver.c                    |  88 ++++++++++++++
src/remote/remote_protocol.x                  |  39 +++++-
tests/cputest.c                               |   4 +-
.../domaincapsdata/qemu_5.2.0-q35.x86_64.xml  |   4 +-
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml  |   4 +-
tests/domaincapsdata/qemu_5.2.0.x86_64.xml    |   4 +-
.../caps_4.1.0.x86_64.xml                     |  16 +--
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |   4 +-
.../caps_4.2.0.x86_64.xml                     |  16 +--
.../caps_5.0.0.x86_64.xml                     |  16 +--
.../caps_5.1.0.x86_64.xml                     |  16 +--
.../caps_5.2.0.x86_64.xml                     |  32 ++---
tests/testutilsqemu.c                         |   8 +-
tools/virsh-domain-monitor.c                  |  25 ++++
39 files changed, 745 insertions(+), 92 deletions(-)
[libvirt PATCH 00/16] expose tainting and deprecations in public API
Posted by Daniel P. Berrangé 3 years, 2 months ago
Libvirt has a notion of "tainting" which we use to mark a guest which
has some undesirable configuration or behaviour from libvirt's POV.
This ends up in the libvirtd logs and in the per-VM log file, but is
not exposed to management applications directly.

QMP has the ability to report whether a CPU or machine type is
deprecated.

QEMU itself prints warnings to stderr which end up in the per VM log:

2021-01-22T12:22:53.566239Z qemu-system-x86_64: Machine type 'pc-1.3' is depr=
ecated: use a newer machine type instead
2021-01-22T12:22:53.566613Z qemu-system-x86_64: warning: CPU model Icelake-Cl=
ient-x86_64-cpu is deprecated -- use Icelake-Server instead

We can use the deprecation info from QMP to add tainting to the
domain too. This will appear in the pre-VM log file again:

2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
ion (machine type 'pc-1.3')
2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
ion (CPU model 'Icelake-Client')

and more usefully in the libvirtd log

2021-01-22 13:18:09.619+0000: 3299849: warning :
qemuDomainObjTaintMsg:6208 :
Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
is tainted: deprecated-configuration (machine type 'pc-1.3')

2021-01-22 13:18:09.619+0000: 3299849: warning :
qemuDomainObjTaintMsg:6208 :
Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
is tainted: deprecated-configuration (CPU model 'Icelake-Client')

This series goes further and also exposes the deprecation info in the
capabilities (machine types) or domain capabilities (CPU) XML. This
lets mgmt apps avoid using the feature upfront if desired.

Finally both deprecation messages and tainting flags are exposed in
new public APIs, and wired into virsh

$ virsh dominfo demo
Id:             3
Name:           demo
UUID:           eadf8ef0-bf14-4c5f-9708-4a19bacf9e81
OS Type:        hvm
State:          running
CPU(s):         2
CPU time:       1.3s
Max memory:     1536000 KiB
Used memory:    1536000 KiB
Persistent:     yes
Autostart:      disable
Managed save:   no
Security model: selinux
Security DOI:   0
Security label: unconfined_u:unconfined_r:svirt_t:s0:c578,c807 (permissive)
Tainting:       custom-monitor
                deprecated-config
Deprecations:   CPU model 'Icelake-Client'
                machine type 'pc-1.3'

The deprecations API is simple, just returning a list of free form
opaque strings, which are eeffectively warning messages.

I'm not entirely convinced by tainting API though. I didn't especially
want to expose the virDomainTaintFlags  enum in the public API since it
feels like the enum flags are (almost) all QEMU driver specific. I thus
took the approach of having an API return opaque strings which are
declared to be hypervisor specific.

I'm worried though that mgmt apps will none the less simply match on
the strings to detect things, at which point we might as well just use
an enum after all.

So perhaps it should just be turned into

  virDomainGetTainting(virDomainPtr obj,
                       int **codes,
		       unsigned int flags);

  enum virDomainTaintCodes {
     ....
  }

Daniel P. Berrang=C3=A9 (16):
  qemu: report whether a CPU model is deprecated in dom capabilities
  qemu: report whether a machine type is deprecated in capabilities
  conf: introduce new taint flag for deprecated configuration
  qemu: add ability to associate a string message with taint warning
  qemu: taint the VM if it is using a deprecated CPU model
  qemu: taint the VM if it is using a deprecated machine type
  conf: record deprecation messages against the domain
  qemu: record deprecation messages against the domain
  src: define virDomainGetDeprecations API
  remote: add RPC support for the virDomainGetDeprecations API
  qemu: implement virDomainGetDeprecations API
  tools: report deprecations for 'dominfo' command
  src: define virDomainGetTainting API
  remote: add RPC support for the virDomainGetTainting API
  qemu: implement virDomainGetTainting API
  tools: report tainting for 'dominfo' command

 docs/formatdomaincaps.html.in                 |  10 +-
 docs/schemas/capability.rng                   |   8 ++
 docs/schemas/domaincaps.rng                   |   8 ++
 include/libvirt/libvirt-domain.h              |   7 ++
 src/conf/capabilities.c                       |   2 +
 src/conf/capabilities.h                       |   1 +
 src/conf/domain_capabilities.c                |  14 ++-
 src/conf/domain_capabilities.h                |   4 +-
 src/conf/domain_conf.c                        |  29 ++++-
 src/conf/domain_conf.h                        |   5 +
 src/driver-hypervisor.h                       |  11 ++
 src/libvirt-domain.c                          |  95 +++++++++++++++
 src/libvirt_private.syms                      |   1 +
 src/libvirt_public.syms                       |   6 +
 src/qemu/qemu_capabilities.c                  |  60 +++++++++-
 src/qemu/qemu_capabilities.h                  |   6 +
 src/qemu/qemu_capspriv.h                      |   3 +-
 src/qemu/qemu_domain.c                        | 111 ++++++++++++++++--
 src/qemu/qemu_domain.h                        |   7 ++
 src/qemu/qemu_driver.c                        |  66 +++++++++++
 src/qemu/qemu_monitor.c                       |   1 +
 src/qemu/qemu_monitor.h                       |   2 +
 src/qemu/qemu_monitor_json.c                  |   8 ++
 src/qemu/qemu_process.c                       |   5 +
 src/remote/remote_daemon_dispatch.c           |  91 ++++++++++++++
 src/remote/remote_driver.c                    |  88 ++++++++++++++
 src/remote/remote_protocol.x                  |  39 +++++-
 tests/cputest.c                               |   4 +-
 .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml  |   4 +-
 .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml  |   4 +-
 tests/domaincapsdata/qemu_5.2.0.x86_64.xml    |   4 +-
 .../caps_4.1.0.x86_64.xml                     |  16 +--
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |   4 +-
 .../caps_4.2.0.x86_64.xml                     |  16 +--
 .../caps_5.0.0.x86_64.xml                     |  16 +--
 .../caps_5.1.0.x86_64.xml                     |  16 +--
 .../caps_5.2.0.x86_64.xml                     |  32 ++---
 tests/testutilsqemu.c                         |   8 +-
 tools/virsh-domain-monitor.c                  |  25 ++++
 39 files changed, 745 insertions(+), 92 deletions(-)

--=20
2.29.2


Re: [libvirt PATCH 00/16] expose tainting and deprecations in public API
Posted by Ján Tomko 3 years, 2 months ago
On a Friday in 2021, Daniel P. Berrangé wrote:
>Libvirt has a notion of "tainting" which we use to mark a guest which
>has some undesirable configuration or behaviour from libvirt's POV.
>This ends up in the libvirtd logs and in the per-VM log file, but is
>not exposed to management applications directly.
>
>QMP has the ability to report whether a CPU or machine type is
>deprecated.
>
>QEMU itself prints warnings to stderr which end up in the per VM log:
>
>2021-01-22T12:22:53.566239Z qemu-system-x86_64: Machine type 'pc-1.3' is depr=
>ecated: use a newer machine type instead
>2021-01-22T12:22:53.566613Z qemu-system-x86_64: warning: CPU model Icelake-Cl=
>ient-x86_64-cpu is deprecated -- use Icelake-Server instead
>
>We can use the deprecation info from QMP to add tainting to the
>domain too. This will appear in the pre-VM log file again:
>
>2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
>ion (machine type 'pc-1.3')
>2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
>ion (CPU model 'Icelake-Client')
>
>and more usefully in the libvirtd log
>
>2021-01-22 13:18:09.619+0000: 3299849: warning :
>qemuDomainObjTaintMsg:6208 :
>Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
>is tainted: deprecated-configuration (machine type 'pc-1.3')
>
>2021-01-22 13:18:09.619+0000: 3299849: warning :
>qemuDomainObjTaintMsg:6208 :
>Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
>is tainted: deprecated-configuration (CPU model 'Icelake-Client')
>
>This series goes further and also exposes the deprecation info in the
>capabilities (machine types) or domain capabilities (CPU) XML. This
>lets mgmt apps avoid using the feature upfront if desired.
>
>Finally both deprecation messages and tainting flags are exposed in
>new public APIs, and wired into virsh
>
>$ virsh dominfo demo
>Id:             3
>Name:           demo
>UUID:           eadf8ef0-bf14-4c5f-9708-4a19bacf9e81
>OS Type:        hvm
>State:          running
>CPU(s):         2
>CPU time:       1.3s
>Max memory:     1536000 KiB
>Used memory:    1536000 KiB
>Persistent:     yes
>Autostart:      disable
>Managed save:   no
>Security model: selinux
>Security DOI:   0
>Security label: unconfined_u:unconfined_r:svirt_t:s0:c578,c807 (permissive)
>Tainting:       custom-monitor
>                deprecated-config
>Deprecations:   CPU model 'Icelake-Client'
>                machine type 'pc-1.3'
>
>The deprecations API is simple, just returning a list of free form
>opaque strings, which are eeffectively warning messages.
>
>I'm not entirely convinced by tainting API though. I didn't especially
>want to expose the virDomainTaintFlags  enum in the public API since it
>feels like the enum flags are (almost) all QEMU driver specific. I thus
>took the approach of having an API return opaque strings which are
>declared to be hypervisor specific.
>
>I'm worried though that mgmt apps will none the less simply match on
>the strings to detect things, at which point we might as well just use
>an enum after all.
>

Depending on the app, there might be no need to even call GetTainting in
the first place - the app already has the power not to use any of the
tainted features, with the exception of the newly-added
deprecated-config, which can happen after migration to a newer QEMU.

So they can just query for deprecations directly.

Also, with the deprecations being an opaque string, can the apps
take any different action than just passing them to the user?
It seems to me the APIs are better for humans and the XML in
capabilities is better for apps, but I've never really written one.

>So perhaps it should just be turned into
>
>  virDomainGetTainting(virDomainPtr obj,
>                       int **codes,
>		       unsigned int flags);
>
>  enum virDomainTaintCodes {
>     ....
>  }
>
>Daniel P. Berrang=C3=A9 (16):
>  qemu: report whether a CPU model is deprecated in dom capabilities
>  qemu: report whether a machine type is deprecated in capabilities
>  conf: introduce new taint flag for deprecated configuration
>  qemu: add ability to associate a string message with taint warning
>  qemu: taint the VM if it is using a deprecated CPU model
>  qemu: taint the VM if it is using a deprecated machine type
>  conf: record deprecation messages against the domain
>  qemu: record deprecation messages against the domain
>  src: define virDomainGetDeprecations API
>  remote: add RPC support for the virDomainGetDeprecations API
>  qemu: implement virDomainGetDeprecations API
>  tools: report deprecations for 'dominfo' command
>  src: define virDomainGetTainting API
>  remote: add RPC support for the virDomainGetTainting API
>  qemu: implement virDomainGetTainting API
>  tools: report tainting for 'dominfo' command
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH 00/16] expose tainting and deprecations in public API
Posted by Daniel P. Berrangé 3 years, 1 month ago
On Mon, Jan 25, 2021 at 11:26:08AM +0100, Ján Tomko wrote:
> On a Friday in 2021, Daniel P. Berrangé wrote:
> > Libvirt has a notion of "tainting" which we use to mark a guest which
> > has some undesirable configuration or behaviour from libvirt's POV.
> > This ends up in the libvirtd logs and in the per-VM log file, but is
> > not exposed to management applications directly.
> > 
> > QMP has the ability to report whether a CPU or machine type is
> > deprecated.
> > 
> > QEMU itself prints warnings to stderr which end up in the per VM log:
> > 
> > 2021-01-22T12:22:53.566239Z qemu-system-x86_64: Machine type 'pc-1.3' is depr=
> > ecated: use a newer machine type instead
> > 2021-01-22T12:22:53.566613Z qemu-system-x86_64: warning: CPU model Icelake-Cl=
> > ient-x86_64-cpu is deprecated -- use Icelake-Server instead
> > 
> > We can use the deprecation info from QMP to add tainting to the
> > domain too. This will appear in the pre-VM log file again:
> > 
> > 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> > ion (machine type 'pc-1.3')
> > 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> > ion (CPU model 'Icelake-Client')
> > 
> > and more usefully in the libvirtd log
> > 
> > 2021-01-22 13:18:09.619+0000: 3299849: warning :
> > qemuDomainObjTaintMsg:6208 :
> > Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> > is tainted: deprecated-configuration (machine type 'pc-1.3')
> > 
> > 2021-01-22 13:18:09.619+0000: 3299849: warning :
> > qemuDomainObjTaintMsg:6208 :
> > Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> > is tainted: deprecated-configuration (CPU model 'Icelake-Client')
> > 
> > This series goes further and also exposes the deprecation info in the
> > capabilities (machine types) or domain capabilities (CPU) XML. This
> > lets mgmt apps avoid using the feature upfront if desired.
> > 
> > Finally both deprecation messages and tainting flags are exposed in
> > new public APIs, and wired into virsh
> > 
> > $ virsh dominfo demo
> > Id:             3
> > Name:           demo
> > UUID:           eadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> > OS Type:        hvm
> > State:          running
> > CPU(s):         2
> > CPU time:       1.3s
> > Max memory:     1536000 KiB
> > Used memory:    1536000 KiB
> > Persistent:     yes
> > Autostart:      disable
> > Managed save:   no
> > Security model: selinux
> > Security DOI:   0
> > Security label: unconfined_u:unconfined_r:svirt_t:s0:c578,c807 (permissive)
> > Tainting:       custom-monitor
> >                deprecated-config
> > Deprecations:   CPU model 'Icelake-Client'
> >                machine type 'pc-1.3'
> > 
> > The deprecations API is simple, just returning a list of free form
> > opaque strings, which are eeffectively warning messages.
> > 
> > I'm not entirely convinced by tainting API though. I didn't especially
> > want to expose the virDomainTaintFlags  enum in the public API since it
> > feels like the enum flags are (almost) all QEMU driver specific. I thus
> > took the approach of having an API return opaque strings which are
> > declared to be hypervisor specific.
> > 
> > I'm worried though that mgmt apps will none the less simply match on
> > the strings to detect things, at which point we might as well just use
> > an enum after all.
> > 
> 
> Depending on the app, there might be no need to even call GetTainting in
> the first place - the app already has the power not to use any of the
> tainted features, with the exception of the newly-added
> deprecated-config, which can happen after migration to a newer QEMU.
> 
> So they can just query for deprecations directly.

It probably depends on the mgmt app to some degree.

With OpenStack there have been cases where they were using deprecated
features because no one told them they were deprecated. We're better
at dociumenting this in QEMU release notes, but not every reads this
until its too late.

In the virt-manager case it will let the user use pretty much
anything that exists in QEMU. My thought was that virt-manager
could display a "💣" icon next to any VM which is marked as
tainted by libvirt, and when opening the VM details, it could
display the list of deprecation messages to the user </handwaving>

> Also, with the deprecations being an opaque string, can the apps
> take any different action than just passing them to the user?
> It seems to me the APIs are better for humans and the XML in
> capabilities is better for apps, but I've never really written one.

I'm not really expecting apps to make significant functional
decisions based on these APIs. By the time the VM is reporting
tainting or deprecations, it is too late - the app/user already
made the bad decisions. So mostly I think it'll just be
displayed to the user "as is", as a way to alert them to something
sub-optimal going on.  The user can read it and figure out what
is wrong.

With that in mind, maybe for the tainting we should also just
display human readable messages too, rather than codes.


> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>

Although you ACKd this, i'm going to just push the first part which
doesn't have public API impact while I think some more.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH 00/16] expose tainting and deprecations in public API
Posted by Eduardo Habkost 3 years, 2 months ago
Related question: does libvirt generate any kind of structured
log records, or all log examples below are going to be stored in
plain text only?

On Fri, Jan 22, 2021 at 05:18:20PM +0000, Daniel P. Berrangé wrote:
[...]
> We can use the deprecation info from QMP to add tainting to the
> domain too. This will appear in the pre-VM log file again:
> 
> 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> ion (machine type 'pc-1.3')
> 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> ion (CPU model 'Icelake-Client')
> 
> and more usefully in the libvirtd log
> 
> 2021-01-22 13:18:09.619+0000: 3299849: warning :
> qemuDomainObjTaintMsg:6208 :
> Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> is tainted: deprecated-configuration (machine type 'pc-1.3')
> 
> 2021-01-22 13:18:09.619+0000: 3299849: warning :
> qemuDomainObjTaintMsg:6208 :
> Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> is tainted: deprecated-configuration (CPU model 'Icelake-Client')
> 
[...]

-- 
Eduardo

Re: [libvirt PATCH 00/16] expose tainting and deprecations in public API
Posted by Daniel P. Berrangé 3 years, 2 months ago
On Mon, Jan 25, 2021 at 02:41:06PM -0500, Eduardo Habkost wrote:
> Related question: does libvirt generate any kind of structured
> log records, or all log examples below are going to be stored in
> plain text only?

Log files are plain text. The API proposals here are what provide
any formal data to apps.

> 
> On Fri, Jan 22, 2021 at 05:18:20PM +0000, Daniel P. Berrangé wrote:
> [...]
> > We can use the deprecation info from QMP to add tainting to the
> > domain too. This will appear in the pre-VM log file again:
> > 
> > 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> > ion (machine type 'pc-1.3')
> > 2021-01-22 12:22:53.492+0000: Domain id=3D2 is tainted: deprecated-configurat=
> > ion (CPU model 'Icelake-Client')
> > 
> > and more usefully in the libvirtd log
> > 
> > 2021-01-22 13:18:09.619+0000: 3299849: warning :
> > qemuDomainObjTaintMsg:6208 :
> > Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> > is tainted: deprecated-configuration (machine type 'pc-1.3')
> > 
> > 2021-01-22 13:18:09.619+0000: 3299849: warning :
> > qemuDomainObjTaintMsg:6208 :
> > Domain id=3D3 name=3D'demo' uuid=3Deadf8ef0-bf14-4c5f-9708-4a19bacf9e81
> > is tainted: deprecated-configuration (CPU model 'Icelake-Client')
> > 
> [...]
> 
> -- 
> Eduardo

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|