diff mbox series

[V4,10/19] migration: cpr channel

Message ID 1733145611-62315-11-git-send-email-steven.sistare@oracle.com (mailing list archive)
State New, archived
Headers show
Series Live update: cpr-transfer | expand

Commit Message

Steven Sistare Dec. 2, 2024, 1:20 p.m. UTC
Add the 'cpr' channel type, and stash the incoming cpr channel for use
in a subsequent patch.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 include/migration/cpr.h |  3 +++
 migration/cpr.c         | 15 +++++++++++++++
 qapi/migration.json     |  3 ++-
 system/vl.c             |  6 ++++++
 4 files changed, 26 insertions(+), 1 deletion(-)

Comments

Markus Armbruster Dec. 5, 2024, 3:37 p.m. UTC | #1
Steve Sistare <steven.sistare@oracle.com> writes:

> Add the 'cpr' channel type, and stash the incoming cpr channel for use
> in a subsequent patch.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

[...]

> diff --git a/qapi/migration.json b/qapi/migration.json
> index a605dc2..a26960b 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1578,11 +1578,12 @@
>  # The migration channel-type request options.
>  #
>  # @main: Main outbound migration channel.
> +# @cpr: cpr state channel.

What does "cpr" stand for?

>  #
>  # Since: 8.1
>  ##
>  { 'enum': 'MigrationChannelType',
> -  'data': [ 'main' ] }
> +  'data': [ 'main', 'cpr' ] }
>  
>  ##
>  # @MigrationChannel:

[...]
Steven Sistare Dec. 5, 2024, 8:46 p.m. UTC | #2
On 12/5/2024 10:37 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Add the 'cpr' channel type, and stash the incoming cpr channel for use
>> in a subsequent patch.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> [...]
> 
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index a605dc2..a26960b 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1578,11 +1578,12 @@
>>   # The migration channel-type request options.
>>   #
>>   # @main: Main outbound migration channel.
>> +# @cpr: cpr state channel.
> 
> What does "cpr" stand for?

docs/devel/migration/CPR.rst:  CheckPoint and Restart (CPR)

- Steve

> 
>>   #
>>   # Since: 8.1
>>   ##
>>   { 'enum': 'MigrationChannelType',
>> -  'data': [ 'main' ] }
>> +  'data': [ 'main', 'cpr' ] }
>>   
>>   ##
>>   # @MigrationChannel:
> 
> [...]
>
Markus Armbruster Dec. 6, 2024, 9:31 a.m. UTC | #3
Steven Sistare <steven.sistare@oracle.com> writes:

> On 12/5/2024 10:37 AM, Markus Armbruster wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>> 
>>> Add the 'cpr' channel type, and stash the incoming cpr channel for use
>>> in a subsequent patch.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> [...]
>> 
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index a605dc2..a26960b 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -1578,11 +1578,12 @@
>>>  # The migration channel-type request options.
>>>  #
>>>  # @main: Main outbound migration channel.
>>> +# @cpr: cpr state channel.
>>>
>> What does "cpr" stand for?
>
> docs/devel/migration/CPR.rst:  CheckPoint and Restart (CPR)

Suggest something like

     # The migration channel-type request options.
     #
     # @main: Main outbound migration channel.
     #
     # @cpr: Checkpoint and restart state channel

A quick glance at docs/devel/migration/CPR.rst makes me wonder: is that
really *developer* documentation?

Should we have something meant for *users*, too?  QAPI docs could then
link to it.
Steven Sistare Dec. 18, 2024, 7:53 p.m. UTC | #4
On 12/6/2024 4:31 AM, Markus Armbruster wrote:
> Steven Sistare <steven.sistare@oracle.com> writes:
> 
>> On 12/5/2024 10:37 AM, Markus Armbruster wrote:
>>> Steve Sistare <steven.sistare@oracle.com> writes:
>>>
>>>> Add the 'cpr' channel type, and stash the incoming cpr channel for use
>>>> in a subsequent patch.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> [...]
>>>
>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>> index a605dc2..a26960b 100644
>>>> --- a/qapi/migration.json
>>>> +++ b/qapi/migration.json
>>>> @@ -1578,11 +1578,12 @@
>>>>   # The migration channel-type request options.
>>>>   #
>>>>   # @main: Main outbound migration channel.
>>>> +# @cpr: cpr state channel.
>>>>
>>> What does "cpr" stand for?
>>
>> docs/devel/migration/CPR.rst:  CheckPoint and Restart (CPR)
> 
> Suggest something like
> 
>       # The migration channel-type request options.
>       #
>       # @main: Main outbound migration channel.
>       #
>       # @cpr: Checkpoint and restart state channel
> 
> A quick glance at docs/devel/migration/CPR.rst makes me wonder: is that
> really *developer* documentation?
> 
> Should we have something meant for *users*, too?  QAPI docs could then
> link to it.

