diff mbox series

[V5,12/25] cpr: QMP interfaces for restart

Message ID 1625678434-240960-13-git-send-email-steven.sistare@oracle.com (mailing list archive)
State New, archived
Headers show
Series Live Update | expand

Commit Message

Steven Sistare July 7, 2021, 5:20 p.m. UTC
cprexec calls cprexec().  Syntax:
  { 'command': 'cprexec', 'data': { 'argv': [ 'str' ] } }

Add the restart mode:
  { 'enum': 'CprMode', 'data': [ 'reboot', 'restart' ] }

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 monitor/qmp-cmds.c |  5 +++++
 qapi/cpr.json      | 16 +++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Marc-André Lureau July 8, 2021, 3:49 p.m. UTC | #1
Hi

On Wed, Jul 7, 2021 at 9:33 PM Steve Sistare <steven.sistare@oracle.com>
wrote:

> cprexec calls cprexec().  Syntax:
>   { 'command': 'cprexec', 'data': { 'argv': [ 'str' ] } }
>
> Add the restart mode:
>   { 'enum': 'CprMode', 'data': [ 'reboot', 'restart' ] }
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  monitor/qmp-cmds.c |  5 +++++
>  qapi/cpr.json      | 16 +++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 1128604..7326f7d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -179,6 +179,11 @@ void qmp_cprsave(const char *file, CprMode mode,
> Error **errp)
>      cprsave(file, mode, errp);
>  }
>
> +void qmp_cprexec(strList *args, Error **errp)
> +{
> +    cprexec(args, errp);
> +}
> +
>  void qmp_cprload(const char *file, Error **errp)
>  {
>      cprload(file, errp);
> diff --git a/qapi/cpr.json b/qapi/cpr.json
> index b6fdc89..2467e48 100644
> --- a/qapi/cpr.json
> +++ b/qapi/cpr.json
> @@ -16,10 +16,12 @@
>  #
>  # @reboot: checkpoint can be cprload'ed after a host kexec reboot.
>  #
> +# @restart: checkpoint can be cprload'ed after restarting qemu.
> +#
>  # Since: 6.1
>  ##
>  { 'enum': 'CprMode',
> -  'data': [ 'reboot' ] }
> +  'data': [ 'reboot', 'restart' ] }
>
>
>  ##
> @@ -61,6 +63,18 @@
>              'mode': 'CprMode' } }
>
>  ##
> +# @cprexec:
> +#
> +# Restart qemu.
> +#
> +# @argv: arguments to exec
>

Why is it not then called cpr-restart ? Why does it take the whole argv?
Could argv be made optional?

+#
> +# Since: 6.1
> +##
> +{ 'command': 'cprexec',
> +  'data': { 'argv': [ 'str' ] } }
> +
> +##
>  # @cprload:
>  #
>  # Start virtual machine from checkpoint file that was created earlier
> using
> --
> 1.8.3.1
>
>
>
Steven Sistare July 12, 2021, 7:19 p.m. UTC | #2
On 7/8/2021 11:49 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 7, 2021 at 9:33 PM Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>> wrote:
> 
>     cprexec calls cprexec().  Syntax:
>       { 'command': 'cprexec', 'data': { 'argv': [ 'str' ] } }
> 
>     Add the restart mode:
>       { 'enum': 'CprMode', 'data': [ 'reboot', 'restart' ] }
> 
>     Signed-off-by: Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>>
>     ---
>      monitor/qmp-cmds.c |  5 +++++
>      qapi/cpr.json      | 16 +++++++++++++++-
>      2 files changed, 20 insertions(+), 1 deletion(-)
> 
>     diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
>     index 1128604..7326f7d 100644
>     --- a/monitor/qmp-cmds.c
>     +++ b/monitor/qmp-cmds.c
>     @@ -179,6 +179,11 @@ void qmp_cprsave(const char *file, CprMode mode, Error **errp)
>          cprsave(file, mode, errp);
>      }
> 
>     +void qmp_cprexec(strList *args, Error **errp)
>     +{
>     +    cprexec(args, errp);
>     +}
>     +
>      void qmp_cprload(const char *file, Error **errp)
>      {
>          cprload(file, errp);
>     diff --git a/qapi/cpr.json b/qapi/cpr.json
>     index b6fdc89..2467e48 100644
>     --- a/qapi/cpr.json
>     +++ b/qapi/cpr.json
>     @@ -16,10 +16,12 @@
>      #
>      # @reboot: checkpoint can be cprload'ed after a host kexec reboot.
>      #
>     +# @restart: checkpoint can be cprload'ed after restarting qemu.
>     +#
>      # Since: 6.1
>      ##
>      { 'enum': 'CprMode',
>     -  'data': [ 'reboot' ] }
>     +  'data': [ 'reboot', 'restart' ] }
> 
> 
>      ##
>     @@ -61,6 +63,18 @@
>                  'mode': 'CprMode' } }
> 
>      ##
>     +# @cprexec:
>     +#
>     +# Restart qemu.
>     +#
>     +# @argv: arguments to exec
> 
> 
> Why is it not then called cpr-restart ? 

I'll change the description.  exec is the key aspect to convey.

< Why does it take the whole argv? 

It takes the whole argv because the caller may provide a prefix command to
modify the process context before executing qemu.  We do that.

> Could argv be made optional?

If argv is omitted, I could exec the qemu binary with no args, but I don't think that 
would be useful.  It may even be confusing, if the caller has a bug and passes no args;
qemu would start and do nothing, rather than fail with an "exec failed" message.

- Steve

>     +#
>     +# Since: 6.1
>     +##
>     +{ 'command': 'cprexec',
>     +  'data': { 'argv': [ 'str' ] } }
>     +
>     +##
>      # @cprload:
>      #
>      # Start virtual machine from checkpoint file that was created earlier using
>     -- 
>     1.8.3.1
> 
> 
> 
> 
> -- 
> Marc-André Lureau
Eric Blake Aug. 4, 2021, 4 p.m. UTC | #3
On Wed, Jul 07, 2021 at 10:20:21AM -0700, Steve Sistare wrote:
> cprexec calls cprexec().  Syntax:
>   { 'command': 'cprexec', 'data': { 'argv': [ 'str' ] } }
> 
> Add the restart mode:
>   { 'enum': 'CprMode', 'data': [ 'reboot', 'restart' ] }
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  monitor/qmp-cmds.c |  5 +++++
>  qapi/cpr.json      | 16 +++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 1128604..7326f7d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -179,6 +179,11 @@ void qmp_cprsave(const char *file, CprMode mode, Error **errp)
>      cprsave(file, mode, errp);
>  }
>  
> +void qmp_cprexec(strList *args, Error **errp)
> +{
> +    cprexec(args, errp);
> +}

Why do you need both qmp_cprexec() and cprexec()?  Can you just name
it qmp_cprexec() in cpr.c from the get-go, rather than having to add a
one-line wrapper in qmp-cmds.c?
Steven Sistare Aug. 4, 2021, 8:22 p.m. UTC | #4
On 8/4/2021 12:00 PM, Eric Blake wrote:
> On Wed, Jul 07, 2021 at 10:20:21AM -0700, Steve Sistare wrote:
>> cprexec calls cprexec().  Syntax:
>>   { 'command': 'cprexec', 'data': { 'argv': [ 'str' ] } }
>>
>> Add the restart mode:
>>   { 'enum': 'CprMode', 'data': [ 'reboot', 'restart' ] }
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  monitor/qmp-cmds.c |  5 +++++
>>  qapi/cpr.json      | 16 +++++++++++++++-
>>  2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
>> index 1128604..7326f7d 100644
>> --- a/monitor/qmp-cmds.c
>> +++ b/monitor/qmp-cmds.c
>> @@ -179,6 +179,11 @@ void qmp_cprsave(const char *file, CprMode mode, Error **errp)
>>      cprsave(file, mode, errp);
>>  }
>>  
>> +void qmp_cprexec(strList *args, Error **errp)
>> +{
>> +    cprexec(args, errp);
>> +}
> 
> Why do you need both qmp_cprexec() and cprexec()?  Can you just name
> it qmp_cprexec() in cpr.c from the get-go, rather than having to add a
> one-line wrapper in qmp-cmds.c?

Will do.

While I'm at it, I will add an underscore to the function names and a dash to the command
names to be consistent with other compound-word commands:

qmp_cpr_save
qmp_cpr_exec
qmp_cpr_load
cpr-save
cpr-exec
cpr-load

- Steve
diff mbox series

Patch

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 1128604..7326f7d 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -179,6 +179,11 @@  void qmp_cprsave(const char *file, CprMode mode, Error **errp)
     cprsave(file, mode, errp);
 }
 
+void qmp_cprexec(strList *args, Error **errp)
+{
+    cprexec(args, errp);
+}
+
 void qmp_cprload(const char *file, Error **errp)
 {
     cprload(file, errp);
diff --git a/qapi/cpr.json b/qapi/cpr.json
index b6fdc89..2467e48 100644
--- a/qapi/cpr.json
+++ b/qapi/cpr.json
@@ -16,10 +16,12 @@ 
 #
 # @reboot: checkpoint can be cprload'ed after a host kexec reboot.
 #
+# @restart: checkpoint can be cprload'ed after restarting qemu.
+#
 # Since: 6.1
 ##
 { 'enum': 'CprMode',
-  'data': [ 'reboot' ] }
+  'data': [ 'reboot', 'restart' ] }
 
 
 ##
@@ -61,6 +63,18 @@ 
             'mode': 'CprMode' } }
 
 ##
+# @cprexec:
+#
+# Restart qemu.
+#
+# @argv: arguments to exec
+#
+# Since: 6.1
+##
+{ 'command': 'cprexec',
+  'data': { 'argv': [ 'str' ] } }
+
+##
 # @cprload:
 #
 # Start virtual machine from checkpoint file that was created earlier using