diff mbox series

[PULL,09/13] hmp: Add hmp_announce_self

Message ID 1550847320-25110-10-git-send-email-jasowang@redhat.com
State New
Headers show
Series [PULL,01/13] net/colo-compare.c: Remove duplicated code | expand

Commit Message

Jason Wang Feb. 22, 2019, 2:55 p.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add an HMP command to trigger self annocements.
Unlike the QMP command (which takes a set of parameters), the HMP
command reuses the set of parameters used for migration.

Signend-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  5 +++++
 hmp.h            |  1 +
 tests/test-hmp.c |  1 +
 4 files changed, 21 insertions(+)

Comments

Thomas Huth Feb. 27, 2019, 8:28 a.m. UTC | #1
On 22/02/2019 15.55, Jason Wang wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add an HMP command to trigger self annocements.
> Unlike the QMP command (which takes a set of parameters), the HMP
> command reuses the set of parameters used for migration.
> 
> Signend-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hmp-commands.hx  | 14 ++++++++++++++
>  hmp.c            |  5 +++++
>  hmp.h            |  1 +
>  tests/test-hmp.c |  1 +
>  4 files changed, 21 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ba71558..9f812bc 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -931,6 +931,20 @@ stops because the size limit is reached.
>  ETEXI
>  
>      {
> +        .name       = "announce_self",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "Trigger GARP/RARP announcements",
> +        .cmd        = hmp_announce_self,
> +    },
> +
> +STEXI
> +@item announce_self
> +@findex announce_self
> +Trigger GARP/RARP announcements.
> +ETEXI

The help text is incredibly sparse. I doubt that the average user
manages to find out the meaning of this command just by looking at this
short information. Also, shouldn't you mention here that the parameters
used for migration are re-used here, like in the patch description?
Thus, could you please send a follow-up patch to increase that
information here a little bit, please?

 Thanks,
  Thomas
Dr. David Alan Gilbert Feb. 27, 2019, 1:11 p.m. UTC | #2
* Thomas Huth (thuth@redhat.com) wrote:
> On 22/02/2019 15.55, Jason Wang wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Add an HMP command to trigger self annocements.
> > Unlike the QMP command (which takes a set of parameters), the HMP
> > command reuses the set of parameters used for migration.
> > 
> > Signend-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  hmp-commands.hx  | 14 ++++++++++++++
> >  hmp.c            |  5 +++++
> >  hmp.h            |  1 +
> >  tests/test-hmp.c |  1 +
> >  4 files changed, 21 insertions(+)
> > 
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index ba71558..9f812bc 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -931,6 +931,20 @@ stops because the size limit is reached.
> >  ETEXI
> >  
> >      {
> > +        .name       = "announce_self",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "Trigger GARP/RARP announcements",
> > +        .cmd        = hmp_announce_self,
> > +    },
> > +
> > +STEXI
> > +@item announce_self
> > +@findex announce_self
> > +Trigger GARP/RARP announcements.
> > +ETEXI
> 
> The help text is incredibly sparse. I doubt that the average user
> manages to find out the meaning of this command just by looking at this
> short information. Also, shouldn't you mention here that the parameters
> used for migration are re-used here, like in the patch description?
> Thus, could you please send a follow-up patch to increase that
> information here a little bit, please?

I've updated that to:
+STEXI
+@item announce_self
+@findex announce_self
+Trigger a round of GARP/RARP broadcasts; this is useful for explicitly updating the
+network infrastructure after a reconfiguration or some forms of migration.
+The timings of the round are set by the migration announce parameters.
 ETEXI

None of these pieces of help seem to be particularly deep.

Dave

>  Thanks,
>   Thomas
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ba71558..9f812bc 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -931,6 +931,20 @@  stops because the size limit is reached.
 ETEXI
 
     {
+        .name       = "announce_self",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Trigger GARP/RARP announcements",
+        .cmd        = hmp_announce_self,
+    },
+
+STEXI
+@item announce_self
+@findex announce_self
+Trigger GARP/RARP announcements.
+ETEXI
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,resume:-r,uri:s",
         .params     = "[-d] [-b] [-i] [-r] uri",
diff --git a/hmp.c b/hmp.c
index f3db0bf..5f13b16 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1570,6 +1570,11 @@  void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 
 }
 
+void hmp_announce_self(Monitor *mon, const QDict *qdict)
+{
+    qmp_announce_self(migrate_announce_params(), NULL);
+}
+
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 {
     qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index 5f1addc..e0f32f0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -46,6 +46,7 @@  void hmp_sync_profile(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
 void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
+void hmp_announce_self(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 1a3a9c5..8c49d2f 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -20,6 +20,7 @@ 
 static int verbose;
 
 static const char *hmp_cmds[] = {
+    "announce_self",
     "boot_set ndc",
     "chardev-add null,id=testchardev1",
     "chardev-send-break testchardev1",