* [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups
@ 2020-11-10 16:16 Stefan Reiter
2020-11-10 16:18 ` Stefan Reiter
2020-11-24 16:32 ` Fabian Grünbichler
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Reiter @ 2020-11-10 16:16 UTC (permalink / raw)
To: pve-devel
...and literal cleanup, as in, call save_cleanup after success or error.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
I found it more confusing to set opaque to a value and then never use it, so I
put it back to NULL and documented it.
Behaviour tested to be the same.
...igrate-dirty-bitmap-state-via-savevm.patch | 33 ++++++++++++-------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
index 77c0c76..a39e3ca 100644
--- a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
+++ b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
@@ -13,13 +13,13 @@ safe migration is possible and makes sense.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
- include/migration/misc.h | 3 ++
- migration/Makefile.objs | 1 +
- migration/pbs-state.c | 97 ++++++++++++++++++++++++++++++++++++++++
- pve-backup.c | 1 +
- qapi/block-core.json | 6 +++
- softmmu/vl.c | 1 +
- 6 files changed, 109 insertions(+)
+ include/migration/misc.h | 3 ++
+ migration/Makefile.objs | 1 +
+ migration/pbs-state.c | 106 +++++++++++++++++++++++++++++++++++++++
+ pve-backup.c | 1 +
+ qapi/block-core.json | 6 +++
+ softmmu/vl.c | 1 +
+ 6 files changed, 118 insertions(+)
create mode 100644 migration/pbs-state.c
diff --git a/include/migration/misc.h b/include/migration/misc.h
@@ -48,10 +48,10 @@ index 0fc619e380..20b3792599 100644
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/pbs-state.c b/migration/pbs-state.c
new file mode 100644
-index 0000000000..c711498c3e
+index 0000000000..29f2b3860d
--- /dev/null
+++ b/migration/pbs-state.c
-@@ -0,0 +1,97 @@
+@@ -0,0 +1,106 @@
+/*
+ * PBS (dirty-bitmap) state migration
+ */
@@ -67,6 +67,7 @@ index 0000000000..c711498c3e
+ bool active;
+} PBSState;
+
++/* state is accessed via this static variable directly, 'opaque' is NULL */
+static PBSState pbs_state;
+
+static void pbs_state_save_pending(QEMUFile *f, void *opaque,
@@ -115,8 +116,9 @@ index 0000000000..c711498c3e
+
+static bool pbs_state_is_active(void *opaque)
+{
-+ // we need to be return active once, else .save_setup is never called, but,
-+ // if we'd just would return true, we'd freeze the migration for ~ 5 - 10s
++ /* we need to return active exactly once, else .save_setup is never called,
++ * but if we'd just return true the migration doesn't make progress since
++ * it'd be waiting for us */
+ return pbs_state.active;
+}
+
@@ -133,6 +135,12 @@ index 0000000000..c711498c3e
+ return false;
+}
+
++static void pbs_state_save_cleanup(void *opaque)
++{
++ /* reset active after migration succeeds or fails */
++ pbs_state.active = false;
++}
++
+static SaveVMHandlers savevm_pbs_state_handlers = {
+ .save_setup = pbs_state_save_setup,
+ .has_postcopy = pbs_state_has_postcopy,
@@ -140,6 +148,7 @@ index 0000000000..c711498c3e
+ .is_active_iterate = pbs_state_is_active_iterate,
+ .load_state = pbs_state_load,
+ .is_active = pbs_state_is_active,
++ .save_cleanup = pbs_state_save_cleanup,
+};
+
+void pbs_state_mig_init(void)
@@ -147,7 +156,7 @@ index 0000000000..c711498c3e
+ pbs_state.active = true;
+ register_savevm_live("pbs-state", 0, 1,
+ &savevm_pbs_state_handlers,
-+ &pbs_state);
++ NULL);
+}
diff --git a/pve-backup.c b/pve-backup.c
index c7cde0fb0e..f65f1dda26 100644
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups
2020-11-10 16:16 [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups Stefan Reiter
@ 2020-11-10 16:18 ` Stefan Reiter
2020-11-24 16:32 ` Fabian Grünbichler
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Reiter @ 2020-11-10 16:18 UTC (permalink / raw)
To: pve-devel
Eh, if it wasn't clear, this is the 'other thing' from the previous
patch [0], so it applies on top.
[0] https://lists.proxmox.com/pipermail/pve-devel/2020-November/045884.html
On 11/10/20 5:16 PM, Stefan Reiter wrote:
> ...and literal cleanup, as in, call save_cleanup after success or error.
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>
> I found it more confusing to set opaque to a value and then never use it, so I
> put it back to NULL and documented it.
>
> Behaviour tested to be the same.
>
> ...igrate-dirty-bitmap-state-via-savevm.patch | 33 ++++++++++++-------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> index 77c0c76..a39e3ca 100644
> --- a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> +++ b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> @@ -13,13 +13,13 @@ safe migration is possible and makes sense.
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> - include/migration/misc.h | 3 ++
> - migration/Makefile.objs | 1 +
> - migration/pbs-state.c | 97 ++++++++++++++++++++++++++++++++++++++++
> - pve-backup.c | 1 +
> - qapi/block-core.json | 6 +++
> - softmmu/vl.c | 1 +
> - 6 files changed, 109 insertions(+)
> + include/migration/misc.h | 3 ++
> + migration/Makefile.objs | 1 +
> + migration/pbs-state.c | 106 +++++++++++++++++++++++++++++++++++++++
> + pve-backup.c | 1 +
> + qapi/block-core.json | 6 +++
> + softmmu/vl.c | 1 +
> + 6 files changed, 118 insertions(+)
> create mode 100644 migration/pbs-state.c
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> @@ -48,10 +48,10 @@ index 0fc619e380..20b3792599 100644
> common-obj-$(CONFIG_RDMA) += rdma.o
> diff --git a/migration/pbs-state.c b/migration/pbs-state.c
> new file mode 100644
> -index 0000000000..c711498c3e
> +index 0000000000..29f2b3860d
> --- /dev/null
> +++ b/migration/pbs-state.c
> -@@ -0,0 +1,97 @@
> +@@ -0,0 +1,106 @@
> +/*
> + * PBS (dirty-bitmap) state migration
> + */
> @@ -67,6 +67,7 @@ index 0000000000..c711498c3e
> + bool active;
> +} PBSState;
> +
> ++/* state is accessed via this static variable directly, 'opaque' is NULL */
> +static PBSState pbs_state;
> +
> +static void pbs_state_save_pending(QEMUFile *f, void *opaque,
> @@ -115,8 +116,9 @@ index 0000000000..c711498c3e
> +
> +static bool pbs_state_is_active(void *opaque)
> +{
> -+ // we need to be return active once, else .save_setup is never called, but,
> -+ // if we'd just would return true, we'd freeze the migration for ~ 5 - 10s
> ++ /* we need to return active exactly once, else .save_setup is never called,
> ++ * but if we'd just return true the migration doesn't make progress since
> ++ * it'd be waiting for us */
> + return pbs_state.active;
> +}
> +
> @@ -133,6 +135,12 @@ index 0000000000..c711498c3e
> + return false;
> +}
> +
> ++static void pbs_state_save_cleanup(void *opaque)
> ++{
> ++ /* reset active after migration succeeds or fails */
> ++ pbs_state.active = false;
> ++}
> ++
> +static SaveVMHandlers savevm_pbs_state_handlers = {
> + .save_setup = pbs_state_save_setup,
> + .has_postcopy = pbs_state_has_postcopy,
> @@ -140,6 +148,7 @@ index 0000000000..c711498c3e
> + .is_active_iterate = pbs_state_is_active_iterate,
> + .load_state = pbs_state_load,
> + .is_active = pbs_state_is_active,
> ++ .save_cleanup = pbs_state_save_cleanup,
> +};
> +
> +void pbs_state_mig_init(void)
> @@ -147,7 +156,7 @@ index 0000000000..c711498c3e
> + pbs_state.active = true;
> + register_savevm_live("pbs-state", 0, 1,
> + &savevm_pbs_state_handlers,
> -+ &pbs_state);
> ++ NULL);
> +}
> diff --git a/pve-backup.c b/pve-backup.c
> index c7cde0fb0e..f65f1dda26 100644
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups
2020-11-10 16:16 [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups Stefan Reiter
2020-11-10 16:18 ` Stefan Reiter
@ 2020-11-24 16:32 ` Fabian Grünbichler
2020-11-25 11:03 ` [pve-devel] applied: " Fabian Grünbichler
1 sibling, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2020-11-24 16:32 UTC (permalink / raw)
To: Proxmox VE development discussion
Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
requires now updated patch from <20201124154122.22202-2-s.reiter@proxmox.com>:
"[PATCH v3 pve-qemu 1/3] update patches with squashed in 'include library version'"
On November 10, 2020 5:16 pm, Stefan Reiter wrote:
> ...and literal cleanup, as in, call save_cleanup after success or error.
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>
> I found it more confusing to set opaque to a value and then never use it, so I
> put it back to NULL and documented it.
>
> Behaviour tested to be the same.
>
> ...igrate-dirty-bitmap-state-via-savevm.patch | 33 ++++++++++++-------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> index 77c0c76..a39e3ca 100644
> --- a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> +++ b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
> @@ -13,13 +13,13 @@ safe migration is possible and makes sense.
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> - include/migration/misc.h | 3 ++
> - migration/Makefile.objs | 1 +
> - migration/pbs-state.c | 97 ++++++++++++++++++++++++++++++++++++++++
> - pve-backup.c | 1 +
> - qapi/block-core.json | 6 +++
> - softmmu/vl.c | 1 +
> - 6 files changed, 109 insertions(+)
> + include/migration/misc.h | 3 ++
> + migration/Makefile.objs | 1 +
> + migration/pbs-state.c | 106 +++++++++++++++++++++++++++++++++++++++
> + pve-backup.c | 1 +
> + qapi/block-core.json | 6 +++
> + softmmu/vl.c | 1 +
> + 6 files changed, 118 insertions(+)
> create mode 100644 migration/pbs-state.c
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> @@ -48,10 +48,10 @@ index 0fc619e380..20b3792599 100644
> common-obj-$(CONFIG_RDMA) += rdma.o
> diff --git a/migration/pbs-state.c b/migration/pbs-state.c
> new file mode 100644
> -index 0000000000..c711498c3e
> +index 0000000000..29f2b3860d
> --- /dev/null
> +++ b/migration/pbs-state.c
> -@@ -0,0 +1,97 @@
> +@@ -0,0 +1,106 @@
> +/*
> + * PBS (dirty-bitmap) state migration
> + */
> @@ -67,6 +67,7 @@ index 0000000000..c711498c3e
> + bool active;
> +} PBSState;
> +
> ++/* state is accessed via this static variable directly, 'opaque' is NULL */
> +static PBSState pbs_state;
> +
> +static void pbs_state_save_pending(QEMUFile *f, void *opaque,
> @@ -115,8 +116,9 @@ index 0000000000..c711498c3e
> +
> +static bool pbs_state_is_active(void *opaque)
> +{
> -+ // we need to be return active once, else .save_setup is never called, but,
> -+ // if we'd just would return true, we'd freeze the migration for ~ 5 - 10s
> ++ /* we need to return active exactly once, else .save_setup is never called,
> ++ * but if we'd just return true the migration doesn't make progress since
> ++ * it'd be waiting for us */
> + return pbs_state.active;
> +}
> +
> @@ -133,6 +135,12 @@ index 0000000000..c711498c3e
> + return false;
> +}
> +
> ++static void pbs_state_save_cleanup(void *opaque)
> ++{
> ++ /* reset active after migration succeeds or fails */
> ++ pbs_state.active = false;
> ++}
> ++
> +static SaveVMHandlers savevm_pbs_state_handlers = {
> + .save_setup = pbs_state_save_setup,
> + .has_postcopy = pbs_state_has_postcopy,
> @@ -140,6 +148,7 @@ index 0000000000..c711498c3e
> + .is_active_iterate = pbs_state_is_active_iterate,
> + .load_state = pbs_state_load,
> + .is_active = pbs_state_is_active,
> ++ .save_cleanup = pbs_state_save_cleanup,
> +};
> +
> +void pbs_state_mig_init(void)
> @@ -147,7 +156,7 @@ index 0000000000..c711498c3e
> + pbs_state.active = true;
> + register_savevm_live("pbs-state", 0, 1,
> + &savevm_pbs_state_handlers,
> -+ &pbs_state);
> ++ NULL);
> +}
> diff --git a/pve-backup.c b/pve-backup.c
> index c7cde0fb0e..f65f1dda26 100644
> --
> 2.20.1
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied: [PATCH pve-qemu] update patches with some pbs-state migration cleanups
2020-11-24 16:32 ` Fabian Grünbichler
@ 2020-11-25 11:03 ` Fabian Grünbichler
0 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2020-11-25 11:03 UTC (permalink / raw)
To: Proxmox VE development discussion
On November 24, 2020 5:32 pm, Fabian Grünbichler wrote:
> Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>
> requires now updated patch from <20201124154122.22202-2-s.reiter@proxmox.com>:
>
> "[PATCH v3 pve-qemu 1/3] update patches with squashed in 'include library version'"
and applied now after that one ;)
>
> On November 10, 2020 5:16 pm, Stefan Reiter wrote:
>> ...and literal cleanup, as in, call save_cleanup after success or error.
>>
>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>> ---
>>
>> I found it more confusing to set opaque to a value and then never use it, so I
>> put it back to NULL and documented it.
>>
>> Behaviour tested to be the same.
>>
>> ...igrate-dirty-bitmap-state-via-savevm.patch | 33 ++++++++++++-------
>> 1 file changed, 21 insertions(+), 12 deletions(-)
>>
>> diff --git a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
>> index 77c0c76..a39e3ca 100644
>> --- a/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
>> +++ b/debian/patches/pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
>> @@ -13,13 +13,13 @@ safe migration is possible and makes sense.
>>
>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>> ---
>> - include/migration/misc.h | 3 ++
>> - migration/Makefile.objs | 1 +
>> - migration/pbs-state.c | 97 ++++++++++++++++++++++++++++++++++++++++
>> - pve-backup.c | 1 +
>> - qapi/block-core.json | 6 +++
>> - softmmu/vl.c | 1 +
>> - 6 files changed, 109 insertions(+)
>> + include/migration/misc.h | 3 ++
>> + migration/Makefile.objs | 1 +
>> + migration/pbs-state.c | 106 +++++++++++++++++++++++++++++++++++++++
>> + pve-backup.c | 1 +
>> + qapi/block-core.json | 6 +++
>> + softmmu/vl.c | 1 +
>> + 6 files changed, 118 insertions(+)
>> create mode 100644 migration/pbs-state.c
>>
>> diff --git a/include/migration/misc.h b/include/migration/misc.h
>> @@ -48,10 +48,10 @@ index 0fc619e380..20b3792599 100644
>> common-obj-$(CONFIG_RDMA) += rdma.o
>> diff --git a/migration/pbs-state.c b/migration/pbs-state.c
>> new file mode 100644
>> -index 0000000000..c711498c3e
>> +index 0000000000..29f2b3860d
>> --- /dev/null
>> +++ b/migration/pbs-state.c
>> -@@ -0,0 +1,97 @@
>> +@@ -0,0 +1,106 @@
>> +/*
>> + * PBS (dirty-bitmap) state migration
>> + */
>> @@ -67,6 +67,7 @@ index 0000000000..c711498c3e
>> + bool active;
>> +} PBSState;
>> +
>> ++/* state is accessed via this static variable directly, 'opaque' is NULL */
>> +static PBSState pbs_state;
>> +
>> +static void pbs_state_save_pending(QEMUFile *f, void *opaque,
>> @@ -115,8 +116,9 @@ index 0000000000..c711498c3e
>> +
>> +static bool pbs_state_is_active(void *opaque)
>> +{
>> -+ // we need to be return active once, else .save_setup is never called, but,
>> -+ // if we'd just would return true, we'd freeze the migration for ~ 5 - 10s
>> ++ /* we need to return active exactly once, else .save_setup is never called,
>> ++ * but if we'd just return true the migration doesn't make progress since
>> ++ * it'd be waiting for us */
>> + return pbs_state.active;
>> +}
>> +
>> @@ -133,6 +135,12 @@ index 0000000000..c711498c3e
>> + return false;
>> +}
>> +
>> ++static void pbs_state_save_cleanup(void *opaque)
>> ++{
>> ++ /* reset active after migration succeeds or fails */
>> ++ pbs_state.active = false;
>> ++}
>> ++
>> +static SaveVMHandlers savevm_pbs_state_handlers = {
>> + .save_setup = pbs_state_save_setup,
>> + .has_postcopy = pbs_state_has_postcopy,
>> @@ -140,6 +148,7 @@ index 0000000000..c711498c3e
>> + .is_active_iterate = pbs_state_is_active_iterate,
>> + .load_state = pbs_state_load,
>> + .is_active = pbs_state_is_active,
>> ++ .save_cleanup = pbs_state_save_cleanup,
>> +};
>> +
>> +void pbs_state_mig_init(void)
>> @@ -147,7 +156,7 @@ index 0000000000..c711498c3e
>> + pbs_state.active = true;
>> + register_savevm_live("pbs-state", 0, 1,
>> + &savevm_pbs_state_handlers,
>> -+ &pbs_state);
>> ++ NULL);
>> +}
>> diff --git a/pve-backup.c b/pve-backup.c
>> index c7cde0fb0e..f65f1dda26 100644
>> --
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-25 11:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 16:16 [pve-devel] [PATCH pve-qemu] update patches with some pbs-state migration cleanups Stefan Reiter
2020-11-10 16:18 ` Stefan Reiter
2020-11-24 16:32 ` Fabian Grünbichler
2020-11-25 11:03 ` [pve-devel] applied: " Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox