Re: [Xen-devel] [PATCH v4 --for 4.6 COLOPre 25/25] tools/libxl: seperate device init/cleanup from checkpoint device layer

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang 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@xxxxxxxxxxxxxx>
> Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> @@ -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);

This change highlights a slightly odd (non-error) flow of code when
there are no NICs or disks. I think having remus_devices_setup (with its
new name) handle by passing its operation and calling
all_devices_setup_cb directly if there are no devices would be more
natural.

A cleanup for another time though.



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