[libvirt] [PATCH 00/10] RFC: conf: partially net model enum conversion
This series is base on my virtio-transitional work, since it touches a lot of the same code: https://www.redhat.com/archives/libvir-list/2019-January/msg00593.html 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. Unwinding it will take some thought. For starters would be populating the enum with any qemu/xen network model that a user could realistically have a working config with. Then maybe tainting the VM if modelstr is used. Then after some time the final round could be causing domains to fail to start, but not fail to parse, so they don't disappear on upgrade but still break in an obvious way that will generate complaints/bugreports. But we should agree on a plan for it. Other caveats: * vz and bhyve drivers are not compile tested * vmx and virtualbox drivers previously would do a case insensitive compare on the model value passed in via the user XML. Current patches don't preserve that. I don't know how much it matters for these drivers for reading fresh XML vs roundtrip XML from converted native formats (which _will_ correct the case issue). If we want to fix this we will need to tolower() the modelstr value in the XML parser. I think there's a gnulib function for it but I haven't explored it deeply Cole Robinson (10): 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 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 | 105 ++++++++++++++---- 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 | 33 +++--- src/qemu/qemu_domain.c | 32 +++--- src/qemu/qemu_domain_address.c | 14 +-- 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 | 37 ++++++ tests/qemuxml2argvtest.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 + 33 files changed, 534 insertions(+), 182 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-many-models.args create mode 100644 tests/qemuxml2argvdata/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/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 + 13 files changed, 253 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-many-models.args create mode 100644 tests/qemuxml2argvdata/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 8da99fc390..e4a9eba575 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1401,6 +1401,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/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 b1eb8a0614..ab3872337e 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -258,6 +258,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
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 | 11 +++++----- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_domain_address.c | 14 ++++++------- 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, 138 insertions(+), 91 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 84fda08943..78f8a88290 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -56,16 +56,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; @@ -78,7 +78,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 1c9191fb96..cf0b2db30a 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -513,7 +513,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 46fbc0befc..778b842979 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25565,9 +25565,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 (virDomainNetHasVirtioModel(def)) { char *str = NULL, *gueststr = NULL, *hoststr = NULL; int rc = 0; @@ -29838,13 +29838,39 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) return iface->trustGuestRxFilters == VIR_TRISTATE_BOOL_YES; } +const char * +virDomainNetGetModelString(const virDomainNetDef *iface) +{ + return iface->model; +} + +int +virDomainNetSetModelString(virDomainNetDefPtr iface, + const char *model) +{ + return VIR_STRDUP(iface->model, model); +} + +int +virDomainNetStreqModelString(const virDomainNetDef *iface, + const char *model) +{ + return STREQ_NULLABLE(iface->model, model); +} + +int +virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, + const char *model) +{ + return iface->model && STRCASEEQ(iface->model, model); +} bool virDomainNetHasVirtioModel(const virDomainNetDef *iface) { - return (STREQ_NULLABLE(iface->model, "virtio") || - STREQ_NULLABLE(iface->model, "virtio-transitional") || - STREQ_NULLABLE(iface->model, "virtio-non-transitional")); + return (virDomainNetStreqModelString(iface, "virtio") || + virDomainNetStreqModelString(iface, "virtio-transitional") || + virDomainNetStreqModelString(iface, "virtio-non-transitional")); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0d801b0d6d..8f856c2d14 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3271,6 +3271,13 @@ virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); +const char *virDomainNetGetModelString(const virDomainNetDef *iface); +int virDomainNetSetModelString(virDomainNetDefPtr iface, + const char *model); +int virDomainNetStreqModelString(const virDomainNetDef *iface, + const char *model); +int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, + const char *model); bool virDomainNetHasVirtioModel(const virDomainNetDef *iface); int virDomainNetAppendIPAddress(virDomainNetDefPtr def, const char *address, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 60a193450b..afb5975352 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -472,6 +472,7 @@ virDomainNetGetActualTrustGuestRxFilters; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; +virDomainNetGetModelString; virDomainNetHasVirtioModel; virDomainNetInsert; virDomainNetNotifyActualDevice; @@ -480,6 +481,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 4102a940b9..29aa04583c 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 940d55b1b1..83222dd0be 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3779,13 +3779,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=" : ""), - (net->model ? net->model : ""), + (netmodel ? ",model=" : ""), + (netmodel ? netmodel : ""), (net->info.alias ? ",id=" : ""), (net->info.alias ? net->info.alias : ""))); return str; @@ -3805,14 +3806,14 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (virDomainNetHasVirtioModel(net)) { if (qemuBuildVirtioTransitional(&buf, "virtio-net", qemuCaps, - net->info.type, - 0, net->model, + net->info.type, 0, + virDomainNetGetModelString(net), VIR_DOMAIN_DEVICE_NET) < 0) goto error; 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 85f7b6a4c8..38e86b8ad1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6565,8 +6565,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 50cc47b7b3..c0843bdad6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -230,10 +230,8 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (net->model && - STREQ(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) goto cleanup; @@ -696,18 +694,18 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, * addresses for other hostdev devices. */ if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || - STREQ(net->model, "usb-net")) { + virDomainNetStreqModelString(net, "usb-net")) { return 0; } - if (STREQ(net->model, "virtio") || - STREQ(net->model, "virtio-non-transitional")) + if (virDomainNetStreqModelString(net, "virtio") || + virDomainNetStreqModelString(net, "virtio-non-transitional")) return virtioFlags; - if (STREQ(net->model, "virtio-transitional")) + if (virDomainNetStreqModelString(net, "virtio-transitional")) return pciFlags; - if (STREQ(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 949c09aba4..ba8b7d13a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7449,20 +7449,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 2fdc71d07b..b0a99ed853 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3702,11 +3702,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 c4650f01e0..0a080f3ece 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1111,8 +1111,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 00d43d9a83..fa8d899fec 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 a853c05e86..7f8357211c 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2670,10 +2670,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 */ @@ -2683,16 +2681,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 { @@ -2702,6 +2696,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) goto cleanup; } + if (virDomainNetSetModelString((*def), virtualDev) < 0) + goto cleanup; + VIR_FREE(virtualDev); + result = 0; cleanup: @@ -3739,28 +3737,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 7e9ef932dc..eab2953759 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -269,9 +269,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 63d013deac..60681591c6 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1105,15 +1105,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: @@ -3378,15 +3378,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 94e0703cf3..74ce8dcc69 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 11af3e747a..b3bbce4e86 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"); @@ -1935,15 +1935,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
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 778b842979..25fbf6f64b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2120,7 +2120,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) if (!def) return; - VIR_FREE(def->model); + VIR_FREE(def->modelstr); switch (def->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -11402,7 +11402,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, _("Model name contains invalid characters")); goto error; } - def->model = model; + def->modelstr = model; model = NULL; } @@ -21971,10 +21971,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; } @@ -29841,28 +29841,28 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *iface) { - return iface->model; + return iface->modelstr; } int virDomainNetSetModelString(virDomainNetDefPtr iface, const char *model) { - return VIR_STRDUP(iface->model, model); + return VIR_STRDUP(iface->modelstr, model); } int virDomainNetStreqModelString(const virDomainNetDef *iface, const char *model) { - return STREQ_NULLABLE(iface->model, model); + return STREQ_NULLABLE(iface->modelstr, model); } int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, const char *model) { - return iface->model && STRCASEEQ(iface->model, model); + return iface->modelstr && STRCASEEQ(iface->modelstr, model); } bool diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f856c2d14..5400deda4c 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 | 56 +++++++++++++++++++++++++++----------- 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(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 25fbf6f64b..129e16bd0b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -436,6 +436,10 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "hostdev", "udp") +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, + "unknown", + "netfront") + VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", "qemu", @@ -2121,6 +2125,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: @@ -11390,21 +11395,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; - } - def->modelstr = model; - model = NULL; - } + if (model != NULL && + virDomainNetSetModelString(def, model) < 0) + goto error; switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -21978,6 +21971,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"), @@ -29841,6 +29842,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) const char * virDomainNetGetModelString(const virDomainNetDef *iface) { + if (iface->model) + return virDomainNetModelTypeToString(iface->model); return iface->modelstr; } @@ -29848,13 +29851,31 @@ int virDomainNetSetModelString(virDomainNetDefPtr iface, const char *model) { - return VIR_STRDUP(iface->modelstr, model); + VIR_FREE(iface->modelstr); + if ((iface->model = virDomainNetModelTypeFromString(model)) >= 0) + return 0; + + iface->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(iface->modelstr, model) < 0) + return -1; + return 0; } int virDomainNetStreqModelString(const virDomainNetDef *iface, const char *model) { + if (iface->model) + return iface->model == virDomainNetModelTypeFromString(model); return STREQ_NULLABLE(iface->modelstr, model); } @@ -29862,6 +29883,9 @@ int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, const char *model) { + if (iface->model) + return STRCASEEQ(virDomainNetModelTypeToString(iface->model), + model); return iface->modelstr && STRCASEEQ(iface->modelstr, model); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5400deda4c..e26b885508 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 { @@ -3493,6 +3502,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 afb5975352..d802535fa0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -475,6 +475,8 @@ virDomainNetGetActualVlan; virDomainNetGetModelString; virDomainNetHasVirtioModel; virDomainNetInsert; +virDomainNetModelTypeFromString; +virDomainNetModelTypeToString; virDomainNetNotifyActualDevice; virDomainNetReleaseActualDevice; virDomainNetRemove; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 29aa04583c..ebb38ba7e8 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 b0a99ed853..168b391f0e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3711,6 +3711,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 (virDomainNetHasVirtioModel(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 74ce8dcc69..dfe240a949 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 b3bbce4e86..071949bff5 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) { @@ -1940,7 +1941,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
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 | 5 ++++- src/conf/domain_conf.h | 3 +++ src/vz/vz_driver.c | 7 +++---- src/vz/vz_sdk.c | 17 +++++++---------- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 129e16bd0b..9b651d06b6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -438,7 +438,10 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "unknown", - "netfront") + "netfront", + "rtl8139", + "virtio", + "e1000") VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e26b885508..2e1235a993 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 eab2953759..10fba0efb7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -269,10 +269,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 60681591c6..13f18cbc19 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1105,16 +1105,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, @@ -3378,15 +3375,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 78f8a88290..ccbd7cd2aa 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -56,16 +56,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; @@ -76,9 +70,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 cf0b2db30a..ba1e3f2cb7 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -494,7 +494,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] */ @@ -513,9 +513,7 @@ bhyveParsePCINet(virDomainDefPtr def, if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0) goto error; - if (virDomainNetSetModelString(net, model) < 0) - goto error; - + net->model = mode; net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; net->info.addr.pci.slot = pcislot; net->info.addr.pci.bus = pcibus; @@ -623,10 +621,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 | 24 +++++++++++------------- src/conf/domain_conf.h | 9 +++++++-- src/libvirt_private.syms | 1 - src/qemu/qemu_command.c | 26 ++++++++++++-------------- src/qemu/qemu_domain.c | 29 +++++++++++++---------------- src/qemu/qemu_domain_address.c | 12 ++++++------ 6 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b651d06b6..4e0e7eabe6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -441,7 +441,14 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "netfront", "rtl8139", "virtio", - "e1000") + "e1000", + "e1000e", + "virtio-transitional", + "virtio-non-transitional", + "usb-net", + "spapr-vlan", + "lan9118", + "scm91c111") VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", @@ -29873,15 +29880,6 @@ virDomainNetSetModelString(virDomainNetDefPtr iface, return 0; } -int -virDomainNetStreqModelString(const virDomainNetDef *iface, - const char *model) -{ - if (iface->model) - return iface->model == virDomainNetModelTypeFromString(model); - return STREQ_NULLABLE(iface->modelstr, model); -} - int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, const char *model) @@ -29895,9 +29893,9 @@ virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, bool virDomainNetHasVirtioModel(const virDomainNetDef *iface) { - return (virDomainNetStreqModelString(iface, "virtio") || - virDomainNetStreqModelString(iface, "virtio-transitional") || - virDomainNetStreqModelString(iface, "virtio-non-transitional")); + return (iface->model == VIR_DOMAIN_NET_MODEL_VIRTIO || + iface->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL || + iface->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2e1235a993..4c42bd18cf 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; @@ -3286,8 +3293,6 @@ bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *iface); int virDomainNetSetModelString(virDomainNetDefPtr iface, const char *model); -int virDomainNetStreqModelString(const virDomainNetDef *iface, - const char *model); int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, const char *model); bool virDomainNetHasVirtioModel(const virDomainNetDef *iface); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d802535fa0..3ed0b3b11c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -485,7 +485,6 @@ virDomainNetResolveActualType; virDomainNetSetDeviceImpl; virDomainNetSetModelString; virDomainNetStrcaseeqModelString; -virDomainNetStreqModelString; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 83222dd0be..48e8b7a9bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -449,7 +449,6 @@ qemuBuildVirtioTransitional(virBufferPtr buf, virQEMUCapsPtr qemuCaps, virDomainDeviceAddressType type, int model, - const char *modelstr, virDomainDeviceType devtype) { int tmodel_cap, ntmodel_cap; @@ -466,8 +465,8 @@ qemuBuildVirtioTransitional(virBufferPtr buf, ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL; break; case VIR_DOMAIN_DEVICE_NET: - has_tmodel = STREQ(modelstr, "virtio-transitional"); - has_ntmodel = STREQ(modelstr, "virtio-non-transitional"); + has_tmodel = model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL; + has_ntmodel = model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL; tmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL; ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL; break; @@ -2186,7 +2185,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, case VIR_DOMAIN_DISK_BUS_VIRTIO: if (qemuBuildVirtioTransitional(&opt, "virtio-blk", qemuCaps, disk->info.type, - disk->model, NULL, + disk->model, VIR_DOMAIN_DEVICE_DISK) < 0) goto error; @@ -2778,7 +2777,7 @@ qemuBuildFSDevStr(const virDomainDef *def, if (qemuBuildVirtioTransitional(&opt, "virtio-9p", qemuCaps, fs->info.type, - fs->model, NULL, + fs->model, VIR_DOMAIN_DEVICE_FS) < 0) goto error; @@ -2989,7 +2988,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL: if (qemuBuildVirtioTransitional(&buf, "virtio-scsi", qemuCaps, def->info.type, - def->model, NULL, + def->model, VIR_DOMAIN_DEVICE_CONTROLLER) < 0) { goto error; } @@ -3034,7 +3033,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: if (qemuBuildVirtioTransitional(&buf, "virtio-serial", qemuCaps, def->info.type, - def->model, NULL, + def->model, VIR_DOMAIN_DEVICE_CONTROLLER) < 0) goto error; @@ -3806,8 +3805,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (virDomainNetHasVirtioModel(net)) { if (qemuBuildVirtioTransitional(&buf, "virtio-net", qemuCaps, - net->info.type, 0, - virDomainNetGetModelString(net), + net->info.type, net->model, VIR_DOMAIN_DEVICE_NET) < 0) goto error; @@ -4204,7 +4202,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, if (qemuBuildVirtioTransitional(&buf, "virtio-balloon", qemuCaps, def->memballoon->info.type, - def->memballoon->model, NULL, + def->memballoon->model, VIR_DOMAIN_DEVICE_MEMBALLOON) < 0) { goto error; } @@ -4317,7 +4315,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: if (qemuBuildVirtioTransitional(&buf, "virtio-input-host", qemuCaps, dev->info.type, - dev->model, NULL, + dev->model, VIR_DOMAIN_DEVICE_INPUT) < 0) goto error; break; @@ -5085,7 +5083,7 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def, if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps, dev->info->type, - hostsrc->model, NULL, + hostsrc->model, VIR_DOMAIN_DEVICE_HOSTDEV) < 0) goto cleanup; @@ -6044,7 +6042,7 @@ qemuBuildRNGDevStr(const virDomainDef *def, if (qemuBuildVirtioTransitional(&buf, "virtio-rng", qemuCaps, dev->info.type, - dev->model, NULL, + dev->model, VIR_DOMAIN_DEVICE_RNG) < 0) goto error; @@ -10510,7 +10508,7 @@ qemuBuildVsockDevStr(virDomainDefPtr def, if (qemuBuildVirtioTransitional(&buf, "vhost-vsock", qemuCaps, vsock->info.type, - vsock->model, NULL, + vsock->model, VIR_DOMAIN_DEVICE_VSOCK) < 0) goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 38e86b8ad1..44d43a85c9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6067,49 +6067,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; } @@ -6565,11 +6565,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 c0843bdad6..0cb557b2ab 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) @@ -694,18 +694,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
Convert the vmware/vmx driver to net model enum, which requires adding enum values for vlance, vmxnet, vmxnet2, and vmxnet3. Previously vmx would accept case insensitive network model names via the domain XML, but now it will require names to exactly match the enum case. For reading from vmx files this won't matter as the parser does case-insensitive comparison. But if a user attempts to define/create a VM via custom XML with the wrong case, it will be rejected. I don't know if that's actually a usecase here so maybe it doesn't matter. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 6 ++++- src/conf/domain_conf.h | 4 ++++ src/vmx/vmx.c | 52 ++++++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4e0e7eabe6..b040b7b983 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -448,7 +448,11 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "usb-net", "spapr-vlan", "lan9118", - "scm91c111") + "scm91c111", + "vlance", + "vmxnet", + "vmxnet2", + "vmxnet3") VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4c42bd18cf..e144d2c87c 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 7f8357211c..ae0373dad2 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2545,6 +2545,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; @@ -2629,11 +2631,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'"), @@ -2641,12 +2649,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 */ @@ -2696,10 +2700,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: @@ -3737,29 +3738,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 Similar to the vmx driver, case insensitive handling of the model passed in via the XML is lost. I don't know if we care Remove the now unused virDomainNetStrcaseeqModelString Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 17 ++++++----------- src/conf/domain_conf.h | 7 +++++-- src/libvirt_private.syms | 1 - src/vbox/vbox_common.c | 31 ++++++++++++++----------------- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b040b7b983..e951e8153b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -452,7 +452,12 @@ VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST, "vlance", "vmxnet", "vmxnet2", - "vmxnet3") + "vmxnet3", + "Am79C970A", + "Am79C973", + "82540EM", + "82545EM", + "82543GC") VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", @@ -29884,16 +29889,6 @@ virDomainNetSetModelString(virDomainNetDefPtr iface, return 0; } -int -virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, - const char *model) -{ - if (iface->model) - return STRCASEEQ(virDomainNetModelTypeToString(iface->model), - model); - return iface->modelstr && STRCASEEQ(iface->modelstr, model); -} - bool virDomainNetHasVirtioModel(const virDomainNetDef *iface) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e144d2c87c..b4e70480ac 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; @@ -3297,8 +3302,6 @@ bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *iface); int virDomainNetSetModelString(virDomainNetDefPtr iface, const char *model); -int virDomainNetStrcaseeqModelString(const virDomainNetDef *iface, - const char *model); bool virDomainNetHasVirtioModel(const virDomainNetDef *iface); int virDomainNetAppendIPAddress(virDomainNetDefPtr def, const char *address, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ed0b3b11c..e4585e4884 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -484,7 +484,6 @@ virDomainNetRemoveHostdev; virDomainNetResolveActualType; virDomainNetSetDeviceImpl; virDomainNetSetModelString; -virDomainNetStrcaseeqModelString; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index fa8d899fec..0474c58730 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 e951e8153b..19564c3e4f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4972,6 +4972,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 b4e70480ac..b25d539cb2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2739,6 +2739,7 @@ typedef enum { VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4), VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5), VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6), + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 7), } virDomainDefFeatures; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 3ada51f517..f4093411e3 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 44d43a85c9..e1b0b08dcc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6804,7 +6804,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | - VIR_DOMAIN_DEF_FEATURE_USER_ALIAS, + VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 3bd30bb417..86abd35de6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -646,7 +646,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 Fri, Jan 18, 2019 at 06:05:18PM -0500, Cole Robinson wrote:
This series is base on my virtio-transitional work, since it touches a lot of the same code: https://www.redhat.com/archives/libvir-list/2019-January/msg00593.html
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
Xen is probably quite easy to deal with as it supports far fewer arches than the qemu driver. On x86 we need to handle the usual rtl8139, e100, ne2k, etc for Xen fully virt. I think for other Xen non-x86 arches, it might be reasonable to only care about net-front even for fully-virt given that people using Xen with hardware acceleration won't want a slow NIC. QEMU is the really hard one as it has a huge set of arches and people using QEMU in TCG mode conceivably care about all the random NIC models no matter how slow & awful they are, because TCG is already slow & awful.
<model type='idontexist'/>
qemu would turn that into
-device idontexist,...
That behavior is untouched by this series. Unwinding it will take some thought. For starters would be populating the enum with any qemu/xen network model that a user could realistically have a working config with. Then maybe tainting the VM if modelstr is used. Then after some time the final round could be causing domains to fail to start, but not fail to parse, so they don't disappear on upgrade but still break in an obvious way that will generate complaints/bugreports. But we should agree on a plan for it.
Yes, I think we should mark the guest as "tainted" as that ensures a log message in the per-QEMU log file. We should also ensure we use VIR_WARN in libvirtd log too i guess. We'd want to keep this loose acceptance for at least a year, possibly two before we consider removing it.
Other caveats: * vz and bhyve drivers are not compile tested
* vmx and virtualbox drivers previously would do a case insensitive compare on the model value passed in via the user XML. Current patches don't preserve that. I don't know how much it matters for these drivers for reading fresh XML vs roundtrip XML from converted native formats (which _will_ correct the case issue). If we want to fix this we will need to tolower() the modelstr value in the XML parser. I think there's a gnulib function for it but I haven't explored it deeply
I guess if we're going to be serious about maintaining compat, we should accept the insensitive strings, at the very least for a transitional period. Normally all our enums would be 100% lowercase, but in the vbox driver you're adding model names which are mixed-case, because that's what vbox would previously output. I think you are right to preserve that mixed case despite it not being our normal practice, because round-tripped XML is important. I fear there there could well be people feeding in vbox XML that uses all-lowercase for these vbox models based on normal libvirt precedent though. This re-inforces the idea that we should allow a case-insensitive match when parsing, and perhaps log a warning if people use the non-preferred case. Perhaps after a year or two we could drop the case-insensitive match, but it would not be a huge burden to just carry it forever. 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 01/22/2019 06:55 AM, Daniel P. Berrangé wrote:
On Fri, Jan 18, 2019 at 06:05:18PM -0500, Cole Robinson wrote:
This series is base on my virtio-transitional work, since it touches a lot of the same code: https://www.redhat.com/archives/libvir-list/2019-January/msg00593.html
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
Xen is probably quite easy to deal with as it supports far fewer arches than the qemu driver. On x86 we need to handle the usual rtl8139, e100, ne2k, etc for Xen fully virt. I think for other Xen non-x86 arches, it might be reasonable to only care about net-front even for fully-virt given that people using Xen with hardware acceleration won't want a slow NIC.
Okay sounds good. I'll save that for a follow up series after doing some research/
QEMU is the really hard one as it has a huge set of arches and people using QEMU in TCG mode conceivably care about all the random NIC models no matter how slow & awful they are, because TCG is already slow & awful.
This is probably not as bad as you suggest. I believe most tcg qemu variants can't even handle -device <netmodel> syntax, but require old style -net nic magic to enable platform devices. Internally in libvirt we have a config whitelist for using that old style syntax, and it only covers a small number, basically arm32/arm64 with non-virtio nic: bool qemuDomainSupportsNicdev(virDomainDefPtr def, virDomainNetDefPtr net) { /* non-virtio ARM nics require legacy -net nic */ if (((def->os.arch == VIR_ARCH_ARMV6L) || (def->os.arch == VIR_ARCH_ARMV7L) || (def->os.arch == VIR_ARCH_AARCH64)) && net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false; return true; } So if we determine a tcg config requires -net nic, and it doesn't meet the above rules, it doesn't work with libvirt anyways.
<model type='idontexist'/>
qemu would turn that into
-device idontexist,...
That behavior is untouched by this series. Unwinding it will take some thought. For starters would be populating the enum with any qemu/xen network model that a user could realistically have a working config with. Then maybe tainting the VM if modelstr is used. Then after some time the final round could be causing domains to fail to start, but not fail to parse, so they don't disappear on upgrade but still break in an obvious way that will generate complaints/bugreports. But we should agree on a plan for it.
Yes, I think we should mark the guest as "tainted" as that ensures a log message in the per-QEMU log file. We should also ensure we use VIR_WARN in libvirtd log too i guess.
We'd want to keep this loose acceptance for at least a year, possibly two before we consider removing it.
Other caveats: * vz and bhyve drivers are not compile tested
* vmx and virtualbox drivers previously would do a case insensitive compare on the model value passed in via the user XML. Current patches don't preserve that. I don't know how much it matters for these drivers for reading fresh XML vs roundtrip XML from converted native formats (which _will_ correct the case issue). If we want to fix this we will need to tolower() the modelstr value in the XML parser. I think there's a gnulib function for it but I haven't explored it deeply
I guess if we're going to be serious about maintaining compat, we should accept the insensitive strings, at the very least for a transitional period.
Normally all our enums would be 100% lowercase, but in the vbox driver you're adding model names which are mixed-case, because that's what vbox would previously output. I think you are right to preserve that mixed case despite it not being our normal practice, because round-tripped XML is important.
I fear there there could well be people feeding in vbox XML that uses all-lowercase for these vbox models based on normal libvirt precedent though. This re-inforces the idea that we should allow a case-insensitive match when parsing, and perhaps log a warning if people use the non-preferred case. Perhaps after a year or two we could drop the case-insensitive match, but it would not be a huge burden to just carry it forever.
Okay I'll explore the tolower() option. I guess then that will imply that all enum strings are lowercase which means generated vbox XML will be different now, but that seems like a minor issue if XML input compat is maintained. - Cole
On Tue, Jan 22, 2019 at 10:52:25AM -0500, Cole Robinson wrote:
On 01/22/2019 06:55 AM, Daniel P. Berrangé wrote:
On Fri, Jan 18, 2019 at 06:05:18PM -0500, Cole Robinson wrote:
This series is base on my virtio-transitional work, since it touches a lot of the same code: https://www.redhat.com/archives/libvir-list/2019-January/msg00593.html
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
Xen is probably quite easy to deal with as it supports far fewer arches than the qemu driver. On x86 we need to handle the usual rtl8139, e100, ne2k, etc for Xen fully virt. I think for other Xen non-x86 arches, it might be reasonable to only care about net-front even for fully-virt given that people using Xen with hardware acceleration won't want a slow NIC.
Okay sounds good. I'll save that for a follow up series after doing some research/
QEMU is the really hard one as it has a huge set of arches and people using QEMU in TCG mode conceivably care about all the random NIC models no matter how slow & awful they are, because TCG is already slow & awful.
This is probably not as bad as you suggest. I believe most tcg qemu variants can't even handle -device <netmodel> syntax, but require old style -net nic magic to enable platform devices. Internally in libvirt we have a config whitelist for using that old style syntax, and it only covers a small number, basically arm32/arm64 with non-virtio nic:
bool qemuDomainSupportsNicdev(virDomainDefPtr def, virDomainNetDefPtr net) { /* non-virtio ARM nics require legacy -net nic */ if (((def->os.arch == VIR_ARCH_ARMV6L) || (def->os.arch == VIR_ARCH_ARMV7L) || (def->os.arch == VIR_ARCH_AARCH64)) && net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false;
return true; }
So if we determine a tcg config requires -net nic, and it doesn't meet the above rules, it doesn't work with libvirt anyways.
Oh that's a very good point - so we really just need to look at 'qemu-system-XXXX -device help' output for each XXXX target and look for which devices are NICs.
* vmx and virtualbox drivers previously would do a case insensitive compare on the model value passed in via the user XML. Current patches don't preserve that. I don't know how much it matters for these drivers for reading fresh XML vs roundtrip XML from converted native formats (which _will_ correct the case issue). If we want to fix this we will need to tolower() the modelstr value in the XML parser. I think there's a gnulib function for it but I haven't explored it deeply
I guess if we're going to be serious about maintaining compat, we should accept the insensitive strings, at the very least for a transitional period.
Normally all our enums would be 100% lowercase, but in the vbox driver you're adding model names which are mixed-case, because that's what vbox would previously output. I think you are right to preserve that mixed case despite it not being our normal practice, because round-tripped XML is important.
I fear there there could well be people feeding in vbox XML that uses all-lowercase for these vbox models based on normal libvirt precedent though. This re-inforces the idea that we should allow a case-insensitive match when parsing, and perhaps log a warning if people use the non-preferred case. Perhaps after a year or two we could drop the case-insensitive match, but it would not be a huge burden to just carry it forever.
Okay I'll explore the tolower() option. I guess then that will imply that all enum strings are lowercase which means generated vbox XML will be different now, but that seems like a minor issue if XML input compat is maintained.
We shouldn't need to change output. I was just thinking that you could do something like this when parsing to an enum: model = virDomainNetModelFromString(modelstr) if model < 0 lowermodelstr = tolower(modelstr) model = virDomainNetModelFromString(lowermodelstr) if model < 0 return -1; VIR_WARN("Model string '%s' uses unexpected case, pleae use '%s", lowermodelstr, virDomainNetModelToString(model)); 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 :|
participants (2)
-
Cole Robinson -
Daniel P. Berrangé