From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v6 17/18] tools/libxl: move remus state into a seperate structure Date: Mon, 25 Jan 2016 14:59:06 -0500 Message-ID: <20160125195906.GW14977@char.us.oracle.com> References: <1451442548-26974-1-git-send-email-wency@cn.fujitsu.com> <1451442548-26974-18-git-send-email-wency@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1451442548-26974-18-git-send-email-wency@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wen Congyang Cc: Lars Kurth , Changlong Xie , Wei Liu , Ian Campbell , Andrew Cooper , Jiang Yunhong , Ian Jackson , xen devel , Dong Eddie , Gui Jianfeng , Shriram Rajagopalan , Yang Hongyang List-Id: xen-devel@lists.xenproject.org On Wed, Dec 30, 2015 at 10:29:07AM +0800, Wen Congyang wrote: > Add a new structure remus state, and move concrete layer's private > member to remus state. > it is pure refactoring and no functional changes. > Init interval in libxl__remus_setup(). It is safe to move this initialisation, > because this value is only used for remus, and remus will use this value after > libxl__remus_setup(). > > Signed-off-by: Wen Congyang > Signed-off-by: Yang Hongyang Reviewed-by: Konrad Rzeszutek Wilk > --- > tools/libxl/libxl.c | 2 +- > tools/libxl/libxl_dom_save.c | 3 +-- > tools/libxl/libxl_internal.h | 35 +++++++++++++++----------- > tools/libxl/libxl_netbuffer.c | 49 +++++++++++++++++++++---------------- > tools/libxl/libxl_remus.c | 24 ++++++++++++------ > tools/libxl/libxl_remus_disk_drbd.c | 8 +++--- > 6 files changed, 72 insertions(+), 49 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 69c8047..481824d 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -882,7 +882,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info, > assert(info); > > /* Point of no return */ > - libxl__remus_setup(egc, dss); > + libxl__remus_setup(egc, &dss->rs); > return AO_INPROGRESS; > > out: > diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c > index 8e8d280..86026ac 100644 > --- a/tools/libxl/libxl_dom_save.c > +++ b/tools/libxl/libxl_dom_save.c > @@ -392,7 +392,6 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss) > } > > if (dss->checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_REMUS) { > - dss->interval = r_info->interval; > if (libxl_defbool_val(r_info->compression)) > dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; > } > @@ -447,7 +446,7 @@ static void domain_save_done(libxl__egc *egc, > * from sending checkpoints. Teardown the network buffers and > * release netlink resources. This is an async op. > */ > - libxl__remus_teardown(egc, dss, rc); > + libxl__remus_teardown(egc, &dss->rs, rc); > return; > } > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 914ce94..b6929a9 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -2894,6 +2894,7 @@ struct libxl__checkpoint_devices_state { > libxl__ao *ao; > uint32_t domid; > libxl__checkpoint_callback *callback; > + void *concrete_data; > int device_kind_flags; > /* The ops must be pointer array, and the last ops must be NULL */ > const libxl__checkpoint_device_instance_ops **ops; > @@ -2917,16 +2918,6 @@ struct libxl__checkpoint_devices_state { > int num_disks; > > libxl__multidev multidev; > - > - /*----- private for concrete (device-specific) layer only -----*/ > - > - /* private for nic device subkind ops */ > - char *netbufscript; > - struct nl_sock *nlsock; > - struct nl_cache *qdisc_cache; > - > - /* private for drbd disk subkind ops */ > - char *drbd_probe_script; > }; > > /* > @@ -2974,6 +2965,23 @@ _hidden void libxl__checkpoint_devices_preresume(libxl__egc *egc, > libxl__checkpoint_devices_state *cds); > _hidden void libxl__checkpoint_devices_commit(libxl__egc *egc, > libxl__checkpoint_devices_state *cds); > + > +/*----- Remus related state structure -----*/ > +typedef struct libxl__remus_state libxl__remus_state; > +struct libxl__remus_state { > + /* private */ > + libxl__ev_time checkpoint_timeout; /* used for Remus checkpoint */ > + int interval; /* checkpoint interval */ > + > + /*----- private for concrete (device-specific) layer only -----*/ > + /* private for nic device subkind ops */ > + char *netbufscript; > + struct nl_sock *nlsock; > + struct nl_cache *qdisc_cache; > + > + /* private for drbd disk subkind ops */ > + char *drbd_probe_script; > +}; > _hidden int libxl__netbuffer_enabled(libxl__gc *gc); > > /*----- Legacy conversion helper -----*/ > @@ -3132,9 +3140,8 @@ struct libxl__domain_save_state { > int hvm; > int xcflags; > libxl__domain_suspend_state dsps; > + libxl__remus_state rs; > libxl__checkpoint_devices_state cds; > - libxl__ev_time checkpoint_timeout; /* used for Remus checkpoint */ > - int interval; /* checkpoint interval (for Remus) */ > libxl__stream_write_state sws; > libxl__logdirty_switch logdirty; > /* private for libxl__domain_save_device_model */ > @@ -3551,9 +3558,9 @@ _hidden void libxl__remus_domain_resume_callback(void *data); > _hidden void libxl__remus_domain_save_checkpoint_callback(void *data); > /* Remus setup and teardown*/ > _hidden void libxl__remus_setup(libxl__egc *egc, > - libxl__domain_save_state *dss); > + libxl__remus_state *rs); > _hidden void libxl__remus_teardown(libxl__egc *egc, > - libxl__domain_save_state *dss, > + libxl__remus_state *rs, > int rc); > /* Remus callbacks for restore */ > _hidden void libxl__remus_domain_restore_checkpoint_callback(void *data); > diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c > index 33c2a42..5c7e8a2 100644 > --- a/tools/libxl/libxl_netbuffer.c > +++ b/tools/libxl/libxl_netbuffer.c > @@ -42,17 +42,18 @@ int init_subkind_nic(libxl__checkpoint_devices_state *cds) > { > int rc, ret; > libxl__domain_save_state *dss = CONTAINER_OF(cds, *dss, cds); > + libxl__remus_state *rs = cds->concrete_data; > > STATE_AO_GC(cds->ao); > > - cds->nlsock = nl_socket_alloc(); > - if (!cds->nlsock) { > + rs->nlsock = nl_socket_alloc(); > + if (!rs->nlsock) { > LOG(ERROR, "cannot allocate nl socket"); > rc = ERROR_FAIL; > goto out; > } > > - ret = nl_connect(cds->nlsock, NETLINK_ROUTE); > + ret = nl_connect(rs->nlsock, NETLINK_ROUTE); > if (ret) { > LOG(ERROR, "failed to open netlink socket: %s", > nl_geterror(ret)); > @@ -61,7 +62,7 @@ int init_subkind_nic(libxl__checkpoint_devices_state *cds) > } > > /* get list of all qdiscs installed on network devs. */ > - ret = rtnl_qdisc_alloc_cache(cds->nlsock, &cds->qdisc_cache); > + ret = rtnl_qdisc_alloc_cache(rs->nlsock, &rs->qdisc_cache); > if (ret) { > LOG(ERROR, "failed to allocate qdisc cache: %s", > nl_geterror(ret)); > @@ -70,10 +71,10 @@ int init_subkind_nic(libxl__checkpoint_devices_state *cds) > } > > if (dss->remus->netbufscript) { > - cds->netbufscript = libxl__strdup(gc, dss->remus->netbufscript); > + rs->netbufscript = libxl__strdup(gc, dss->remus->netbufscript); > } else { > - cds->netbufscript = GCSPRINTF("%s/remus-netbuf-setup", > - libxl__xen_script_dir_path()); > + rs->netbufscript = GCSPRINTF("%s/remus-netbuf-setup", > + libxl__xen_script_dir_path()); > } > > rc = 0; > @@ -84,20 +85,22 @@ out: > > void cleanup_subkind_nic(libxl__checkpoint_devices_state *cds) > { > + libxl__remus_state *rs = cds->concrete_data; > + > STATE_AO_GC(cds->ao); > > /* free qdisc cache */ > - if (cds->qdisc_cache) { > - nl_cache_clear(cds->qdisc_cache); > - nl_cache_free(cds->qdisc_cache); > - cds->qdisc_cache = NULL; > + if (rs->qdisc_cache) { > + nl_cache_clear(rs->qdisc_cache); > + nl_cache_free(rs->qdisc_cache); > + rs->qdisc_cache = NULL; > } > > /* close & free nlsock */ > - if (cds->nlsock) { > - nl_close(cds->nlsock); > - nl_socket_free(cds->nlsock); > - cds->nlsock = NULL; > + if (rs->nlsock) { > + nl_close(rs->nlsock); > + nl_socket_free(rs->nlsock); > + rs->nlsock = NULL; > } > } > > @@ -150,13 +153,14 @@ static int init_qdisc(libxl__checkpoint_devices_state *cds, > int rc, ret, ifindex; > struct rtnl_link *ifb = NULL; > struct rtnl_qdisc *qdisc = NULL; > + libxl__remus_state *rs = cds->concrete_data; > > STATE_AO_GC(cds->ao); > > /* Now that we have brought up REMUS_IFB device with plug qdisc for > * this vif, so we need to refill the qdisc cache. > */ > - ret = nl_cache_refill(cds->nlsock, cds->qdisc_cache); > + ret = nl_cache_refill(rs->nlsock, rs->qdisc_cache); > if (ret) { > LOG(ERROR, "cannot refill qdisc cache: %s", nl_geterror(ret)); > rc = ERROR_FAIL; > @@ -164,7 +168,7 @@ static int init_qdisc(libxl__checkpoint_devices_state *cds, > } > > /* get a handle to the REMUS_IFB interface */ > - ret = rtnl_link_get_kernel(cds->nlsock, 0, remus_nic->ifb, &ifb); > + ret = rtnl_link_get_kernel(rs->nlsock, 0, remus_nic->ifb, &ifb); > if (ret) { > LOG(ERROR, "cannot obtain handle for %s: %s", remus_nic->ifb, > nl_geterror(ret)); > @@ -187,7 +191,7 @@ static int init_qdisc(libxl__checkpoint_devices_state *cds, > * There is no need to explicitly free this qdisc as its just a > * reference from the qdisc cache we allocated earlier. > */ > - qdisc = rtnl_qdisc_get_by_parent(cds->qdisc_cache, ifindex, TC_H_ROOT); > + qdisc = rtnl_qdisc_get_by_parent(rs->qdisc_cache, ifindex, TC_H_ROOT); > if (qdisc) { > const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc)); > /* Sanity check: Ensure that the root qdisc is a plug qdisc. */ > @@ -238,11 +242,12 @@ static void setup_async_exec(libxl__checkpoint_device *dev, char *op) > libxl__remus_device_nic *remus_nic = dev->concrete_data; > libxl__checkpoint_devices_state *cds = dev->cds; > libxl__async_exec_state *aes = &dev->aodev.aes; > + libxl__remus_state *rs = cds->concrete_data; > > STATE_AO_GC(cds->ao); > > /* Convenience aliases */ > - char *const script = libxl__strdup(gc, cds->netbufscript); > + char *const script = libxl__strdup(gc, rs->netbufscript); > const uint32_t domid = cds->domid; > const int dev_id = remus_nic->devid; > const char *const vif = remus_nic->vif; > @@ -333,6 +338,7 @@ static void netbuf_setup_script_cb(libxl__egc *egc, > libxl__checkpoint_device *dev = CONTAINER_OF(aodev, *dev, aodev); > libxl__remus_device_nic *remus_nic = dev->concrete_data; > libxl__checkpoint_devices_state *cds = dev->cds; > + libxl__remus_state *rs = cds->concrete_data; > const char *out_path_base, *hotplug_error = NULL; > > STATE_AO_GC(cds->ao); > @@ -377,7 +383,7 @@ static void netbuf_setup_script_cb(libxl__egc *egc, > > if (hotplug_error) { > LOG(ERROR, "netbuf script %s setup failed for vif %s: %s", > - cds->netbufscript, vif, hotplug_error); > + rs->netbufscript, vif, hotplug_error); > rc = ERROR_FAIL; > goto out; > } > @@ -445,6 +451,7 @@ static int remus_netbuf_op(libxl__remus_device_nic *remus_nic, > int buffer_op) > { > int rc, ret; > + libxl__remus_state *rs = cds->concrete_data; > > STATE_AO_GC(cds->ao); > > @@ -458,7 +465,7 @@ static int remus_netbuf_op(libxl__remus_device_nic *remus_nic, > goto out; > } > > - ret = rtnl_qdisc_add(cds->nlsock, remus_nic->qdisc, NLM_F_REQUEST); > + ret = rtnl_qdisc_add(rs->nlsock, remus_nic->qdisc, NLM_F_REQUEST); > if (ret) { > rc = ERROR_FAIL; > goto out; > diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c > index 3375331..00e3c80 100644 > --- a/tools/libxl/libxl_remus.c > +++ b/tools/libxl/libxl_remus.c > @@ -35,9 +35,10 @@ static void remus_setup_failed(libxl__egc *egc, > static void remus_checkpoint_stream_written( > libxl__egc *egc, libxl__stream_write_state *sws, int rc); > > -void libxl__remus_setup(libxl__egc *egc, > - libxl__domain_save_state *dss) > +void libxl__remus_setup(libxl__egc *egc, libxl__remus_state *rs) > { > + libxl__domain_save_state *dss = CONTAINER_OF(rs, *dss, rs); > + > /* Convenience aliases */ > libxl__checkpoint_devices_state *const cds = &dss->cds; > const libxl_domain_remus_info *const info = dss->remus; > @@ -59,6 +60,8 @@ void libxl__remus_setup(libxl__egc *egc, > cds->domid = dss->domid; > cds->callback = remus_setup_done; > cds->ops = remus_ops; > + cds->concrete_data = rs; > + rs->interval = info->interval; > > dss->sws.checkpoint_callback = remus_checkpoint_stream_written; > > @@ -103,15 +106,20 @@ static void remus_teardown_done(libxl__egc *egc, > libxl__checkpoint_devices_state *cds, > int rc); > void libxl__remus_teardown(libxl__egc *egc, > - libxl__domain_save_state *dss, > + libxl__remus_state *rs, > int rc) > { > + libxl__domain_save_state *dss = CONTAINER_OF(rs, *dss, rs); > + > + /* Convenience aliases */ > + libxl__checkpoint_devices_state *const cds = &dss->cds; > + > EGC_GC; > > LOG(WARN, "Remus: Domain suspend terminated with rc %d," > " teardown Remus devices...", rc); > - dss->cds.callback = remus_teardown_done; > - libxl__checkpoint_devices_teardown(egc, &dss->cds); > + cds->callback = remus_teardown_done; > + libxl__checkpoint_devices_teardown(egc, cds); > } > > static void remus_teardown_done(libxl__egc *egc, > @@ -285,9 +293,9 @@ static void remus_devices_commit_cb(libxl__egc *egc, > */ > > /* Set checkpoint interval timeout */ > - rc = libxl__ev_time_register_rel(ao, &dss->checkpoint_timeout, > + rc = libxl__ev_time_register_rel(ao, &dss->rs.checkpoint_timeout, > remus_next_checkpoint, > - dss->interval); > + dss->rs.interval); > > if (rc) > goto out; > @@ -303,7 +311,7 @@ static void remus_next_checkpoint(libxl__egc *egc, libxl__ev_time *ev, > int rc) > { > libxl__domain_save_state *dss = > - CONTAINER_OF(ev, *dss, checkpoint_timeout); > + CONTAINER_OF(ev, *dss, rs.checkpoint_timeout); > > STATE_AO_GC(dss->ao); > > diff --git a/tools/libxl/libxl_remus_disk_drbd.c b/tools/libxl/libxl_remus_disk_drbd.c > index 4dddc58..844dd66 100644 > --- a/tools/libxl/libxl_remus_disk_drbd.c > +++ b/tools/libxl/libxl_remus_disk_drbd.c > @@ -28,10 +28,11 @@ typedef struct libxl__remus_drbd_disk { > > int init_subkind_drbd_disk(libxl__checkpoint_devices_state *cds) > { > + libxl__remus_state *rs = cds->concrete_data; > STATE_AO_GC(cds->ao); > > - cds->drbd_probe_script = GCSPRINTF("%s/block-drbd-probe", > - libxl__xen_script_dir_path()); > + rs->drbd_probe_script = GCSPRINTF("%s/block-drbd-probe", > + libxl__xen_script_dir_path()); > > return 0; > } > @@ -96,6 +97,7 @@ static void match_async_exec(libxl__egc *egc, libxl__checkpoint_device *dev) > int arraysize, nr = 0, rc; > const libxl_device_disk *disk = dev->backend_dev; > libxl__async_exec_state *aes = &dev->aodev.aes; > + libxl__remus_state *rs = dev->cds->concrete_data; > STATE_AO_GC(dev->cds->ao); > > /* setup env & args */ > @@ -107,7 +109,7 @@ static void match_async_exec(libxl__egc *egc, libxl__checkpoint_device *dev) > arraysize = 3; > nr = 0; > GCNEW_ARRAY(aes->args, arraysize); > - aes->args[nr++] = dev->cds->drbd_probe_script; > + aes->args[nr++] = rs->drbd_probe_script; > aes->args[nr++] = disk->pdev_path; > aes->args[nr++] = NULL; > assert(nr <= arraysize); > -- > 2.5.0 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel