For <interface type='hostdev'> the <driver> subelement (including the
backupAlias attribute) is parsed directly into the hostdev child
object (virDomaniHostdevDef) of the interface (using
virDomainHostdevDefParseXMLSubsys()). But for <interface
type='network'> where the network is a pool of hostdevs, the hostdev
object doesn't exist until the network port is allocated at runtime,
and so virDomainHostdevDefParseXMLSubsys() can't be called during XML
parsing, and any backupAlias in the driver subelement of the XML will
be lost.
For this case, we need to add a backupAlias member to the interface
object (virDomainNetDef), and parse it during the <driver> parsing
that happens for all non-hostdev interfaces. Then when the network
port is allocated at runtime and the hostdev child object is created,
we can copy the backupAlias into the hostdev so it is available when
building the QEMU commandline.
An example usage:
<interface type='network'>
<source network='mybridge'/>
<mac address='00:11:22:33:44:55'/>
<model type='virtio'/>
<alias name='ua-backup0'/>
<driver failover='on'/>
</interface>
<interface type='network'>
<source network='hostdev-pool'/>
<mac address='00:11:22:33:44:55'/>
<model type='virtio'/>
<driver backupAlias='ua-backup0'/>
</interface>
Signed-off-by: Laine Stump <laine@redhat.com>
---
src/conf/domain_conf.c | 11 ++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_domain.c | 15 +++---
.../net-virtio-failover-network.xml | 37 ++++++++++++++
.../net-virtio-failover-network.xml | 51 +++++++++++++++++++
tests/qemuxml2xmltest.c | 3 ++
6 files changed, 112 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/net-virtio-failover-network.xml
create mode 100644 tests/qemuxml2xmloutdata/net-virtio-failover-network.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e0e47415ed..89cccd22bc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2428,6 +2428,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
break;
}
+ VIR_FREE(def->driver.backupAlias);
VIR_FREE(def->backend.tap);
VIR_FREE(def->backend.vhost);
VIR_FREE(def->virtPortProfile);
@@ -11563,6 +11564,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *localaddr = NULL;
g_autofree char *localport = NULL;
g_autofree char *model = NULL;
+ g_autofree char *backupAlias = NULL;
g_autofree char *backend = NULL;
g_autofree char *txmode = NULL;
g_autofree char *ioeventfd = NULL;
@@ -11735,6 +11737,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (virXMLNodeNameEqual(cur, "model")) {
model = virXMLPropString(cur, "type");
} else if (virXMLNodeNameEqual(cur, "driver")) {
+ backupAlias = virXMLPropString(cur, "backupAlias");
backend = virXMLPropString(cur, "name");
txmode = virXMLPropString(cur, "txmode");
ioeventfd = virXMLPropString(cur, "ioeventfd");
@@ -12075,6 +12078,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
* the <driver> subelement of <interface type='hostdev'> has
* already been parsed by virDomainHostdevDefParseXMLSubsys()
*/
+
+ if (backupAlias)
+ def->driver.backupAlias = g_steal_pointer(&backupAlias);
+
if (virDomainNetIsVirtioModel(def)) {
if (backend != NULL) {
if ((val = virDomainNetBackendTypeFromString(backend)) < 0 ||
@@ -25455,6 +25462,8 @@ virDomainNetDriverAttributesFormat(char **outstr,
virDomainVirtioOptionsFormat(&buf, def->virtio);
}
+ virBufferEscapeString(&buf, " backupAlias='%s'", def->driver.backupAlias);
+
*outstr = virBufferContentAndReset(&buf);
}
@@ -30675,6 +30684,8 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface,
}
actual->data.hostdev.def.source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
actual->data.hostdev.def.source.subsys.u.pci.addr = port->plug.hostdevpci.addr;
+ actual->data.hostdev.def.source.subsys.u.pci.backupAlias
+ = g_strdup(iface->driver.backupAlias);
switch ((virNetworkForwardDriverNameType)port->plug.hostdevpci.driver) {
case VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT:
actual->data.hostdev.def.source.subsys.u.pci.backend =
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0bf1d2b6bf..2b6d9bab75 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -929,6 +929,7 @@ struct _virDomainNetDef {
int model; /* virDomainNetModelType */
char *modelstr;
struct {
+ char *backupAlias; /* alias id of backup virtio device for failover */
union {
struct {
virDomainNetBackendType name; /* which driver backend to use */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d32a3c0935..b8acbc6248 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6472,13 +6472,16 @@ 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->driver.backupAlias ||
+ (virDomainNetIsVirtioModel(net) &&
+ 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 &&
diff --git a/tests/qemuxml2argvdata/net-virtio-failover-network.xml b/tests/qemuxml2argvdata/net-virtio-failover-network.xml
new file mode 100644
index 0000000000..e2674ed57d
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-virtio-failover-network.xml
@@ -0,0 +1,37 @@
+<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='network'>
+ <mac address='00:11:22:33:44:55'/>
+ <source network='mybridge'/>
+ <model type='virtio'/>
+ <driver failover='on'/>
+ <alias name='ua-backup0'/>
+ </interface>
+ <interface type='network'>
+ <mac address='00:11:22:33:44:55'/>
+ <source network='myhostdevpool'/>
+ <model type='virtio'/>
+ <driver backupAlias='ua-backup0'/>
+ </interface>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/net-virtio-failover-network.xml b/tests/qemuxml2xmloutdata/net-virtio-failover-network.xml
new file mode 100644
index 0000000000..b803de1da2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/net-virtio-failover-network.xml
@@ -0,0 +1,51 @@
+<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='network'>
+ <mac address='00:11:22:33:44:55'/>
+ <source network='mybridge'/>
+ <model type='virtio'/>
+ <driver failover='on'/>
+ <alias name='ua-backup0'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ </interface>
+ <interface type='network'>
+ <mac address='00:11:22:33:44:55'/>
+ <source network='myhostdevpool'/>
+ <model type='virtio'/>
+ <driver backupAlias='ua-backup0'/>
+ <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 c91690a030..45ba4db30c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -454,6 +454,9 @@ mymain(void)
DO_TEST("net-virtio-failover",
QEMU_CAPS_VIRTIO_NET_FAILOVER,
QEMU_CAPS_DEVICE_VFIO_PCI);
+ DO_TEST("net-virtio-failover-network",
+ QEMU_CAPS_VIRTIO_NET_FAILOVER,
+ QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST("net-hostdev", NONE);
DO_TEST("net-hostdev-bootorder", NONE);
DO_TEST("net-hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI);
--
2.24.1
On Sun, Jan 19, 2020 at 10:24:15PM -0500, Laine Stump wrote:
> For <interface type='hostdev'> the <driver> subelement (including the
> backupAlias attribute) is parsed directly into the hostdev child
> object (virDomaniHostdevDef) of the interface (using
> virDomainHostdevDefParseXMLSubsys()). But for <interface
> type='network'> where the network is a pool of hostdevs, the hostdev
> object doesn't exist until the network port is allocated at runtime,
> and so virDomainHostdevDefParseXMLSubsys() can't be called during XML
> parsing, and any backupAlias in the driver subelement of the XML will
> be lost.
I'm thinking this is a sign that we're storing the data in the
wrong way / structure internally, and possibly with the XML
schema too. Overall I'm not a fan of having the data scattered
across three different structs for different scenarios....
> For this case, we need to add a backupAlias member to the interface
> object (virDomainNetDef), and parse it during the <driver> parsing
> that happens for all non-hostdev interfaces. Then when the network
> port is allocated at runtime and the hostdev child object is created,
> we can copy the backupAlias into the hostdev so it is available when
> building the QEMU commandline.
>
> An example usage:
>
> <interface type='network'>
> <source network='mybridge'/>
> <mac address='00:11:22:33:44:55'/>
> <model type='virtio'/>
> <alias name='ua-backup0'/>
> <driver failover='on'/>
> </interface>
> <interface type='network'>
> <source network='hostdev-pool'/>
> <mac address='00:11:22:33:44:55'/>
> <model type='virtio'/>
> <driver backupAlias='ua-backup0'/>
> </interface>
I get that a big part of the problem is that the <driver> element
is parsed by different bits of code depending on the type of
interface being use, so it is hard to get consistent handling
of this.
We could perhaps better deal with this by not using <driver> at all
and instead have a new <failover> element we store in virDomainNetDef
separately from the driver struct that's causing us trouble.
eg
<interface type='network'>
<source network='mybridge'/>
<mac address='00:11:22:33:44:55'/>
<model type='virtio'/>
<alias name='ua-backup0'/>
<failover type="primary"/>
</interface>
<interface type='network'>
<source network='hostdev-pool'/>
<mac address='00:11:22:33:44:55'/>
<model type='virtio'/>
<failover type="secondary" primary="ua-backup0"/>
</interface>
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 :|
© 2016 - 2026 Red Hat, Inc.