mbox series

[v2,00/11] conf: partial net model enum conversion

Message ID cover.1552492022.git.crobinso@redhat.com
Headers show
Series conf: partial net model enum conversion | expand

Message

Cole Robinson March 13, 2019, 3:51 p.m. UTC
v1 here:
https://www.redhat.com/archives/libvir-list/2019-January/msg00763.html

Changes since v1:
- patch #7, case insensitive model input comparison
- Add xml2xml testing
- compile tested on freebsd12.0

This series partially converts the net->model value from a string
to an enum. We wrap the existing ->model string in accessor functions,
rename it to ->modelstr, add a ->model enum, and convert internal
driver usage bit by bit. At the end, all driver code that is acting
on specific network model values is comparing against an enum, not
a string.

This is only partial because of xen/libxl/xm and qemu drivers, which
if they don't know anything particular about the model string will
just place it on the qemu command line/xen config and see what happens.
So basically if I were to pass in

  <model type='idontexist'/>

qemu would turn that into

  -device idontexist,...

That behavior is untouched by this series, as fully unwinding that
will take some more work:

 * Figuring out all reasonable qemu + xen values that could actually
   result in a working VM config, and adding them to the enum
 * Figuring out a long term plan for disabling passthrough entirely.
   There's some discussion in the v1 thread about this.

Some caveats:
 * vz driver is not compile tested. What's the sdk magic to actually
   get this building?
 * net model enum lookup is done case insensitive. this is to maintain
   the behavior of the vmx and virtualbox drivers, but it's different
   than all our other enum usage.

Cole Robinson (11):
  tests: Add several net model passthrough tests
  conf: net: Add wrapper functions for <model> value
  conf: net: Rename 'model' to 'modelstr'
  conf: net: Add model enum, and netfront value
  vz: convert to net model enum
  bhyve: convert to net model enum
  qemu: Partially convert to net model enum
  conf: Make net model enum compare case insensitive
  vmx: convert to net model enum
  vbox: Convert to net enum model
  conf: Add VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING

 src/bhyve/bhyve_command.c                     |  15 +--
 src/bhyve/bhyve_parse_command.c               |  10 +-
 src/conf/domain_conf.c                        | 111 ++++++++++++++----
 src/conf/domain_conf.h                        |  35 +++++-
 src/libvirt_private.syms                      |   4 +
 src/libxl/libxl_conf.c                        |   8 +-
 src/libxl/libxl_domain.c                      |   1 +
 src/qemu/qemu_command.c                       |  13 +-
 src/qemu/qemu_domain.c                        |  32 +++--
 src/qemu/qemu_domain_address.c                |  13 +-
 src/qemu/qemu_driver.c                        |  14 ++-
 src/qemu/qemu_hotplug.c                       |  15 ++-
 src/qemu/qemu_parse_command.c                 |   5 +-
 src/security/virt-aa-helper.c                 |   3 +-
 src/vbox/vbox_common.c                        |  29 ++---
 src/vmx/vmx.c                                 |  55 ++++-----
 src/vz/vz_driver.c                            |   7 +-
 src/vz/vz_sdk.c                               |  17 ++-
 src/xenconfig/xen_common.c                    |  31 ++---
 src/xenconfig/xen_sxpr.c                      |  30 ++---
 tests/qemuxml2argvdata/net-many-models.args   |  39 ++++++
 tests/qemuxml2argvdata/net-many-models.xml    |  38 ++++++
 tests/qemuxml2argvtest.c                      |   1 +
 tests/qemuxml2xmloutdata/net-many-models.xml  |  53 +++++++++
 tests/qemuxml2xmltest.c                       |   1 +
 tests/xlconfigdata/test-net-fakemodel.cfg     |  24 ++++
 tests/xlconfigdata/test-net-fakemodel.xml     |  39 ++++++
 tests/xlconfigtest.c                          |   1 +
 .../test-paravirt-net-fakemodel.cfg           |  13 ++
 .../test-paravirt-net-fakemodel.xml           |  40 +++++++
 .../test-paravirt-net-modelstr.cfg            |  13 ++
 tests/xmconfigtest.c                          |   1 +
 .../xml2sexpr-fv-net-many-models.sexpr        |   1 +
 .../xml2sexpr-fv-net-many-models.xml          |  43 +++++++
 tests/xml2sexprtest.c                         |   1 +
 35 files changed, 586 insertions(+), 170 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/net-many-models.args
 create mode 100644 tests/qemuxml2argvdata/net-many-models.xml
 create mode 100644 tests/qemuxml2xmloutdata/net-many-models.xml
 create mode 100644 tests/xlconfigdata/test-net-fakemodel.cfg
 create mode 100644 tests/xlconfigdata/test-net-fakemodel.xml
 create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.cfg
 create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.xml
 create mode 100644 tests/xmconfigdata/test-paravirt-net-modelstr.cfg
 create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr
 create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml

-- 
2.20.1

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

Comments

Cole Robinson March 28, 2019, 5:14 p.m. UTC | #1
ping

On 3/13/19 11:51 AM, Cole Robinson wrote:
> v1 here:

