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

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Ping?

Or do I need to repost yet again?

Tks,

John

On 05/17/2018 08:42 AM, 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
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/25/2018 01:10 PM, John Ferlan wrote:
> 
> 
> 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.

I meant answers to my questions. Esp. on the capability check and usage
of flags. If you fix the capability check like I'm suggesting in 4/6 you
have my ACK for the whole patch set.

Also, sorry for leaving these patches so long without any review.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/25/2018 07:39 AM, Michal Privoznik wrote:
> On 05/25/2018 01:10 PM, John Ferlan wrote:
>>
>>
>> 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.
> 
> I meant answers to my questions. Esp. on the capability check and usage
> of flags. If you fix the capability check like I'm suggesting in 4/6 you
> have my ACK for the whole patch set.

Ironically, as part of review of v2 patch 10, I was asked to remove the
check from qemuBuildVMGenIDCommandLine:

https://www.redhat.com/archives/libvir-list/2018-April/msg02253.html

Perhaps it's best to just move it from qemu_process to qemu_command -
that just means passing @qemuCaps into qemuBuildVMGenIDCommandLine and
using it as well as removing @priv from qemuProcessGenID.

It really doesn't matter in qemu_process whether the capability exists,
it was an optimization to put it there rather than an error path in
qemu_command... and with that change GenIDCommandLine now could return
-1 or 0; whereas, without the caps check it was only ever returning 0.

I can repost the entire series if so desired.

Tks -

John

> 
> Also, sorry for leaving these patches so long without any review.
> 
> Michal
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/25/2018 02:04 PM, John Ferlan wrote:
> 
> 
> On 05/25/2018 07:39 AM, Michal Privoznik wrote:
>> On 05/25/2018 01:10 PM, John Ferlan wrote:
>>>
>>>
>>> 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.
>>
>> I meant answers to my questions. Esp. on the capability check and usage
>> of flags. If you fix the capability check like I'm suggesting in 4/6 you
>> have my ACK for the whole patch set.
> 
> Ironically, as part of review of v2 patch 10, I was asked to remove the
> check from qemuBuildVMGenIDCommandLine:
> 
> https://www.redhat.com/archives/libvir-list/2018-April/msg02253.html
> 
> Perhaps it's best to just move it from qemu_process to qemu_command -
> that just means passing @qemuCaps into qemuBuildVMGenIDCommandLine and
> using it as well as removing @priv from qemuProcessGenID.
> 
> It really doesn't matter in qemu_process whether the capability exists,
> it was an optimization to put it there rather than an error path in
> qemu_command... and with that change GenIDCommandLine now could return
> -1 or 0; whereas, without the caps check it was only ever returning 0.
> 
> I can repost the entire series if so desired.

I don't think it's needed. Just fix and push.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[...]

>>> of flags. If you fix the capability check like I'm suggesting in 4/6 you
>>> have my ACK for the whole patch set.
>>
>> Ironically, as part of review of v2 patch 10, I was asked to remove the
>> check from qemuBuildVMGenIDCommandLine:
>>
>> https://www.redhat.com/archives/libvir-list/2018-April/msg02253.html
>>
>> Perhaps it's best to just move it from qemu_process to qemu_command -
>> that just means passing @qemuCaps into qemuBuildVMGenIDCommandLine and
>> using it as well as removing @priv from qemuProcessGenID.
>>
>> It really doesn't matter in qemu_process whether the capability exists,
>> it was an optimization to put it there rather than an error path in
>> qemu_command... and with that change GenIDCommandLine now could return
>> -1 or 0; whereas, without the caps check it was only ever returning 0.
>>
>> I can repost the entire series if so desired.
> 
> I don't think it's needed. Just fix and push.
> 

OK - thanks. The removal of the caps check will be done in patch 3 since
that's where qemuProcessGenID was introduced, I've also beefed up the
[sparse] comments to the function to be:

 * If this domain is requesting to use genid, then update the GUID
 * value if the VIR_QEMU_PROCESS_START_GEN_VMID flag is set. This
 * flag is set on specific paths during domain start processing when
 * there is the possibility that the VM is potentially re-executing
 * something that has already been executed before.

FWIW: The last part of the wording is taken from Daniel's v2 response.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list