Re: [PATCH v6 18/18] tools/libxl: seperate device init/cleanup from checkpoint device layer

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Dec 30, 2015 at 10:29:08AM +0800, Wen Congyang wrote:
> we call (init|cleanup)_subkind_nic and (init|cleanup)_subkind_drbd_disk
> directly in checkpoint device. Move them to libxl_remus.c, Call them before
> calling libxl__checkpoint_devices_setup() or after calling
> libxl__checkpoint_devices_teardown().
> it is pure refactoring and no functional changes.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  tools/libxl/libxl_checkpoint_device.c | 42 ++---------------------------------
>  tools/libxl/libxl_remus.c             | 42 +++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/libxl/libxl_checkpoint_device.c b/tools/libxl/libxl_checkpoint_device.c
> index bbc6dc4..0a16dbb 100644
> --- a/tools/libxl/libxl_checkpoint_device.c
> +++ b/tools/libxl/libxl_checkpoint_device.c
> @@ -17,38 +17,6 @@
>  
>  #include "libxl_internal.h"
>  
> -/*----- helper functions -----*/
> -
> -static int init_device_subkind(libxl__checkpoint_devices_state *cds)
> -{
> -    /* init device subkind-specific state in the libxl ctx */
> -    int rc;
> -    STATE_AO_GC(cds->ao);
> -
> -    if (libxl__netbuffer_enabled(gc)) {
> -        rc = init_subkind_nic(cds);
> -        if (rc) goto out;
> -    }
> -
> -    rc = init_subkind_drbd_disk(cds);
> -    if (rc) goto out;
> -
> -    rc = 0;
> -out:
> -    return rc;
> -}
> -
> -static void cleanup_device_subkind(libxl__checkpoint_devices_state *cds)
> -{
> -    /* cleanup device subkind-specific state in the libxl ctx */
> -    STATE_AO_GC(cds->ao);
> -
> -    if (libxl__netbuffer_enabled(gc))
> -        cleanup_subkind_nic(cds);
> -
> -    cleanup_subkind_drbd_disk(cds);
> -}
> -
>  /*----- setup() and teardown() -----*/
>  
>  /* callbacks */
> @@ -86,14 +54,10 @@ static void checkpoint_devices_setup(libxl__egc *egc,
>  void libxl__checkpoint_devices_setup(libxl__egc *egc,
>                                       libxl__checkpoint_devices_state *cds)
>  {
> -    int i, rc;
> +    int i;
>  
>      STATE_AO_GC(cds->ao);
>  
> -    rc = init_device_subkind(cds);
> -    if (rc)
> -        goto out;
> -
>      cds->num_devices = 0;
>      cds->num_nics = 0;
>      cds->num_disks = 0;
> @@ -126,7 +90,7 @@ void libxl__checkpoint_devices_setup(libxl__egc *egc,
>      return;
>  
>  out:
> -    cds->callback(egc, cds, rc);
> +    cds->callback(egc, cds, 0);
>  }
>  
>  static void checkpoint_devices_setup(libxl__egc *egc,
> @@ -263,8 +227,6 @@ static void devices_teardown_cb(libxl__egc *egc,
>      cds->disks = NULL;
>      cds->num_disks = 0;
>  
> -    cleanup_device_subkind(cds);
> -
>      cds->callback(egc, cds, rc);
>  }
>  
> diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
> index 00e3c80..07a1699 100644
> --- a/tools/libxl/libxl_remus.c
> +++ b/tools/libxl/libxl_remus.c
> @@ -26,6 +26,38 @@ static const libxl__checkpoint_device_instance_ops *remus_ops[] = {
>      NULL,
>  };
>  
> +/*----- helper functions -----*/
> +
> +static int init_device_subkind(libxl__checkpoint_devices_state *cds)
> +{
> +    /* init device subkind-specific state in the libxl ctx */
> +    int rc;
> +    STATE_AO_GC(cds->ao);
> +
> +    if (libxl__netbuffer_enabled(gc)) {
> +        rc = init_subkind_nic(cds);
> +        if (rc) goto out;
> +    }
> +
> +    rc = init_subkind_drbd_disk(cds);
> +    if (rc) goto out;
> +
> +    rc = 0;
> +out:
> +    return rc;
> +}
> +
> +static void cleanup_device_subkind(libxl__checkpoint_devices_state *cds)
> +{
> +    /* cleanup device subkind-specific state in the libxl ctx */
> +    STATE_AO_GC(cds->ao);
> +
> +    if (libxl__netbuffer_enabled(gc))
> +        cleanup_subkind_nic(cds);
> +
> +    cleanup_subkind_drbd_disk(cds);
> +}
> +
>  /*-------------------- Remus setup and teardown ---------------------*/
>  
>  static void remus_setup_done(libxl__egc *egc,
> @@ -63,6 +95,12 @@ void libxl__remus_setup(libxl__egc *egc, libxl__remus_state *rs)
>      cds->concrete_data = rs;
>      rs->interval = info->interval;
>  
> +    if (init_device_subkind(cds)) {
> +        LOG(ERROR, "Remus: failed to init device subkind for guest %u",
> +            dss->domid);
> +        goto out;
> +    }
> +
>      dss->sws.checkpoint_callback = remus_checkpoint_stream_written;
>  
>      libxl__checkpoint_devices_setup(egc, cds);
> @@ -99,6 +137,8 @@ static void remus_setup_failed(libxl__egc *egc,
>          LOG(ERROR, "Remus: failed to teardown device after setup failed"
>              " for guest with domid %u, rc %d", dss->domid, rc);
>  
> +    cleanup_device_subkind(cds);
> +
>      dss->callback(egc, dss, rc);
>  }
>  
> @@ -133,6 +173,8 @@ static void remus_teardown_done(libxl__egc *egc,
>          LOG(ERROR, "Remus: failed to teardown device for guest with domid %u,"
>              " rc %d", dss->domid, rc);
>  
> +    cleanup_device_subkind(cds);
> +
>      dss->callback(egc, dss, rc);
>  }
>  
> -- 
> 2.5.0
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel