diff mbox

[v4,3/3] libxl: rename checkpointed_stream to stream_type

Message ID 1458007117-22697-3-git-send-email-wency@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wen Congyang March 15, 2016, 1:58 a.m. UTC
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
v3->v4: Remove the new macro, and updte the macro LIBXL_HAVE_CHECKPOINTED_STREAM
 tools/libxl/libxl.c              |  4 ++--
 tools/libxl/libxl.h              |  4 +++-
 tools/libxl/libxl_create.c       |  4 ++--
 tools/libxl/libxl_dom_save.c     |  6 +++---
 tools/libxl/libxl_internal.h     |  2 +-
 tools/libxl/libxl_save_callout.c |  4 ++--
 tools/libxl/libxl_stream_read.c  |  4 ++--
 tools/libxl/libxl_stream_write.c |  2 +-
 tools/libxl/libxl_types.idl      |  2 +-
 tools/libxl/xl_cmdimpl.c         | 16 ++++++++--------
 10 files changed, 25 insertions(+), 23 deletions(-)

Comments

Wei Liu March 15, 2016, 12:54 p.m. UTC | #1
On Tue, Mar 15, 2016 at 09:58:37AM +0800, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> v3->v4: Remove the new macro, and updte the macro LIBXL_HAVE_CHECKPOINTED_STREAM
>  tools/libxl/libxl.c              |  4 ++--
>  tools/libxl/libxl.h              |  4 +++-
>  tools/libxl/libxl_create.c       |  4 ++--
>  tools/libxl/libxl_dom_save.c     |  6 +++---
>  tools/libxl/libxl_internal.h     |  2 +-
>  tools/libxl/libxl_save_callout.c |  4 ++--
>  tools/libxl/libxl_stream_read.c  |  4 ++--
>  tools/libxl/libxl_stream_write.c |  2 +-
>  tools/libxl/libxl_types.idl      |  2 +-
>  tools/libxl/xl_cmdimpl.c         | 16 ++++++++--------
>  10 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 93e228d..7579dd2 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -876,7 +876,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
>      dss->live = 1;
>      dss->debug = 0;
>      dss->remus = info;
> -    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_REMUS;
> +    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_REMUS;
>  
>      assert(info);
>  
> @@ -937,7 +937,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
>      dss->type = type;
>      dss->live = flags & LIBXL_SUSPEND_LIVE;
>      dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> -    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
> +    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_NONE;
>  
>      rc = libxl__fd_flags_modify_save(gc, dss->fd,
>                                       ~(O_NONBLOCK|O_NDELAY), 0,
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index f9e3ef5..c55094b 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -879,7 +879,9 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
>  /*
>   * LIBXL_HAVE_CHECKPOINTED_STREAM
>   *
> - * If this is defined, then libxl_checkpointed_stream exists.
> + * If this is defined, then libxl_checkpointed_stream exists, and the
> + * libxl_domain_create_restore() interface's parameter checkpointed_stream
> + * is renamed to stream_type
>   */
>  #define LIBXL_HAVE_CHECKPOINTED_STREAM 1

I just realised this patch have not provided compatibility shim for
older version of LIBXL_API.

It now breaks backward compatibility because it changes existing API.
It's my fault for not observing this earlier, sorry.

On the other hand, I don't think this patch is strictly necessary.
Andrew's original complain was about libxc not exporting XC_MIG_* types.
I think libxl can function just fine as is with the old name.

So if you really wish to change the name of that field, I'm afraid
compatibility shim for all functions that accept
libxl_domain_restore_params is required.

I suggest we drop this patch for now and focus on core COLO
functionality, what do you think?

Wei.
Wen Congyang March 16, 2016, 1:08 a.m. UTC | #2
On 03/15/2016 08:54 PM, Wei Liu wrote:
> On Tue, Mar 15, 2016 at 09:58:37AM +0800, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> v3->v4: Remove the new macro, and updte the macro LIBXL_HAVE_CHECKPOINTED_STREAM
>>  tools/libxl/libxl.c              |  4 ++--
>>  tools/libxl/libxl.h              |  4 +++-
>>  tools/libxl/libxl_create.c       |  4 ++--
>>  tools/libxl/libxl_dom_save.c     |  6 +++---
>>  tools/libxl/libxl_internal.h     |  2 +-
>>  tools/libxl/libxl_save_callout.c |  4 ++--
>>  tools/libxl/libxl_stream_read.c  |  4 ++--
>>  tools/libxl/libxl_stream_write.c |  2 +-
>>  tools/libxl/libxl_types.idl      |  2 +-
>>  tools/libxl/xl_cmdimpl.c         | 16 ++++++++--------
>>  10 files changed, 25 insertions(+), 23 deletions(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 93e228d..7579dd2 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -876,7 +876,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
>>      dss->live = 1;
>>      dss->debug = 0;
>>      dss->remus = info;
>> -    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_REMUS;
>> +    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_REMUS;
>>  
>>      assert(info);
>>  
>> @@ -937,7 +937,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
>>      dss->type = type;
>>      dss->live = flags & LIBXL_SUSPEND_LIVE;
>>      dss->debug = flags & LIBXL_SUSPEND_DEBUG;
>> -    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
>> +    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_NONE;
>>  
>>      rc = libxl__fd_flags_modify_save(gc, dss->fd,
>>                                       ~(O_NONBLOCK|O_NDELAY), 0,
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> index f9e3ef5..c55094b 100644
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>> @@ -879,7 +879,9 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
>>  /*
>>   * LIBXL_HAVE_CHECKPOINTED_STREAM
>>   *
>> - * If this is defined, then libxl_checkpointed_stream exists.
>> + * If this is defined, then libxl_checkpointed_stream exists, and the
>> + * libxl_domain_create_restore() interface's parameter checkpointed_stream
>> + * is renamed to stream_type
>>   */
>>  #define LIBXL_HAVE_CHECKPOINTED_STREAM 1
> 
> I just realised this patch have not provided compatibility shim for
> older version of LIBXL_API.
> 
> It now breaks backward compatibility because it changes existing API.
> It's my fault for not observing this earlier, sorry.
> 
> On the other hand, I don't think this patch is strictly necessary.
> Andrew's original complain was about libxc not exporting XC_MIG_* types.
> I think libxl can function just fine as is with the old name.
> 
> So if you really wish to change the name of that field, I'm afraid
> compatibility shim for all functions that accept
> libxl_domain_restore_params is required.
> 
> I suggest we drop this patch for now and focus on core COLO
> functionality, what do you think?

It is OK to drop this patch.

Thanks
Wen Congyang

> 
> Wei.
> 
> 
> .
>
diff mbox

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 93e228d..7579dd2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -876,7 +876,7 @@  int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
     dss->live = 1;
     dss->debug = 0;
     dss->remus = info;
-    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_REMUS;
+    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_REMUS;
 
     assert(info);
 
@@ -937,7 +937,7 @@  int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
-    dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
+    dss->stream_type = LIBXL_CHECKPOINTED_STREAM_NONE;
 
     rc = libxl__fd_flags_modify_save(gc, dss->fd,
                                      ~(O_NONBLOCK|O_NDELAY), 0,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f9e3ef5..c55094b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -879,7 +879,9 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 /*
  * LIBXL_HAVE_CHECKPOINTED_STREAM
  *
- * If this is defined, then libxl_checkpointed_stream exists.
+ * If this is defined, then libxl_checkpointed_stream exists, and the
+ * libxl_domain_create_restore() interface's parameter checkpointed_stream
+ * is renamed to stream_type
  */
 #define LIBXL_HAVE_CHECKPOINTED_STREAM 1
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f1028bc..b28eb89 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -993,7 +993,7 @@  static void domcreate_bootloader_done(libxl__egc *egc,
     libxl_domain_config *const d_config = dcs->guest_config;
     const int restore_fd = dcs->restore_fd;
     libxl__domain_build_state *const state = &dcs->build_state;
-    const int checkpointed_stream = dcs->restore_params.checkpointed_stream;
+    const int stream_type = dcs->restore_params.stream_type;
 
     if (rc) {
         domcreate_rebuild_done(egc, dcs, rc);
@@ -1033,7 +1033,7 @@  static void domcreate_bootloader_done(libxl__egc *egc,
     dcs->srs.completion_callback = domcreate_stream_done;
 
     if (restore_fd >= 0) {
-        switch (checkpointed_stream) {
+        switch (stream_type) {
         case LIBXL_CHECKPOINTED_STREAM_REMUS:
             libxl__remus_restore_setup(egc, dcs);
             /* fall through */
diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index f3288b9..ff92ef0 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -338,7 +338,7 @@  void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss)
     unsigned int nr_vnodes = 0, nr_vmemranges = 0, nr_vcpus = 0;
     libxl__domain_suspend_state *dsps = &dss->dsps;
 
-    if (dss->checkpointed_stream != LIBXL_CHECKPOINTED_STREAM_NONE && !r_info) {
+    if (dss->stream_type != LIBXL_CHECKPOINTED_STREAM_NONE && !r_info) {
         LOG(ERROR, "Migration stream is checkpointed, but there's no "
                    "checkpoint info!");
         rc = ERROR_INVAL;
@@ -383,12 +383,12 @@  void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss)
         goto out;
     }
 
-    if (dss->checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_REMUS) {
+    if (dss->stream_type == LIBXL_CHECKPOINTED_STREAM_REMUS) {
         if (libxl_defbool_val(r_info->compression))
             dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
     }
 
-    if (dss->checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_NONE)
+    if (dss->stream_type == LIBXL_CHECKPOINTED_STREAM_NONE)
         callbacks->suspend = libxl__domain_suspend_callback;
 
     callbacks->switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 005fe53..0aada0d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3128,7 +3128,7 @@  struct libxl__domain_save_state {
     libxl_domain_type type;
     int live;
     int debug;
-    int checkpointed_stream;
+    libxl_checkpointed_stream stream_type;
     const libxl_domain_remus_info *remus;
     /* private */
     int rc;
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 7f1f5d4..133d74e 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -61,7 +61,7 @@  void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
         state->store_domid, state->console_port,
         state->console_domid,
         hvm, pae, superpages,
-        cbflags, dcs->restore_params.checkpointed_stream,
+        cbflags, dcs->restore_params.stream_type,
     };
 
     shs->ao = ao;
@@ -85,7 +85,7 @@  void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_save_state *dss,
 
     const unsigned long argnums[] = {
         dss->domid, 0, 0, dss->xcflags, dss->hvm,
-        cbflags, dss->checkpointed_stream,
+        cbflags, dss->stream_type,
     };
 
     shs->ao = ao;
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index f4781eb..2c480c0 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -773,7 +773,7 @@  void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
     STATE_AO_GC(dcs->ao);
 
     /* convenience aliases */
-    const int checkpointed_stream = dcs->restore_params.checkpointed_stream;
+    const int stream_type = dcs->restore_params.stream_type;
 
     if (rc)
         goto err;
@@ -794,7 +794,7 @@  void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
      * If the stream is not still alive, we must not continue any work.
      */
     if (libxl__stream_read_inuse(stream)) {
-        switch (checkpointed_stream) {
+        switch (stream_type) {
         case LIBXL_CHECKPOINTED_STREAM_REMUS:
             /*
              * Failover from primary. Domain state is currently at a
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index f6ea55d..4894f51 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -355,7 +355,7 @@  void libxl__xc_domain_save_done(libxl__egc *egc, void *dss_void,
      * If the stream is not still alive, we must not continue any work.
      */
     if (libxl__stream_write_inuse(stream)) {
-        if (dss->checkpointed_stream != LIBXL_CHECKPOINTED_STREAM_NONE)
+        if (dss->stream_type != LIBXL_CHECKPOINTED_STREAM_NONE)
             /*
              * For remus, if libxl__xc_domain_save_done() completes,
              * there was an error sending data to the secondary.
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 632c009..7cfeb83 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -382,7 +382,7 @@  libxl_domain_create_info = Struct("domain_create_info",[
     ], dir=DIR_IN)
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
-    ("checkpointed_stream", integer),
+    ("stream_type", integer),
     ("stream_version", uint32, {'init_val': '1'}),
     ])
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 990d3c9..006f3e1 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -154,7 +154,7 @@  struct domain_create {
     int vnc;
     int vncautopass;
     int console_autoconnect;
-    int checkpointed_stream;
+    int stream_type;
     const char *config_file;
     char *extra_config; /* extra config string */
     const char *restore_file;
@@ -2890,7 +2890,7 @@  start:
 
         libxl_domain_restore_params_init(&params);
 
-        params.checkpointed_stream = dom_info->checkpointed_stream;
+        params.stream_type = dom_info->stream_type;
         params.stream_version =
             (hdr.mandatory_flags & XL_MANDATORY_FLAG_STREAMv2) ? 2 : 1;
 
@@ -4435,7 +4435,7 @@  static void migrate_domain(uint32_t domid, const char *rune, int debug,
 
 static void migrate_receive(int debug, int daemonize, int monitor,
                             int send_fd, int recv_fd,
-                            libxl_checkpointed_stream checkpointed)
+                            libxl_checkpointed_stream stream_type)
 {
     uint32_t domid;
     int rc, rc2;
@@ -4460,7 +4460,7 @@  static void migrate_receive(int debug, int daemonize, int monitor,
     dom_info.paused = 1;
     dom_info.migrate_fd = recv_fd;
     dom_info.migration_domname_r = &migration_domname;
-    dom_info.checkpointed_stream = checkpointed;
+    dom_info.stream_type = stream_type;
 
     rc = create_domain(&dom_info);
     if (rc < 0) {
@@ -4471,7 +4471,7 @@  static void migrate_receive(int debug, int daemonize, int monitor,
 
     domid = rc;
 
-    switch (checkpointed) {
+    switch (stream_type) {
     case LIBXL_CHECKPOINTED_STREAM_REMUS:
         /* If we are here, it means that the sender (primary) has crashed.
          * TODO: Split-Brain Check.
@@ -4646,7 +4646,7 @@  int main_restore(int argc, char **argv)
 int main_migrate_receive(int argc, char **argv)
 {
     int debug = 0, daemonize = 1, monitor = 1;
-    libxl_checkpointed_stream checkpointed = LIBXL_CHECKPOINTED_STREAM_NONE;
+    libxl_checkpointed_stream stream_type = LIBXL_CHECKPOINTED_STREAM_NONE;
     int opt;
 
     SWITCH_FOREACH_OPT(opt, "Fedr", NULL, "migrate-receive", 0) {
@@ -4661,7 +4661,7 @@  int main_migrate_receive(int argc, char **argv)
         debug = 1;
         break;
     case 'r':
-        checkpointed = LIBXL_CHECKPOINTED_STREAM_REMUS;
+        stream_type = LIBXL_CHECKPOINTED_STREAM_REMUS;
         break;
     }
 
@@ -4671,7 +4671,7 @@  int main_migrate_receive(int argc, char **argv)
     }
     migrate_receive(debug, daemonize, monitor,
                     STDOUT_FILENO, STDIN_FILENO,
-                    checkpointed);
+                    stream_type);
 
     return 0;
 }