I agree, CPR.rst is user documentation.

Peter, are you OK with me moving it to the "System Emulation" section of
the documention?

- Steve
Peter Xu Dec. 18, 2024, 8:27 p.m. UTC | #5
On Wed, Dec 18, 2024 at 02:53:16PM -0500, Steven Sistare wrote:
> On 12/6/2024 4:31 AM, Markus Armbruster wrote:
> > Steven Sistare <steven.sistare@oracle.com> writes:
> > 
> > > On 12/5/2024 10:37 AM, Markus Armbruster wrote:
> > > > Steve Sistare <steven.sistare@oracle.com> writes:
> > > > 
> > > > > Add the 'cpr' channel type, and stash the incoming cpr channel for use
> > > > > in a subsequent patch.
> > > > > 
> > > > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > > > [...]
> > > > 
> > > > > diff --git a/qapi/migration.json b/qapi/migration.json
> > > > > index a605dc2..a26960b 100644
> > > > > --- a/qapi/migration.json
> > > > > +++ b/qapi/migration.json
> > > > > @@ -1578,11 +1578,12 @@
> > > > >   # The migration channel-type request options.
> > > > >   #
> > > > >   # @main: Main outbound migration channel.
> > > > > +# @cpr: cpr state channel.
> > > > > 
> > > > What does "cpr" stand for?
> > > 
> > > docs/devel/migration/CPR.rst:  CheckPoint and Restart (CPR)
> > 
> > Suggest something like
> > 
> >       # The migration channel-type request options.
> >       #
> >       # @main: Main outbound migration channel.
> >       #
> >       # @cpr: Checkpoint and restart state channel
> > 
> > A quick glance at docs/devel/migration/CPR.rst makes me wonder: is that
> > really *developer* documentation?
> > 
> > Should we have something meant for *users*, too?  QAPI docs could then
> > link to it.
> 
> I agree, CPR.rst is user documentation.
> 
> Peter, are you OK with me moving it to the "System Emulation" section of
> the documention?

Considering CPR is very closely attached to migration, while we do have the
migration doc in one place right now in devel/... it may make it harder for
people to find relevant info.

