[v5,06/11] qapi: add failover negotiated event
diff mbox series

Message ID 20191023082711.16694-7-jfreimann@redhat.com
State New
Headers show
Series
  • add failover feature for assigned network devices
Related show

Commit Message

Jens Freimann Oct. 23, 2019, 8:27 a.m. UTC
This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
feature was not negotiated during virtio feature negotiation. If this
event is received it means any primary devices hotplugged before
this were were never really added to QEMU devices.

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

Comments

Dr. David Alan Gilbert Oct. 24, 2019, 5:32 p.m. UTC | #1
* Jens Freimann (jfreimann@redhat.com) wrote:
> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
> feature was not negotiated during virtio feature negotiation. If this
> event is received it means any primary devices hotplugged before
> this were were never really added to QEMU devices.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>

Can I just understand a bit more about what the meaning of this is.

Say my VM boots:
   a) BIOS
   b) Boot loader
   c) Linux
   d) Reboots
      (possibly a',b', different c')

When would I get that event?
When can libvirt know it can use it?

Dave

> ---
>  qapi/net.json | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/qapi/net.json b/qapi/net.json
> index 728990f4fb..8c5f3f1fb2 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -737,3 +737,19 @@
>  ##
>  { 'command': 'announce-self', 'boxed': true,
>    'data' : 'AnnounceParameters'}
> +
> +##
> +# @FAILOVER_NEGOTIATED:
> +#
> +# Emitted when VIRTIO_NET_F_STANDBY was negotiated during feature negotiation
> +#
> +# Since: 4.2
> +#
> +# Example:
> +#
> +# <- { "event": "FAILOVER_NEGOTIATED",
> +#      "data": {} }
> +#
> +##
> +{ 'event': 'FAILOVER_NEGOTIATED',
> +  'data': {} }
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Jens Freimann Oct. 24, 2019, 8:03 p.m. UTC | #2
On Thu, Oct 24, 2019 at 06:32:45PM +0100, Dr. David Alan Gilbert wrote:
>* Jens Freimann (jfreimann@redhat.com) wrote:
>> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
>> feature was not negotiated during virtio feature negotiation. If this
>> event is received it means any primary devices hotplugged before
>> this were were never really added to QEMU devices.
>>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>
>Can I just understand a bit more about what the meaning of this is.
>
>Say my VM boots:
>   a) BIOS
>   b) Boot loader
>   c) Linux
>   d) Reboots
>      (possibly a',b', different c')
>
>When would I get that event?
>When can libvirt know it can use it?

The event is sent every time we do feature negotiation for the virtio-net
device, so you'll get it during Linux boot and reboots.

In v6, I add a data field 'id' to the event to pass the device id. 

regards,
Jens
Markus Armbruster Oct. 25, 2019, 5:35 a.m. UTC | #3
We ask patch submitters to cc: subject matter experts for review.  You
did.  When such patches touch the QAPI schema, it's best to cc the qapi
schema maintainers (Eric Blake and me) as well, because we can't require
all subject matter experts to be fluent in the QAPI schema language and
conventions.  I found this one more or less by chance.

Jens Freimann <jfreimann@redhat.com> writes:

> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
> feature was not negotiated during virtio feature negotiation. If this
> event is received it means any primary devices hotplugged before
> this were were never really added to QEMU devices.

Too many negations for my poor old brain to process.

>
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
>  qapi/net.json | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/qapi/net.json b/qapi/net.json
> index 728990f4fb..8c5f3f1fb2 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -737,3 +737,19 @@
>  ##
>  { 'command': 'announce-self', 'boxed': true,
>    'data' : 'AnnounceParameters'}
> +
> +##
> +# @FAILOVER_NEGOTIATED:
> +#
> +# Emitted when VIRTIO_NET_F_STANDBY was negotiated during feature negotiation
> +#
> +# Since: 4.2
> +#
> +# Example:
> +#
> +# <- { "event": "FAILOVER_NEGOTIATED",
> +#      "data": {} }
> +#
> +##
> +{ 'event': 'FAILOVER_NEGOTIATED',
> +  'data': {} }

The commit message at least tries to explain intended use.  The doc
string does not.  Should it?
Jens Freimann Oct. 25, 2019, 7:51 a.m. UTC | #4
On Fri, Oct 25, 2019 at 07:35:28AM +0200, Markus Armbruster wrote:
>We ask patch submitters to cc: subject matter experts for review.  You
>did.  When such patches touch the QAPI schema, it's best to cc the qapi
>schema maintainers (Eric Blake and me) as well, because we can't require
>all subject matter experts to be fluent in the QAPI schema language and
>conventions.  I found this one more or less by chance.

Sorry about that, I'll make sure to get the right people next time.
>
>Jens Freimann <jfreimann@redhat.com> writes:
>
>> This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
>> feature was not negotiated during virtio feature negotiation. If this
>> event is received it means any primary devices hotplugged before
>> this were were never really added to QEMU devices.
>
>Too many negations for my poor old brain to process.

I'll try to explain better :) 
>
>>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>> ---
>>  qapi/net.json | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/qapi/net.json b/qapi/net.json
>> index 728990f4fb..8c5f3f1fb2 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -737,3 +737,19 @@
>>  ##
>>  { 'command': 'announce-self', 'boxed': true,
>>    'data' : 'AnnounceParameters'}
>> +
>> +##
>> +# @FAILOVER_NEGOTIATED:
>> +#
>> +# Emitted when VIRTIO_NET_F_STANDBY was negotiated during feature negotiation
>> +#
>> +# Since: 4.2
>> +#
>> +# Example:
>> +#
>> +# <- { "event": "FAILOVER_NEGOTIATED",
>> +#      "data": {} }
>> +#
>> +##
>> +{ 'event': 'FAILOVER_NEGOTIATED',
>> +  'data': {} }
>
>The commit message at least tries to explain intended use.  The doc
>string does not.  Should it?

Sure, I'll add it. 

Thanks for the review!

regards,
Jens

Patch
diff mbox series

diff --git a/qapi/net.json b/qapi/net.json
index 728990f4fb..8c5f3f1fb2 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -737,3 +737,19 @@ 
 ##
 { 'command': 'announce-self', 'boxed': true,
   'data' : 'AnnounceParameters'}
+
+##
+# @FAILOVER_NEGOTIATED:
+#
+# Emitted when VIRTIO_NET_F_STANDBY was negotiated during feature negotiation
+#
+# Since: 4.2
+#
+# Example:
+#
+# <- { "event": "FAILOVER_NEGOTIATED",
+#      "data": {} }
+#
+##
+{ 'event': 'FAILOVER_NEGOTIATED',
+  'data': {} }