On 4/29/19 8:35 AM, Michal Privoznik wrote: > 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. > So I completely forgot about this series and never even saw the ACKs! I rebased the patches and they applied cleanly besides the driver->privileged piece that was already mentioned. So I addressed all the original review comments, add RB tags for you and Daniel, and pushed now. So no worries on the review delay :) Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list