> https://www.redhat.com/archives/libvir-list/2019-January/msg00763.html

> 

> Changes since v1:

> - patch #7, case insensitive model input comparison

> - Add xml2xml testing

> - compile tested on freebsd12.0

> 

> This series partially converts the net->model value from a string

> to an enum. We wrap the existing ->model string in accessor functions,

> rename it to ->modelstr, add a ->model enum, and convert internal

> driver usage bit by bit. At the end, all driver code that is acting

> on specific network model values is comparing against an enum, not

> a string.

> 

> This is only partial because of xen/libxl/xm and qemu drivers, which

> if they don't know anything particular about the model string will

> just place it on the qemu command line/xen config and see what happens.

> So basically if I were to pass in

> 

>   <model type='idontexist'/>

> 

> qemu would turn that into

> 

>   -device idontexist,...

> 

> That behavior is untouched by this series, as fully unwinding that

> will take some more work:

> 

>  * Figuring out all reasonable qemu + xen values that could actually

>    result in a working VM config, and adding them to the enum

>  * Figuring out a long term plan for disabling passthrough entirely.

>    There's some discussion in the v1 thread about this.

> 

> Some caveats:

>  * vz driver is not compile tested. What's the sdk magic to actually

>    get this building?

>  * net model enum lookup is done case insensitive. this is to maintain

>    the behavior of the vmx and virtualbox drivers, but it's different

>    than all our other enum usage.

> 

> Cole Robinson (11):

>   tests: Add several net model passthrough tests

>   conf: net: Add wrapper functions for <model> value

>   conf: net: Rename 'model' to 'modelstr'

>   conf: net: Add model enum, and netfront value

>   vz: convert to net model enum

>   bhyve: convert to net model enum

>   qemu: Partially convert to net model enum

>   conf: Make net model enum compare case insensitive

>   vmx: convert to net model enum

>   vbox: Convert to net enum model

>   conf: Add VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING

> 

>  src/bhyve/bhyve_command.c                     |  15 +--

>  src/bhyve/bhyve_parse_command.c               |  10 +-

>  src/conf/domain_conf.c                        | 111 ++++++++++++++----

>  src/conf/domain_conf.h                        |  35 +++++-

>  src/libvirt_private.syms                      |   4 +

>  src/libxl/libxl_conf.c                        |   8 +-

>  src/libxl/libxl_domain.c                      |   1 +

>  src/qemu/qemu_command.c                       |  13 +-

>  src/qemu/qemu_domain.c                        |  32 +++--

>  src/qemu/qemu_domain_address.c                |  13 +-

>  src/qemu/qemu_driver.c                        |  14 ++-

>  src/qemu/qemu_hotplug.c                       |  15 ++-

>  src/qemu/qemu_parse_command.c                 |   5 +-

>  src/security/virt-aa-helper.c                 |   3 +-

>  src/vbox/vbox_common.c                        |  29 ++---

>  src/vmx/vmx.c                                 |  55 ++++-----

>  src/vz/vz_driver.c                            |   7 +-

>  src/vz/vz_sdk.c                               |  17 ++-

>  src/xenconfig/xen_common.c                    |  31 ++---

>  src/xenconfig/xen_sxpr.c                      |  30 ++---

>  tests/qemuxml2argvdata/net-many-models.args   |  39 ++++++

>  tests/qemuxml2argvdata/net-many-models.xml    |  38 ++++++

>  tests/qemuxml2argvtest.c                      |   1 +

>  tests/qemuxml2xmloutdata/net-many-models.xml  |  53 +++++++++

>  tests/qemuxml2xmltest.c                       |   1 +

>  tests/xlconfigdata/test-net-fakemodel.cfg     |  24 ++++

>  tests/xlconfigdata/test-net-fakemodel.xml     |  39 ++++++

>  tests/xlconfigtest.c                          |   1 +

>  .../test-paravirt-net-fakemodel.cfg           |  13 ++

>  .../test-paravirt-net-fakemodel.xml           |  40 +++++++

>  .../test-paravirt-net-modelstr.cfg            |  13 ++

>  tests/xmconfigtest.c                          |   1 +

>  .../xml2sexpr-fv-net-many-models.sexpr        |   1 +

>  .../xml2sexpr-fv-net-many-models.xml          |  43 +++++++

>  tests/xml2sexprtest.c                         |   1 +

>  35 files changed, 586 insertions(+), 170 deletions(-)

>  create mode 100644 tests/qemuxml2argvdata/net-many-models.args

>  create mode 100644 tests/qemuxml2argvdata/net-many-models.xml

>  create mode 100644 tests/qemuxml2xmloutdata/net-many-models.xml

>  create mode 100644 tests/xlconfigdata/test-net-fakemodel.cfg

>  create mode 100644 tests/xlconfigdata/test-net-fakemodel.xml

>  create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.cfg

>  create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.xml

>  create mode 100644 tests/xmconfigdata/test-paravirt-net-modelstr.cfg

>  create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr

>  create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml

> 



- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Michal Prívozník April 16, 2019, 3:26 p.m. UTC | #2
On 3/13/19 4:51 PM, Cole Robinson wrote:
> v1 here:

> https://www.redhat.com/archives/libvir-list/2019-January/msg00763.html

> 

> Changes since v1:

> - patch #7, case insensitive model input comparison

> - Add xml2xml testing

> - compile tested on freebsd12.0

> 

> This series partially converts the net->model value from a string

> to an enum. We wrap the existing ->model string in accessor functions,

> rename it to ->modelstr, add a ->model enum, and convert internal

> driver usage bit by bit. At the end, all driver code that is acting

> on specific network model values is comparing against an enum, not

> a string.

> 

> This is only partial because of xen/libxl/xm and qemu drivers, which

> if they don't know anything particular about the model string will

> just place it on the qemu command line/xen config and see what happens.

> So basically if I were to pass in

> 

>    <model type='idontexist'/>

> 

> qemu would turn that into

> 

>    -device idontexist,...

> 

> That behavior is untouched by this series, as fully unwinding that

> will take some more work:

> 

>   * Figuring out all reasonable qemu + xen values that could actually

>     result in a working VM config, and adding them to the enum

>   * Figuring out a long term plan for disabling passthrough entirely.

>     There's some discussion in the v1 thread about this.

> 

> Some caveats:

>   * vz driver is not compile tested. What's the sdk magic to actually

>     get this building?

>   * net model enum lookup is done case insensitive. this is to maintain

>     the behavior of the vmx and virtualbox drivers, but it's different

>     than all our other enum usage.

> 

> Cole Robinson (11):

>    tests: Add several net model passthrough tests

>    conf: net: Add wrapper functions for <model> value

>    conf: net: Rename 'model' to 'modelstr'

>    conf: net: Add model enum, and netfront value

>    vz: convert to net model enum

>    bhyve: convert to net model enum

>    qemu: Partially convert to net model enum

>    conf: Make net model enum compare case insensitive

>    vmx: convert to net model enum

>    vbox: Convert to net enum model

>    conf: Add VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING

> 

>   src/bhyve/bhyve_command.c                     |  15 +--

>   src/bhyve/bhyve_parse_command.c               |  10 +-

>   src/conf/domain_conf.c                        | 111 ++++++++++++++----

>   src/conf/domain_conf.h                        |  35 +++++-

>   src/libvirt_private.syms                      |   4 +

>   src/libxl/libxl_conf.c                        |   8 +-

>   src/libxl/libxl_domain.c                      |   1 +

>   src/qemu/qemu_command.c                       |  13 +-

>   src/qemu/qemu_domain.c                        |  32 +++--

>   src/qemu/qemu_domain_address.c                |  13 +-

>   src/qemu/qemu_driver.c                        |  14 ++-

>   src/qemu/qemu_hotplug.c                       |  15 ++-

>   src/qemu/qemu_parse_command.c                 |   5 +-

>   src/security/virt-aa-helper.c                 |   3 +-

>   src/vbox/vbox_common.c                        |  29 ++---

>   src/vmx/vmx.c                                 |  55 ++++-----

>   src/vz/vz_driver.c                            |   7 +-

>   src/vz/vz_sdk.c                               |  17 ++-

>   src/xenconfig/xen_common.c                    |  31 ++---

>   src/xenconfig/xen_sxpr.c                      |  30 ++---

>   tests/qemuxml2argvdata/net-many-models.args   |  39 ++++++

>   tests/qemuxml2argvdata/net-many-models.xml    |  38 ++++++

>   tests/qemuxml2argvtest.c                      |   1 +

>   tests/qemuxml2xmloutdata/net-many-models.xml  |  53 +++++++++

>   tests/qemuxml2xmltest.c                       |   1 +

>   tests/xlconfigdata/test-net-fakemodel.cfg     |  24 ++++

>   tests/xlconfigdata/test-net-fakemodel.xml     |  39 ++++++

>   tests/xlconfigtest.c                          |   1 +

>   .../test-paravirt-net-fakemodel.cfg           |  13 ++

>   .../test-paravirt-net-fakemodel.xml           |  40 +++++++

>   .../test-paravirt-net-modelstr.cfg            |  13 ++

>   tests/xmconfigtest.c                          |   1 +

>   .../xml2sexpr-fv-net-many-models.sexpr        |   1 +

>   .../xml2sexpr-fv-net-many-models.xml          |  43 +++++++

>   tests/xml2sexprtest.c                         |   1 +

>   35 files changed, 586 insertions(+), 170 deletions(-)

>   create mode 100644 tests/qemuxml2argvdata/net-many-models.args

>   create mode 100644 tests/qemuxml2argvdata/net-many-models.xml

>   create mode 100644 tests/qemuxml2xmloutdata/net-many-models.xml

>   create mode 100644 tests/xlconfigdata/test-net-fakemodel.cfg

>   create mode 100644 tests/xlconfigdata/test-net-fakemodel.xml

>   create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.cfg

>   create mode 100644 tests/xmconfigdata/test-paravirt-net-fakemodel.xml

>   create mode 100644 tests/xmconfigdata/test-paravirt-net-modelstr.cfg

>   create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr

>   create mode 100644 tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml

> 



ACK series, but please see my comments before pushing.

Michal

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