It might indeed be an issue, and it can be a more generic that migration
doc (no matter whether it's user or devel oriented..) always stays in
devel/ so far..

As of now.. How about we still keep it in devel/migration/ so migration
stuff is together, but then we move user-relevant migration docs out
instead?  That may contain more than CPR.
Steven Sistare Dec. 18, 2024, 8:31 p.m. UTC | #6
On 12/18/2024 3:27 PM, Peter Xu wrote:
> On Wed, Dec 18, 2024 at 02:53:16PM -0500, Steven Sistare wrote:
>> On 12/6/2024 4:31 AM, Markus Armbruster wrote:
>>> Steven Sistare <steven.sistare@oracle.com> writes:
>>>
>>>> On 12/5/2024 10:37 AM, Markus Armbruster wrote:
>>>>> Steve Sistare <steven.sistare@oracle.com> writes:
>>>>>
>>>>>> Add the 'cpr' channel type, and stash the incoming cpr channel for use
>>>>>> in a subsequent patch.
>>>>>>
>>>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>>> [...]
>>>>>
>>>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>>>> index a605dc2..a26960b 100644
>>>>>> --- a/qapi/migration.json
>>>>>> +++ b/qapi/migration.json
>>>>>> @@ -1578,11 +1578,12 @@
>>>>>>    # The migration channel-type request options.
>>>>>>    #
>>>>>>    # @main: Main outbound migration channel.
>>>>>> +# @cpr: cpr state channel.
>>>>>>
>>>>> What does "cpr" stand for?
>>>>
>>>> docs/devel/migration/CPR.rst:  CheckPoint and Restart (CPR)
>>>
>>> Suggest something like
>>>
>>>        # The migration channel-type request options.
>>>        #
>>>        # @main: Main outbound migration channel.
>>>        #
>>>        # @cpr: Checkpoint and restart state channel
>>>
>>> A quick glance at docs/devel/migration/CPR.rst makes me wonder: is that
>>> really *developer* documentation?
>>>
>>> Should we have something meant for *users*, too?  QAPI docs could then
>>> link to it.
>>
>> I agree, CPR.rst is user documentation.
>>
>> Peter, are you OK with me moving it to the "System Emulation" section of
>> the documention?
> 
> Considering CPR is very closely attached to migration, while we do have the
> migration doc in one place right now in devel/... it may make it harder for
> people to find relevant info.
> 
> It might indeed be an issue, and it can be a more generic that migration
> doc (no matter whether it's user or devel oriented..) always stays in
> devel/ so far..
> 
> As of now.. How about we still keep it in devel/migration/ so migration
> stuff is together, but then we move user-relevant migration docs out
> instead?  That may contain more than CPR.

Fine with me - steve
diff mbox series

Patch

diff --git a/include/migration/cpr.h b/include/migration/cpr.h
index 201d66d..e833fae 100644
--- a/include/migration/cpr.h
+++ b/include/migration/cpr.h
@@ -17,6 +17,9 @@  void cpr_save_fd(const char *name, int id, int fd);
 void cpr_delete_fd(const char *name, int id);
 int cpr_find_fd(const char *name, int id);
 
+void cpr_set_cpr_channel(MigrationChannel *channel);
+MigrationChannel *cpr_get_cpr_channel(void);
+
 int cpr_state_save(MigrationChannel *channel, Error **errp);
 int cpr_state_load(Error **errp);
 void cpr_state_close(void);
diff --git a/migration/cpr.c b/migration/cpr.c
index 1e2878c..f4a795f 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -116,6 +116,21 @@  QIOChannel *cpr_state_ioc(void)
     return qemu_file_get_ioc(cpr_state_file);
 }
 
+static MigrationChannel *cpr_channel;
+
+void cpr_set_cpr_channel(MigrationChannel *channel)
+{
+    if (cpr_channel) {
+        qapi_free_MigrationChannel(cpr_channel);
+    }
+    cpr_channel = channel;
+}
+
+MigrationChannel *cpr_get_cpr_channel(void)
+{
+    return cpr_channel;
+}
+
 int cpr_state_save(MigrationChannel *channel, Error **errp)
 {
     int ret;
diff --git a/qapi/migration.json b/qapi/migration.json
index a605dc2..a26960b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1578,11 +1578,12 @@ 
 # The migration channel-type request options.
 #
 # @main: Main outbound migration channel.
+# @cpr: cpr state channel.
 #
 # Since: 8.1
 ##
 { 'enum': 'MigrationChannelType',
-  'data': [ 'main' ] }
+  'data': [ 'main', 'cpr' ] }
 
 ##
 # @MigrationChannel:
diff --git a/system/vl.c b/system/vl.c
index 2c24c60..40e049e 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -77,6 +77,7 @@ 
 #include "hw/block/block.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "migration/cpr.h"
 #include "migration/misc.h"
 #include "migration/snapshot.h"
 #include "sysemu/tpm.h"
@@ -1834,6 +1835,11 @@  static void incoming_option_parse(const char *str)
         qobject_unref(obj);
         visit_type_MigrationChannel(v, "channel", &channel, &error_fatal);
         visit_free(v);
+
+        if (channel->channel_type == MIGRATION_CHANNEL_TYPE_CPR) {
+            cpr_set_cpr_channel(channel);
+            return;
+        }
     } else if (!strcmp(str, "defer")) {
         channel = NULL;
     } else {