Re: [libvirt] [REPOSTv2 PATCH v3 0/6] Add support for VM Generation ID (vmgenid)

On 05/25/2018 04:24 AM, Michal Privoznik wrote: > On 05/17/2018 02:42 PM, John Ferlan wrote: >> Second reposting of: >> >> https://www.redhat.com/archives/libvir-list/2018-May/msg00813.html >> >> To update patches with more conflicts for patch 2 (capabilities) and >> patch 6 (news) >> >> Cover from the v3 posting: >> >> v2: https://www.redhat.com/archives/libvir-list/2018-April/msg02234.html >> >> Changes since v2: >> >> * Essentially handle comments from code review of original series from >> comments received for patch 6: >> >> https://www.redhat.com/archives/libvir-list/2018-April/msg02240.html >> >> It's a somewhat simplified approach removing the ABI checks and the >> adjustment to the genid value as part of domain def copy. >> >> * (NEW) Patch 5 - add a 'genid' domain capability (similar to how Cole >> added support for vmcoreinfo). Since the apps need a way to determine >> whether this is enabled, this seems to be the best way. >> >> >> John Ferlan (6): >> conf: Add VM Generation ID parse/format support >> qemu: Add VM Generation ID device capability >> qemu: Alter VM Generation ID for specific startup/launch transitions >> qemu: Add VM Generation ID to qemu command line >> domcaps: Add 'genid' to domain capabilities >> docs: Add news article for VM Generation ID >> >> docs/formatdomain.html.in | 27 +++++++++++ >> docs/formatdomaincaps.html.in | 7 ++- >> docs/news.xml | 13 ++++++ >> docs/schemas/domaincaps.rng | 7 +++ >> docs/schemas/domaincommon.rng | 8 ++++ >> src/conf/domain_capabilities.c | 3 ++ >> src/conf/domain_capabilities.h | 1 + >> src/conf/domain_conf.c | 54 ++++++++++++++++++++++ >> src/conf/domain_conf.h | 5 ++ >> src/qemu/qemu_capabilities.c | 4 ++ >> src/qemu/qemu_capabilities.h | 1 + >> src/qemu/qemu_command.c | 24 ++++++++++ >> src/qemu/qemu_driver.c | 17 +++++-- >> src/qemu/qemu_process.c | 46 +++++++++++++++++- >> src/qemu/qemu_process.h | 1 + >> tests/domaincapsschemadata/basic.xml | 1 + >> tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 1 + >> tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 1 + >> tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 1 + >> tests/domaincapsschemadata/full.xml | 1 + >> tests/domaincapsschemadata/libxl-xenfv-usb.xml | 1 + >> tests/domaincapsschemadata/libxl-xenfv.xml | 1 + >> tests/domaincapsschemadata/libxl-xenpv-usb.xml | 1 + >> tests/domaincapsschemadata/libxl-xenpv.xml | 1 + >> tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 + >> .../qemu_2.12.0-virt.aarch64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.12.0.s390x.xml | 1 + >> tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 1 + >> .../qemu_2.6.0-virt.aarch64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 + >> .../domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 + >> tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml | 1 + >> .../domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 1 + >> .../domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml | 1 + >> tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + >> .../qemuxml2argvdata/genid-auto.x86_64-latest.args | 30 ++++++++++++ >> tests/qemuxml2argvdata/genid-auto.xml | 32 +++++++++++++ >> tests/qemuxml2argvdata/genid.x86_64-latest.args | 30 ++++++++++++ >> tests/qemuxml2argvdata/genid.xml | 32 +++++++++++++ >> tests/qemuxml2argvtest.c | 4 ++ >> tests/qemuxml2xmloutdata/genid-active.xml | 32 +++++++++++++ >> tests/qemuxml2xmloutdata/genid-auto-active.xml | 32 +++++++++++++ >> tests/qemuxml2xmloutdata/genid-auto-inactive.xml | 32 +++++++++++++ >> tests/qemuxml2xmloutdata/genid-inactive.xml | 32 +++++++++++++ >> tests/qemuxml2xmltest.c | 5 +- >> 53 files changed, 500 insertions(+), 7 deletions(-) >> create mode 100644 tests/qemuxml2argvdata/genid-auto.x86_64-latest.args >> create mode 100644 tests/qemuxml2argvdata/genid-auto.xml >> create mode 100644 tests/qemuxml2argvdata/genid.x86_64-latest.args >> create mode 100644 tests/qemuxml2argvdata/genid.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-active.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-active.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-inactive.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-inactive.xml >> > > I like the patches, I do. I'd ACK them but some discussion is needed > first in my opinion. > Discussion about what? Honestly this "version" of the patches has been languishing for the entire month without any review. I know "everyone" is "busy" with their own stuff and facing their own deadlines so I don't expect immediate review, but a whole month without any feedback is a bitter pill to swallow. Now that we reach the end of the month and release time - the design is being called into question. That's fine - although IMO I got review on the original design from the RFC posted in March and the v1 posted in early April. The v2 series posted a couple weeks later gave a lot more important feedback and resulted in the adjustments posted here. So as noted in another response, the v2 review gave the best feedback and is the basis for this simplified approach.