Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On 01/26/2016 10:27 PM, Konrad Rzeszutek Wilk wrote: > On Tue, Jan 26, 2016 at 03:04:39PM +0800, Wen Congyang wrote: >> On 01/26/2016 02:59 AM, Konrad Rzeszutek Wilk wrote: >>> On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote: Secondary vm is running in colo mode, we need to send secondary vm's dirty page information to master at checkpoint, >>> >>> In previous patch you called it primary, so perhaps: >>> s/master/primary/ ? >>> so we have to enable qemu logdirty on secondary. libxl__domain_suspend_common_switch_qemu_logdirty() is to enable qemu logdirty. But it uses domain_save_state, and calls >>> >>> s/domain_save_state/libxl__domain_save_state/ libxl__xc_domain_saverestore_async_callback_done() before exits. This can not be used for secondary vm. Update libxl__domain_suspend_common_switch_qemu_logdirty() to introduce a new API libxl__domain_common_switch_qemu_logdirty(). This API only uses libxl__logdirty_switch, and calls lds->callback before exits. >>> >>> One question - that perhaps had been part of the review earlier >>> (if so it may be good to include this in the description >>> so I don't ask silly questions): >>> >>> Why add this extra API? You could squash >>> libxl__domain_suspend_common_switch_qemu_logdirty >>> and libxl__domain_common_switch_qemu_logdirty code together >>> and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty >>> (ok, just kidding on the name). But - why not have one function >>> instead of splitting the functionality in two? >> >> Do you mean that auto switch qemu logdirty when suspend the guest? > > Squash the two functions - libxl__domain_common_switch_qemu_logdirty and > libxl__domain_suspend_common_switch_qemu_logdirty together? No, libxl__domain_suspend_common_switch_qemu_logdirty() is used by save side. libxl__domain_common_switch_qemu_logdirty() will be used by restore side. Please see the patch 11 in another series. Thanks Wen Congyang > > > . > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On 01/27/2016 08:53 AM, Wen Congyang wrote: > On 01/26/2016 10:27 PM, Konrad Rzeszutek Wilk wrote: >> On Tue, Jan 26, 2016 at 03:04:39PM +0800, Wen Congyang wrote: >>> On 01/26/2016 02:59 AM, Konrad Rzeszutek Wilk wrote: On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote: > Secondary vm is running in colo mode, we need to send > secondary vm's dirty page information to master at checkpoint, In previous patch you called it primary, so perhaps: s/master/primary/ ? > so we have to enable qemu logdirty on secondary. > > libxl__domain_suspend_common_switch_qemu_logdirty() is to enable > qemu logdirty. But it uses domain_save_state, and calls s/domain_save_state/libxl__domain_save_state/ > libxl__xc_domain_saverestore_async_callback_done() > before exits. This can not be used for secondary vm. > > Update libxl__domain_suspend_common_switch_qemu_logdirty() to > introduce a new API libxl__domain_common_switch_qemu_logdirty(). > This API only uses libxl__logdirty_switch, and calls > lds->callback before exits. One question - that perhaps had been part of the review earlier (if so it may be good to include this in the description so I don't ask silly questions): Why add this extra API? You could squash libxl__domain_suspend_common_switch_qemu_logdirty and libxl__domain_common_switch_qemu_logdirty code together and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty (ok, just kidding on the name). But - why not have one function instead of splitting the functionality in two? >>> >>> Do you mean that auto switch qemu logdirty when suspend the guest? >> >> Squash the two functions - libxl__domain_common_switch_qemu_logdirty and >> libxl__domain_suspend_common_switch_qemu_logdirty together? > > IIRC, no codes need such API now. Sorry for the mistake. I understand it now. Thanks Wen Congyang > > Thanks > Wen Congyang > >> >> >> . >> > > > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > . > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On 01/26/2016 10:27 PM, Konrad Rzeszutek Wilk wrote: > On Tue, Jan 26, 2016 at 03:04:39PM +0800, Wen Congyang wrote: >> On 01/26/2016 02:59 AM, Konrad Rzeszutek Wilk wrote: >>> On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote: Secondary vm is running in colo mode, we need to send secondary vm's dirty page information to master at checkpoint, >>> >>> In previous patch you called it primary, so perhaps: >>> s/master/primary/ ? >>> so we have to enable qemu logdirty on secondary. libxl__domain_suspend_common_switch_qemu_logdirty() is to enable qemu logdirty. But it uses domain_save_state, and calls >>> >>> s/domain_save_state/libxl__domain_save_state/ libxl__xc_domain_saverestore_async_callback_done() before exits. This can not be used for secondary vm. Update libxl__domain_suspend_common_switch_qemu_logdirty() to introduce a new API libxl__domain_common_switch_qemu_logdirty(). This API only uses libxl__logdirty_switch, and calls lds->callback before exits. >>> >>> One question - that perhaps had been part of the review earlier >>> (if so it may be good to include this in the description >>> so I don't ask silly questions): >>> >>> Why add this extra API? You could squash >>> libxl__domain_suspend_common_switch_qemu_logdirty >>> and libxl__domain_common_switch_qemu_logdirty code together >>> and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty >>> (ok, just kidding on the name). But - why not have one function >>> instead of splitting the functionality in two? >> >> Do you mean that auto switch qemu logdirty when suspend the guest? > > Squash the two functions - libxl__domain_common_switch_qemu_logdirty and > libxl__domain_suspend_common_switch_qemu_logdirty together? IIRC, no codes need such API now. Thanks Wen Congyang > > > . > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On Tue, Jan 26, 2016 at 03:04:39PM +0800, Wen Congyang wrote: > On 01/26/2016 02:59 AM, Konrad Rzeszutek Wilk wrote: > > On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote: > >> Secondary vm is running in colo mode, we need to send > >> secondary vm's dirty page information to master at checkpoint, > > > > In previous patch you called it primary, so perhaps: > > s/master/primary/ ? > > > >> so we have to enable qemu logdirty on secondary. > >> > >> libxl__domain_suspend_common_switch_qemu_logdirty() is to enable > >> qemu logdirty. But it uses domain_save_state, and calls > > > > s/domain_save_state/libxl__domain_save_state/ > >> libxl__xc_domain_saverestore_async_callback_done() > >> before exits. This can not be used for secondary vm. > >> > >> Update libxl__domain_suspend_common_switch_qemu_logdirty() to > >> introduce a new API libxl__domain_common_switch_qemu_logdirty(). > >> This API only uses libxl__logdirty_switch, and calls > >> lds->callback before exits. > > > > One question - that perhaps had been part of the review earlier > > (if so it may be good to include this in the description > > so I don't ask silly questions): > > > > Why add this extra API? You could squash > > libxl__domain_suspend_common_switch_qemu_logdirty > > and libxl__domain_common_switch_qemu_logdirty code together > > and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty > > (ok, just kidding on the name). But - why not have one function > > instead of splitting the functionality in two? > > Do you mean that auto switch qemu logdirty when suspend the guest? Squash the two functions - libxl__domain_common_switch_qemu_logdirty and libxl__domain_suspend_common_switch_qemu_logdirty together? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On 01/26/2016 02:59 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote:
>> Secondary vm is running in colo mode, we need to send
>> secondary vm's dirty page information to master at checkpoint,
>
> In previous patch you called it primary, so perhaps:
> s/master/primary/ ?
>
>> so we have to enable qemu logdirty on secondary.
>>
>> libxl__domain_suspend_common_switch_qemu_logdirty() is to enable
>> qemu logdirty. But it uses domain_save_state, and calls
>
> s/domain_save_state/libxl__domain_save_state/
>> libxl__xc_domain_saverestore_async_callback_done()
>> before exits. This can not be used for secondary vm.
>>
>> Update libxl__domain_suspend_common_switch_qemu_logdirty() to
>> introduce a new API libxl__domain_common_switch_qemu_logdirty().
>> This API only uses libxl__logdirty_switch, and calls
>> lds->callback before exits.
>
> One question - that perhaps had been part of the review earlier
> (if so it may be good to include this in the description
> so I don't ask silly questions):
>
> Why add this extra API? You could squash
> libxl__domain_suspend_common_switch_qemu_logdirty
> and libxl__domain_common_switch_qemu_logdirty code together
> and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty
> (ok, just kidding on the name). But - why not have one function
> instead of splitting the functionality in two?
Do you mean that auto switch qemu logdirty when suspend the guest?
Thanks
Wen Congyang
>
> Is there another patch that depends on it? If so it may be good
> to spell it out, like:
>
> Patch blah blah is going to use it.
>
> Thanks!
>>
>> Signed-off-by: Yang Hongyang
>> Signed-off-by: Wen Congyang
>> CC: Andrew Cooper
>> Acked-by: Ian Campbell
>> ---
>> tools/libxl/libxl_dom_save.c | 95
>>
>> tools/libxl/libxl_internal.h | 8
>> 2 files changed, 60 insertions(+), 43 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
>> index b3ecad7..79e43f1 100644
>> --- a/tools/libxl/libxl_dom_save.c
>> +++ b/tools/libxl/libxl_dom_save.c
>> @@ -42,7 +42,7 @@ static void switch_logdirty_timeout(libxl__egc *egc,
>> libxl__ev_time *ev,
>> static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
>> const char *watch_path, const char *event_path);
>> static void switch_logdirty_done(libxl__egc *egc,
>> - libxl__domain_save_state *dss, int rc);
>> + libxl__logdirty_switch *lds, int rc);
>>
>> static void logdirty_init(libxl__logdirty_switch *lds)
>> {
>> @@ -52,13 +52,10 @@ static void logdirty_init(libxl__logdirty_switch *lds)
>> }
>>
>> static void domain_suspend_switch_qemu_xen_traditional_logdirty
>> - (int domid, unsigned enable,
>> -libxl__save_helper_state *shs)
>> + (libxl__egc *egc, int domid, unsigned enable,
>> +libxl__logdirty_switch *lds)
>> {
>> -libxl__egc *egc = shs->egc;
>> -libxl__domain_save_state *dss = shs->caller_state;
>> -libxl__logdirty_switch *lds = &dss->logdirty;
>> -STATE_AO_GC(dss->ao);
>> +STATE_AO_GC(lds->ao);
>> int rc;
>> xs_transaction_t t = 0;
>> const char *got;
>> @@ -120,26 +117,34 @@ static void
>> domain_suspend_switch_qemu_xen_traditional_logdirty
>> out:
>> LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
>> libxl__xs_transaction_abort(gc, &t);
>> -switch_logdirty_done(egc,dss,rc);
>> +switch_logdirty_done(egc,lds,rc);
>> }
>>
>> static void domain_suspend_switch_qemu_xen_logdirty
>> - (int domid, unsigned enable,
>> -libxl__save_helper_state *shs)
>> + (libxl__egc *egc, int domid, unsigned enable,
>> +libxl__logdirty_switch *lds)
>> {
>> -libxl__egc *egc = shs->egc;
>> -libxl__domain_save_state *dss = shs->caller_state;
>> -STATE_AO_GC(dss->ao);
>> +STATE_AO_GC(lds->ao);
>> int rc;
>>
>> rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
>> -if (!rc) {
>> -libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
>> -} else {
>> +if (rc)
>> LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
>> +
>> +lds->callback(egc, lds, rc);
>> +}
>> +
>> +static void domain_suspend_switch_qemu_logdirty_done
>> +(libxl__egc *egc, libxl__logdirty_switch *lds, int
>> rc)
>> +{
>> +libxl__domain_save_state *dss = CONTAINER_OF(lds, *dss, logdirty);
>> +
>> +if (rc) {
>> dss->rc = rc;
>> -libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
>> -}
>> +libxl__xc_domain_saverestore_async_callback_done(egc,
>> +
Re: [Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
On Wed, Dec 30, 2015 at 10:28:59AM +0800, Wen Congyang wrote:
> Secondary vm is running in colo mode, we need to send
> secondary vm's dirty page information to master at checkpoint,
In previous patch you called it primary, so perhaps:
s/master/primary/ ?
> so we have to enable qemu logdirty on secondary.
>
> libxl__domain_suspend_common_switch_qemu_logdirty() is to enable
> qemu logdirty. But it uses domain_save_state, and calls
s/domain_save_state/libxl__domain_save_state/
> libxl__xc_domain_saverestore_async_callback_done()
> before exits. This can not be used for secondary vm.
>
> Update libxl__domain_suspend_common_switch_qemu_logdirty() to
> introduce a new API libxl__domain_common_switch_qemu_logdirty().
> This API only uses libxl__logdirty_switch, and calls
> lds->callback before exits.
One question - that perhaps had been part of the review earlier
(if so it may be good to include this in the description
so I don't ask silly questions):
Why add this extra API? You could squash
libxl__domain_suspend_common_switch_qemu_logdirty
and libxl__domain_common_switch_qemu_logdirty code together
and call it libxl_domain_common_and_suspend_common_switch_qemu_logdirty
(ok, just kidding on the name). But - why not have one function
instead of splitting the functionality in two?
Is there another patch that depends on it? If so it may be good
to spell it out, like:
Patch blah blah is going to use it.
Thanks!
>
> Signed-off-by: Yang Hongyang
> Signed-off-by: Wen Congyang
> CC: Andrew Cooper
> Acked-by: Ian Campbell
> ---
> tools/libxl/libxl_dom_save.c | 95
>
> tools/libxl/libxl_internal.h | 8
> 2 files changed, 60 insertions(+), 43 deletions(-)
>
> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
> index b3ecad7..79e43f1 100644
> --- a/tools/libxl/libxl_dom_save.c
> +++ b/tools/libxl/libxl_dom_save.c
> @@ -42,7 +42,7 @@ static void switch_logdirty_timeout(libxl__egc *egc,
> libxl__ev_time *ev,
> static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
> const char *watch_path, const char *event_path);
> static void switch_logdirty_done(libxl__egc *egc,
> - libxl__domain_save_state *dss, int rc);
> + libxl__logdirty_switch *lds, int rc);
>
> static void logdirty_init(libxl__logdirty_switch *lds)
> {
> @@ -52,13 +52,10 @@ static void logdirty_init(libxl__logdirty_switch *lds)
> }
>
> static void domain_suspend_switch_qemu_xen_traditional_logdirty
> - (int domid, unsigned enable,
> -libxl__save_helper_state *shs)
> + (libxl__egc *egc, int domid, unsigned enable,
> +libxl__logdirty_switch *lds)
> {
> -libxl__egc *egc = shs->egc;
> -libxl__domain_save_state *dss = shs->caller_state;
> -libxl__logdirty_switch *lds = &dss->logdirty;
> -STATE_AO_GC(dss->ao);
> +STATE_AO_GC(lds->ao);
> int rc;
> xs_transaction_t t = 0;
> const char *got;
> @@ -120,26 +117,34 @@ static void
> domain_suspend_switch_qemu_xen_traditional_logdirty
> out:
> LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
> libxl__xs_transaction_abort(gc, &t);
> -switch_logdirty_done(egc,dss,rc);
> +switch_logdirty_done(egc,lds,rc);
> }
>
> static void domain_suspend_switch_qemu_xen_logdirty
> - (int domid, unsigned enable,
> -libxl__save_helper_state *shs)
> + (libxl__egc *egc, int domid, unsigned enable,
> +libxl__logdirty_switch *lds)
> {
> -libxl__egc *egc = shs->egc;
> -libxl__domain_save_state *dss = shs->caller_state;
> -STATE_AO_GC(dss->ao);
> +STATE_AO_GC(lds->ao);
> int rc;
>
> rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
> -if (!rc) {
> -libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
> -} else {
> +if (rc)
> LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
> +
> +lds->callback(egc, lds, rc);
> +}
> +
> +static void domain_suspend_switch_qemu_logdirty_done
> +(libxl__egc *egc, libxl__logdirty_switch *lds, int
> rc)
> +{
> +libxl__domain_save_state *dss = CONTAINER_OF(lds, *dss, logdirty);
> +
> +if (rc) {
> dss->rc = rc;
> -libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
> -}
> +libxl__xc_domain_saverestore_async_callback_done(egc,
> + &dss->sws.shs, -1);
> +} else
> +libxl__xc_domain_saverestore_async_callback_done(egc, &dss->sws.shs,
> 0);
> }
>
> void libxl__domain_suspend_common_switch_qemu_logdirty
> @@ -148,42 +153,52 @@ void libxl__domain_suspend_common_swi
[Xen-devel] [PATCH v6 09/18] tools/libxl: introduce libxl__domain_common_switch_qemu_logdirty()
Secondary vm is running in colo mode, we need to send
secondary vm's dirty page information to master at checkpoint,
so we have to enable qemu logdirty on secondary.
libxl__domain_suspend_common_switch_qemu_logdirty() is to enable
qemu logdirty. But it uses domain_save_state, and calls
libxl__xc_domain_saverestore_async_callback_done()
before exits. This can not be used for secondary vm.
Update libxl__domain_suspend_common_switch_qemu_logdirty() to
introduce a new API libxl__domain_common_switch_qemu_logdirty().
This API only uses libxl__logdirty_switch, and calls
lds->callback before exits.
Signed-off-by: Yang Hongyang
Signed-off-by: Wen Congyang
CC: Andrew Cooper
Acked-by: Ian Campbell
---
tools/libxl/libxl_dom_save.c | 95
tools/libxl/libxl_internal.h | 8
2 files changed, 60 insertions(+), 43 deletions(-)
diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index b3ecad7..79e43f1 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -42,7 +42,7 @@ static void switch_logdirty_timeout(libxl__egc *egc,
libxl__ev_time *ev,
static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
const char *watch_path, const char *event_path);
static void switch_logdirty_done(libxl__egc *egc,
- libxl__domain_save_state *dss, int rc);
+ libxl__logdirty_switch *lds, int rc);
static void logdirty_init(libxl__logdirty_switch *lds)
{
@@ -52,13 +52,10 @@ static void logdirty_init(libxl__logdirty_switch *lds)
}
static void domain_suspend_switch_qemu_xen_traditional_logdirty
- (int domid, unsigned enable,
-libxl__save_helper_state *shs)
+ (libxl__egc *egc, int domid, unsigned enable,
+libxl__logdirty_switch *lds)
{
-libxl__egc *egc = shs->egc;
-libxl__domain_save_state *dss = shs->caller_state;
-libxl__logdirty_switch *lds = &dss->logdirty;
-STATE_AO_GC(dss->ao);
+STATE_AO_GC(lds->ao);
int rc;
xs_transaction_t t = 0;
const char *got;
@@ -120,26 +117,34 @@ static void
domain_suspend_switch_qemu_xen_traditional_logdirty
out:
LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
libxl__xs_transaction_abort(gc, &t);
-switch_logdirty_done(egc,dss,rc);
+switch_logdirty_done(egc,lds,rc);
}
static void domain_suspend_switch_qemu_xen_logdirty
- (int domid, unsigned enable,
-libxl__save_helper_state *shs)
+ (libxl__egc *egc, int domid, unsigned enable,
+libxl__logdirty_switch *lds)
{
-libxl__egc *egc = shs->egc;
-libxl__domain_save_state *dss = shs->caller_state;
-STATE_AO_GC(dss->ao);
+STATE_AO_GC(lds->ao);
int rc;
rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
-if (!rc) {
-libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
-} else {
+if (rc)
LOG(ERROR,"logdirty switch failed (rc=%d), abandoning suspend",rc);
+
+lds->callback(egc, lds, rc);
+}
+
+static void domain_suspend_switch_qemu_logdirty_done
+(libxl__egc *egc, libxl__logdirty_switch *lds, int rc)
+{
+libxl__domain_save_state *dss = CONTAINER_OF(lds, *dss, logdirty);
+
+if (rc) {
dss->rc = rc;
-libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
-}
+libxl__xc_domain_saverestore_async_callback_done(egc,
+ &dss->sws.shs, -1);
+} else
+libxl__xc_domain_saverestore_async_callback_done(egc, &dss->sws.shs,
0);
}
void libxl__domain_suspend_common_switch_qemu_logdirty
@@ -148,42 +153,52 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
libxl__save_helper_state *shs = user;
libxl__egc *egc = shs->egc;
libxl__domain_save_state *dss = shs->caller_state;
-STATE_AO_GC(dss->ao);
+
+/* convenience aliases */
+libxl__logdirty_switch *const lds = &dss->logdirty;
+
+lds->callback = domain_suspend_switch_qemu_logdirty_done;
+libxl__domain_common_switch_qemu_logdirty(egc, domid, enable, lds);
+}
+
+void libxl__domain_common_switch_qemu_logdirty(libxl__egc *egc,
+ int domid, unsigned enable,
+ libxl__logdirty_switch *lds)
+{
+STATE_AO_GC(lds->ao);
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-domain_suspend_switch_qemu_xen_traditional_logdirty(domid, enable,
shs);
+domain_suspend_switch_qemu_xen_traditional_logdirty(egc, domid, enable,
+lds);
