diff mbox series

[v6,06/11] qapi: add failover negotiated event

Message ID 20191025121930.6855-7-jfreimann@redhat.com
State New
Headers show
Series add failover feature for assigned network devices | expand

Commit Message

Jens Freimann Oct. 25, 2019, 12:19 p.m. UTC 
This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
was enabled. The primary device this virtio-net device is associated
with, will now be hotplugged via qdev_device_add().

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
---
 qapi/net.json | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Markus Armbruster Oct. 25, 2019, 2:03 p.m. UTC | #1 
Bear with me, I know next to nothing about failover.

Jens Freimann <jfreimann@redhat.com> writes:

> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
> was enabled. The primary device this virtio-net device is associated
> with, will now be hotplugged via qdev_device_add().

Passive voice deftly avoids telling the reader who will do the
hot-plugging.  Intentional?

> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  qapi/net.json | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/qapi/net.json b/qapi/net.json
> index 728990f4fb..ea6eeee4f7 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -737,3 +737,22 @@
>  ##
>  { 'command': 'announce-self', 'boxed': true,
>    'data' : 'AnnounceParameters'}
> +
> +##
> +# @FAILOVER_NEGOTIATED:
> +#
> +# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
> +# Failover primary devices which were hidden (not hotplugged when requested)
> +# before will now be hotplugged by the virtio-net standby device.
> +#
> +# device-id: QEMU device id of the unplugged device

@device-id is new since v5.

A quick skim of
https://www.kernel.org/doc/html/latest/networking/net_failover.html
tells me there are three devices involved: master, primary slave,
standby slave.  Which one is @device-id?  Or am I confused?

> +# Since: 4.2
> +#
> +# Example:
> +#
> +# <- { "event": "FAILOVER_NEGOTIATED",
> +#      "data": "net1" }
> +#
> +##
> +{ 'event': 'FAILOVER_NEGOTIATED',
> +  'data': {'device-id': 'str'} }
Jens Freimann Oct. 25, 2019, 5:39 p.m. UTC | #2 
On Fri, Oct 25, 2019 at 04:03:54PM +0200, Markus Armbruster wrote:
>Bear with me, I know next to nothing about failover.
>
>Jens Freimann <jfreimann@redhat.com> writes:
>
>> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
>> was enabled. The primary device this virtio-net device is associated
>> with, will now be hotplugged via qdev_device_add().
>
>Passive voice deftly avoids telling the reader who will do the
>hot-plugging.  Intentional?

Not really, it's in the comment to the event. The hotplug will be
 done by the virtio-net device code that activates the feature, in
virtio_net_set_features().

>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  qapi/net.json | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/qapi/net.json b/qapi/net.json
>> index 728990f4fb..ea6eeee4f7 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -737,3 +737,22 @@
>>  ##
>>  { 'command': 'announce-self', 'boxed': true,
>>    'data' : 'AnnounceParameters'}
>> +
>> +##
>> +# @FAILOVER_NEGOTIATED:
>> +#
>> +# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
>> +# Failover primary devices which were hidden (not hotplugged when requested)
>> +# before will now be hotplugged by the virtio-net standby device.
>> +#
>> +# device-id: QEMU device id of the unplugged device
>
>@device-id is new since v5.
>
>A quick skim of
>https://www.kernel.org/doc/html/latest/networking/net_failover.html
>tells me there are three devices involved: master, primary slave,
>standby slave.  Which one is @device-id?  Or am I confused?

Yes, the device-id is new and it's the device-id of the standby (i.e.
virtio-net) device.

regards,
Jens
Markus Armbruster Oct. 29, 2019, 12:13 p.m. UTC | #3 
Jens Freimann <jfreimann@redhat.com> writes:

> On Fri, Oct 25, 2019 at 04:03:54PM +0200, Markus Armbruster wrote:
>>Bear with me, I know next to nothing about failover.
>>
>>Jens Freimann <jfreimann@redhat.com> writes:
>>
>>> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
>>> was enabled. The primary device this virtio-net device is associated
>>> with, will now be hotplugged via qdev_device_add().
>>
>>Passive voice deftly avoids telling the reader who will do the
>>hot-plugging.  Intentional?
>
> Not really, it's in the comment to the event. The hotplug will be
> done by the virtio-net device code that activates the feature, in
> virtio_net_set_features().

If you need to respin, I suggest to work this bit of information into
the commit message.

>>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>  qapi/net.json | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/qapi/net.json b/qapi/net.json
>>> index 728990f4fb..ea6eeee4f7 100644
>>> --- a/qapi/net.json
>>> +++ b/qapi/net.json
>>> @@ -737,3 +737,22 @@
>>>  ##
>>>  { 'command': 'announce-self', 'boxed': true,
>>>    'data' : 'AnnounceParameters'}
>>> +
>>> +##
>>> +# @FAILOVER_NEGOTIATED:
>>> +#
>>> +# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
>>> +# Failover primary devices which were hidden (not hotplugged when requested)
>>> +# before will now be hotplugged by the virtio-net standby device.
>>> +#
>>> +# device-id: QEMU device id of the unplugged device
>>
>>@device-id is new since v5.
>>
>>A quick skim of
>>https://www.kernel.org/doc/html/latest/networking/net_failover.html
>>tells me there are three devices involved: master, primary slave,
>>standby slave.  Which one is @device-id?  Or am I confused?
>
> Yes, the device-id is new and it's the device-id of the standby (i.e.
> virtio-net) device.

Would working this into the doc comment make it clearer?
Michael S. Tsirkin Oct. 29, 2019, 10:41 p.m. UTC | #4 
On Fri, Oct 25, 2019 at 07:39:21PM +0200, Jens Freimann wrote:
> On Fri, Oct 25, 2019 at 04:03:54PM +0200, Markus Armbruster wrote:
> > Bear with me, I know next to nothing about failover.
> > 
> > Jens Freimann <jfreimann@redhat.com> writes:
> > 
> > > This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
> > > was enabled. The primary device this virtio-net device is associated
> > > with, will now be hotplugged via qdev_device_add().
> > 
> > Passive voice deftly avoids telling the reader who will do the
> > hot-plugging.  Intentional?
> 
> Not really, it's in the comment to the event. The hotplug will be
> done by the virtio-net device code that activates the feature, in
> virtio_net_set_features().
> 
> > 
> > > Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> > > Acked-by: Cornelia Huck <cohuck@redhat.com>
> > > ---
> > >  qapi/net.json | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/qapi/net.json b/qapi/net.json
> > > index 728990f4fb..ea6eeee4f7 100644
> > > --- a/qapi/net.json
> > > +++ b/qapi/net.json
> > > @@ -737,3 +737,22 @@
> > >  ##
> > >  { 'command': 'announce-self', 'boxed': true,
> > >    'data' : 'AnnounceParameters'}
> > > +
> > > +##
> > > +# @FAILOVER_NEGOTIATED:
> > > +#
> > > +# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
> > > +# Failover primary devices which were hidden (not hotplugged when requested)
> > > +# before will now be hotplugged by the virtio-net standby device.
> > > +#
> > > +# device-id: QEMU device id of the unplugged device
> > 
> > @device-id is new since v5.
> > 
> > A quick skim of
> > https://www.kernel.org/doc/html/latest/networking/net_failover.html
> > tells me there are three devices involved: master, primary slave,
> > standby slave.  Which one is @device-id?  Or am I confused?
> 
> Yes, the device-id is new and it's the device-id of the standby (i.e.
> virtio-net) device.
> 
> regards,
> Jens

And now I am confused.
How is standby "the unplugged device"?
Why not just say "the standby device"?
Jens Freimann Oct. 30, 2019, 10:09 a.m. UTC | #5 
On Tue, Oct 29, 2019 at 06:41:58PM -0400, Michael S. Tsirkin wrote:
>On Fri, Oct 25, 2019 at 07:39:21PM +0200, Jens Freimann wrote:
>> On Fri, Oct 25, 2019 at 04:03:54PM +0200, Markus Armbruster wrote:
>> > Bear with me, I know next to nothing about failover.
>> >
>> > Jens Freimann <jfreimann@redhat.com> writes:
>> >
>> > > This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY feature
>> > > was enabled. The primary device this virtio-net device is associated
>> > > with, will now be hotplugged via qdev_device_add().
>> >
>> > Passive voice deftly avoids telling the reader who will do the
>> > hot-plugging.  Intentional?
>>
>> Not really, it's in the comment to the event. The hotplug will be
>> done by the virtio-net device code that activates the feature, in
>> virtio_net_set_features().
>>
>> >
>> > > Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>> > > Acked-by: Cornelia Huck <cohuck@redhat.com>
>> > > ---
>> > >  qapi/net.json | 19 +++++++++++++++++++
>> > >  1 file changed, 19 insertions(+)
>> > >
>> > > diff --git a/qapi/net.json b/qapi/net.json
>> > > index 728990f4fb..ea6eeee4f7 100644
>> > > --- a/qapi/net.json
>> > > +++ b/qapi/net.json
>> > > @@ -737,3 +737,22 @@
>> > >  ##
>> > >  { 'command': 'announce-self', 'boxed': true,
>> > >    'data' : 'AnnounceParameters'}
>> > > +
>> > > +##
>> > > +# @FAILOVER_NEGOTIATED:
>> > > +#
>> > > +# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
>> > > +# Failover primary devices which were hidden (not hotplugged when requested)
>> > > +# before will now be hotplugged by the virtio-net standby device.
>> > > +#
>> > > +# device-id: QEMU device id of the unplugged device
>> >
>> > @device-id is new since v5.
>> >
>> > A quick skim of
>> > https://www.kernel.org/doc/html/latest/networking/net_failover.html
>> > tells me there are three devices involved: master, primary slave,
>> > standby slave.  Which one is @device-id?  Or am I confused?
>>
>> Yes, the device-id is new and it's the device-id of the standby (i.e.
>> virtio-net) device.
>>
>> regards,
>> Jens
>
>And now I am confused.
>How is standby "the unplugged device"?
>Why not just say "the standby device"?

Yes, that's better. Do you want a patch on top?

diff --git a/qapi/net.json b/qapi/net.json
index ea6eeee4f7..0f225cb900 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -745,7 +745,7 @@
 # Failover primary devices which were hidden (not hotplugged when requested)
 # before will now be hotplugged by the virtio-net standby device.
 #
-# device-id: QEMU device id of the unplugged device
+# device-id: QEMU device id of the standby device
 # Since: 4.2
 #
 # Example:


Signed-off-by: Jens Freimann <jfreimann@redhat.com>

regards,
Jens 

>
>-- 
>MST
diff mbox series

Patch

diff --git a/qapi/net.json b/qapi/net.json
index 728990f4fb..ea6eeee4f7 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -737,3 +737,22 @@ 
 ##
 { 'command': 'announce-self', 'boxed': true,
   'data' : 'AnnounceParameters'}
+
+##
+# @FAILOVER_NEGOTIATED:
+#
+# Emitted when VIRTIO_NET_F_STANDBY was enabled during feature negotiation.
+# Failover primary devices which were hidden (not hotplugged when requested)
+# before will now be hotplugged by the virtio-net standby device.
+#
+# device-id: QEMU device id of the unplugged device
+# Since: 4.2
+#
+# Example:
+#
+# <- { "event": "FAILOVER_NEGOTIATED",
+#      "data": "net1" }
+#
+##
+{ 'event': 'FAILOVER_NEGOTIATED',
+  'data': {'device-id': 'str'} }