[libvirt] [PATCH v2 00/11] conf: partial net model enum conversion
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
Examples of passing unknown strings through <interface> <model type=X/> Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvdata/net-many-models.args | 39 ++++++++++++++ tests/qemuxml2argvdata/net-many-models.xml | 37 +++++++++++++ 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 + 15 files changed, 307 insertions(+) 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 diff --git a/tests/qemuxml2argvdata/net-many-models.args b/tests/qemuxml2argvdata/net-many-models.args new file mode 100644 index 0000000000..69909fc699 --- /dev/null +++ b/tests/qemuxml2argvdata/net-many-models.args @@ -0,0 +1,39 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-netdev user,id=hostnet0 \ +-device idontexist,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,\ +addr=0x3 \ +-netdev user,id=hostnet1 \ +-device vmxnet3,netdev=hostnet1,id=net1,mac=00:11:22:33:44:56,bus=pci.0,\ +addr=0x4 \ +-netdev user,id=hostnet2 \ +-device netfront,netdev=hostnet2,id=net2,mac=00:11:22:33:44:57,bus=pci.0,\ +addr=0x5 \ +-netdev user,id=hostnet3 \ +-device virtio-net-pci,netdev=hostnet3,id=net3,mac=00:11:22:33:44:58,bus=pci.0,\ +addr=0x6 \ +-netdev user,id=hostnet4 \ +-device ne2k_pci,netdev=hostnet4,id=net4,mac=00:11:22:33:44:58,bus=pci.0,\ +addr=0x7 \ +-netdev user,id=hostnet5 \ +-device pcnet,netdev=hostnet5,id=net5,mac=00:11:22:33:44:58,bus=pci.0,addr=0x8 diff --git a/tests/qemuxml2argvdata/net-many-models.xml b/tests/qemuxml2argvdata/net-many-models.xml new file mode 100644 index 0000000000..2b8f9b18eb --- /dev/null +++ b/tests/qemuxml2argvdata/net-many-models.xml @@ -0,0 +1,37 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='idontexist'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:56'/> + <model type='vmxnet3'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:57'/> + <model type='netfront'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='virtio'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='ne2k_pci'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='pcnet'/> + </interface> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 03e8d79595..26e869f881 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1347,6 +1347,7 @@ mymain(void) DO_TEST("net-eth-hostip", NONE); DO_TEST("net-client", NONE); DO_TEST("net-server", NONE); + DO_TEST("net-many-models", NONE); DO_TEST("net-mcast", NONE); DO_TEST("net-udp", NONE); DO_TEST("net-hostdev", NONE); diff --git a/tests/qemuxml2xmloutdata/net-many-models.xml b/tests/qemuxml2xmloutdata/net-many-models.xml new file mode 100644 index 0000000000..8fb3be920b --- /dev/null +++ b/tests/qemuxml2xmloutdata/net-many-models.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='idontexist'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:56'/> + <model type='vmxnet3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:57'/> + <model type='netfront'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='ne2k_pci'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:58'/> + <model type='pcnet'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 83a0d1cf7b..03a6b8741e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -433,6 +433,7 @@ mymain(void) DO_TEST("net-bandwidth2", NONE); DO_TEST("net-mtu", NONE); DO_TEST("net-coalesce", NONE); + DO_TEST("net-many-models", NONE); DO_TEST("serial-tcp-tlsx509-chardev", NONE); DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE); diff --git a/tests/xlconfigdata/test-net-fakemodel.cfg b/tests/xlconfigdata/test-net-fakemodel.cfg new file mode 100644 index 0000000000..70bd922ac0 --- /dev/null +++ b/tests/xlconfigdata/test-net-fakemodel.cfg @@ -0,0 +1,24 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=fakemodel" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" diff --git a/tests/xlconfigdata/test-net-fakemodel.xml b/tests/xlconfigdata/test-net-fakemodel.xml new file mode 100644 index 0000000000..b9afbac78d --- /dev/null +++ b/tests/xlconfigdata/test-net-fakemodel.xml @@ -0,0 +1,39 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>592896</memory> + <currentMemory unit='KiB'>403456</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='fakemodel'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 492bda2e63..ff27c96e87 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -259,6 +259,7 @@ mymain(void) #ifdef LIBXL_HAVE_QED DO_TEST_FORMAT("disk-qed", false); #endif + DO_TEST("net-fakemodel"); DO_TEST("spice"); DO_TEST("spice-features"); DO_TEST("vif-rate"); diff --git a/tests/xmconfigdata/test-paravirt-net-fakemodel.cfg b/tests/xmconfigdata/test-paravirt-net-fakemodel.cfg new file mode 100644 index 0000000000..bf00cb555d --- /dev/null +++ b/tests/xmconfigdata/test-paravirt-net-fakemodel.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +vfb = [ "type=vnc,vncunused=1,vnclisten=127.0.0.1,vncpasswd=123poi" ] +vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge,model=fakemodel" ] +bootloader = "/usr/bin/pygrub" +disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ] diff --git a/tests/xmconfigdata/test-paravirt-net-fakemodel.xml b/tests/xmconfigdata/test-paravirt-net-fakemodel.xml new file mode 100644 index 0000000000..80819fa55e --- /dev/null +++ b/tests/xmconfigdata/test-paravirt-net-fakemodel.xml @@ -0,0 +1,40 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>c7a5fdb0-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>592896</memory> + <currentMemory unit='KiB'>403456</currentMemory> + <vcpu placement='static'>1</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest1'/> + <target dev='xvda' bus='xen'/> + </disk> + <interface type='bridge'> + <mac address='00:16:3e:66:94:9c'/> + <source bridge='br0'/> + <script path='vif-bridge'/> + <model type='fakemodel'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='xen' vram='4096' heads='1' primary='yes'/> + </video> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xmconfigdata/test-paravirt-net-modelstr.cfg b/tests/xmconfigdata/test-paravirt-net-modelstr.cfg new file mode 100644 index 0000000000..bf00cb555d --- /dev/null +++ b/tests/xmconfigdata/test-paravirt-net-modelstr.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +vfb = [ "type=vnc,vncunused=1,vnclisten=127.0.0.1,vncpasswd=123poi" ] +vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge,model=fakemodel" ] +bootloader = "/usr/bin/pygrub" +disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ] diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index cae1ed15d4..1de3c33f69 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -193,6 +193,7 @@ mymain(void) DO_TEST("paravirt-new-pvfb"); DO_TEST("paravirt-new-pvfb-vncdisplay"); DO_TEST("paravirt-net-e1000"); + DO_TEST("paravirt-net-fakemodel"); DO_TEST("paravirt-net-vifname"); DO_TEST("paravirt-vcpu"); DO_TEST("paravirt-maxvcpus"); diff --git a/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr b/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr new file mode 100644 index 0000000000..118dfd87bb --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.sexpr @@ -0,0 +1 @@ +(vm (name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd2-75cd-aca5-1776-9660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'destroy')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(vcpus 1)(boot c)(pae 1)(usb 1)(parallel none)(serial none)(device_model '/usr/lib64/xen/bin/qemu-dm')(rtc_timeoffset 0)(localtime 0)))(localtime 0)(device (vif (mac '00:11:22:33:44:55')(bridge 'xenbr0')(script 'vif-bridge')(model 'idontexist')))(device (vif (mac '00:11:22:33:44:56')(bridge 'xenbr0')(script 'vif-bridge')(model 'vmxnet3')))(device (vif (mac '00:11:22:33:44:57')(bridge 'xenbr0')(script 'vif-bridge')(type netfront)))(device (vif (mac '00:11:22:33:44:58')(bridge 'xenbr0')(script 'vif-bridge')(model 'virtio')))(device (vif (mac '00:11:22:33:44:58')(bridge 'xenbr0')(script 'vif-bridge')(model 'ne2k_pci')))(device (vif (mac '00:11:22:33:44:58')(bridge 'xenbr0')(script 'vif-bridge')(model 'pcnet')))) diff --git a/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml b/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml new file mode 100644 index 0000000000..e95cba7f6a --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-fv-net-many-models.xml @@ -0,0 +1,43 @@ +<domain type='xen'> + <name>fvtest</name> + <uuid>b5d70dd275cdaca517769660b059d8bc</uuid> + <memory unit='KiB'>409600</memory> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:55'/> + <model type='idontexist'/> + </interface> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:56'/> + <model type='vmxnet3'/> + </interface> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:57'/> + <model type='netfront'/> + </interface> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:58'/> + <model type='virtio'/> + </interface> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:58'/> + <model type='ne2k_pci'/> + </interface> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:11:22:33:44:58'/> + <model type='pcnet'/> + </interface> + </devices> +</domain> diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index de68ea1f05..64383876ab 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -151,6 +151,7 @@ mymain(void) DO_TEST("fv-sound", "fv-sound", "fvtest"); + DO_TEST("fv-net-many-models", "fv-net-many-models", "fvtest"); DO_TEST("fv-net-netfront", "fv-net-netfront", "fvtest"); DO_TEST("fv-net-rate", "fv-net-rate", "fvtest"); -- 2.20.1
On 3/13/19 4:51 PM, Cole Robinson wrote:
Examples of passing unknown strings through <interface> <model type=X/>
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvdata/net-many-models.args | 39 ++++++++++++++ tests/qemuxml2argvdata/net-many-models.xml | 37 +++++++++++++ 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 + 15 files changed, 307 insertions(+) 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
You'll need to regenerate the test outputs though, but this is because patches were sitting unreviewed for quite some time. Michal
To ease converting the net->model value to an enum, add the wrapper functions: virDomainNetGetModelString virDomainNetSetModelString virDomainNetStreqModelString virDomainNetStrcaseeqModelString Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/bhyve/bhyve_command.c | 8 ++++---- src/bhyve/bhyve_parse_command.c | 2 +- src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++----- src/conf/domain_conf.h | 7 +++++++ src/libvirt_private.syms | 4 ++++ src/libxl/libxl_conf.c | 8 ++++---- src/qemu/qemu_command.c | 15 +++++++------- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_domain_address.c | 13 ++++++------ src/qemu/qemu_driver.c | 14 ++++++++++--- src/qemu/qemu_hotplug.c | 7 ++++--- src/qemu/qemu_parse_command.c | 5 +++-- src/vbox/vbox_common.c | 18 ++++++++--------- src/vmx/vmx.c | 31 ++++++++++++++-------------- src/vz/vz_driver.c | 4 ++-- src/vz/vz_sdk.c | 14 ++++++------- src/xenconfig/xen_common.c | 21 ++++++++++--------- src/xenconfig/xen_sxpr.c | 21 ++++++++++--------- 18 files changed, 140 insertions(+), 92 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 10340ee436..d3d790f6b6 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -57,16 +57,16 @@ bhyveBuildNetArgStr(virConnectPtr conn, int ret = -1; virDomainNetType actualType = virDomainNetGetActualType(net); - if (net->model == NULL) { + if (!virDomainNetGetModelString(net)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("NIC model must be specified")); return -1; } - if (STREQ(net->model, "virtio")) { + if (virDomainNetStreqModelString(net, "virtio")) { if (VIR_STRDUP(nic_model, "virtio-net") < 0) return -1; - } else if (STREQ(net->model, "e1000")) { + } else if (virDomainNetStreqModelString(net, "e1000")) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) { if (VIR_STRDUP(nic_model, "e1000") < 0) return -1; @@ -79,7 +79,7 @@ bhyveBuildNetArgStr(virConnectPtr conn, } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("NIC model '%s' is not supported"), - net->model); + virDomainNetGetModelString(net)); return -1; } diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index bd93070dfb..60eb4c5412 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -511,7 +511,7 @@ bhyveParsePCINet(virDomainDefPtr def, if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0) goto error; - if (VIR_STRDUP(net->model, model) < 0) + if (virDomainNetSetModelString(net, model) < 0) goto error; net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 778c386ee2..d282b4d3ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25282,9 +25282,9 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " actual='%s'", def->ifname_guest_actual); virBufferAddLit(buf, "/>\n"); } - if (def->model) { + if (virDomainNetGetModelString(def)) { virBufferEscapeString(buf, "<model type='%s'/>\n", - def->model); + virDomainNetGetModelString(def)); if (virDomainNetIsVirtioModel(def)) { int rc = 0; VIR_AUTOFREE(char *) str = NULL; @@ -29376,13 +29376,39 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) return iface->trustGuestRxFilters == VIR_TRISTATE_BOOL_YES; } +const char * +virDomainNetGetModelString(const virDomainNetDef *net) +{ + return net->model; +} + +int +virDomainNetSetModelString(virDomainNetDefPtr net, + const char *model) +{ + return VIR_STRDUP(net->model, model); +} + +int +virDomainNetStreqModelString(const virDomainNetDef *net, + const char *model) +{ + return STREQ_NULLABLE(net->model, model); +} + +int +virDomainNetStrcaseeqModelString(const virDomainNetDef *net, + const char *model) +{ + return net->model && STRCASEEQ(net->model, model); +} bool virDomainNetIsVirtioModel(const virDomainNetDef *net) { - return (STREQ_NULLABLE(net->model, "virtio") || - STREQ_NULLABLE(net->model, "virtio-transitional") || - STREQ_NULLABLE(net->model, "virtio-non-transitional")); + return (virDomainNetStreqModelString(net, "virtio") || + virDomainNetStreqModelString(net, "virtio-transitional") || + virDomainNetStreqModelString(net, "virtio-non-transitional")); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fe9d4fb81a..39618928f7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3314,6 +3314,13 @@ virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); +const char *virDomainNetGetModelString(const virDomainNetDef *net); +int virDomainNetSetModelString(virDomainNetDefPtr et, + const char *model); +int virDomainNetStreqModelString(const virDomainNetDef *net, + const char *model); +int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, + const char *model); bool virDomainNetIsVirtioModel(const virDomainNetDef *net); int virDomainNetAppendIPAddress(virDomainNetDefPtr def, const char *address, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 960a97cf1d..a01f158946 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -474,6 +474,7 @@ virDomainNetGetActualTrustGuestRxFilters; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; +virDomainNetGetModelString; virDomainNetInsert; virDomainNetIsVirtioModel; virDomainNetNotifyActualDevice; @@ -482,6 +483,9 @@ virDomainNetRemove; virDomainNetRemoveHostdev; virDomainNetResolveActualType; virDomainNetSetDeviceImpl; +virDomainNetSetModelString; +virDomainNetStrcaseeqModelString; +virDomainNetStreqModelString; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index c769050ff1..9236820bfe 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1266,18 +1266,18 @@ libxlMakeNic(virDomainDefPtr def, * xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for * hvm guest"). */ - if (l_nic->model) { + if (virDomainNetGetModelString(l_nic)) { if ((def->os.type == VIR_DOMAIN_OSTYPE_XEN || def->os.type == VIR_DOMAIN_OSTYPE_XENPVH) && - STRNEQ(l_nic->model, "netfront")) { + !virDomainNetStreqModelString(l_nic, "netfront")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only model 'netfront' is supported for " "Xen PV(H) domains")); return -1; } - if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) + if (VIR_STRDUP(x_nic->model, virDomainNetGetModelString(l_nic)) < 0) goto cleanup; - if (STREQ(l_nic->model, "netfront")) + if (virDomainNetStreqModelString(l_nic, "netfront")) x_nic->nictype = LIBXL_NIC_TYPE_VIF; else x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5e56447b76..0388ebc5af 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -479,10 +479,10 @@ qemuBuildVirtioDevStr(virBufferPtr buf, break; case VIR_DOMAIN_DEVICE_NET: - has_tmodel = STREQ_NULLABLE(device.data.net->model, - "virtio-transitional"); - has_ntmodel = STREQ_NULLABLE(device.data.net->model, - "virtio-non-transitional"); + has_tmodel = virDomainNetStreqModelString(device.data.net, + "virtio-transitional"); + has_ntmodel = virDomainNetStreqModelString(device.data.net, + "virtio-non-transitional"); break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -3842,13 +3842,14 @@ qemuBuildLegacyNicStr(virDomainNetDefPtr net) { char *str; char macaddr[VIR_MAC_STRING_BUFLEN]; + const char *netmodel = virDomainNetGetModelString(net); ignore_value(virAsprintf(&str, "nic,macaddr=%s,netdev=host%s%s%s%s%s", virMacAddrFormat(&net->mac, macaddr), net->info.alias, - (net->model ? ",model=" : ""), - NULLSTR_EMPTY(net->model), + netmodel ? ",model=" : "", + NULLSTR_EMPTY(netmodel), (net->info.alias ? ",id=" : ""), NULLSTR_EMPTY(net->info.alias))); return str; @@ -3874,7 +3875,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, usingVirtio = true; } else { - virBufferAddStr(&buf, net->model); + virBufferAddStr(&buf, virDomainNetGetModelString(net)); } if (usingVirtio && net->driver.virtio.txmode) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 788c19c248..7b001bca52 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6727,8 +6727,8 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps) { if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - !net->model) { - if (VIR_STRDUP(net->model, + !virDomainNetGetModelString(net)) { + if (virDomainNetSetModelString(net, qemuDomainDefaultNetModel(def, qemuCaps)) < 0) return -1; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 4740536d82..380c5bffcc 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -230,7 +230,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (STREQ_NULLABLE(net->model, "spapr-vlan")) + if (virDomainNetStreqModelString(net, "spapr-vlan")) net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &net->info, VIO_ADDR_NET) < 0) @@ -715,19 +715,18 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, * addresses for other hostdev devices. */ if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || - STREQ_NULLABLE(net->model, "usb-net")) { + virDomainNetStreqModelString(net, "usb-net")) { return 0; } - if (STREQ_NULLABLE(net->model, "virtio") || - STREQ_NULLABLE(net->model, "virtio-non-transitional")) + if (virDomainNetStreqModelString(net, "virtio") || + virDomainNetStreqModelString(net, "virtio-non-transitional")) return virtioFlags; - /* Transitional devices only work in conventional PCI slots */ - if (STREQ_NULLABLE(net->model, "virtio-transitional")) + if (virDomainNetStreqModelString(net, "virtio-transitional")) return pciFlags; - if (STREQ_NULLABLE(net->model, "e1000e")) + if (virDomainNetStreqModelString(net, "e1000e")) return pcieFlags; return pciFlags; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e461fb51b0..a3d52a5dc0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7444,20 +7444,28 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, for (i = 0; i < vm->def->nnets; i++) { virDomainNetDefPtr net = vm->def->nets[i]; unsigned int bootIndex = net->info.bootIndex; - char *model = net->model; + char *model; virMacAddr mac = net->mac; char *script = net->script; - net->model = NULL; + if (virDomainNetGetModelString(net) && + VIR_STRDUP(model, virDomainNetGetModelString(net)) < 0) + goto cleanup; + net->script = NULL; virDomainNetDefClear(net); net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->info.bootIndex = bootIndex; - net->model = model; net->mac = mac; net->script = script; + + if (model && virDomainNetSetModelString(net, model) < 0) { + VIR_FREE(model); + goto cleanup; + } + VIR_FREE(model); } if (!(cmd = qemuProcessCreatePretendCmd(driver, vm, NULL, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f43f80668c..872d8e2a6f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3692,11 +3692,12 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, goto cleanup; } - if (STRNEQ_NULLABLE(olddev->model, newdev->model)) { + if (STRNEQ_NULLABLE(virDomainNetGetModelString(olddev), + virDomainNetGetModelString(newdev))) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("cannot modify network device model from %s to %s"), - olddev->model ? olddev->model : "(default)", - newdev->model ? newdev->model : "(default)"); + NULLSTR(virDomainNetGetModelString(olddev)), + NULLSTR(virDomainNetGetModelString(newdev))); goto cleanup; } diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 49b34b1c17..fc3f70fcde 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1109,8 +1109,9 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt, goto error; } } else if (STREQ(keywords[i], "model")) { - def->model = values[i]; - values[i] = NULL; + if (virDomainNetSetModelString(def, values[i]) < 0) + goto error; + VIR_FREE(values[i]); } else if (STREQ(keywords[i], "vhost")) { if ((values[i] == NULL) || STREQ(values[i], "on")) { def->driver.virtio.name = VIR_DOMAIN_NET_BACKEND_TYPE_VHOST; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index c410514d37..8c28bba8ee 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1309,7 +1309,7 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) macaddrvbox[VIR_MAC_STRING_BUFLEN - 6] = '\0'; VIR_DEBUG("NIC(%zu): Type: %d", i, def->nets[i]->type); - VIR_DEBUG("NIC(%zu): Model: %s", i, def->nets[i]->model); + VIR_DEBUG("NIC(%zu): Model: %s", i, virDomainNetGetModelString(def->nets[i])); VIR_DEBUG("NIC(%zu): Mac: %s", i, macaddr); VIR_DEBUG("NIC(%zu): ifname: %s", i, def->nets[i]->ifname); if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -1338,19 +1338,19 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) gVBoxAPI.UINetworkAdapter.SetEnabled(adapter, 1); - if (def->nets[i]->model) { - if (STRCASEEQ(def->nets[i]->model, "Am79C970A")) { + if (virDomainNetGetModelString(def->nets[i])) { + if (virDomainNetStrcaseeqModelString(def->nets[i], "Am79C970A")) { adapterType = NetworkAdapterType_Am79C970A; - } else if (STRCASEEQ(def->nets[i]->model, "Am79C973")) { + } else if (virDomainNetStrcaseeqModelString(def->nets[i], "Am79C973")) { adapterType = NetworkAdapterType_Am79C973; - } else if (STRCASEEQ(def->nets[i]->model, "82540EM")) { + } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82540EM")) { adapterType = NetworkAdapterType_I82540EM; - } else if (STRCASEEQ(def->nets[i]->model, "82545EM")) { + } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82545EM")) { adapterType = NetworkAdapterType_I82545EM; - } else if (STRCASEEQ(def->nets[i]->model, "82543GC")) { + } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82543GC")) { adapterType = NetworkAdapterType_I82543GC; } else if (gVBoxAPI.APIVersion >= 3000051 && - STRCASEEQ(def->nets[i]->model, "virtio")) { + virDomainNetStrcaseeqModelString(def->nets[i], "virtio")) { /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ adapterType = NetworkAdapterType_Virtio; } @@ -3762,7 +3762,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) model = "virtio"; break; } - if (VIR_STRDUP(net->model, model) < 0) + if (virDomainNetSetModelString(net, model) < 0) goto error; gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 8ffd5ff088..7a557847ba 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2671,10 +2671,8 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) /* Setup virDomainNetDef */ if (connectionType == NULL || STRCASEEQ(connectionType, "bridged")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - (*def)->model = virtualDev; (*def)->data.bridge.brname = networkName; - virtualDev = NULL; networkName = NULL; } else if (STRCASEEQ(connectionType, "hostonly")) { /* FIXME */ @@ -2684,16 +2682,12 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } else if (STRCASEEQ(connectionType, "nat")) { (*def)->type = VIR_DOMAIN_NET_TYPE_USER; - (*def)->model = virtualDev; - virtualDev = NULL; } else if (STRCASEEQ(connectionType, "custom")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - (*def)->model = virtualDev; (*def)->data.bridge.brname = networkName; (*def)->ifname = vnet; - virtualDev = NULL; networkName = NULL; vnet = NULL; } else { @@ -2703,6 +2697,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } + if (virDomainNetSetModelString((*def), virtualDev) < 0) + goto cleanup; + VIR_FREE(virtualDev); + result = 0; cleanup: @@ -3740,28 +3738,29 @@ virVMXFormatEthernet(virDomainNetDefPtr def, int controller, virBufferAsprintf(buffer, "ethernet%d.present = \"true\"\n", controller); /* def:model -> vmx:virtualDev, vmx:features */ - if (def->model != NULL) { - if (STRCASENEQ(def->model, "vlance") && - STRCASENEQ(def->model, "vmxnet") && - STRCASENEQ(def->model, "vmxnet2") && - STRCASENEQ(def->model, "vmxnet3") && - STRCASENEQ(def->model, "e1000") && - STRCASENEQ(def->model, "e1000e")) { + if (virDomainNetGetModelString(def)) { + if (!virDomainNetStrcaseeqModelString(def, "vlance") && + !virDomainNetStrcaseeqModelString(def, "vmxnet") && + !virDomainNetStrcaseeqModelString(def, "vmxnet2") && + !virDomainNetStrcaseeqModelString(def, "vmxnet3") && + !virDomainNetStrcaseeqModelString(def, "e1000") && + !virDomainNetStrcaseeqModelString(def, "e1000e")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML entry 'devices/interface/model' " "to be 'vlance' or 'vmxnet' or 'vmxnet2' or 'vmxnet3' " - "or 'e1000' or 'e1000e' but found '%s'"), def->model); + "or 'e1000' or 'e1000e' but found '%s'"), + virDomainNetGetModelString(def)); return -1; } - if (STRCASEEQ(def->model, "vmxnet2")) { + if (virDomainNetStrcaseeqModelString(def, "vmxnet2")) { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"vmxnet\"\n", controller); virBufferAsprintf(buffer, "ethernet%d.features = \"15\"\n", controller); } else { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"%s\"\n", - controller, def->model); + controller, virDomainNetGetModelString(def)); } } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 066d617524..66d019378f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -265,9 +265,9 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET && (dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK || dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && - !dev->data.net->model && + !virDomainNetGetModelString(dev->data.net) && def->os.type == VIR_DOMAIN_OSTYPE_HVM && - VIR_STRDUP(dev->data.net->model, "e1000") < 0) + virDomainNetSetModelString(dev->data.net, "e1000") < 0) return -1; return 0; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b9fd03c0d2..ed8345a874 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1104,15 +1104,15 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) switch ((int)type) { case PNT_RTL: - if (VIR_STRDUP(net->model, "rtl8139") < 0) + if (virDomainNetSetModelString(net, "rtl8139") < 0) goto cleanup; break; case PNT_E1000: - if (VIR_STRDUP(net->model, "e1000") < 0) + if (virDomainNetSetModelString(net, "e1000") < 0) goto cleanup; break; case PNT_VIRTIO: - if (VIR_STRDUP(net->model, "virtio") < 0) + if (virDomainNetSetModelString(net, "virtio") < 0) goto cleanup; break; default: @@ -3377,15 +3377,15 @@ static int prlsdkConfigureNet(vzDriverPtr driver ATTRIBUTE_UNUSED, goto cleanup; if (isCt) { - if (net->model) + if (virDomainNetGetModelString(net)) VIR_WARN("Setting network adapter for containers is not " "supported by vz driver."); } else { - if (STREQ(net->model, "rtl8139")) { + if (virDomainNetStreqModelString(net, "rtl8139")) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); - } else if (STREQ(net->model, "e1000")) { + } else if (virDomainNetStreqModelString(net, "e1000")) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); - } else if (STREQ(net->model, "virtio")) { + } else if (virDomainNetStreqModelString(net, "virtio")) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 21c56edd58..170378fab5 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1069,11 +1069,11 @@ xenParseVif(char *entry, const char *vif_typename) goto cleanup; if (model[0] && - VIR_STRDUP(net->model, model) < 0) + virDomainNetSetModelString(net, model) < 0) goto cleanup; if (!model[0] && type[0] && STREQ(type, vif_typename) && - VIR_STRDUP(net->model, "netfront") < 0) + virDomainNetSetModelString(net, "netfront") < 0) goto cleanup; if (vifname[0] && @@ -1422,15 +1422,16 @@ xenFormatNet(virConnectPtr conn, goto cleanup; } - if (!hvm) { - if (net->model != NULL) - virBufferAsprintf(&buf, ",model=%s", net->model); - } else { - if (net->model != NULL && STREQ(net->model, "netfront")) { - virBufferAsprintf(&buf, ",type=%s", vif_typename); + if (virDomainNetGetModelString(net)) { + if (!hvm) { + virBufferAsprintf(&buf, ",model=%s", + virDomainNetGetModelString(net)); } else { - if (net->model != NULL) - virBufferAsprintf(&buf, ",model=%s", net->model); + if (virDomainNetStreqModelString(net, "netfront")) + virBufferAsprintf(&buf, ",type=%s", vif_typename); + else + virBufferAsprintf(&buf, ",model=%s", + virDomainNetGetModelString(net)); } } diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index eb535cde19..224c874b90 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -642,11 +642,11 @@ xenParseSxprNets(virDomainDefPtr def, } } - if (VIR_STRDUP(net->model, model) < 0) + if (virDomainNetSetModelString(net, model) < 0) goto cleanup; if (!model && type && STREQ(type, "netfront") && - VIR_STRDUP(net->model, "netfront") < 0) + virDomainNetSetModelString(net, "netfront") < 0) goto cleanup; tmp = sexpr_node(node, "device/vif/rate"); @@ -1929,15 +1929,16 @@ xenFormatSxprNet(virConnectPtr conn, !STRPREFIX(def->ifname, "vif")) virBufferEscapeSexpr(buf, "(vifname '%s')", def->ifname); - if (!hvm) { - if (def->model != NULL) - virBufferEscapeSexpr(buf, "(model '%s')", def->model); - } else { - if (def->model != NULL && STREQ(def->model, "netfront")) { - virBufferAddLit(buf, "(type netfront)"); + if (virDomainNetGetModelString(def)) { + if (!hvm) { + virBufferEscapeSexpr(buf, "(model '%s')", + virDomainNetGetModelString(def)); } else { - if (def->model != NULL) - virBufferEscapeSexpr(buf, "(model '%s')", def->model); + if (virDomainNetStreqModelString(def, "netfront")) + virBufferAddLit(buf, "(type netfront)"); + else + virBufferEscapeSexpr(buf, "(model '%s')", + virDomainNetGetModelString(def)); } } -- 2.20.1
On 3/13/19 4:51 PM, Cole Robinson wrote:
To ease converting the net->model value to an enum, add the wrapper functions:
virDomainNetGetModelString virDomainNetSetModelString virDomainNetStreqModelString virDomainNetStrcaseeqModelString
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/bhyve/bhyve_command.c | 8 ++++---- src/bhyve/bhyve_parse_command.c | 2 +- src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++----- src/conf/domain_conf.h | 7 +++++++ src/libvirt_private.syms | 4 ++++ src/libxl/libxl_conf.c | 8 ++++---- src/qemu/qemu_command.c | 15 +++++++------- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_domain_address.c | 13 ++++++------ src/qemu/qemu_driver.c | 14 ++++++++++--- src/qemu/qemu_hotplug.c | 7 ++++--- src/qemu/qemu_parse_command.c | 5 +++-- src/vbox/vbox_common.c | 18 ++++++++--------- src/vmx/vmx.c | 31 ++++++++++++++-------------- src/vz/vz_driver.c | 4 ++-- src/vz/vz_sdk.c | 14 ++++++------- src/xenconfig/xen_common.c | 21 ++++++++++--------- src/xenconfig/xen_sxpr.c | 21 ++++++++++--------- 18 files changed, 140 insertions(+), 92 deletions(-)
Missed aa-helper: diff --git i/src/security/virt-aa-helper.c w/src/security/virt-aa-helper.c index 989dcf1784..158b614757 100644 --- i/src/security/virt-aa-helper.c +++ w/src/security/virt-aa-helper.c @@ -1253,7 +1253,7 @@ get_files(vahControl * ctl) if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i]; - if (net && net->model) { + if (net && virDomainNetGetModelString(net)) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) continue; if (!virDomainNetIsVirtioModel(net)) Michal
We will be adding a 'model' enum in upcoming patches. Rename the existing value to make the differentiation clear Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 16 ++++++++-------- src/conf/domain_conf.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d282b4d3ae..fe1945b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2227,7 +2227,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) if (!def) return; - VIR_FREE(def->model); + VIR_FREE(def->modelstr); switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11495,7 +11495,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, _("Model name contains invalid characters")); goto error; } - VIR_STEAL_PTR(def->model, model); + VIR_STEAL_PTR(def->modelstr, model); } switch (def->type) { @@ -21732,10 +21732,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } - if (STRNEQ_NULLABLE(src->model, dst->model)) { + if (STRNEQ_NULLABLE(src->modelstr, dst->modelstr)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card model %s does not match source %s"), - NULLSTR(dst->model), NULLSTR(src->model)); + NULLSTR(dst->modelstr), NULLSTR(src->modelstr)); return false; } @@ -29379,28 +29379,28 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { - return net->model; + return net->modelstr; } int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->model, model); + return VIR_STRDUP(net->modelstr, model); } int virDomainNetStreqModelString(const virDomainNetDef *net, const char *model) { - return STREQ_NULLABLE(net->model, model); + return STREQ_NULLABLE(net->modelstr, model); } int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, const char *model) { - return net->model && STRCASEEQ(net->model, model); + return net->modelstr && STRCASEEQ(net->modelstr, model); } bool diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 39618928f7..a0d443ee1b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1021,7 +1021,7 @@ struct _virDomainNetDef { virDomainNetType type; virMacAddr mac; bool mac_generated; /* true if mac was *just now* auto-generated by libvirt */ - char *model; + char *modelstr; union { struct { virDomainNetBackendType name; /* which driver backend to use */ -- 2.20.1
This adds a network model enum. The virDomainNetDef property is named 'model' like most other devices. When the XML parser or a driver calls NetSetModelString, if the passed string is in the enum, we will set net->model, otherwise we copy the string into net->modelstr Add a single example for the 'netfront' xen model, and wire that up, just to verify it's all working Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 4 +-- src/qemu/qemu_hotplug.c | 8 ++++++ src/xenconfig/xen_common.c | 16 +++++------ src/xenconfig/xen_sxpr.c | 15 ++++++----- 7 files changed, 78 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe1945b644..571f2eea39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "udp", ); +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, + "unknown", + "netfront", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) return; VIR_FREE(def->modelstr); + def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - /* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); - goto error; - } - VIR_STEAL_PTR(def->modelstr, model); - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error; switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card model %s does not match source %s"), + virDomainNetModelTypeToString(dst->model), + virDomainNetModelTypeToString(src->model)); + return false; + } + if (src->mtu != dst->mtu) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card MTU %d does not match source %d"), @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { + if (net->model) + return virDomainNetModelTypeToString(net->model); return net->modelstr; } @@ -29386,13 +29391,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->modelstr, model); + VIR_FREE(net->modelstr); + if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (!model) + return 0; + + if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Model name contains invalid characters")); + return -1; + } + + if (VIR_STRDUP(net->modelstr, model) < 0) + return -1; + return 0; } int virDomainNetStreqModelString(const virDomainNetDef *net, const char *model) { + if (net->model) + return net->model == virDomainNetModelTypeFromString(model); return STREQ_NULLABLE(net->modelstr, model); } @@ -29400,6 +29423,8 @@ int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, const char *model) { + if (net->model) + return STRCASEEQ(virDomainNetModelTypeToString(net->model), model); return net->modelstr && STRCASEEQ(net->modelstr, model); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a0d443ee1b..92d54b0348 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -960,6 +960,14 @@ typedef enum { VIR_DOMAIN_NET_TYPE_LAST } virDomainNetType; +/* network model types */ +typedef enum { + VIR_DOMAIN_NET_MODEL_UNKNOWN, + VIR_DOMAIN_NET_MODEL_NETFRONT, + + VIR_DOMAIN_NET_MODEL_LAST +} virDomainNetModelType; + /* the backend driver used for virtio interfaces */ typedef enum { VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */ @@ -1021,6 +1029,7 @@ struct _virDomainNetDef { virDomainNetType type; virMacAddr mac; bool mac_generated; /* true if mac was *just now* auto-generated by libvirt */ + int model; /* virDomainNetModelType */ char *modelstr; union { struct { @@ -3536,6 +3545,7 @@ VIR_ENUM_DECL(virDomainNet); VIR_ENUM_DECL(virDomainNetBackend); VIR_ENUM_DECL(virDomainNetVirtioTxMode); VIR_ENUM_DECL(virDomainNetInterfaceLinkState); +VIR_ENUM_DECL(virDomainNetModel); VIR_ENUM_DECL(virDomainChrDevice); VIR_ENUM_DECL(virDomainChrChannelTarget); VIR_ENUM_DECL(virDomainChrConsoleTarget); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a01f158946..3a0699d724 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -477,6 +477,8 @@ virDomainNetGetActualVlan; virDomainNetGetModelString; virDomainNetInsert; virDomainNetIsVirtioModel; +virDomainNetModelTypeFromString; +virDomainNetModelTypeToString; virDomainNetNotifyActualDevice; virDomainNetReleaseActualDevice; virDomainNetRemove; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 9236820bfe..007ef9c372 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1269,7 +1269,7 @@ libxlMakeNic(virDomainDefPtr def, if (virDomainNetGetModelString(l_nic)) { if ((def->os.type == VIR_DOMAIN_OSTYPE_XEN || def->os.type == VIR_DOMAIN_OSTYPE_XENPVH) && - !virDomainNetStreqModelString(l_nic, "netfront")) { + l_nic->model != VIR_DOMAIN_NET_MODEL_NETFRONT) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only model 'netfront' is supported for " "Xen PV(H) domains")); @@ -1277,7 +1277,7 @@ libxlMakeNic(virDomainDefPtr def, } if (VIR_STRDUP(x_nic->model, virDomainNetGetModelString(l_nic)) < 0) goto cleanup; - if (virDomainNetStreqModelString(l_nic, "netfront")) + if (l_nic->model == VIR_DOMAIN_NET_MODEL_NETFRONT) x_nic->nictype = LIBXL_NIC_TYPE_VIF; else x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 872d8e2a6f..7bd3a8a408 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3701,6 +3701,14 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, goto cleanup; } + if (olddev->model != newdev->model) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify network device model from %s to %s"), + virDomainNetModelTypeToString(olddev->model), + virDomainNetModelTypeToString(newdev->model)); + goto cleanup; + } + if (virDomainNetIsVirtioModel(olddev) && (olddev->driver.virtio.name != newdev->driver.virtio.name || olddev->driver.virtio.txmode != newdev->driver.virtio.txmode || diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 170378fab5..69a6f53507 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1068,13 +1068,13 @@ xenParseVif(char *entry, const char *vif_typename) VIR_STRDUP(net->script, script) < 0) goto cleanup; - if (model[0] && - virDomainNetSetModelString(net, model) < 0) - goto cleanup; - - if (!model[0] && type[0] && STREQ(type, vif_typename) && - virDomainNetSetModelString(net, "netfront") < 0) - goto cleanup; + if (model[0]) { + if (virDomainNetSetModelString(net, model) < 0) + goto cleanup; + } else { + if (type[0] && STREQ(type, vif_typename)) + net->model = VIR_DOMAIN_NET_MODEL_NETFRONT; + } if (vifname[0] && VIR_STRDUP(net->ifname, vifname) < 0) @@ -1427,7 +1427,7 @@ xenFormatNet(virConnectPtr conn, virBufferAsprintf(&buf, ",model=%s", virDomainNetGetModelString(net)); } else { - if (virDomainNetStreqModelString(net, "netfront")) + if (net->model == VIR_DOMAIN_NET_MODEL_NETFRONT) virBufferAsprintf(&buf, ",type=%s", vif_typename); else virBufferAsprintf(&buf, ",model=%s", diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 224c874b90..0bff94ead5 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -642,12 +642,13 @@ xenParseSxprNets(virDomainDefPtr def, } } - if (virDomainNetSetModelString(net, model) < 0) - goto cleanup; - - if (!model && type && STREQ(type, "netfront") && - virDomainNetSetModelString(net, "netfront") < 0) - goto cleanup; + if (model) { + if (virDomainNetSetModelString(net, model) < 0) + goto cleanup; + } else { + if (type && STREQ(type, "netfront")) + net->model = VIR_DOMAIN_NET_MODEL_NETFRONT; + } tmp = sexpr_node(node, "device/vif/rate"); if (tmp) { @@ -1934,7 +1935,7 @@ xenFormatSxprNet(virConnectPtr conn, virBufferEscapeSexpr(buf, "(model '%s')", virDomainNetGetModelString(def)); } else { - if (virDomainNetStreqModelString(def, "netfront")) + if (def->model == VIR_DOMAIN_NET_MODEL_NETFRONT) virBufferAddLit(buf, "(type netfront)"); else virBufferEscapeSexpr(buf, "(model '%s')", -- 2.20.1
On 3/13/19 4:51 PM, Cole Robinson wrote:
This adds a network model enum. The virDomainNetDef property is named 'model' like most other devices.
When the XML parser or a driver calls NetSetModelString, if the passed string is in the enum, we will set net->model, otherwise we copy the string into net->modelstr
Add a single example for the 'netfront' xen model, and wire that up, just to verify it's all working
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 4 +-- src/qemu/qemu_hotplug.c | 8 ++++++ src/xenconfig/xen_common.c | 16 +++++------ src/xenconfig/xen_sxpr.c | 15 ++++++----- 7 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe1945b644..571f2eea39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "udp", );
+VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,
This is supposed to be on a separate line now ;-)
+ "unknown", + "netfront", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) return;
VIR_FREE(def->modelstr); + def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN;
switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
- /* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); - goto error; - } - VIR_STEAL_PTR(def->modelstr, model); - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error;
switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; }
+ if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card model %s does not match source %s"), + virDomainNetModelTypeToString(dst->model), + virDomainNetModelTypeToString(src->model)); + return false; + } + if (src->mtu != dst->mtu) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card MTU %d does not match source %d"), @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { + if (net->model) + return virDomainNetModelTypeToString(net->model); return net->modelstr; }
@@ -29386,13 +29391,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->modelstr, model); + VIR_FREE(net->modelstr); + if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (!model) + return 0;
Is this a safe thing to do? I mean virEnumFromString() (which is called by any TypeFromString() in the end) doesn't handle NULL gracefully. You'll need to swap some lines and probably have a temp variable to store virDomainNetModelTypeFromString() retval, ...
+ + if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Model name contains invalid characters")); + return -1; + } + + if (VIR_STRDUP(net->modelstr, model) < 0) + return -1; + return 0;
Or simply 'return VIR_STRDUP();'
}
Michal
On 4/16/19 11:27 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This adds a network model enum. The virDomainNetDef property is named 'model' like most other devices.
When the XML parser or a driver calls NetSetModelString, if the passed string is in the enum, we will set net->model, otherwise we copy the string into net->modelstr
Add a single example for the 'netfront' xen model, and wire that up, just to verify it's all working
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 4 +-- src/qemu/qemu_hotplug.c | 8 ++++++ src/xenconfig/xen_common.c | 16 +++++------ src/xenconfig/xen_sxpr.c | 15 ++++++----- 7 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe1945b644..571f2eea39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "udp", ); +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,
This is supposed to be on a separate line now ;-)
+ "unknown", + "netfront", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) return; VIR_FREE(def->modelstr); + def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - /* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); - goto error; - } - VIR_STEAL_PTR(def->modelstr, model); - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error; switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card model %s does not match source %s"), + virDomainNetModelTypeToString(dst->model), + virDomainNetModelTypeToString(src->model)); + return false; + } + if (src->mtu != dst->mtu) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card MTU %d does not match source %d"), @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { + if (net->model) + return virDomainNetModelTypeToString(net->model); return net->modelstr; } @@ -29386,13 +29391,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->modelstr, model); + VIR_FREE(net->modelstr); + if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (!model) + return 0;
Is this a safe thing to do? I mean virEnumFromString() (which is called by any TypeFromString() in the end) doesn't handle NULL gracefully. You'll need to swap some lines and probably have a temp variable to store virDomainNetModelTypeFromString() retval, ...
Not completely sure I follow, but I think you mean: this function looks like it should operate as virEnumFromString does, meaning return -1 on NULL value. But consider that this is a hybrid enum (net->model) and string (net->modelstr) approach, in which modelstr=NULL is a valid case, so I'm not sure it should be an error. Later patches change the code here a bit so the end result looks different and the NULL check happens earlier, but still returns 0. It doesn't look like any code is depending on passing NULL there but it's still arguably a valid case. We are essentially trying to hide the ->modelstr detail from API users. So not really sure whether to change it or not I will push the series as is (with the other fixes included) but I'll send additional patches here if I misunderstood. Thanks for the reviews - Cole
On 4/16/19 7:40 PM, Cole Robinson wrote:
On 4/16/19 11:27 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This adds a network model enum. The virDomainNetDef property is named 'model' like most other devices.
When the XML parser or a driver calls NetSetModelString, if the passed string is in the enum, we will set net->model, otherwise we copy the string into net->modelstr
Add a single example for the 'netfront' xen model, and wire that up, just to verify it's all working
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 4 +-- src/qemu/qemu_hotplug.c | 8 ++++++ src/xenconfig/xen_common.c | 16 +++++------ src/xenconfig/xen_sxpr.c | 15 ++++++----- 7 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe1945b644..571f2eea39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "udp", ); +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,
This is supposed to be on a separate line now ;-)
+ "unknown", + "netfront", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) return; VIR_FREE(def->modelstr); + def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - /* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); - goto error; - } - VIR_STEAL_PTR(def->modelstr, model); - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error; switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card model %s does not match source %s"), + virDomainNetModelTypeToString(dst->model), + virDomainNetModelTypeToString(src->model)); + return false; + } + if (src->mtu != dst->mtu) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card MTU %d does not match source %d"), @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { + if (net->model) + return virDomainNetModelTypeToString(net->model); return net->modelstr; } @@ -29386,13 +29391,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->modelstr, model); + VIR_FREE(net->modelstr); + if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (!model) + return 0;
Is this a safe thing to do? I mean virEnumFromString() (which is called by any TypeFromString() in the end) doesn't handle NULL gracefully. You'll need to swap some lines and probably have a temp variable to store virDomainNetModelTypeFromString() retval, ...
Not completely sure I follow, but I think you mean: this function looks like it should operate as virEnumFromString does, meaning return -1 on NULL value. But consider that this is a hybrid enum (net->model) and string (net->modelstr) approach, in which modelstr=NULL is a valid case, so I'm not sure it should be an error.
I know this is a hybrid, but calling virDomainNetSetModelString(net, NULL) will lead to instant crash. Because model(=NULL) is passed to virDomainNetModeTypeFromString() which dereferences it, and only after that there's the check if (!model). True, in 08/11 you're fixing this so not big of a deal, but if somebody wants to cherry-pick this one they also need to back port 08/11. Michal
On 4/17/19 3:33 AM, Michal Privoznik wrote:
On 4/16/19 7:40 PM, Cole Robinson wrote:
On 4/16/19 11:27 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This adds a network model enum. The virDomainNetDef property is named 'model' like most other devices.
When the XML parser or a driver calls NetSetModelString, if the passed string is in the enum, we will set net->model, otherwise we copy the string into net->modelstr
Add a single example for the 'netfront' xen model, and wire that up, just to verify it's all working
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 4 +-- src/qemu/qemu_hotplug.c | 8 ++++++ src/xenconfig/xen_common.c | 16 +++++------ src/xenconfig/xen_sxpr.c | 15 ++++++----- 7 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe1945b644..571f2eea39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "udp", ); +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,
This is supposed to be on a separate line now ;-)
+ "unknown", + "netfront", +); + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) return; VIR_FREE(def->modelstr); + def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - /* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ - if (model != NULL) { - if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Model name contains invalid characters")); - goto error; - } - VIR_STEAL_PTR(def->modelstr, model); - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error; switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card model %s does not match source %s"), + virDomainNetModelTypeToString(dst->model), + virDomainNetModelTypeToString(src->model)); + return false; + } + if (src->mtu != dst->mtu) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card MTU %d does not match source %d"), @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *net) { + if (net->model) + return virDomainNetModelTypeToString(net->model); return net->modelstr; } @@ -29386,13 +29391,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - return VIR_STRDUP(net->modelstr, model); + VIR_FREE(net->modelstr); + if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (!model) + return 0;
Is this a safe thing to do? I mean virEnumFromString() (which is called by any TypeFromString() in the end) doesn't handle NULL gracefully. You'll need to swap some lines and probably have a temp variable to store virDomainNetModelTypeFromString() retval, ...
Not completely sure I follow, but I think you mean: this function looks like it should operate as virEnumFromString does, meaning return -1 on NULL value. But consider that this is a hybrid enum (net->model) and string (net->modelstr) approach, in which modelstr=NULL is a valid case, so I'm not sure it should be an error.
I know this is a hybrid, but calling virDomainNetSetModelString(net, NULL) will lead to instant crash. Because model(=NULL) is passed to virDomainNetModeTypeFromString() which dereferences it, and only after that there's the check if (!model). True, in 08/11 you're fixing this so not big of a deal, but if somebody wants to cherry-pick this one they also need to back port 08/11.
virDomainNetModeTypeFromString is just virEnumFromString which doesn't deference NULL int virEnumFromString(const char * const *types, unsigned int ntypes, const char *type) { size_t i; if (!type) return -1; - Cole
On 4/17/19 5:58 PM, Cole Robinson wrote:
<snip/> virDomainNetModeTypeFromString is just virEnumFromString which doesn't deference NULL
int
virEnumFromString(const char * const *types,
unsigned int ntypes,
const char *type)
{
size_t i;
if (!type)
return -1;
Ah, I somehow missed that. Yep, your code was right from the beginning. Sorry for the noise. Michal
The vz driver only handles three models: virtio, e1000, and rtl8139. Add enum values for those models, and convert the vz driver to handling net->model natively Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/conf/domain_conf.h | 3 +++ src/vz/vz_driver.c | 7 +++---- src/vz/vz_sdk.c | 17 +++++++---------- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 571f2eea39..854b8eb045 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -475,6 +475,9 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "unknown", "netfront", + "rtl8139", + "virtio", + "e1000", ); VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 92d54b0348..780f878fd8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -964,6 +964,9 @@ typedef enum { typedef enum { VIR_DOMAIN_NET_MODEL_UNKNOWN, VIR_DOMAIN_NET_MODEL_NETFRONT, + VIR_DOMAIN_NET_MODEL_RTL8139, + VIR_DOMAIN_NET_MODEL_VIRTIO, + VIR_DOMAIN_NET_MODEL_E1000, VIR_DOMAIN_NET_MODEL_LAST } virDomainNetModelType; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 66d019378f..00ee87c8d1 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -265,10 +265,9 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET && (dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK || dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && - !virDomainNetGetModelString(dev->data.net) && - def->os.type == VIR_DOMAIN_OSTYPE_HVM && - virDomainNetSetModelString(dev->data.net, "e1000") < 0) - return -1; + dev->data.net->model == VIR_DOMAIN_NET_MODEL_UNKNOWN && + def->os.type == VIR_DOMAIN_OSTYPE_HVM) + dev->data.net->model = VIR_DOMAIN_NET_MODEL_E1000; return 0; } diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index ed8345a874..2316ca5a31 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1104,16 +1104,13 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) switch ((int)type) { case PNT_RTL: - if (virDomainNetSetModelString(net, "rtl8139") < 0) - goto cleanup; + net->model = VIR_DOMAIN_NET_MODEL_RTL8139; break; case PNT_E1000: - if (virDomainNetSetModelString(net, "e1000") < 0) - goto cleanup; + net->model = VIR_DOMAIN_NET_MODEL_E1000; break; case PNT_VIRTIO: - if (virDomainNetSetModelString(net, "virtio") < 0) - goto cleanup; + net->model = VIR_DOMAIN_NET_MODEL_VIRTIO; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3377,15 +3374,15 @@ static int prlsdkConfigureNet(vzDriverPtr driver ATTRIBUTE_UNUSED, goto cleanup; if (isCt) { - if (virDomainNetGetModelString(net)) + if (net->model != VIR_DOMAIN_NET_MODEL_UNKNOWN) VIR_WARN("Setting network adapter for containers is not " "supported by vz driver."); } else { - if (virDomainNetStreqModelString(net, "rtl8139")) { + if (net->model == VIR_DOMAIN_NET_MODEL_RTL8139) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); - } else if (virDomainNetStreqModelString(net, "e1000")) { + } else if (net->model == VIR_DOMAIN_NET_MODEL_E1000) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); - } else if (virDomainNetStreqModelString(net, "virtio")) { + } else if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.20.1
The bhyve driver only works with the virtio and e1000 models, which we already have in the enum. Some error reporting is slightly downgraded to avoid some subtle usage of modelstr Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/bhyve/bhyve_command.c | 15 ++++----------- src/bhyve/bhyve_parse_command.c | 10 ++++------ 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index d3d790f6b6..219e2b8d9e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -57,16 +57,10 @@ bhyveBuildNetArgStr(virConnectPtr conn, int ret = -1; virDomainNetType actualType = virDomainNetGetActualType(net); - if (!virDomainNetGetModelString(net)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("NIC model must be specified")); - return -1; - } - - if (virDomainNetStreqModelString(net, "virtio")) { + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { if (VIR_STRDUP(nic_model, "virtio-net") < 0) return -1; - } else if (virDomainNetStreqModelString(net, "e1000")) { + } else if (net->model == VIR_DOMAIN_NET_MODEL_E1000) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) { if (VIR_STRDUP(nic_model, "e1000") < 0) return -1; @@ -77,9 +71,8 @@ bhyveBuildNetArgStr(virConnectPtr conn, return -1; } } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("NIC model '%s' is not supported"), - virDomainNetGetModelString(net)); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NIC model is not supported")); return -1; } diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 60eb4c5412..490381688c 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -492,7 +492,7 @@ bhyveParsePCINet(virDomainDefPtr def, unsigned pcislot, unsigned pcibus, unsigned function, - const char *model, + int model, const char *config) { /* -s slot,virtio-net,tapN[,mac=xx:xx:xx:xx:xx:xx] */ @@ -511,9 +511,7 @@ bhyveParsePCINet(virDomainDefPtr def, if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0) goto error; - if (virDomainNetSetModelString(net, model) < 0) - goto error; - + net->model = model; net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; net->info.addr.pci.slot = pcislot; net->info.addr.pci.bus = pcibus; @@ -621,10 +619,10 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def, conf); else if (STREQ(emulation, "virtio-net")) bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function, - "virtio", conf); + VIR_DOMAIN_NET_MODEL_VIRTIO, conf); else if (STREQ(emulation, "e1000")) bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function, - "e1000", conf); + VIR_DOMAIN_NET_MODEL_E1000, conf); VIR_FREE(emulation); VIR_FREE(slotdef); -- 2.20.1
This converts the qemu driver to the net model enum, for all the model values that we have hardcoded for various checks, which adds e1000e, virtio-transitional, virtio-non-transitional, usb-net, spapr-vlan, lan9118, smc91c111 Because the qemu driver has historically also allowed the raw model string onto the qemu command line, this isn't a full conversion. Unwinding that will require more thought. However for all new driver code we should be adding explicit enum values for any model name we have special handling for. Remove the now unused virDomainNetStreqModelString Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++++------------ src/conf/domain_conf.h | 9 +++++++-- src/libvirt_private.syms | 1 - src/qemu/qemu_command.c | 6 ++---- src/qemu/qemu_domain.c | 29 +++++++++++++---------------- src/qemu/qemu_domain_address.c | 12 ++++++------ 6 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 854b8eb045..a5882f27d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -478,6 +478,13 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "rtl8139", "virtio", "e1000", + "e1000e", + "virtio-transitional", + "virtio-non-transitional", + "usb-net", + "spapr-vlan", + "lan9118", + "scm91c111", ); VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, @@ -29413,15 +29420,6 @@ virDomainNetSetModelString(virDomainNetDefPtr net, return 0; } -int -virDomainNetStreqModelString(const virDomainNetDef *net, - const char *model) -{ - if (net->model) - return net->model == virDomainNetModelTypeFromString(model); - return STREQ_NULLABLE(net->modelstr, model); -} - int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, const char *model) @@ -29434,9 +29432,9 @@ virDomainNetStrcaseeqModelString(const virDomainNetDef *net, bool virDomainNetIsVirtioModel(const virDomainNetDef *net) { - return (virDomainNetStreqModelString(net, "virtio") || - virDomainNetStreqModelString(net, "virtio-transitional") || - virDomainNetStreqModelString(net, "virtio-non-transitional")); + return (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO || + net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL || + net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 780f878fd8..ed95deebf1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -967,6 +967,13 @@ typedef enum { VIR_DOMAIN_NET_MODEL_RTL8139, VIR_DOMAIN_NET_MODEL_VIRTIO, VIR_DOMAIN_NET_MODEL_E1000, + VIR_DOMAIN_NET_MODEL_E1000E, + VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL, + VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL, + VIR_DOMAIN_NET_MODEL_USB_NET, + VIR_DOMAIN_NET_MODEL_SPAPR_VLAN, + VIR_DOMAIN_NET_MODEL_LAN9118, + VIR_DOMAIN_NET_MODEL_SMC91C111, VIR_DOMAIN_NET_MODEL_LAST } virDomainNetModelType; @@ -3329,8 +3336,6 @@ bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); int virDomainNetSetModelString(virDomainNetDefPtr et, const char *model); -int virDomainNetStreqModelString(const virDomainNetDef *net, - const char *model); int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, const char *model); bool virDomainNetIsVirtioModel(const virDomainNetDef *net); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3a0699d724..2639446720 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -487,7 +487,6 @@ virDomainNetResolveActualType; virDomainNetSetDeviceImpl; virDomainNetSetModelString; virDomainNetStrcaseeqModelString; -virDomainNetStreqModelString; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0388ebc5af..a5deffbef2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -479,10 +479,8 @@ qemuBuildVirtioDevStr(virBufferPtr buf, break; case VIR_DOMAIN_DEVICE_NET: - has_tmodel = virDomainNetStreqModelString(device.data.net, - "virtio-transitional"); - has_ntmodel = virDomainNetStreqModelString(device.data.net, - "virtio-non-transitional"); + has_tmodel = device.data.net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL; + has_ntmodel = device.data.net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL; break; case VIR_DOMAIN_DEVICE_HOSTDEV: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7b001bca52..6718dfbc22 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6230,49 +6230,49 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, * is NULL this function may return NULL if the default model depends on the * capabilities. */ -static const char * +static int qemuDomainDefaultNetModel(const virDomainDef *def, virQEMUCapsPtr qemuCaps) { if (ARCH_IS_S390(def->os.arch)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; if (def->os.arch == VIR_ARCH_ARMV6L || def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) { if (STREQ(def->os.machine, "versatilepb")) - return "smc91c111"; + return VIR_DOMAIN_NET_MODEL_SMC91C111; if (qemuDomainIsARMVirt(def)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; /* Incomplete. vexpress (and a few others) use this, but not all * arm boards */ - return "lan9118"; + return VIR_DOMAIN_NET_MODEL_LAN9118; } /* virtio is a sensible default for RISC-V virt guests */ if (qemuDomainIsRISCVVirt(def)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; /* In all other cases the model depends on the capabilities. If they were * not provided don't report any default. */ if (!qemuCaps) - return NULL; + return VIR_DOMAIN_NET_MODEL_UNKNOWN; /* Try several network devices in turn; each of these devices is * less likely be supported out-of-the-box by the guest operating * system than the previous one */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RTL8139)) - return "rtl8139"; + return VIR_DOMAIN_NET_MODEL_RTL8139; else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000)) - return "e1000"; + return VIR_DOMAIN_NET_MODEL_E1000; else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET)) - return "virtio"; + return VIR_DOMAIN_NET_MODEL_VIRTIO; /* We've had no luck detecting support for any network device, * but we have to return something: might as well be rtl8139 */ - return "rtl8139"; + return VIR_DOMAIN_NET_MODEL_RTL8139; } @@ -6727,11 +6727,8 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps) { if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - !virDomainNetGetModelString(net)) { - if (virDomainNetSetModelString(net, - qemuDomainDefaultNetModel(def, qemuCaps)) < 0) - return -1; - } + !virDomainNetGetModelString(net)) + net->model = qemuDomainDefaultNetModel(def, qemuCaps); return 0; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 380c5bffcc..976ddaf3e0 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -230,7 +230,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (virDomainNetStreqModelString(net, "spapr-vlan")) + if (net->model == VIR_DOMAIN_NET_MODEL_SPAPR_VLAN) net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &net->info, VIO_ADDR_NET) < 0) @@ -715,18 +715,18 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, * addresses for other hostdev devices. */ if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || - virDomainNetStreqModelString(net, "usb-net")) { + net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { return 0; } - if (virDomainNetStreqModelString(net, "virtio") || - virDomainNetStreqModelString(net, "virtio-non-transitional")) + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO || + net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL) return virtioFlags; - if (virDomainNetStreqModelString(net, "virtio-transitional")) + if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL) return pciFlags; - if (virDomainNetStreqModelString(net, "e1000e")) + if (net->model == VIR_DOMAIN_NET_MODEL_E1000E) return pcieFlags; return pciFlags; -- 2.20.1
vbox and vmx drivers do net case insensitive net model comparisons, so for example 'VMXNET3' and 'vmxnet3' and 'VmxNeT3' in the XML will translate to the same driver configuration. To convert these drivers to use net model enum, we will need to do case insensitive comparisons as well. Essentially we implement virEnumToString, but with case insensitive comparison. XML will always be formatted with the enum model string we track internally, but we will accept any case insensitive variant. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 12 +++++++++--- tests/qemuxml2argvdata/net-many-models.xml | 3 ++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a5882f27d9..93b511d9bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29401,14 +29401,20 @@ int virDomainNetSetModelString(virDomainNetDefPtr net, const char *model) { - VIR_FREE(net->modelstr); - if ((net->model = virDomainNetModelTypeFromString(model)) >= 0) - return 0; + size_t i; + VIR_FREE(net->modelstr); net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN; if (!model) return 0; + for (i = 0; i < ARRAY_CARDINALITY(virDomainNetModelTypeList); i++) { + if (STRCASEEQ(virDomainNetModelTypeList[i], model)) { + net->model = i; + return 0; + } + } + if (strspn(model, NET_MODEL_CHARS) < strlen(model)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Model name contains invalid characters")); diff --git a/tests/qemuxml2argvdata/net-many-models.xml b/tests/qemuxml2argvdata/net-many-models.xml index 2b8f9b18eb..40fc5de06c 100644 --- a/tests/qemuxml2argvdata/net-many-models.xml +++ b/tests/qemuxml2argvdata/net-many-models.xml @@ -21,7 +21,8 @@ </interface> <interface type='user'> <mac address='00:11:22:33:44:58'/> - <model type='virtio'/> + <!-- explicitly testing case insensitive model compare --> + <model type='ViRtIo'/> </interface> <interface type='user'> <mac address='00:11:22:33:44:58'/> -- 2.20.1
Convert the vmware/vmx driver to net model enum, which requires adding enum values for vlance, vmxnet, vmxnet2, and vmxnet3. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 4 ++++ src/vmx/vmx.c | 52 ++++++++++++++++++++++-------------------- 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 93b511d9bc..3b5f27300e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -485,6 +485,10 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "spapr-vlan", "lan9118", "scm91c111", + "vlance", + "vmxnet", + "vmxnet2", + "vmxnet3", ); VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ed95deebf1..71ab8e5bf4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -974,6 +974,10 @@ typedef enum { VIR_DOMAIN_NET_MODEL_SPAPR_VLAN, VIR_DOMAIN_NET_MODEL_LAN9118, VIR_DOMAIN_NET_MODEL_SMC91C111, + VIR_DOMAIN_NET_MODEL_VLANCE, + VIR_DOMAIN_NET_MODEL_VMXNET, + VIR_DOMAIN_NET_MODEL_VMXNET2, + VIR_DOMAIN_NET_MODEL_VMXNET3, VIR_DOMAIN_NET_MODEL_LAST } virDomainNetModelType; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 7a557847ba..eb68c3acc4 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2546,6 +2546,8 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) char networkName_name[48] = ""; char *networkName = NULL; + int netmodel = VIR_DOMAIN_NET_MODEL_UNKNOWN; + if (def == NULL || *def != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; @@ -2630,11 +2632,17 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) } if (virtualDev != NULL) { - if (STRCASENEQ(virtualDev, "vlance") && - STRCASENEQ(virtualDev, "vmxnet") && - STRCASENEQ(virtualDev, "vmxnet3") && - STRCASENEQ(virtualDev, "e1000") && - STRCASENEQ(virtualDev, "e1000e")) { + if (STRCASEEQ(virtualDev, "vlance")) { + netmodel = VIR_DOMAIN_NET_MODEL_VLANCE; + } else if (STRCASEEQ(virtualDev, "vmxnet")) { + netmodel = VIR_DOMAIN_NET_MODEL_VMXNET; + } else if (STRCASEEQ(virtualDev, "vmxnet3")) { + netmodel = VIR_DOMAIN_NET_MODEL_VMXNET3; + } else if (STRCASEEQ(virtualDev, "e1000")) { + netmodel = VIR_DOMAIN_NET_MODEL_E1000; + } else if (STRCASEEQ(virtualDev, "e1000e")) { + netmodel = VIR_DOMAIN_NET_MODEL_E1000E; + } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting VMX entry '%s' to be 'vlance' or 'vmxnet' or " "'vmxnet3' or 'e1000' or 'e1000e' but found '%s'"), @@ -2642,12 +2650,8 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } - if (STRCASEEQ(virtualDev, "vmxnet") && features == 15) { - VIR_FREE(virtualDev); - - if (VIR_STRDUP(virtualDev, "vmxnet2") < 0) - goto cleanup; - } + if (netmodel == VIR_DOMAIN_NET_MODEL_VMXNET && features == 15) + netmodel = VIR_DOMAIN_NET_MODEL_VMXNET2; } /* vmx:networkName -> def:data.bridge.brname */ @@ -2697,10 +2701,7 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } - if (virDomainNetSetModelString((*def), virtualDev) < 0) - goto cleanup; - VIR_FREE(virtualDev); - + (*def)->model = netmodel; result = 0; cleanup: @@ -3738,29 +3739,30 @@ virVMXFormatEthernet(virDomainNetDefPtr def, int controller, virBufferAsprintf(buffer, "ethernet%d.present = \"true\"\n", controller); /* def:model -> vmx:virtualDev, vmx:features */ - if (virDomainNetGetModelString(def)) { - if (!virDomainNetStrcaseeqModelString(def, "vlance") && - !virDomainNetStrcaseeqModelString(def, "vmxnet") && - !virDomainNetStrcaseeqModelString(def, "vmxnet2") && - !virDomainNetStrcaseeqModelString(def, "vmxnet3") && - !virDomainNetStrcaseeqModelString(def, "e1000") && - !virDomainNetStrcaseeqModelString(def, "e1000e")) { + if (def->model) { + if (def->model != VIR_DOMAIN_NET_MODEL_VLANCE && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET2 && + def->model != VIR_DOMAIN_NET_MODEL_VMXNET3 && + def->model != VIR_DOMAIN_NET_MODEL_E1000 && + def->model != VIR_DOMAIN_NET_MODEL_E1000E) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML entry 'devices/interface/model' " "to be 'vlance' or 'vmxnet' or 'vmxnet2' or 'vmxnet3' " "or 'e1000' or 'e1000e' but found '%s'"), - virDomainNetGetModelString(def)); + virDomainNetModelTypeToString(def->model)); return -1; } - if (virDomainNetStrcaseeqModelString(def, "vmxnet2")) { + if (def->model == VIR_DOMAIN_NET_MODEL_VMXNET2) { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"vmxnet\"\n", controller); virBufferAsprintf(buffer, "ethernet%d.features = \"15\"\n", controller); } else { virBufferAsprintf(buffer, "ethernet%d.virtualDev = \"%s\"\n", - controller, virDomainNetGetModelString(def)); + controller, + virDomainNetModelTypeToString(def->model)); } } -- 2.20.1
Convert the vbox driver to net model enum, which requires adding enum values for Am79C970A, Am79C973, 82540EM, 82545EM, 82543GC. We preserve the same casing that vbox historically used for these model names. Remove the now unused virDomainNetStrcaseeqModelString Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 14 +++++--------- src/conf/domain_conf.h | 7 +++++-- src/libvirt_private.syms | 1 - src/vbox/vbox_common.c | 31 ++++++++++++++----------------- 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3b5f27300e..f58a6d77b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -489,6 +489,11 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "vmxnet", "vmxnet2", "vmxnet3", + "Am79C970A", + "Am79C973", + "82540EM", + "82545EM", + "82543GC", ); VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, @@ -29430,15 +29435,6 @@ virDomainNetSetModelString(virDomainNetDefPtr net, return 0; } -int -virDomainNetStrcaseeqModelString(const virDomainNetDef *net, - const char *model) -{ - if (net->model) - return STRCASEEQ(virDomainNetModelTypeToString(net->model), model); - return net->modelstr && STRCASEEQ(net->modelstr, model); -} - bool virDomainNetIsVirtioModel(const virDomainNetDef *net) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 71ab8e5bf4..505982ec0e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -978,6 +978,11 @@ typedef enum { VIR_DOMAIN_NET_MODEL_VMXNET, VIR_DOMAIN_NET_MODEL_VMXNET2, VIR_DOMAIN_NET_MODEL_VMXNET3, + VIR_DOMAIN_NET_MODEL_AM79C970A, + VIR_DOMAIN_NET_MODEL_AM79C973, + VIR_DOMAIN_NET_MODEL_82540EM, + VIR_DOMAIN_NET_MODEL_82545EM, + VIR_DOMAIN_NET_MODEL_82543GC, VIR_DOMAIN_NET_MODEL_LAST } virDomainNetModelType; @@ -3340,8 +3345,6 @@ bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); int virDomainNetSetModelString(virDomainNetDefPtr et, const char *model); -int virDomainNetStrcaseeqModelString(const virDomainNetDef *net, - const char *model); bool virDomainNetIsVirtioModel(const virDomainNetDef *net); int virDomainNetAppendIPAddress(virDomainNetDefPtr def, const char *address, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2639446720..afb8cf92d8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -486,7 +486,6 @@ virDomainNetRemoveHostdev; virDomainNetResolveActualType; virDomainNetSetDeviceImpl; virDomainNetSetModelString; -virDomainNetStrcaseeqModelString; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 8c28bba8ee..4612b1f6bd 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1309,7 +1309,7 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) macaddrvbox[VIR_MAC_STRING_BUFLEN - 6] = '\0'; VIR_DEBUG("NIC(%zu): Type: %d", i, def->nets[i]->type); - VIR_DEBUG("NIC(%zu): Model: %s", i, virDomainNetGetModelString(def->nets[i])); + VIR_DEBUG("NIC(%zu): Model: %s", i, virDomainNetModelTypeToString(def->nets[i]->model)); VIR_DEBUG("NIC(%zu): Mac: %s", i, macaddr); VIR_DEBUG("NIC(%zu): ifname: %s", i, def->nets[i]->ifname); if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -1338,19 +1338,19 @@ vboxAttachNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) gVBoxAPI.UINetworkAdapter.SetEnabled(adapter, 1); - if (virDomainNetGetModelString(def->nets[i])) { - if (virDomainNetStrcaseeqModelString(def->nets[i], "Am79C970A")) { + if (def->nets[i]->model) { + if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_AM79C970A) { adapterType = NetworkAdapterType_Am79C970A; - } else if (virDomainNetStrcaseeqModelString(def->nets[i], "Am79C973")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_AM79C973) { adapterType = NetworkAdapterType_Am79C973; - } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82540EM")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82540EM) { adapterType = NetworkAdapterType_I82540EM; - } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82545EM")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82545EM) { adapterType = NetworkAdapterType_I82545EM; - } else if (virDomainNetStrcaseeqModelString(def->nets[i], "82543GC")) { + } else if (def->nets[i]->model == VIR_DOMAIN_NET_MODEL_82543GC) { adapterType = NetworkAdapterType_I82543GC; } else if (gVBoxAPI.APIVersion >= 3000051 && - virDomainNetStrcaseeqModelString(def->nets[i], "virtio")) { + def->nets[i]->model == VIR_DOMAIN_NET_MODEL_VIRTIO) { /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ adapterType = NetworkAdapterType_Virtio; } @@ -3687,7 +3687,6 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) { PRUint32 attachmentType = NetworkAttachmentType_Null; PRUint32 adapterType = NetworkAdapterType_Null; - const char *model = NULL; PRUnichar *utf16 = NULL; char *utf8 = NULL; virDomainNetDefPtr net = NULL; @@ -3742,28 +3741,26 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); switch (adapterType) { case NetworkAdapterType_Am79C970A: - model = "Am79C970A"; + net->model = VIR_DOMAIN_NET_MODEL_AM79C970A; break; case NetworkAdapterType_Am79C973: - model = "Am79C973"; + net->model = VIR_DOMAIN_NET_MODEL_AM79C973; break; case NetworkAdapterType_I82540EM: - model = "82540EM"; + net->model = VIR_DOMAIN_NET_MODEL_82540EM; break; case NetworkAdapterType_I82545EM: - model = "82545EM"; + net->model = VIR_DOMAIN_NET_MODEL_82545EM; break; case NetworkAdapterType_I82543GC: - model = "82543GC"; + net->model = VIR_DOMAIN_NET_MODEL_82543GC; break; case NetworkAdapterType_Virtio: /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ if (gVBoxAPI.APIVersion >= 3000051) - model = "virtio"; + net->model = VIR_DOMAIN_NET_MODEL_VIRTIO; break; } - if (virDomainNetSetModelString(net, model) < 0) - goto error; gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16); VBOX_UTF16_TO_UTF8(utf16, &utf8); -- 2.20.1
This requires drivers to opt in to handle the raw modelstr network model, all others will error if a passed in XML value is not in the model enum. Enable this feature for libxl/xen/xm and qemu drivers Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_domain.c | 1 + src/qemu/qemu_domain.c | 3 ++- src/security/virt-aa-helper.c | 3 ++- 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f58a6d77b9..f06070d740 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5265,6 +5265,15 @@ virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev, virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) return -1; + if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) && + dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->modelstr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("driver does not support net model '%s'"), + dev->data.net->modelstr); + return -1; + } + return 0; } #undef UNSUPPORTED diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 505982ec0e..14b831ffac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2769,6 +2769,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8), } virDomainDefFeatures; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ffafa7967d..dc974dc926 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -424,6 +424,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6718dfbc22..1214e32be4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6967,7 +6967,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | - VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 989dcf1784..c250c61cb6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) virDomainDefParserConfig virAAHelperDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | - VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS, + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; static int -- 2.20.1
On 3/13/19 4:51 PM, Cole Robinson wrote:
This requires drivers to opt in to handle the raw modelstr network model, all others will error if a passed in XML value is not in the model enum.
Enable this feature for libxl/xen/xm and qemu drivers
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_domain.c | 1 + src/qemu/qemu_domain.c | 3 ++- src/security/virt-aa-helper.c | 3 ++- 5 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f58a6d77b9..f06070d740 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5265,6 +5265,15 @@ virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev, virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) return -1;
+ if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) && + dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->modelstr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("driver does not support net model '%s'"), + dev->data.net->modelstr); + return -1; + } + return 0; } #undef UNSUPPORTED diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 505982ec0e..14b831ffac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2769,6 +2769,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8), } virDomainDefFeatures;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ffafa7967d..dc974dc926 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -424,6 +424,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, };
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6718dfbc22..1214e32be4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6967,7 +6967,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | - VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, };
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 989dcf1784..c250c61cb6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) virDomainDefParserConfig virAAHelperDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | - VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS, + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, };
static int
You've changed some other drivers too. Should we enable this feature for them too? Michal
On 4/16/19 11:26 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This requires drivers to opt in to handle the raw modelstr network model, all others will error if a passed in XML value is not in the model enum.
Enable this feature for libxl/xen/xm and qemu drivers
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_domain.c | 1 + src/qemu/qemu_domain.c | 3 ++- src/security/virt-aa-helper.c | 3 ++- 5 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f58a6d77b9..f06070d740 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5265,6 +5265,15 @@ virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev, virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) return -1; + if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) && + dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->modelstr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("driver does not support net model '%s'"), + dev->data.net->modelstr); + return -1; + } + return 0; } #undef UNSUPPORTED diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 505982ec0e..14b831ffac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2769,6 +2769,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8), } virDomainDefFeatures; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ffafa7967d..dc974dc926 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -424,6 +424,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6718dfbc22..1214e32be4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6967,7 +6967,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | - VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 989dcf1784..c250c61cb6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) virDomainDefParserConfig virAAHelperDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | - VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS, + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; static int
You've changed some other drivers too. Should we enable this feature for them too?
The other drivers are all changed to only handle known enum values. This feature is only enabled for drivers that still contain code which may process the raw modelstr. So that's libxl and qemu (and by extension virt-aa-helper which processes XML). So I think this piece is correct. Thanks, Cole
On Tue, Apr 16, 2019 at 12:59:23PM -0400, Cole Robinson wrote:
On 4/16/19 11:26 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This requires drivers to opt in to handle the raw modelstr network model, all others will error if a passed in XML value is not in the model enum.
Enable this feature for libxl/xen/xm and qemu drivers
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_domain.c | 1 + src/qemu/qemu_domain.c | 3 ++- src/security/virt-aa-helper.c | 3 ++- 5 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f58a6d77b9..f06070d740 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5265,6 +5265,15 @@ virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev, virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) return -1; + if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) && + dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->modelstr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("driver does not support net model '%s'"), + dev->data.net->modelstr); + return -1; + } + return 0; } #undef UNSUPPORTED diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 505982ec0e..14b831ffac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2769,6 +2769,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8), } virDomainDefFeatures; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ffafa7967d..dc974dc926 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -424,6 +424,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6718dfbc22..1214e32be4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6967,7 +6967,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | - VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 989dcf1784..c250c61cb6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) virDomainDefParserConfig virAAHelperDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | - VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS, + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; static int
You've changed some other drivers too. Should we enable this feature for them too?
The other drivers are all changed to only handle known enum values. This feature is only enabled for drivers that still contain code which may process the raw modelstr. So that's libxl and qemu (and by extension virt-aa-helper which processes XML). So I think this piece is correct.
I see this has broken the virt-manager test suite which passes various invented model names to the test driver. Given that this is the test driver, this isn't too serious, but probably should be called out in the release notes if you've not already done so. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 4/17/19 7:28 AM, Daniel P. Berrangé wrote:
On Tue, Apr 16, 2019 at 12:59:23PM -0400, Cole Robinson wrote:
On 4/16/19 11:26 AM, Michal Privoznik wrote:
On 3/13/19 4:51 PM, Cole Robinson wrote:
This requires drivers to opt in to handle the raw modelstr network model, all others will error if a passed in XML value is not in the model enum.
Enable this feature for libxl/xen/xm and qemu drivers
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + src/libxl/libxl_domain.c | 1 + src/qemu/qemu_domain.c | 3 ++- src/security/virt-aa-helper.c | 3 ++- 5 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f58a6d77b9..f06070d740 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5265,6 +5265,15 @@ virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev, virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0) return -1; + if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) && + dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->modelstr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("driver does not support net model '%s'"), + dev->data.net->modelstr); + return -1; + } + return 0; } #undef UNSUPPORTED diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 505982ec0e..14b831ffac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2769,6 +2769,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8), } virDomainDefFeatures; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ffafa7967d..dc974dc926 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -424,6 +424,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6718dfbc22..1214e32be4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6967,7 +6967,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | - VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 989dcf1784..c250c61cb6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) virDomainDefParserConfig virAAHelperDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | - VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS, + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; static int
You've changed some other drivers too. Should we enable this feature for them too?
The other drivers are all changed to only handle known enum values. This feature is only enabled for drivers that still contain code which may process the raw modelstr. So that's libxl and qemu (and by extension virt-aa-helper which processes XML). So I think this piece is correct.
I see this has broken the virt-manager test suite which passes various invented model names to the test driver. Given that this is the test driver, this isn't too serious, but probably should be called out in the release notes if you've not already done so.
I fixed the virt-manager test suite to not pass in unknown models so CI should be fixed This XML feature should be enabled for the test driver though, at least until we get 'full' enum coverage in xen and qemu drivers (for example right now test driver with model=pcnet will be rejected because it's not yet in the enum, even though it definitely should be). I'll send a patch for that. I think release notes should only be added after the next round of changes which will filling in more enum content, and maybe taint VMs if they are using model passthrough (not sure yet, patches aren't written yet). Thanks, Cole
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
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
participants (3)
-
Cole Robinson -
Daniel P. Berrangé -
Michal Privoznik