| Message ID | 20191018202040.30349-3-jfreimann@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add failover feature for assigned network devices | expand |
On Fri, 18 Oct 2019 22:20:31 +0200 Jens Freimann <jfreimann@redhat.com> wrote: > This patch adds a net_failover_pair_id property to PCIDev which is > used to link the primary device in a failover pair (the PCI dev) to > a standby (a virtio-net-pci) device. > > It only supports ethernet devices. Also currently it only supports > PCIe devices. QEMU will exit with an error message otherwise. Doesn't the PCIe device also need to be hotpluggable? We can have PCIe devices attached to the root bus where we don't currently support hotplug. Thanks, Alex > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > --- > hw/pci/pci.c | 17 +++++++++++++++++ > include/hw/pci/pci.h | 3 +++ > 2 files changed, 20 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index aa05c2b9b2..fa9b5219f8 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -75,6 +75,8 @@ static Property pci_props[] = { > QEMU_PCIE_LNKSTA_DLLLA_BITNR, true), > DEFINE_PROP_BIT("x-pcie-extcap-init", PCIDevice, cap_present, > QEMU_PCIE_EXTCAP_INIT_BITNR, true), > + DEFINE_PROP_STRING("net_failover_pair_id", PCIDevice, > + net_failover_pair_id), > DEFINE_PROP_END_OF_LIST() > }; > > @@ -2077,6 +2079,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) > ObjectClass *klass = OBJECT_CLASS(pc); > Error *local_err = NULL; > bool is_default_rom; > + uint16_t class_id; > > /* initialize cap_present for pci_is_express() and pci_config_size(), > * Note that hybrid PCIs are not set automatically and need to manage > @@ -2101,6 +2104,20 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) > } > } > > + if (pci_dev->net_failover_pair_id) { > + if (!pci_is_express(pci_dev)) { > + error_setg(errp, "failover device is not a PCIExpress device"); > + error_propagate(errp, local_err); > + return; > + } > + class_id = pci_get_word(pci_dev->config + PCI_CLASS_DEVICE); > + if (class_id != PCI_CLASS_NETWORK_ETHERNET) { > + error_setg(errp, "failover device is not an Ethernet device"); > + error_propagate(errp, local_err); > + return; > + } > + } > + > /* rom loading */ > is_default_rom = false; > if (pci_dev->romfile == NULL && pc->romfile != NULL) { > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index f3f0ffd5fb..def5435685 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -352,6 +352,9 @@ struct PCIDevice { > MSIVectorUseNotifier msix_vector_use_notifier; > MSIVectorReleaseNotifier msix_vector_release_notifier; > MSIVectorPollNotifier msix_vector_poll_notifier; > + > + /* ID of standby device in net_failover pair */ > + char *net_failover_pair_id; > }; > > void pci_register_bar(PCIDevice *pci_dev, int region_num,
On Mon, Oct 21, 2019 at 08:58:23AM -0600, Alex Williamson wrote: >On Fri, 18 Oct 2019 22:20:31 +0200 >Jens Freimann <jfreimann@redhat.com> wrote: > >> This patch adds a net_failover_pair_id property to PCIDev which is >> used to link the primary device in a failover pair (the PCI dev) to >> a standby (a virtio-net-pci) device. >> >> It only supports ethernet devices. Also currently it only supports >> PCIe devices. QEMU will exit with an error message otherwise. > >Doesn't the PCIe device also need to be hotpluggable? We can have PCIe >devices attached to the root bus where we don't currently support >hotplug. Thanks, How do I recognize such a device? hotpluggable in DeviceClass? regards, Jens
On Mon, 21 Oct 2019 20:45:46 +0200 Jens Freimann <jfreimann@redhat.com> wrote: > On Mon, Oct 21, 2019 at 08:58:23AM -0600, Alex Williamson wrote: > >On Fri, 18 Oct 2019 22:20:31 +0200 > >Jens Freimann <jfreimann@redhat.com> wrote: > > > >> This patch adds a net_failover_pair_id property to PCIDev which is > >> used to link the primary device in a failover pair (the PCI dev) to > >> a standby (a virtio-net-pci) device. > >> > >> It only supports ethernet devices. Also currently it only supports > >> PCIe devices. QEMU will exit with an error message otherwise. > > > >Doesn't the PCIe device also need to be hotpluggable? We can have PCIe > >devices attached to the root bus where we don't currently support > >hotplug. Thanks, > > How do I recognize such a device? hotpluggable in DeviceClass? I wouldn't expect it to be a device property, it's more of a function of whether the bus that it's attached to supports hotplug, so probably something related to the bus controller. Thanks, Alex
On Mon, Oct 21, 2019 at 01:01:22PM -0600, Alex Williamson wrote: >On Mon, 21 Oct 2019 20:45:46 +0200 >Jens Freimann <jfreimann@redhat.com> wrote: > >> On Mon, Oct 21, 2019 at 08:58:23AM -0600, Alex Williamson wrote: >> >On Fri, 18 Oct 2019 22:20:31 +0200 >> >Jens Freimann <jfreimann@redhat.com> wrote: >> > >> >> This patch adds a net_failover_pair_id property to PCIDev which is >> >> used to link the primary device in a failover pair (the PCI dev) to >> >> a standby (a virtio-net-pci) device. >> >> >> >> It only supports ethernet devices. Also currently it only supports >> >> PCIe devices. QEMU will exit with an error message otherwise. >> > >> >Doesn't the PCIe device also need to be hotpluggable? We can have PCIe >> >devices attached to the root bus where we don't currently support >> >hotplug. Thanks, >> >> How do I recognize such a device? hotpluggable in DeviceClass? > >I wouldn't expect it to be a device property, it's more of a function >of whether the bus that it's attached to supports hotplug, so probably >something related to the bus controller. Thanks, IIUC this is checked for in qdev_device_add() by this: if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); return NULL; } So qemu will fail with 'Bus pcie.0 does not allow hotplug' when we try to hotplug the device. I tried a primary with bus=pcie.0 and it aborted. Aborting qemu is not nice so I'll make it print a error message QERR_BUS_NO_HOTPLUG instead but let it continue. This will be a change in the virtio-net patch, not in this one. regards, Jens
On Mon, 21 Oct 2019 22:28:57 +0200 Jens Freimann <jfreimann@redhat.com> wrote: > On Mon, Oct 21, 2019 at 01:01:22PM -0600, Alex Williamson wrote: > >On Mon, 21 Oct 2019 20:45:46 +0200 > >Jens Freimann <jfreimann@redhat.com> wrote: > > > >> On Mon, Oct 21, 2019 at 08:58:23AM -0600, Alex Williamson wrote: > >> >On Fri, 18 Oct 2019 22:20:31 +0200 > >> >Jens Freimann <jfreimann@redhat.com> wrote: > >> > > >> >> This patch adds a net_failover_pair_id property to PCIDev which is > >> >> used to link the primary device in a failover pair (the PCI dev) to > >> >> a standby (a virtio-net-pci) device. > >> >> > >> >> It only supports ethernet devices. Also currently it only supports > >> >> PCIe devices. QEMU will exit with an error message otherwise. > >> > > >> >Doesn't the PCIe device also need to be hotpluggable? We can have PCIe > >> >devices attached to the root bus where we don't currently support > >> >hotplug. Thanks, > >> > >> How do I recognize such a device? hotpluggable in DeviceClass? > > > >I wouldn't expect it to be a device property, it's more of a function > >of whether the bus that it's attached to supports hotplug, so probably > >something related to the bus controller. Thanks, > > IIUC this is checked for in qdev_device_add() by this: > > if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { > error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); > return NULL; > } > > So qemu will fail with 'Bus pcie.0 does not allow hotplug' when we try > to hotplug the device. I tried a primary with bus=pcie.0 and it aborted. > Aborting qemu is not nice so I'll make it print a error message > QERR_BUS_NO_HOTPLUG instead but let it continue. This will be > a change in the virtio-net patch, not in this one. qdev_hotplug is only set to true after qdev_machine_creation_done(), so if we start a VM with cold-plugged primary and failover, wouldn't the user only learn the configuration is invalid at the time they try to use it? I agree that the above should prevent a device from being hot-added to the bus, but I don't think that's our starting position, is it? Thanks, Alex
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index aa05c2b9b2..fa9b5219f8 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -75,6 +75,8 @@ static Property pci_props[] = { QEMU_PCIE_LNKSTA_DLLLA_BITNR, true), DEFINE_PROP_BIT("x-pcie-extcap-init", PCIDevice, cap_present, QEMU_PCIE_EXTCAP_INIT_BITNR, true), + DEFINE_PROP_STRING("net_failover_pair_id", PCIDevice, + net_failover_pair_id), DEFINE_PROP_END_OF_LIST() }; @@ -2077,6 +2079,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) ObjectClass *klass = OBJECT_CLASS(pc); Error *local_err = NULL; bool is_default_rom; + uint16_t class_id; /* initialize cap_present for pci_is_express() and pci_config_size(), * Note that hybrid PCIs are not set automatically and need to manage @@ -2101,6 +2104,20 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) } } + if (pci_dev->net_failover_pair_id) { + if (!pci_is_express(pci_dev)) { + error_setg(errp, "failover device is not a PCIExpress device"); + error_propagate(errp, local_err); + return; + } + class_id = pci_get_word(pci_dev->config + PCI_CLASS_DEVICE); + if (class_id != PCI_CLASS_NETWORK_ETHERNET) { + error_setg(errp, "failover device is not an Ethernet device"); + error_propagate(errp, local_err); + return; + } + } + /* rom loading */ is_default_rom = false; if (pci_dev->romfile == NULL && pc->romfile != NULL) { diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index f3f0ffd5fb..def5435685 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -352,6 +352,9 @@ struct PCIDevice { MSIVectorUseNotifier msix_vector_use_notifier; MSIVectorReleaseNotifier msix_vector_release_notifier; MSIVectorPollNotifier msix_vector_poll_notifier; + + /* ID of standby device in net_failover pair */ + char *net_failover_pair_id; }; void pci_register_bar(PCIDevice *pci_dev, int region_num,
This patch adds a net_failover_pair_id property to PCIDev which is used to link the primary device in a failover pair (the PCI dev) to a standby (a virtio-net-pci) device. It only supports ethernet devices. Also currently it only supports PCIe devices. QEMU will exit with an error message otherwise. Signed-off-by: Jens Freimann <jfreimann@redhat.com> --- hw/pci/pci.c | 17 +++++++++++++++++ include/hw/pci/pci.h | 3 +++ 2 files changed, 20 insertions(+)