On Sun, Jan 19, 2020 at 10:24:13PM -0500, Laine Stump wrote: > This attribute is only used for virtio-net devices, so it is stored in > the virtio part of the anonymous union in virDomainNetDef::driver. An I'm not convinced that storing it only for virtio-net is the right approach. This feels like a generic concept, that just happens to only be implemented by virtio-net in QEMU today. IOW, it is valid to store it in the virDOmainNetDef in a place that's generally applicable. Any restriction to virtio-net belongs in the QEMU driver, rather than the XML parser. > example of the new option: > > <interface type='network'> > <source network='mybridge'/> > <mac address='00:11:22:33:44:55'/> > <model type='virtio'/> > <driver failover='on'/> > </interface> > > The corresponding qemu commandline option can only be set if the qemu > binary supports it, so we check for the qemu capability before adding > it. > > Signed-off-by: Laine Stump <laine@xxxxxxxxxx> > --- > docs/schemas/domaincommon.rng | 5 ++ > src/conf/domain_conf.c | 15 ++++++ > src/conf/domain_conf.h | 1 + > src/qemu/qemu_command.c | 3 ++ > src/qemu/qemu_domain.c | 12 ++++- > .../qemuxml2argvdata/net-virtio-failover.args | 36 +++++++++++++ > .../qemuxml2argvdata/net-virtio-failover.xml | 36 +++++++++++++ > tests/qemuxml2argvtest.c | 3 ++ > .../net-virtio-failover.xml | 50 +++++++++++++++++++ > tests/qemuxml2xmltest.c | 2 + > 10 files changed, 161 insertions(+), 2 deletions(-) > create mode 100644 tests/qemuxml2argvdata/net-virtio-failover.args > create mode 100644 tests/qemuxml2argvdata/net-virtio-failover.xml > create mode 100644 tests/qemuxml2xmloutdata/net-virtio-failover.xml > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 76d94b156f..80aea47e36 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3050,6 +3050,11 @@ > <optional> > <ref name="event_idx"/> > </optional> > + <optional> > + <attribute name="failover"> > + <ref name="virOnOff"/> > + </attribute> > + </optional> > </group> > </choice> > <ref name="virtioOptions"/> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 1379ae1600..29636617a2 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -11558,6 +11558,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > g_autofree char *txmode = NULL; > g_autofree char *ioeventfd = NULL; > g_autofree char *event_idx = NULL; > + g_autofree char *failover = NULL; > g_autofree char *queues = NULL; > g_autofree char *rx_queue_size = NULL; > g_autofree char *tx_queue_size = NULL; > @@ -11729,6 +11730,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > txmode = virXMLPropString(cur, "txmode"); > ioeventfd = virXMLPropString(cur, "ioeventfd"); > event_idx = virXMLPropString(cur, "event_idx"); > + failover = virXMLPropString(cur, "failover"); > queues = virXMLPropString(cur, "queues"); > rx_queue_size = virXMLPropString(cur, "rx_queue_size"); > tx_queue_size = virXMLPropString(cur, "tx_queue_size"); > @@ -12105,6 +12107,15 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > } > def->driver.virtio.event_idx = val; > } > + if (failover) { > + if ((val = virTristateSwitchTypeFromString(failover)) <= 0) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unknown interface failover setting '%s'"), > + failover); > + goto error; > + } > + def->driver.virtio.failover = val; > + } > if (queues) { > unsigned int q; > if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { > @@ -25416,6 +25427,10 @@ virDomainNetDriverAttributesFormat(char **outstr, > virBufferAsprintf(&buf, " event_idx='%s'", > virTristateSwitchTypeToString(def->driver.virtio.event_idx)); > } > + if (def->driver.virtio.failover) { > + virBufferAsprintf(&buf, " failover='%s'", > + virTristateSwitchTypeToString(def->driver.virtio.failover)); > + } > if (def->driver.virtio.queues) > virBufferAsprintf(&buf, " queues='%u'", def->driver.virtio.queues); > if (def->driver.virtio.rx_queue_size) > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 5a44113681..af9691d62b 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -934,6 +934,7 @@ struct _virDomainNetDef { > virDomainNetVirtioTxModeType txmode; > virTristateSwitch ioeventfd; > virTristateSwitch event_idx; > + virTristateSwitch failover; > unsigned int queues; /* Multiqueue virtio-net */ > unsigned int rx_queue_size; > unsigned int tx_queue_size; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 904d2beab5..d3c0cc0506 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -3834,6 +3834,9 @@ qemuBuildNicDevStr(virDomainDefPtr def, > virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu); > } > > + if (usingVirtio && net->driver.virtio.failover == VIR_TRISTATE_SWITCH_ON) > + virBufferAddLit(&buf, ",failover=on"); > + > virBufferAsprintf(&buf, ",netdev=host%s", net->info.alias); > virBufferAsprintf(&buf, ",id=%s", net->info.alias); > virBufferAsprintf(&buf, ",mac=%s", > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index a6dde15bad..6f45d74bde 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -6396,7 +6396,8 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, > > > static int > -qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net) > +qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net, > + virQEMUCapsPtr qemuCaps) > { > bool hasIPv4 = false; > bool hasIPv6 = false; > @@ -6471,6 +6472,13 @@ qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net) > _("tx_queue_size has to be a power of two")); > return -1; > } > + > + if (net->driver.virtio.failover == VIR_TRISTATE_SWITCH_ON && > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_FAILOVER)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("virtio-net failover is not supported with this QEMU binary")); > + return -1; > + } > } > > if (net->mtu && > @@ -8377,7 +8385,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, > > switch ((virDomainDeviceType)dev->type) { > case VIR_DOMAIN_DEVICE_NET: > - ret = qemuDomainDeviceDefValidateNetwork(dev->data.net); > + ret = qemuDomainDeviceDefValidateNetwork(dev->data.net, qemuCaps); > break; > > case VIR_DOMAIN_DEVICE_CHR: > diff --git a/tests/qemuxml2argvdata/net-virtio-failover.args b/tests/qemuxml2argvdata/net-virtio-failover.args > new file mode 100644 > index 0000000000..da41e19628 > --- /dev/null > +++ b/tests/qemuxml2argvdata/net-virtio-failover.args > @@ -0,0 +1,36 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-QEMUGuest1 \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ > +QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu-system-i386 \ > +-name QEMUGuest1 \ > +-S \ > +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ > +-m 214 \ > +-realtime mlock=off \ > +-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 \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ > +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ > +-netdev user,id=hostua-backup0 \ > +-device virtio-net-pci,failover=on,netdev=hostua-backup0,id=ua-backup0,\ > +mac=00:11:22:33:44:55,bus=pci.0,addr=0x3 \ > +-netdev user,id=hostua-backup1 \ > +-device virtio-net-pci,failover=on,netdev=hostua-backup1,id=ua-backup1,\ > +mac=66:44:33:22:11:00,bus=pci.0,addr=0x4 \ > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 > diff --git a/tests/qemuxml2argvdata/net-virtio-failover.xml b/tests/qemuxml2argvdata/net-virtio-failover.xml > new file mode 100644 > index 0000000000..1f545b8d73 > --- /dev/null > +++ b/tests/qemuxml2argvdata/net-virtio-failover.xml > @@ -0,0 +1,36 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + </disk> > + <controller type='usb' index='0'/> > + <interface type='user'> > + <mac address='00:11:22:33:44:55'/> > + <model type='virtio'/> > + <driver failover='on'/> > + <alias name='ua-backup0'/> > + </interface> > + <interface type='user'> > + <mac address='66:44:33:22:11:00'/> > + <model type='virtio'/> > + <driver failover='on'/> > + <alias name='ua-backup1'/> > + </interface> > + <memballoon model='virtio'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 58b4deefc6..c4fbe321d8 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -1308,6 +1308,9 @@ mymain(void) > QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, > QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE); > DO_TEST_PARSE_ERROR("net-virtio-rxqueuesize-invalid-size", NONE); > + DO_TEST("net-virtio-failover", > + QEMU_CAPS_VIRTIO_NET_FAILOVER); > + DO_TEST_PARSE_ERROR("net-virtio-failover", NONE); > DO_TEST("net-eth", NONE); > DO_TEST("net-eth-ifname", NONE); > DO_TEST("net-eth-names", NONE); > diff --git a/tests/qemuxml2xmloutdata/net-virtio-failover.xml b/tests/qemuxml2xmloutdata/net-virtio-failover.xml > new file mode 100644 > index 0000000000..7895c03dd7 > --- /dev/null > +++ b/tests/qemuxml2xmloutdata/net-virtio-failover.xml > @@ -0,0 +1,50 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='qemu' type='raw'/> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > + <controller type='usb' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> > + </controller> > + <controller type='pci' index='0' model='pci-root'/> > + <controller type='ide' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> > + </controller> > + <interface type='user'> > + <mac address='00:11:22:33:44:55'/> > + <model type='virtio'/> > + <driver failover='on'/> > + <alias name='ua-backup0'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> > + </interface> > + <interface type='user'> > + <mac address='66:44:33:22:11:00'/> > + <model type='virtio'/> > + <driver failover='on'/> > + <alias name='ua-backup1'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> > + </interface> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='virtio'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> > + </memballoon> > + </devices> > +</domain> > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 3cefc64833..326e49fbcd 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -451,6 +451,8 @@ mymain(void) > DO_TEST("net-eth-unmanaged-tap", NONE); > DO_TEST("net-virtio-network-portgroup", NONE); > DO_TEST("net-virtio-rxtxqueuesize", NONE); > + DO_TEST("net-virtio-failover", > + QEMU_CAPS_VIRTIO_NET_FAILOVER); > DO_TEST("net-hostdev", NONE); > DO_TEST("net-hostdev-bootorder", NONE); > DO_TEST("net-hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI); > -- > 2.24.1 > 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 :|