Re: [Xen-devel] [PATCH v6 16/18] tools/libxl: store remus_ops in checkpoint device state

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Dec 30, 2015 at 10:29:06AM +0800, Wen Congyang wrote:
> Checkpoint device is an abstract layer to do checkpoint.
> COLO can also use it to do checkpoint. But there are
> still some codes in checkpoint device which touch remus.
> 
> This patch and the following 2 will seperate remus from

s/and the following 2/and:

 tools/libxl: move remus state into a seperate structure 
 tools/libxl: seperate device init/cleanup from checkpoint device layer    

> checkpoint device layer.
> 
> We use remus ops directly in checkpoint device. Store it
> in checkpoint device state so that we do not aware of
> remus_ops in the checkpoint device layer.
> 
> it is pure refactoring and no functional changes.
s/it/It/

> 
> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
> Signed-off-by: Yang Hongyang <hongyang.yang@xxxxxxxxxxxx>
> Acked-by:Ian Campbell <ian.campbell@xxxxxxxxxx>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

with the changes I mentioned.
> ---
>  tools/libxl/libxl_checkpoint_device.c | 10 +---------
>  tools/libxl/libxl_internal.h          |  2 ++
>  tools/libxl/libxl_remus.c             |  9 +++++++++
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl_checkpoint_device.c 
> b/tools/libxl/libxl_checkpoint_device.c
> index 226f159..bbc6dc4 100644
> --- a/tools/libxl/libxl_checkpoint_device.c
> +++ b/tools/libxl/libxl_checkpoint_device.c
> @@ -17,14 +17,6 @@
>  
>  #include "libxl_internal.h"
>  
> -extern const libxl__checkpoint_device_instance_ops remus_device_nic;
> -extern const libxl__checkpoint_device_instance_ops remus_device_drbd_disk;
> -static const libxl__checkpoint_device_instance_ops *remus_ops[] = {
> -    &remus_device_nic,
> -    &remus_device_drbd_disk,
> -    NULL,
> -};
> -
>  /*----- helper functions -----*/
>  
>  static int init_device_subkind(libxl__checkpoint_devices_state *cds)
> @@ -172,7 +164,7 @@ static void device_setup_iterate(libxl__egc *egc, 
> libxl__ao_device *aodev)
>          goto out;
>  
>      do {
> -        dev->ops = remus_ops[++dev->ops_index];
> +        dev->ops = dev->cds->ops[++dev->ops_index];
>          if (!dev->ops) {
>              libxl_device_nic * nic = NULL;
>              libxl_device_disk * disk = NULL;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 5b99d6e..914ce94 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2895,6 +2895,8 @@ struct libxl__checkpoint_devices_state {
>      uint32_t domid;
>      libxl__checkpoint_callback *callback;
>      int device_kind_flags;
> +    /* The ops must be pointer array, and the last ops must be NULL */

s/NULL/NULL./

> +    const libxl__checkpoint_device_instance_ops **ops;
>  
>      /*----- private for abstract layer only -----*/
>  
> diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
> index d088dad..3375331 100644
> --- a/tools/libxl/libxl_remus.c
> +++ b/tools/libxl/libxl_remus.c
> @@ -18,6 +18,14 @@
>  
>  #include "libxl_internal.h"
>  
> +extern const libxl__checkpoint_device_instance_ops remus_device_nic;
> +extern const libxl__checkpoint_device_instance_ops remus_device_drbd_disk;
> +static const libxl__checkpoint_device_instance_ops *remus_ops[] = {
> +    &remus_device_nic,
> +    &remus_device_drbd_disk,
> +    NULL,
> +};
> +
>  /*-------------------- Remus setup and teardown ---------------------*/
>  
>  static void remus_setup_done(libxl__egc *egc,
> @@ -50,6 +58,7 @@ void libxl__remus_setup(libxl__egc *egc,
>      cds->ao = ao;
>      cds->domid = dss->domid;
>      cds->callback = remus_setup_done;
> +    cds->ops = remus_ops;
>  
>      dss->sws.checkpoint_callback = remus_checkpoint_stream_written;
>  
> -- 
> 2.5.0
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel