Sridhar Samudrala
2018-May-07 23:09 UTC
[PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
This feature bit can be used by hypervisor to indicate virtio_net device to act as a standby for another device with the same MAC address. I tested this with a small change to the patch to mark the STANDBY feature 'true' by default as i am using libvirt to start the VMs. Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt XML file? Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> --- hw/net/virtio-net.c | 2 ++ include/standard-headers/linux/virtio_net.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 90502fca7c..38b3140670 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { true), DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, + false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h index e9f255ea3f..01ec09684c 100644 --- a/include/standard-headers/linux/virtio_net.h +++ b/include/standard-headers/linux/virtio_net.h @@ -57,6 +57,9 @@ * Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device + * with the same MAC. + */ #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ #ifndef VIRTIO_NET_NO_LEGACY -- 2.14.3
Samudrala, Sridhar
2018-Jun-05 01:41 UTC
[PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
Ping on this patch now that the kernel patches are accepted into davem's net-next tree. https://patchwork.ozlabs.org/cover/920005/ On 5/7/2018 4:09 PM, Sridhar Samudrala wrote:> This feature bit can be used by hypervisor to indicate virtio_net device to > act as a standby for another device with the same MAC address. > > I tested this with a small change to the patch to mark the STANDBY feature 'true' > by default as i am using libvirt to start the VMs. > Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt > XML file? > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > --- > hw/net/virtio-net.c | 2 ++ > include/standard-headers/linux/virtio_net.h | 3 +++ > 2 files changed, 5 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 90502fca7c..38b3140670 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { > true), > DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), > DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), > + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, > + false), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h > index e9f255ea3f..01ec09684c 100644 > --- a/include/standard-headers/linux/virtio_net.h > +++ b/include/standard-headers/linux/virtio_net.h > @@ -57,6 +57,9 @@ > * Steering */ > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > + * with the same MAC. > + */ > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ > > #ifndef VIRTIO_NET_NO_LEGACY
Jason Wang
2018-Jun-05 02:06 UTC
[PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
On 2018?06?05? 09:41, Samudrala, Sridhar wrote:> Ping on this patch now that the kernel patches are accepted into > davem's net-next tree. > https://patchwork.ozlabs.org/cover/920005/ > > > On 5/7/2018 4:09 PM, Sridhar Samudrala wrote: >> This feature bit can be used by hypervisor to indicate virtio_net >> device to >> act as a standby for another device with the same MAC address. >> >> I tested this with a small change to the patch to mark the STANDBY >> feature 'true' >> by default as i am using libvirt to start the VMs. >> Is there a way to pass the newly added feature bit 'standby' to qemu >> via libvirt >> XML file? >>Maybe you can try qemu command line passthrough: https://libvirt.org/drvqemu.html#qemucommand The patch looks good to me. Let's see if Michael have any comment. Thanks>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> --- >> ? hw/net/virtio-net.c???????????????????????? | 2 ++ >> ? include/standard-headers/linux/virtio_net.h | 3 +++ >> ? 2 files changed, 5 insertions(+) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 90502fca7c..38b3140670 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { >> ?????????????????????? true), >> ????? DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, >> SPEED_UNKNOWN), >> ????? DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), >> +??? DEFINE_PROP_BIT64("standby", VirtIONet, host_features, >> VIRTIO_NET_F_STANDBY, >> +????????????????????? false), >> ????? DEFINE_PROP_END_OF_LIST(), >> ? }; >> ? diff --git a/include/standard-headers/linux/virtio_net.h >> b/include/standard-headers/linux/virtio_net.h >> index e9f255ea3f..01ec09684c 100644 >> --- a/include/standard-headers/linux/virtio_net.h >> +++ b/include/standard-headers/linux/virtio_net.h >> @@ -57,6 +57,9 @@ >> ?????????????????????? * Steering */ >> ? #define VIRTIO_NET_F_CTRL_MAC_ADDR 23??? /* Set MAC address */ >> ? +#define VIRTIO_NET_F_STANDBY????? 62??? /* Act as standby for >> another device >> +???????????????????????????????????????? * with the same MAC. >> +???????????????????????????????????????? */ >> ? #define VIRTIO_NET_F_SPEED_DUPLEX 63??? /* Device set linkspeed and >> duplex */ >> ? ? #ifndef VIRTIO_NET_NO_LEGACY >
Michael S. Tsirkin
2018-Jun-05 12:33 UTC
[PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
I don't think this is sufficient. If both primary and standby devices are present, a legacy guest without support for the feature might see two devices with same mac and get confused. I think that we should only make primary visible after guest acked the backup feature bit. And on reset or when backup is cleared in some other way, unplug the primary. Something like the below will do the job: Primary device is added with a special "primary-failover" flag. A virtual machine is then initialized with just a standby virtio device. Primary is not yet added. Later QEMU detects that guest driver device set DRIVER_OK. It then exposes the primary device to the guest, and triggers a device addition event (hot-plug event) for it. If QEMU detects guest driver removal, it initiates a hot-unplug sequence to remove the primary driver. In particular, if QEMU detects guest re-initialization (e.g. by detecting guest reset) it immediately removes the primary device. We can move some of this code to management as well, architecturally it does not make too much sense but it might be easier implementation-wise. HTH On Mon, Jun 04, 2018 at 06:41:48PM -0700, Samudrala, Sridhar wrote:> Ping on this patch now that the kernel patches are accepted into davem's net-next tree. > https://patchwork.ozlabs.org/cover/920005/ > > > On 5/7/2018 4:09 PM, Sridhar Samudrala wrote: > > This feature bit can be used by hypervisor to indicate virtio_net device to > > act as a standby for another device with the same MAC address. > > > > I tested this with a small change to the patch to mark the STANDBY feature 'true' > > by default as i am using libvirt to start the VMs. > > Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt > > XML file? > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > > --- > > hw/net/virtio-net.c | 2 ++ > > include/standard-headers/linux/virtio_net.h | 3 +++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index 90502fca7c..38b3140670 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { > > true), > > DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), > > DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), > > + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, > > + false), > > DEFINE_PROP_END_OF_LIST(), > > }; > > diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h > > index e9f255ea3f..01ec09684c 100644 > > --- a/include/standard-headers/linux/virtio_net.h > > +++ b/include/standard-headers/linux/virtio_net.h > > @@ -57,6 +57,9 @@ > > * Steering */ > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > > + * with the same MAC. > > + */ > > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ > > #ifndef VIRTIO_NET_NO_LEGACY
Michael S. Tsirkin
2018-Jun-11 17:26 UTC
[PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
On Mon, May 07, 2018 at 04:09:54PM -0700, Sridhar Samudrala wrote:> This feature bit can be used by hypervisor to indicate virtio_net device to > act as a standby for another device with the same MAC address. > > I tested this with a small change to the patch to mark the STANDBY feature 'true' > by default as i am using libvirt to start the VMs. > Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt > XML file? > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com>So I do not think we can commit to this interface: we really need to control visibility of the primary device. However just for testing purposes, we could add a non-stable interface "x-standby" with the understanding that as any x- prefix it's unstable and will be changed down the road, likely in the next release.> --- > hw/net/virtio-net.c | 2 ++ > include/standard-headers/linux/virtio_net.h | 3 +++ > 2 files changed, 5 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 90502fca7c..38b3140670 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { > true), > DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), > DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), > + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, > + false), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h > index e9f255ea3f..01ec09684c 100644 > --- a/include/standard-headers/linux/virtio_net.h > +++ b/include/standard-headers/linux/virtio_net.h > @@ -57,6 +57,9 @@ > * Steering */ > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > + * with the same MAC. > + */ > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ > > #ifndef VIRTIO_NET_NO_LEGACY > -- > 2.14.3
Jason Wang
2018-Jun-12 01:54 UTC
[Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
On 2018?06?12? 01:26, Michael S. Tsirkin wrote:> On Mon, May 07, 2018 at 04:09:54PM -0700, Sridhar Samudrala wrote: >> This feature bit can be used by hypervisor to indicate virtio_net device to >> act as a standby for another device with the same MAC address. >> >> I tested this with a small change to the patch to mark the STANDBY feature 'true' >> by default as i am using libvirt to start the VMs. >> Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt >> XML file? >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > So I do not think we can commit to this interface: we > really need to control visibility of the primary device.The problem is legacy guest won't use primary device at all if we do this. How about control the visibility of standby device? Thanks> However just for testing purposes, we could add a non-stable > interface "x-standby" with the understanding that as any > x- prefix it's unstable and will be changed down the road, > likely in the next release. > > >> --- >> hw/net/virtio-net.c | 2 ++ >> include/standard-headers/linux/virtio_net.h | 3 +++ >> 2 files changed, 5 insertions(+) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 90502fca7c..38b3140670 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { >> true), >> DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), >> DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), >> + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, >> + false), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h >> index e9f255ea3f..01ec09684c 100644 >> --- a/include/standard-headers/linux/virtio_net.h >> +++ b/include/standard-headers/linux/virtio_net.h >> @@ -57,6 +57,9 @@ >> * Steering */ >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ >> >> +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device >> + * with the same MAC. >> + */ >> #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ >> >> #ifndef VIRTIO_NET_NO_LEGACY >> -- >> 2.14.3
Reasonably Related Threads
- [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
- [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
- [Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
- [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
- [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net