Re: [libvirt] [PATCH 0/7] qemu: use domCaps for validation

On 4/5/19 1:37 AM, Cole Robinson wrote: > I'm trying to remove some hurdles and pitfalls WRT extending > domaincapabilities data. One issue is that it's too easy to add > invalid data to it, or let the data become out of date. > > For example the first two patches of this series add <rng model=X> > domcaps reporting. The logic to fill in the domcaps data from qemuCaps > is nearly identical to the logic we use to validate rng->model in > qemuDomainRNGDefValidate. If just those patches are added, and later > a new qemu rng model was introduced, a future patch could easily > miss updated domaincapabilities output. > > This series aims to set up a pattern to prevent these types of issues > from sneaking in. A function virDomainCapsDeviceDefValidate is added > which will use domcaps data to perform validation against a devicedef. > The existing qemu <rng> model validation is moved there. This ensures > that any future <rng> model additions, if they want to work in the > qemu driver, effectively need to extend domaincapabilities as well. > It's also theoretically useful for other drivers too. > > One issue is that at DomainDefValidate time we don't have domCaps > handy, or any cache layer for domCaps assembling. Patch #4 adds > a domCapsCache hashtable to the virQEMUCaps class for caching domCaps > builds based on the full tuple of emulator+machine+arch+virttype. > If qemuCaps need to be regenerated, the domCaps cache is wiped out > for us so we don't need to worry about the data being stale, it's > tied to the lifetime of a qemuCaps instance. > > Cole Robinson (7): >    conf: domcaps: Report device <rng> >    qemu: capabilities: fill in domcaps <rng> >    qemu: conf: add virQEMUDriverGetDomainCapabilities >    qemu: conf: Cache domCaps in qemuCaps >    conf: domcaps: Add virDomainCapsDeviceDefValidate >    qemu: domain: Call virDomainCapsDeviceDefValidate >    qemu: Move rng model validation to domcaps > >   docs/formatdomaincaps.html.in                 | 35 ++++++++ >   docs/schemas/domaincaps.rng                   | 10 +++ >   src/conf/domain_capabilities.c                | 83 ++++++++++++++++++ >   src/conf/domain_capabilities.h                | 14 ++++ >   src/libvirt_private.syms                      |  1 + >   src/qemu/qemu_capabilities.c                  | 41 +++++++++ >   src/qemu/qemu_capabilities.h                  |  1 + >   src/qemu/qemu_conf.c                          | 84 +++++++++++++++++++ >   src/qemu/qemu_conf.h                          |  7 ++ >   src/qemu/qemu_domain.c                        | 38 +++------ >   src/qemu/qemu_driver.c                        | 18 +--- >   .../qemu_1.7.0.x86_64.xml                     |  9 ++ >   .../qemu_2.12.0-virt.aarch64.xml              | 11 +++ >   .../qemu_2.12.0.ppc64.xml                     | 11 +++ >   .../qemu_2.12.0.s390x.xml                     | 11 +++ >   .../qemu_2.12.0.x86_64.xml                    | 11 +++ >   .../qemu_2.6.0-virt.aarch64.xml               | 11 +++ >   .../qemu_2.6.0.aarch64.xml                    | 11 +++ >   .../domaincapsschemadata/qemu_2.6.0.ppc64.xml | 11 +++ >   .../qemu_2.6.0.x86_64.xml                     | 11 +++ >   .../domaincapsschemadata/qemu_2.7.0.s390x.xml | 11 +++ >   .../qemu_2.8.0-tcg.x86_64.xml                 | 11 +++ >   .../domaincapsschemadata/qemu_2.8.0.s390x.xml | 11 +++ >   .../qemu_2.8.0.x86_64.xml                     | 11 +++ >   .../qemu_2.9.0-q35.x86_64.xml                 | 11 +++ >   .../qemu_2.9.0-tcg.x86_64.xml                 | 11 +++ >   .../qemu_2.9.0.x86_64.xml                     | 11 +++ >   .../domaincapsschemadata/qemu_3.0.0.s390x.xml | 11 +++ >   .../qemu_4.0.0.x86_64.xml                     | 11 +++ >   29 files changed, 488 insertions(+), 40 deletions(-) > ACK if you address Daniel's and mine findings. Sorry for delayed review.