* [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 11:00 ` Dr. David Alan Gilbert
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process Sanidhya Kashyap
` (8 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Added prefix qemu_ to the function as pointed out by Juan.
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
arch_init.c | 19 +++++++++++--------
include/exec/ram_addr.h | 4 ++++
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8ddaf35..9ac4602 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -434,20 +434,22 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
return (next - base) << TARGET_PAGE_BITS;
}
-static inline bool migration_bitmap_set_dirty(ram_addr_t addr)
+static inline bool qemu_bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap,
+ bool migration_flag)
{
bool ret;
int nr = addr >> TARGET_PAGE_BITS;
- ret = test_and_set_bit(nr, migration_bitmap);
+ ret = test_and_set_bit(nr, bitmap);
- if (!ret) {
+ if (!ret && migration_flag) {
migration_dirty_pages++;
}
return ret;
}
-static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
+void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+ unsigned long *bitmap, bool migration_flag)
{
ram_addr_t addr;
unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
@@ -461,8 +463,8 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
for (k = page; k < page + nr; k++) {
if (src[k]) {
unsigned long new_dirty;
- new_dirty = ~migration_bitmap[k];
- migration_bitmap[k] |= src[k];
+ new_dirty = ~bitmap[k];
+ bitmap[k] |= src[k];
new_dirty &= src[k];
migration_dirty_pages += ctpopl(new_dirty);
src[k] = 0;
@@ -476,7 +478,7 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
cpu_physical_memory_reset_dirty(start + addr,
TARGET_PAGE_SIZE,
DIRTY_MEMORY_MIGRATION);
- migration_bitmap_set_dirty(start + addr);
+ qemu_bitmap_set_dirty(start + addr, bitmap, migration_flag);
}
}
}
@@ -512,7 +514,8 @@ static void migration_bitmap_sync(void)
address_space_sync_dirty_bitmap(&address_space_memory);
QTAILQ_FOREACH(block, &ram_list.blocks, next) {
- migration_bitmap_sync_range(block->mr->ram_addr, block->length);
+ qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
+ migration_bitmap, true);
}
trace_migration_bitmap_sync_end(migration_dirty_pages
- num_dirty_pages_init);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index e9eb831..a5809b4 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -151,5 +151,9 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
unsigned client);
+
+void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+ unsigned long *bitmap, bool migration_flag);
+
#endif
#endif
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump Sanidhya Kashyap
@ 2014-07-18 11:00 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:00 UTC (permalink / raw)
To: Sanidhya Kashyap
Cc: Amit Shah, qemu list, Dr. David Alan Gilbert, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Added prefix qemu_ to the function as pointed out by Juan.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> arch_init.c | 19 +++++++++++--------
> include/exec/ram_addr.h | 4 ++++
> 2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 8ddaf35..9ac4602 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -434,20 +434,22 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
> return (next - base) << TARGET_PAGE_BITS;
> }
>
> -static inline bool migration_bitmap_set_dirty(ram_addr_t addr)
> +static inline bool qemu_bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap,
> + bool migration_flag)
> {
> bool ret;
> int nr = addr >> TARGET_PAGE_BITS;
>
> - ret = test_and_set_bit(nr, migration_bitmap);
> + ret = test_and_set_bit(nr, bitmap);
>
> - if (!ret) {
> + if (!ret && migration_flag) {
> migration_dirty_pages++;
> }
> return ret;
> }
One small suggestion; instead of taking migration_flag you could give it
uint64_t *counter
and then make it:
if (!ret && counter) {
*counter++;
}
that keeps the functions more general.
> -static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> +void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length,
> + unsigned long *bitmap, bool migration_flag)
> {
> ram_addr_t addr;
> unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
> @@ -461,8 +463,8 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> for (k = page; k < page + nr; k++) {
> if (src[k]) {
> unsigned long new_dirty;
> - new_dirty = ~migration_bitmap[k];
> - migration_bitmap[k] |= src[k];
> + new_dirty = ~bitmap[k];
> + bitmap[k] |= src[k];
> new_dirty &= src[k];
> migration_dirty_pages += ctpopl(new_dirty);
> src[k] = 0;
> @@ -476,7 +478,7 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> cpu_physical_memory_reset_dirty(start + addr,
> TARGET_PAGE_SIZE,
> DIRTY_MEMORY_MIGRATION);
> - migration_bitmap_set_dirty(start + addr);
> + qemu_bitmap_set_dirty(start + addr, bitmap, migration_flag);
> }
> }
> }
> @@ -512,7 +514,8 @@ static void migration_bitmap_sync(void)
> address_space_sync_dirty_bitmap(&address_space_memory);
>
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> - migration_bitmap_sync_range(block->mr->ram_addr, block->length);
> + qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
> + migration_bitmap, true);
> }
> trace_migration_bitmap_sync_end(migration_dirty_pages
> - num_dirty_pages_init);
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index e9eb831..a5809b4 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -151,5 +151,9 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
> void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
> unsigned client);
>
> +
> +void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length,
> + unsigned long *bitmap, bool migration_flag);
> +
> #endif
> #endif
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 11:02 ` Dr. David Alan Gilbert
2014-07-18 12:16 ` Eric Blake
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates Sanidhya Kashyap
` (7 subsequent siblings)
9 siblings, 2 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Changed those files that were directly using the RUN_STATE_RUNNING flag. Now,
they have been replaced by the runstate_is_running() function.
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
hw/usb/hcd-ehci.c | 2 +-
hw/usb/redirect.c | 6 +++---
qapi-schema.json | 7 ++++++-
vl.c | 29 ++++++++++++++++++++++++++++-
4 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a00a93c..5487edc 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2393,7 +2393,7 @@ static void usb_ehci_vm_state_change(void *opaque, int running, RunState state)
* USB-devices which have async handled packages have a packet in the
* ep queue to match the completion with.
*/
- if (state == RUN_STATE_RUNNING) {
+ if (runstate_is_running()) {
ehci_advance_async_state(ehci);
}
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 44522d9..2c16ad5 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -283,7 +283,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
}
/* Don't send new data to the chardev until our state is fully synced */
- if (!runstate_check(RUN_STATE_RUNNING)) {
+ if (!runstate_is_running()) {
return 0;
}
@@ -1290,7 +1290,7 @@ static int usbredir_chardev_can_read(void *opaque)
}
/* Don't read new data from the chardev until our state is fully synced */
- if (!runstate_check(RUN_STATE_RUNNING)) {
+ if (!runstate_is_running()) {
return 0;
}
@@ -1340,7 +1340,7 @@ static void usbredir_vm_state_change(void *priv, int running, RunState state)
{
USBRedirDevice *dev = priv;
- if (state == RUN_STATE_RUNNING && dev->parser != NULL) {
+ if (runstate_is_running() && dev->parser != NULL) {
usbredirparser_do_write(dev->parser); /* Flush any pending writes */
}
}
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..501b8d0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -145,12 +145,17 @@
# @watchdog: the watchdog action is configured to pause and has been triggered
#
# @guest-panicked: guest has been panicked as a result of guest OS panic
+#
+# @migrate: migration process is being executed
+#
+# @dump-bitmap: dump the writable working set of the guest
+#
##
{ 'enum': 'RunState',
'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
- 'guest-panicked' ] }
+ 'guest-panicked', 'migrate', 'dump-bitmap' ] }
##
# @StatusInfo:
diff --git a/vl.c b/vl.c
index 6e084c2..30d6fb7 100644
--- a/vl.c
+++ b/vl.c
@@ -593,31 +593,39 @@ static const RunStateTransition runstate_transitions_def[] = {
/* from -> to */
{ RUN_STATE_DEBUG, RUN_STATE_RUNNING },
{ RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
{ RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
{ RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
{ RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
{ RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
{ RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
{ RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_IO_ERROR, RUN_STATE_MIGRATE },
{ RUN_STATE_PAUSED, RUN_STATE_RUNNING },
{ RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_PAUSED, RUN_STATE_MIGRATE },
{ RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
{ RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_POSTMIGRATE, RUN_STATE_MIGRATE },
{ RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
{ RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
{ RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
+ { RUN_STATE_PRELAUNCH, RUN_STATE_MIGRATE },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
{ RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
+ { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING},
+
{ RUN_STATE_RUNNING, RUN_STATE_DEBUG },
{ RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
{ RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
@@ -628,6 +636,8 @@ static const RunStateTransition runstate_transitions_def[] = {
{ RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
{ RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
{ RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
+ { RUN_STATE_RUNNING, RUN_STATE_DUMP_BITMAP },
+ { RUN_STATE_RUNNING, RUN_STATE_MIGRATE },
{ RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
@@ -638,12 +648,27 @@ static const RunStateTransition runstate_transitions_def[] = {
{ RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
{ RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
{ RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_SUSPENDED, RUN_STATE_MIGRATE },
{ RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
{ RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_WATCHDOG, RUN_STATE_MIGRATE },
{ RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
{ RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_GUEST_PANICKED, RUN_STATE_MIGRATE },
+
+ { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING },
+
+ { RUN_STATE_MIGRATE, RUN_STATE_POSTMIGRATE },
+ { RUN_STATE_MIGRATE, RUN_STATE_PAUSED },
+ { RUN_STATE_MIGRATE, RUN_STATE_SHUTDOWN },
+ { RUN_STATE_MIGRATE, RUN_STATE_GUEST_PANICKED },
+ { RUN_STATE_MIGRATE, RUN_STATE_DEBUG },
+ { RUN_STATE_MIGRATE, RUN_STATE_RUNNING },
+ { RUN_STATE_MIGRATE, RUN_STATE_INTERNAL_ERROR },
+ { RUN_STATE_MIGRATE, RUN_STATE_IO_ERROR },
+ { RUN_STATE_MIGRATE, RUN_STATE_WATCHDOG },
{ RUN_STATE_MAX, RUN_STATE_MAX },
};
@@ -684,7 +709,9 @@ void runstate_set(RunState new_state)
int runstate_is_running(void)
{
- return runstate_check(RUN_STATE_RUNNING);
+ return runstate_check(RUN_STATE_RUNNING) ||
+ runstate_check(RUN_STATE_MIGRATE) ||
+ runstate_check(RUN_STATE_DUMP_BITMAP);
}
bool runstate_needs_reset(void)
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process Sanidhya Kashyap
@ 2014-07-18 11:02 ` Dr. David Alan Gilbert
2014-07-18 12:16 ` Eric Blake
1 sibling, 0 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:02 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, kraxel, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Changed those files that were directly using the RUN_STATE_RUNNING flag. Now,
> they have been replaced by the runstate_is_running() function.
I've cc'd Gerd (aka kraxel) in since he's the USB maintainer.
(and those fixes look worth while irrespective of the rest of the patch).
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> hw/usb/hcd-ehci.c | 2 +-
> hw/usb/redirect.c | 6 +++---
> qapi-schema.json | 7 ++++++-
> vl.c | 29 ++++++++++++++++++++++++++++-
> 4 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index a00a93c..5487edc 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -2393,7 +2393,7 @@ static void usb_ehci_vm_state_change(void *opaque, int running, RunState state)
> * USB-devices which have async handled packages have a packet in the
> * ep queue to match the completion with.
> */
> - if (state == RUN_STATE_RUNNING) {
> + if (runstate_is_running()) {
> ehci_advance_async_state(ehci);
> }
>
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 44522d9..2c16ad5 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -283,7 +283,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
> }
>
> /* Don't send new data to the chardev until our state is fully synced */
> - if (!runstate_check(RUN_STATE_RUNNING)) {
> + if (!runstate_is_running()) {
> return 0;
> }
>
> @@ -1290,7 +1290,7 @@ static int usbredir_chardev_can_read(void *opaque)
> }
>
> /* Don't read new data from the chardev until our state is fully synced */
> - if (!runstate_check(RUN_STATE_RUNNING)) {
> + if (!runstate_is_running()) {
> return 0;
> }
>
> @@ -1340,7 +1340,7 @@ static void usbredir_vm_state_change(void *priv, int running, RunState state)
> {
> USBRedirDevice *dev = priv;
>
> - if (state == RUN_STATE_RUNNING && dev->parser != NULL) {
> + if (runstate_is_running() && dev->parser != NULL) {
> usbredirparser_do_write(dev->parser); /* Flush any pending writes */
> }
> }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b11aad2..501b8d0 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -145,12 +145,17 @@
> # @watchdog: the watchdog action is configured to pause and has been triggered
> #
> # @guest-panicked: guest has been panicked as a result of guest OS panic
> +#
> +# @migrate: migration process is being executed
> +#
> +# @dump-bitmap: dump the writable working set of the guest
> +#
> ##
> { 'enum': 'RunState',
> 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
> 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> - 'guest-panicked' ] }
> + 'guest-panicked', 'migrate', 'dump-bitmap' ] }
>
> ##
> # @StatusInfo:
> diff --git a/vl.c b/vl.c
> index 6e084c2..30d6fb7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -593,31 +593,39 @@ static const RunStateTransition runstate_transitions_def[] = {
> /* from -> to */
> { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
> { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
> + { RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
>
> { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
> { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
>
> { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
> { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> + { RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
Are all of these new transitions really possible?
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process Sanidhya Kashyap
2014-07-18 11:02 ` Dr. David Alan Gilbert
@ 2014-07-18 12:16 ` Eric Blake
2014-07-18 18:01 ` Sanidhya Kashyap
1 sibling, 1 reply; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:16 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Changed those files that were directly using the RUN_STATE_RUNNING flag. Now,
> they have been replaced by the runstate_is_running() function.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> hw/usb/hcd-ehci.c | 2 +-
> hw/usb/redirect.c | 6 +++---
> qapi-schema.json | 7 ++++++-
> vl.c | 29 ++++++++++++++++++++++++++++-
> 4 files changed, 38 insertions(+), 6 deletions(-)
>
> +++ b/qapi-schema.json
> @@ -145,12 +145,17 @@
> # @watchdog: the watchdog action is configured to pause and has been triggered
> #
> # @guest-panicked: guest has been panicked as a result of guest OS panic
> +#
> +# @migrate: migration process is being executed
> +#
> +# @dump-bitmap: dump the writable working set of the guest
Please document these new flags as (since 2.2).
How does 'migrate' differ from 'inmigrate' and 'finish-migrate'? Why do
you need to introduce 'dump-bitmap' again? How will this interact with
older libvirt that doesn't know about the state?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process
2014-07-18 12:16 ` Eric Blake
@ 2014-07-18 18:01 ` Sanidhya Kashyap
0 siblings, 0 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 18:01 UTC (permalink / raw)
To: Eric Blake, qemu list; +Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
>> +# +# @migrate: migration process is being executed +# +#
>> @dump-bitmap: dump the writable working set of the guest
>
> Please document these new flags as (since 2.2).
>
> How does 'migrate' differ from 'inmigrate' and 'finish-migrate'?
> Why do you need to introduce 'dump-bitmap' again? How will this
> interact with older libvirt that doesn't know about the state?
>
What I know is that 'inmigrate' is used on the destination side,
'finish-migrate' is the state for the last stage of migration on the
source when the VM is stopped. Whereas 'migrate' is the state when the
user gives a migrate command for which the runstate changes to
'migrate', if there is a satisfactory condition that allows migration.
I wanted to separate both visualize all the states properly. That's
why I introduced the 'dump-bitmap'. Another reason to support the
introduction is that there is no point in dumping the bitmap when the
VM is halted, so that's why 'dump-bitmap' is required. If the VM is
halted, then the dump-bitmap process will terminate. Well, that is my
thinking and it can either be redundant or useless.
I have no idea what will happen to the older libvirt. :-/
- --
- -----
Sanidhya Kashyap
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTyWESAAoJEFt9RLmoahlnesAIAJi2ZbPkZTvllPl0/n5oQmRm
hqegnl4ggsv8R9lpElcEtJKiwI0JpqwNzmNGp/nUmHqlq9cFgsC58AtdbRQGjDmx
52B7ySpa9Ptne5YTxHbNMj+o6RADdMCjENcYaWToFNftFQvfU8zgeQFnBn+wKKdL
hSqGdaZQ5ak1sQ8VJGjjBKWCiD/4dKZd5VVBk8FLPvVBxI/eVf0e5BKB+XnmpmNW
vElmJXckEOUGd/Uzj4sXRq9RLJ7VQxRD7YJgSuDFFqJShGLxjylOHlsVHg7BOsr7
Zd08j+v8LTRaKDHnKy5VccmA1JdL2RC19YC0vg4SzEjpwPVCDH9dhOAqNwhT3co=
=eITb
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump Sanidhya Kashyap
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 2/8] RunState: added two new flags for bitmap dump and migration process Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 11:12 ` Dr. David Alan Gilbert
` (2 more replies)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 4/8] BitmapLog: hmp interface for dirty bitmap dump Sanidhya Kashyap
` (6 subsequent siblings)
9 siblings, 3 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Reformatted the code and added the functionality of dumping the blocks' info
which can be read by the user when required. I have also made the block name
length global.
Some minor modification to the structure which is now storing all the
information.
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
include/exec/cpu-all.h | 4 +-
migration.c | 7 ++
qapi-schema.json | 18 +++
qmp-commands.hx | 30 +++++
savevm.c | 325 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 383 insertions(+), 1 deletion(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..b459301 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -297,13 +297,15 @@ CPUArchState *cpu_copy(CPUArchState *env);
/* memory API */
+#define RAMBLOCK_NAME_LENGTH (1<<8)
+
typedef struct RAMBlock {
struct MemoryRegion *mr;
uint8_t *host;
ram_addr_t offset;
ram_addr_t length;
uint32_t flags;
- char idstr[256];
+ char idstr[RAMBLOCK_NAME_LENGTH];
/* Reads can take either the iothread or the ramlist lock.
* Writes must take both locks.
*/
diff --git a/migration.c b/migration.c
index 8d675b3..e2e313c 100644
--- a/migration.c
+++ b/migration.c
@@ -436,6 +436,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
+ if (runstate_check(RUN_STATE_DUMP_BITMAP)) {
+ error_setg(errp, "bitmap dump in progress");
+ return;
+ }
+
+ runstate_set(RUN_STATE_MIGRATE);
+
s = migrate_init(¶ms);
if (strstart(uri, "tcp:", &p)) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 501b8d0..924d6bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3485,3 +3485,21 @@
# Since: 2.1
##
{ 'command': 'rtc-reset-reinjection' }
+
+##
+# @log-dirty-bitmap
+#
+# dumps the dirty bitmap to a file by logging the
+# memory for a specified number of times with a
+# a defined time differnce
+#
+# @filename: name of the file in which the bitmap will be saved.
+# @epochs: number of times the memory will be logged (optional).
+# @frequency: time difference in milliseconds between each epoch (optional).
+#
+# Since 2.2
+##
+{ 'command' : 'log-dirty-bitmap',
+ 'data' : { 'filename' : 'str',
+ '*epochs' : 'int',
+ '*frequency' : 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4be4765..200f57e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3753,5 +3753,35 @@ Example:
-> { "execute": "rtc-reset-reinjection" }
<- { "return": {} }
+EQMP
+
+ {
+ .name = "log-dirty-bitmap",
+ .args_type = "filename:s,epochs:i?,frequency:i?,readable:-r?",
+ .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap,
+ },
+
+SQMP
+log-dirty-bitmap
+--------------------
+
+start logging the memory of the VM for writable working set
+
+Arguments:
+
+- "filename": name of the file, in which the bitmap will be saved
+- "epochs": number of times, the memory will be logged
+- "frequency": time difference in milliseconds between each epoch
+
+Examples:
+-> { "execute" : "log-dirty-bitmap",
+ "arguments" : {
+ "filename" : "/tmp/fileXXX",
+ "epochs" : 3,
+ "frequency" : 10 } }
+
+<- { "return": {} }
+Note: The epochs, frequency and readable are optional. epochs default
+value is 3 while that of frequency is 10.
EQMP
diff --git a/savevm.c b/savevm.c
index e19ae0a..ecb334e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -42,6 +42,9 @@
#include "qemu/iov.h"
#include "block/snapshot.h"
#include "block/qapi.h"
+#include "exec/address-spaces.h"
+#include "exec/ram_addr.h"
+#include "qemu/bitmap.h"
#ifndef ETH_P_RARP
@@ -1137,6 +1140,328 @@ void do_savevm(Monitor *mon, const QDict *qdict)
}
}
+/*
+ * Adding the functionality of continuous logging of the
+ * dirty bitmap which is almost similar to the migration
+ * thread
+ */
+
+enum {
+ LOG_BITMAP_STATE_ERROR = -1,
+ LOG_BITMAP_STATE_NONE,
+ LOG_BITMAP_STATE_SETUP,
+ LOG_BITMAP_STATE_ACTIVE,
+ LOG_BITMAP_STATE_CANCELING,
+ LOG_BITMAP_STATE_COMPLETED
+};
+
+typedef struct BitmapLogState BitmapLogState;
+static unsigned long *logging_bitmap;
+static int64_t MIN_EPOCH_VALUE = 3;
+static int64_t MIN_FREQUENCY_VALUE = 10;
+static int64_t MAX_EPOCH_VALUE = 100000;
+static int64_t MAX_FREQUENCY_VALUE = 100000;
+
+struct BitmapLogState {
+ int state;
+ int fd;
+ int64_t current_frequency;
+ int64_t current_epoch;
+ int64_t total_epochs;
+ QemuThread thread;
+};
+
+/*
+ * helper functions
+ */
+
+static inline void logging_lock(void)
+{
+ qemu_mutex_lock_iothread();
+ qemu_mutex_lock_ramlist();
+}
+
+static inline void logging_unlock(void)
+{
+ qemu_mutex_unlock_ramlist();
+ qemu_mutex_unlock_iothread();
+}
+
+static inline void logging_bitmap_set_dirty(ram_addr_t addr)
+{
+ int nr = addr >> TARGET_PAGE_BITS;
+ set_bit(nr, logging_bitmap);
+}
+
+static bool logging_state_set_status(BitmapLogState *b,
+ int old_state,
+ int new_state)
+{
+ return atomic_cmpxchg(&b->state, old_state, new_state);
+}
+
+static inline bool value_in_range(int64_t value, int64_t min_value,
+ int64_t max_value, const char *str,
+ Error **errp)
+{
+ if (value < min_value) {
+ error_setg(errp, "%s's value must be greater than %ld",
+ str, min_value);
+ return false;
+ }
+ if (value > max_value) {
+ error_setg(errp, "%s's value must be less than %ld",
+ str, max_value);
+ return false;
+ }
+ return true;
+}
+
+/*
+ * inspired from migration mechanism
+ */
+
+static BitmapLogState *logging_current_state(void)
+{
+ static BitmapLogState current_bitmaplogstate = {
+ .state = LOG_BITMAP_STATE_NONE,
+ };
+
+ return ¤t_bitmaplogstate;
+}
+
+/*
+ * syncing the logging_bitmap with the ram_list dirty bitmap
+ */
+
+static void dirty_bitmap_sync(void)
+{
+ RAMBlock *block;
+ address_space_sync_dirty_bitmap(&address_space_memory);
+ QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+ qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
+ logging_bitmap, false);
+ }
+}
+
+static inline void logging_bitmap_close(BitmapLogState *b)
+{
+ logging_lock();
+ memory_global_dirty_log_stop();
+ logging_unlock();
+
+ g_free(logging_bitmap);
+ logging_bitmap = NULL;
+ qemu_close(b->fd);
+ b->fd = -1;
+}
+
+static bool ram_block_info_dump(int fd)
+{
+ int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+ int block_count = 0;
+ RAMBlock *block;
+ int ret;
+
+ if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
+ return true;
+ }
+
+ QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+ block_count++;
+ }
+
+ ret = qemu_write_full(fd, &block_count, sizeof(int));
+ if (ret < sizeof(int)) {
+ return true;
+ }
+
+ QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+ ret = qemu_write_full(fd, &(block->idstr), sizeof(char) *
+ RAMBLOCK_NAME_LENGTH);
+ if (ret < sizeof(char) * RAMBLOCK_NAME_LENGTH) {
+ return true;
+ }
+ ret = qemu_write_full(fd, &(block->offset), sizeof(ram_addr_t));
+ if (ret < sizeof(ram_addr_t)) {
+ return true;
+ }
+ ret = qemu_write_full(fd, &(block->length), sizeof(ram_addr_t));
+ if (ret < sizeof(ram_addr_t)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+static void logging_state_update_status(BitmapLogState *b)
+{
+ switch (b->state) {
+ case LOG_BITMAP_STATE_ACTIVE:
+ logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
+ LOG_BITMAP_STATE_COMPLETED);
+ return;
+ case LOG_BITMAP_STATE_CANCELING:
+ logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
+ LOG_BITMAP_STATE_COMPLETED);
+ return;
+ case LOG_BITMAP_STATE_ERROR:
+ logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
+ LOG_BITMAP_STATE_COMPLETED);
+ }
+ return;
+}
+
+static void *bitmap_logging_thread(void *opaque)
+{
+ /*
+ * setup basic structures
+ */
+
+ BitmapLogState *b = opaque;
+ int fd = b->fd;
+ b->current_epoch = 1;
+ int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+ size_t bitmap_size = BITS_TO_LONGS(ram_bitmap_pages) *
+ sizeof(unsigned long);
+ int ret;
+ char marker = 'M';
+
+ logging_state_set_status(b, LOG_BITMAP_STATE_NONE,
+ LOG_BITMAP_STATE_SETUP);
+
+ logging_bitmap = bitmap_new(ram_bitmap_pages);
+
+ if (logging_bitmap == NULL) {
+ b->state = LOG_BITMAP_STATE_ERROR;
+ goto log_thread_end;
+ }
+
+ logging_state_set_status(b, LOG_BITMAP_STATE_SETUP,
+ LOG_BITMAP_STATE_ACTIVE);
+ /*
+ * start the logging period
+ */
+ logging_lock();
+ memory_global_dirty_log_start();
+ dirty_bitmap_sync();
+ bitmap_zero(logging_bitmap, ram_bitmap_pages);
+ logging_unlock();
+
+ if (ram_block_info_dump(fd)) {
+ b->state = LOG_BITMAP_STATE_ERROR;
+ goto log_thread_end;
+ }
+
+ /*
+ * sync the dirty bitmap along with saving it
+ * using the FILE pointer f.
+ */
+ while (b->current_epoch <= b->total_epochs) {
+ if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
+ b->state != LOG_BITMAP_STATE_ACTIVE) {
+ goto log_thread_end;
+ }
+ bitmap_zero(logging_bitmap, ram_bitmap_pages);
+ logging_lock();
+ dirty_bitmap_sync();
+ logging_unlock();
+
+ ret = qemu_write_full(fd, logging_bitmap, bitmap_size);
+ if (ret < bitmap_size) {
+ b->state = LOG_BITMAP_STATE_ERROR;
+ goto log_thread_end;
+ }
+
+ ret = qemu_write_full(fd, &marker, sizeof(char));
+ if (ret < sizeof(char)) {
+ b->state = LOG_BITMAP_STATE_ERROR;
+ goto log_thread_end;
+ }
+ g_usleep(b->current_frequency * 1000);
+ b->current_epoch++;
+ }
+
+ /*
+ * stop the logging period.
+ */
+ log_thread_end:
+ logging_bitmap_close(b);
+ logging_state_update_status(b);
+ runstate_set(RUN_STATE_RUNNING);
+ return NULL;
+}
+
+void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
+ int64_t epochs, bool has_frequency,
+ int64_t frequency, Error **errp)
+{
+ int fd = -1;
+ BitmapLogState *b = logging_current_state();
+ Error *local_err = NULL;
+ if (runstate_check(RUN_STATE_DUMP_BITMAP) ||
+ b->state == LOG_BITMAP_STATE_ACTIVE ||
+ b->state == LOG_BITMAP_STATE_SETUP ||
+ b->state == LOG_BITMAP_STATE_CANCELING) {
+ error_setg(errp, "dirty bitmap dump in progress");
+ return;
+ }
+
+ if (!runstate_is_running()) {
+ error_setg(errp, "Guest is not in a running state");
+ return;
+ }
+
+ runstate_set(RUN_STATE_DUMP_BITMAP);
+ b->state = LOG_BITMAP_STATE_NONE;
+
+ /*
+ * checking the epoch range
+ */
+ if (!has_epochs) {
+ b->total_epochs = MIN_EPOCH_VALUE;
+ } else if (!value_in_range(epochs, MIN_EPOCH_VALUE,
+ MAX_EPOCH_VALUE, "epoch", &local_err)) {
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+ runstate_set(RUN_STATE_RUNNING);
+ return;
+ } else {
+ b->total_epochs = epochs;
+ }
+
+ /*
+ * checking the frequency range
+ */
+ if (!has_frequency) {
+ b->current_frequency = MIN_FREQUENCY_VALUE;
+ } else if (!value_in_range(frequency, MIN_FREQUENCY_VALUE,
+ MAX_FREQUENCY_VALUE, "frequency", &local_err)) {
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+ runstate_set(RUN_STATE_RUNNING);
+ return;
+ } else {
+ b->current_frequency = frequency;
+ }
+
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
+ if (fd < 0) {
+ error_setg_file_open(errp, errno, filename);
+ runstate_set(RUN_STATE_RUNNING);
+ return;
+ }
+
+ b->fd = fd;
+ qemu_thread_create(&b->thread, "dirty-bitmap-dump",
+ bitmap_logging_thread, b,
+ QEMU_THREAD_JOINABLE);
+
+ return;
+}
+
void qmp_xen_save_devices_state(const char *filename, Error **errp)
{
QEMUFile *f;
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates Sanidhya Kashyap
@ 2014-07-18 11:12 ` Dr. David Alan Gilbert
2014-07-18 18:18 ` Sanidhya Kashyap
2014-07-18 11:14 ` Dr. David Alan Gilbert
2014-07-18 12:20 ` Eric Blake
2 siblings, 1 reply; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:12 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Reformatted the code and added the functionality of dumping the blocks' info
> which can be read by the user when required. I have also made the block name
> length global.
> Some minor modification to the structure which is now storing all the
> information.
One thought, I wonder how much of the savevm.c changes could move into
a separate file rather than making savevm even bigger?
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> include/exec/cpu-all.h | 4 +-
> migration.c | 7 ++
> qapi-schema.json | 18 +++
> qmp-commands.hx | 30 +++++
> savevm.c | 325 +++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 383 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f91581f..b459301 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -297,13 +297,15 @@ CPUArchState *cpu_copy(CPUArchState *env);
>
> /* memory API */
>
> +#define RAMBLOCK_NAME_LENGTH (1<<8)
Be careful; making this bigger would break migration formats,
making it smaller would probably break migration loading.
> typedef struct RAMBlock {
> struct MemoryRegion *mr;
> uint8_t *host;
> ram_addr_t offset;
> ram_addr_t length;
> uint32_t flags;
> - char idstr[256];
> + char idstr[RAMBLOCK_NAME_LENGTH];
> /* Reads can take either the iothread or the ramlist lock.
> * Writes must take both locks.
> */
> diff --git a/migration.c b/migration.c
> index 8d675b3..e2e313c 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -436,6 +436,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> return;
> }
>
> + if (runstate_check(RUN_STATE_DUMP_BITMAP)) {
> + error_setg(errp, "bitmap dump in progress");
> + return;
> + }
> +
> + runstate_set(RUN_STATE_MIGRATE);
> +
> s = migrate_init(¶ms);
>
> if (strstart(uri, "tcp:", &p)) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 501b8d0..924d6bc 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3485,3 +3485,21 @@
> # Since: 2.1
> ##
> { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @log-dirty-bitmap
> +#
> +# dumps the dirty bitmap to a file by logging the
> +# memory for a specified number of times with a
> +# a defined time differnce
> +#
> +# @filename: name of the file in which the bitmap will be saved.
> +# @epochs: number of times the memory will be logged (optional).
> +# @frequency: time difference in milliseconds between each epoch (optional).
> +#
> +# Since 2.2
> +##
> +{ 'command' : 'log-dirty-bitmap',
> + 'data' : { 'filename' : 'str',
> + '*epochs' : 'int',
> + '*frequency' : 'int' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..200f57e 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3753,5 +3753,35 @@ Example:
>
> -> { "execute": "rtc-reset-reinjection" }
> <- { "return": {} }
> +EQMP
> +
> + {
> + .name = "log-dirty-bitmap",
> + .args_type = "filename:s,epochs:i?,frequency:i?,readable:-r?",
> + .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap,
> + },
> +
> +SQMP
> +log-dirty-bitmap
> +--------------------
> +
> +start logging the memory of the VM for writable working set
> +
> +Arguments:
> +
> +- "filename": name of the file, in which the bitmap will be saved
> +- "epochs": number of times, the memory will be logged
> +- "frequency": time difference in milliseconds between each epoch
> +
> +Examples:
> +-> { "execute" : "log-dirty-bitmap",
> + "arguments" : {
> + "filename" : "/tmp/fileXXX",
> + "epochs" : 3,
> + "frequency" : 10 } }
> +
> +<- { "return": {} }
>
> +Note: The epochs, frequency and readable are optional. epochs default
> +value is 3 while that of frequency is 10.
> EQMP
> diff --git a/savevm.c b/savevm.c
> index e19ae0a..ecb334e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -42,6 +42,9 @@
> #include "qemu/iov.h"
> #include "block/snapshot.h"
> #include "block/qapi.h"
> +#include "exec/address-spaces.h"
> +#include "exec/ram_addr.h"
> +#include "qemu/bitmap.h"
>
>
> #ifndef ETH_P_RARP
> @@ -1137,6 +1140,328 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
> }
>
> +/*
> + * Adding the functionality of continuous logging of the
> + * dirty bitmap which is almost similar to the migration
> + * thread
> + */
> +
> +enum {
> + LOG_BITMAP_STATE_ERROR = -1,
> + LOG_BITMAP_STATE_NONE,
> + LOG_BITMAP_STATE_SETUP,
> + LOG_BITMAP_STATE_ACTIVE,
> + LOG_BITMAP_STATE_CANCELING,
> + LOG_BITMAP_STATE_COMPLETED
> +};
I'd be tempted to give that enum a name and use it as the type
for functions that pass the state around (although I realise
you have to be careful with the variable having to be an int
for the atomic).
> +
> +typedef struct BitmapLogState BitmapLogState;
> +static unsigned long *logging_bitmap;
> +static int64_t MIN_EPOCH_VALUE = 3;
> +static int64_t MIN_FREQUENCY_VALUE = 10;
> +static int64_t MAX_EPOCH_VALUE = 100000;
> +static int64_t MAX_FREQUENCY_VALUE = 100000;
> +
> +struct BitmapLogState {
> + int state;
> + int fd;
> + int64_t current_frequency;
> + int64_t current_epoch;
> + int64_t total_epochs;
> + QemuThread thread;
> +};
> +
> +/*
> + * helper functions
> + */
> +
> +static inline void logging_lock(void)
> +{
> + qemu_mutex_lock_iothread();
> + qemu_mutex_lock_ramlist();
> +}
I wonder how often you can really not have the ramlist locked; if stuff
is added/removed the last_ram_offset would change, so to really be safe
you probably need to hold it for much longer than you currently do -
but that might not be practical.
> +
> +static inline void logging_unlock(void)
> +{
> + qemu_mutex_unlock_ramlist();
> + qemu_mutex_unlock_iothread();
> +}
> +
> +static inline void logging_bitmap_set_dirty(ram_addr_t addr)
> +{
> + int nr = addr >> TARGET_PAGE_BITS;
Be careful; int is too small; long is probably what's
needed (which I think is the type of the parameter to set_bit).
> + set_bit(nr, logging_bitmap);
> +}
> +
> +static bool logging_state_set_status(BitmapLogState *b,
> + int old_state,
> + int new_state)
> +{
> + return atomic_cmpxchg(&b->state, old_state, new_state);
> +}
> +
> +static inline bool value_in_range(int64_t value, int64_t min_value,
> + int64_t max_value, const char *str,
> + Error **errp)
> +{
> + if (value < min_value) {
> + error_setg(errp, "%s's value must be greater than %ld",
> + str, min_value);
> + return false;
> + }
> + if (value > max_value) {
> + error_setg(errp, "%s's value must be less than %ld",
> + str, max_value);
> + return false;
> + }
> + return true;
> +}
This seems a pretty generic function; could it go somewhere
in util or the like?
> +
> +/*
> + * inspired from migration mechanism
> + */
> +
> +static BitmapLogState *logging_current_state(void)
> +{
> + static BitmapLogState current_bitmaplogstate = {
> + .state = LOG_BITMAP_STATE_NONE,
> + };
> +
> + return ¤t_bitmaplogstate;
> +}
> +
> +/*
> + * syncing the logging_bitmap with the ram_list dirty bitmap
> + */
> +
> +static void dirty_bitmap_sync(void)
> +{
> + RAMBlock *block;
> + address_space_sync_dirty_bitmap(&address_space_memory);
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
> + logging_bitmap, false);
> + }
> +}
I think that should be logging_dirty_bitmap_sync since it's
specific to logging_bitmap.
> +
> +static inline void logging_bitmap_close(BitmapLogState *b)
> +{
> + logging_lock();
> + memory_global_dirty_log_stop();
> + logging_unlock();
> +
> + g_free(logging_bitmap);
> + logging_bitmap = NULL;
> + qemu_close(b->fd);
> + b->fd = -1;
> +}
> +
> +static bool ram_block_info_dump(int fd)
> +{
> + int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> + int block_count = 0;
> + RAMBlock *block;
> + int ret;
> +
> + if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
> + return true;
> + }
> +
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + block_count++;
> + }
> +
> + ret = qemu_write_full(fd, &block_count, sizeof(int));
> + if (ret < sizeof(int)) {
> + return true;
> + }
> +
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + ret = qemu_write_full(fd, &(block->idstr), sizeof(char) *
> + RAMBLOCK_NAME_LENGTH);
> + if (ret < sizeof(char) * RAMBLOCK_NAME_LENGTH) {
> + return true;
> + }
> + ret = qemu_write_full(fd, &(block->offset), sizeof(ram_addr_t));
> + if (ret < sizeof(ram_addr_t)) {
> + return true;
> + }
> + ret = qemu_write_full(fd, &(block->length), sizeof(ram_addr_t));
> + if (ret < sizeof(ram_addr_t)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static void logging_state_update_status(BitmapLogState *b)
> +{
> + switch (b->state) {
> + case LOG_BITMAP_STATE_ACTIVE:
> + logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
> + LOG_BITMAP_STATE_COMPLETED);
> + return;
> + case LOG_BITMAP_STATE_CANCELING:
> + logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
> + LOG_BITMAP_STATE_COMPLETED);
> + return;
> + case LOG_BITMAP_STATE_ERROR:
> + logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
> + LOG_BITMAP_STATE_COMPLETED);
> + }
> + return;
> +}
I didn't really see the point of this at first, but I guess
it always moves to 'COMPLETED' unless you're already at completed
or in NONE; but then perhaps:
int s = b->state;
switch (s) {
case LOG_BITMAP_STATE_ACTIVE:
case LOG_BITMAP_STATE_CANCELING:
case LOG_BITMAP_STATE_ERROR:
logging_state_set_status(b, s,
LOG_BITMAP_STATE_COMPLETED);
return;
}
return;
would be more obvious (note I only read the state once)
Dave
> +static void *bitmap_logging_thread(void *opaque)
> +{
> + /*
> + * setup basic structures
> + */
> +
> + BitmapLogState *b = opaque;
> + int fd = b->fd;
> + b->current_epoch = 1;
> + int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> + size_t bitmap_size = BITS_TO_LONGS(ram_bitmap_pages) *
> + sizeof(unsigned long);
> + int ret;
> + char marker = 'M';
> +
> + logging_state_set_status(b, LOG_BITMAP_STATE_NONE,
> + LOG_BITMAP_STATE_SETUP);
> +
> + logging_bitmap = bitmap_new(ram_bitmap_pages);
> +
> + if (logging_bitmap == NULL) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + logging_state_set_status(b, LOG_BITMAP_STATE_SETUP,
> + LOG_BITMAP_STATE_ACTIVE);
> + /*
> + * start the logging period
> + */
> + logging_lock();
> + memory_global_dirty_log_start();
> + dirty_bitmap_sync();
> + bitmap_zero(logging_bitmap, ram_bitmap_pages);
> + logging_unlock();
> +
> + if (ram_block_info_dump(fd)) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + /*
> + * sync the dirty bitmap along with saving it
> + * using the FILE pointer f.
> + */
> + while (b->current_epoch <= b->total_epochs) {
> + if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
> + b->state != LOG_BITMAP_STATE_ACTIVE) {
> + goto log_thread_end;
> + }
> + bitmap_zero(logging_bitmap, ram_bitmap_pages);
> + logging_lock();
> + dirty_bitmap_sync();
> + logging_unlock();
> +
> + ret = qemu_write_full(fd, logging_bitmap, bitmap_size);
> + if (ret < bitmap_size) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + ret = qemu_write_full(fd, &marker, sizeof(char));
> + if (ret < sizeof(char)) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> + g_usleep(b->current_frequency * 1000);
> + b->current_epoch++;
> + }
> +
> + /*
> + * stop the logging period.
> + */
> + log_thread_end:
> + logging_bitmap_close(b);
> + logging_state_update_status(b);
> + runstate_set(RUN_STATE_RUNNING);
> + return NULL;
> +}
> +
> +void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
> + int64_t epochs, bool has_frequency,
> + int64_t frequency, Error **errp)
> +{
> + int fd = -1;
> + BitmapLogState *b = logging_current_state();
> + Error *local_err = NULL;
> + if (runstate_check(RUN_STATE_DUMP_BITMAP) ||
> + b->state == LOG_BITMAP_STATE_ACTIVE ||
> + b->state == LOG_BITMAP_STATE_SETUP ||
> + b->state == LOG_BITMAP_STATE_CANCELING) {
> + error_setg(errp, "dirty bitmap dump in progress");
> + return;
> + }
> +
> + if (!runstate_is_running()) {
> + error_setg(errp, "Guest is not in a running state");
> + return;
> + }
> +
> + runstate_set(RUN_STATE_DUMP_BITMAP);
> + b->state = LOG_BITMAP_STATE_NONE;
> +
> + /*
> + * checking the epoch range
> + */
> + if (!has_epochs) {
> + b->total_epochs = MIN_EPOCH_VALUE;
> + } else if (!value_in_range(epochs, MIN_EPOCH_VALUE,
> + MAX_EPOCH_VALUE, "epoch", &local_err)) {
> + if (local_err) {
> + error_propagate(errp, local_err);
> + }
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + } else {
> + b->total_epochs = epochs;
> + }
> +
> + /*
> + * checking the frequency range
> + */
> + if (!has_frequency) {
> + b->current_frequency = MIN_FREQUENCY_VALUE;
> + } else if (!value_in_range(frequency, MIN_FREQUENCY_VALUE,
> + MAX_FREQUENCY_VALUE, "frequency", &local_err)) {
> + if (local_err) {
> + error_propagate(errp, local_err);
> + }
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + } else {
> + b->current_frequency = frequency;
> + }
> +
> + fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
> + if (fd < 0) {
> + error_setg_file_open(errp, errno, filename);
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + }
> +
> + b->fd = fd;
> + qemu_thread_create(&b->thread, "dirty-bitmap-dump",
> + bitmap_logging_thread, b,
> + QEMU_THREAD_JOINABLE);
> +
> + return;
> +}
> +
> void qmp_xen_save_devices_state(const char *filename, Error **errp)
> {
> QEMUFile *f;
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-18 11:12 ` Dr. David Alan Gilbert
@ 2014-07-18 18:18 ` Sanidhya Kashyap
0 siblings, 0 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 18:18 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Amit Shah, qemu list, Juan Quintela
>> +#define RAMBLOCK_NAME_LENGTH (1<<8)
>
> Be careful; making this bigger would break migration formats,
> making it smaller would probably break migration loading.
>
its the same as previous, will write it explicitly. What I think is that
we should make name_length to be global. I have seen at most of the
places that we are using 256 as length, why can't we use a variable?
>> +
>> +enum {
>> + LOG_BITMAP_STATE_ERROR = -1,
>> + LOG_BITMAP_STATE_NONE,
>> + LOG_BITMAP_STATE_SETUP,
>> + LOG_BITMAP_STATE_ACTIVE,
>> + LOG_BITMAP_STATE_CANCELING,
>> + LOG_BITMAP_STATE_COMPLETED
>> +};
>
> I'd be tempted to give that enum a name and use it as the type
> for functions that pass the state around (although I realise
> you have to be careful with the variable having to be an int
> for the atomic).
>
will see what can be done.
>> +static inline void logging_lock(void)
>> +{
>> + qemu_mutex_lock_iothread();
>> + qemu_mutex_lock_ramlist();
>> +}
>
> I wonder how often you can really not have the ramlist locked; if stuff
> is added/removed the last_ram_offset would change, so to really be safe
> you probably need to hold it for much longer than you currently do -
> but that might not be practical.
>
I didn't realize that. I will try to make suitable changes in my code to
cater to those added/removed last_ram_offset.
>> +static inline void logging_bitmap_set_dirty(ram_addr_t addr)
>> +{
>> + int nr = addr >> TARGET_PAGE_BITS;
>
> Be careful; int is too small; long is probably what's
> needed (which I think is the type of the parameter to set_bit).
>
My mistake. I will rectify it!
>> +static inline bool value_in_range(int64_t value, int64_t min_value,
>> + int64_t max_value, const char *str,
>> + Error **errp)
>> +{
>> + if (value < min_value) {
>> + error_setg(errp, "%s's value must be greater than %ld",
>> + str, min_value);
>> + return false;
>> + }
>> + if (value > max_value) {
>> + error_setg(errp, "%s's value must be less than %ld",
>> + str, max_value);
>> + return false;
>> + }
>> + return true;
>> +}
>
> This seems a pretty generic function; could it go somewhere
> in util or the like?
>
okay.
>> +
>> +static void dirty_bitmap_sync(void)
>> +{
>> + RAMBlock *block;
>> + address_space_sync_dirty_bitmap(&address_space_memory);
>> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> + qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
>> + logging_bitmap, false);
>> + }
>> +}
>
> I think that should be logging_dirty_bitmap_sync since it's
> specific to logging_bitmap.
>
okay. Will change it.
>> +
>> +static void logging_state_update_status(BitmapLogState *b)
>> +{
>> + switch (b->state) {
>> + case LOG_BITMAP_STATE_ACTIVE:
>> + logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
>> + LOG_BITMAP_STATE_COMPLETED);
>> + return;
>> + case LOG_BITMAP_STATE_CANCELING:
>> + logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
>> + LOG_BITMAP_STATE_COMPLETED);
>> + return;
>> + case LOG_BITMAP_STATE_ERROR:
>> + logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
>> + LOG_BITMAP_STATE_COMPLETED);
>> + }
>> + return;
>> +}
>
> I didn't really see the point of this at first, but I guess
> it always moves to 'COMPLETED' unless you're already at completed
> or in NONE; but then perhaps:
>
> int s = b->state;
> switch (s) {
> case LOG_BITMAP_STATE_ACTIVE:
> case LOG_BITMAP_STATE_CANCELING:
> case LOG_BITMAP_STATE_ERROR:
> logging_state_set_status(b, s,
> LOG_BITMAP_STATE_COMPLETED);
> return;
> }
> return;
>
> would be more obvious (note I only read the state once)
>
Yup, will save some code and reduce confusion.
--
-----
Sanidhya Kashyap
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates Sanidhya Kashyap
2014-07-18 11:12 ` Dr. David Alan Gilbert
@ 2014-07-18 11:14 ` Dr. David Alan Gilbert
2014-07-18 18:09 ` Sanidhya Kashyap
2014-07-18 12:20 ` Eric Blake
2 siblings, 1 reply; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:14 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
<snip>
One more I forgot:
> + while (b->current_epoch <= b->total_epochs) {
> + if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
> + b->state != LOG_BITMAP_STATE_ACTIVE) {
> + goto log_thread_end;
> + }
> + bitmap_zero(logging_bitmap, ram_bitmap_pages);
> + logging_lock();
> + dirty_bitmap_sync();
> + logging_unlock();
> +
> + ret = qemu_write_full(fd, logging_bitmap, bitmap_size);
> + if (ret < bitmap_size) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + ret = qemu_write_full(fd, &marker, sizeof(char));
> + if (ret < sizeof(char)) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> + g_usleep(b->current_frequency * 1000);
Be careful; lets say that was set to 20ms, you wouldn't
get 50 dumps a second, since you *add* that delay to the
rest of the time in the loop.
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-18 11:14 ` Dr. David Alan Gilbert
@ 2014-07-18 18:09 ` Sanidhya Kashyap
0 siblings, 0 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 18:09 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu list, Juan Quintela
>
> Be careful; lets say that was set to 20ms, you wouldn't
> get 50 dumps a second, since you *add* that delay to the
> rest of the time in the loop.
>
> Dave
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
yup, true. will definitely change the misleading name and will specify
this point in the documentation as well as in the code.
--
-----
Sanidhya Kashyap
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates Sanidhya Kashyap
2014-07-18 11:12 ` Dr. David Alan Gilbert
2014-07-18 11:14 ` Dr. David Alan Gilbert
@ 2014-07-18 12:20 ` Eric Blake
2 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:20 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Reformatted the code and added the functionality of dumping the blocks' info
> which can be read by the user when required. I have also made the block name
> length global.
> Some minor modification to the structure which is now storing all the
> information.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> include/exec/cpu-all.h | 4 +-
> migration.c | 7 ++
> qapi-schema.json | 18 +++
> qmp-commands.hx | 30 +++++
> savevm.c | 325 +++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 383 insertions(+), 1 deletion(-)
>
> +++ b/qapi-schema.json
> @@ -3485,3 +3485,21 @@
> # Since: 2.1
> ##
> { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @log-dirty-bitmap
> +#
> +# dumps the dirty bitmap to a file by logging the
> +# memory for a specified number of times with a
> +# a defined time differnce
s/differnce/difference/
> +#
> +# @filename: name of the file in which the bitmap will be saved.
> +# @epochs: number of times the memory will be logged (optional).
> +# @frequency: time difference in milliseconds between each epoch (optional).
and their default values?
When I see epoch in relation to programming, my first thought is
1970-01-01 - not something that can be plural. Would that variable be
better named 'iterations'?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 4/8] BitmapLog: hmp interface for dirty bitmap dump
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (2 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 3/8] BitmapLog: bitmap dump code via QAPI framework with runstates Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 11:15 ` Dr. David Alan Gilbert
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process Sanidhya Kashyap
` (5 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
hmp-commands.hx | 16 ++++++++++++++++
hmp.c | 16 ++++++++++++++++
hmp.h | 1 +
3 files changed, 33 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0943b1..575df78 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1788,6 +1788,22 @@ STEXI
show available trace events and their state
ETEXI
+ {
+ .name = "ldb|log-dirty-bitmap",
+ .args_type = "filename:s,epochs:i?,frequency:i?",
+ .params = "filename epochs frequency",
+ .help = "dumps the memory's dirty bitmap to file\n\t\t\t"
+ "filename: name of the file in which the bitmap will be saved\n\t\t\t"
+ "epochs: number of times, the memory will be logged\n\t\t\t"
+ "frequency: time difference in milliseconds between each epoch",
+ .mhandler.cmd = hmp_log_dirty_bitmap,
+ },
+STEXI
+@item ldb or log-dirty-bitmap @var{filename}
+@findex log-dirty-bitmap
+dumps the writable working set of a VM's memory to a file
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 4d1838e..3c8e56d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1318,6 +1318,22 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+ const char *filename = qdict_get_str(qdict, "filename");
+ int64_t epochs = qdict_get_try_int(qdict, "epochs", 3);
+ int64_t frequency = qdict_get_try_int(qdict, "frequency", 10);
+ Error *err = NULL;
+
+ qmp_log_dirty_bitmap(filename, !!epochs, epochs, !!frequency,
+ frequency, &err);
+ if (err) {
+ monitor_printf(mon, "log-dirty-bitmap: %s\n", error_get_pretty(err));
+ error_free(err);
+ return;
+ }
+}
+
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 4fd3c4a..0895182 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 4/8] BitmapLog: hmp interface for dirty bitmap dump
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 4/8] BitmapLog: hmp interface for dirty bitmap dump Sanidhya Kashyap
@ 2014-07-18 11:15 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:15 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> hmp-commands.hx | 16 ++++++++++++++++
> hmp.c | 16 ++++++++++++++++
> hmp.h | 1 +
> 3 files changed, 33 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d0943b1..575df78 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1788,6 +1788,22 @@ STEXI
> show available trace events and their state
> ETEXI
>
> + {
> + .name = "ldb|log-dirty-bitmap",
> + .args_type = "filename:s,epochs:i?,frequency:i?",
> + .params = "filename epochs frequency",
> + .help = "dumps the memory's dirty bitmap to file\n\t\t\t"
> + "filename: name of the file in which the bitmap will be saved\n\t\t\t"
> + "epochs: number of times, the memory will be logged\n\t\t\t"
> + "frequency: time difference in milliseconds between each epoch",
> + .mhandler.cmd = hmp_log_dirty_bitmap,
> + },
> +STEXI
> +@item ldb or log-dirty-bitmap @var{filename}
> +@findex log-dirty-bitmap
> +dumps the writable working set of a VM's memory to a file
> +ETEXI
> +
> STEXI
> @end table
> ETEXI
> diff --git a/hmp.c b/hmp.c
> index 4d1838e..3c8e56d 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1318,6 +1318,22 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> +void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
> +{
> + const char *filename = qdict_get_str(qdict, "filename");
> + int64_t epochs = qdict_get_try_int(qdict, "epochs", 3);
> + int64_t frequency = qdict_get_try_int(qdict, "frequency", 10);
> + Error *err = NULL;
> +
> + qmp_log_dirty_bitmap(filename, !!epochs, epochs, !!frequency,
> + frequency, &err);
Other hmp functions that have optional integer parameters use
qdict_haskey, e.g.
int has_hold_time = qdict_haskey(qdict, "hold-time");
int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
Dave
> + if (err) {
> + monitor_printf(mon, "log-dirty-bitmap: %s\n", error_get_pretty(err));
> + error_free(err);
> + return;
> + }
> +}
> +
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> diff --git a/hmp.h b/hmp.h
> index 4fd3c4a..0895182 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
> void hmp_object_add(Monitor *mon, const QDict *qdict);
> void hmp_object_del(Monitor *mon, const QDict *qdict);
> void hmp_info_memdev(Monitor *mon, const QDict *qdict);
> +void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
> void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
> void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
> void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (3 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 4/8] BitmapLog: hmp interface for dirty bitmap dump Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 12:22 ` Eric Blake
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the " Sanidhya Kashyap
` (4 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
hmp-commands.hx | 14 ++++++++++++++
hmp.c | 5 +++++
hmp.h | 1 +
qapi-schema.json | 8 ++++++++
qmp-commands.hx | 21 +++++++++++++++++++++
savevm.c | 19 +++++++++++++++++++
6 files changed, 68 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 575df78..61eca66 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1804,6 +1804,20 @@ STEXI
dumps the writable working set of a VM's memory to a file
ETEXI
+ {
+ .name = "ldbc|log-dirty-bitmap-cancel",
+ .args_type = "",
+ .params = "",
+ .help = "cancel the current bitmap dump process",
+ .mhandler.cmd = hmp_log_dirty_bitmap_cancel,
+},
+
+STEXI
+@item ldbc or log-dirty-bitmap-cancel
+@findex log-dirty-bitmap-cancel
+Cancel the current bitmap dump process
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 3c8e56d..782f788 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1334,6 +1334,11 @@ void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
}
}
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict)
+{
+ qmp_log_dirty_bitmap_cancel(NULL);
+}
+
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 0895182..12691f9 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 924d6bc..70e07e9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3503,3 +3503,11 @@
'data' : { 'filename' : 'str',
'*epochs' : 'int',
'*frequency' : 'int' } }
+##
+# @log-dirty-bitmap-cancel
+#
+# cancel the dirty bitmap logging process
+#
+# Since 2.2
+##
+{ 'command': 'log-dirty-bitmap-cancel' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 200f57e..69d4a07 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3785,3 +3785,24 @@ Examples:
Note: The epochs, frequency and readable are optional. epochs default
value is 3 while that of frequency is 10.
EQMP
+
+ {
+ .name = "log-dirty-bitmap-cancel",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_cancel,
+ },
+
+SQMP
+log_bitmap_cancel
+--------------
+
+Cancel the current bitmap dump process.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-cancel" }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index ecb334e..b1b0421 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1462,6 +1462,25 @@ void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
return;
}
+static void logging_bitmap_cancel(BitmapLogState *b)
+{
+ int old_state;
+ do {
+ old_state = b->state;
+ if (old_state != LOG_BITMAP_STATE_SETUP &&
+ old_state != LOG_BITMAP_STATE_ACTIVE) {
+ break;
+ }
+ logging_state_set_status(b, old_state,
+ LOG_BITMAP_STATE_CANCELING);
+ } while (b->state != LOG_BITMAP_STATE_CANCELING);
+}
+
+void qmp_log_dirty_bitmap_cancel(Error **errp)
+{
+ logging_bitmap_cancel(logging_current_state());
+}
+
void qmp_xen_save_devices_state(const char *filename, Error **errp)
{
QEMUFile *f;
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process Sanidhya Kashyap
@ 2014-07-18 12:22 ` Eric Blake
2014-07-18 17:51 ` Sanidhya Kashyap
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:22 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> hmp-commands.hx | 14 ++++++++++++++
> hmp.c | 5 +++++
> hmp.h | 1 +
> qapi-schema.json | 8 ++++++++
> qmp-commands.hx | 21 +++++++++++++++++++++
> savevm.c | 19 +++++++++++++++++++
> 6 files changed, 68 insertions(+)
So here you provide HMP and QMP in one patch (fine); but earlier you
separated them between 3/8 and 4/8. Might be nice to be consistent in
the series.
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 575df78..61eca66 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1804,6 +1804,20 @@ STEXI
> dumps the writable working set of a VM's memory to a file
> ETEXI
>
> + {
> + .name = "ldbc|log-dirty-bitmap-cancel",
HMP commands still use _ in their names; it's only QMP where we prefer dash.
> +++ b/qapi-schema.json
> @@ -3503,3 +3503,11 @@
> 'data' : { 'filename' : 'str',
> '*epochs' : 'int',
> '*frequency' : 'int' } }
> +##
> +# @log-dirty-bitmap-cancel
> +#
> +# cancel the dirty bitmap logging process
> +#
> +# Since 2.2
> +##
> +{ 'command': 'log-dirty-bitmap-cancel' }
Correctly named with dash here...
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 200f57e..69d4a07 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3785,3 +3785,24 @@ Examples:
> Note: The epochs, frequency and readable are optional. epochs default
> value is 3 while that of frequency is 10.
> EQMP
> +
> + {
> + .name = "log-dirty-bitmap-cancel",
> + .args_type = "",
> + .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_cancel,
> + },
> +
> +SQMP
> +log_bitmap_cancel
...but completely wrong name here.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process
2014-07-18 12:22 ` Eric Blake
@ 2014-07-18 17:51 ` Sanidhya Kashyap
0 siblings, 0 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 17:51 UTC (permalink / raw)
To: Eric Blake, qemu list; +Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> So here you provide HMP and QMP in one patch (fine); but earlier
> you separated them between 3/8 and 4/8. Might be nice to be
> consistent in the series.
>
Will separate the patch.
>> + { + .name = "ldbc|log-dirty-bitmap-cancel",
>
> HMP commands still use _ in their names; it's only QMP where we
> prefer dash.
>
I didn't notice. Will change it.
- -----
Sanidhya Kashyap
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTyV6sAAoJEFt9RLmoahlnPM8H/38Ey44B5PiWkQwvXlz/xFmm
hJZXsP/icaLJBcTYKBLWHTWUs39wmdeOO0tNpm196nBZlUP+3wRJEgajmUVu4rFg
uM8dNwh6HxGn5Hu4u7fbEfEXh1wHrupApHfxOwoFK9dmdInyvVLfalarwWXfshxu
mxMdm6kq3uz3W+I1pNWiYN2UUG62CLPIpcmJEjAYpWyhouwEJqwPER3CS5K7QxWb
41F5MF6VoFBCKF7VWM36dlFag0yVOOVi1OPytfKT/CBH+kDkNlJyoy8+Oimo/Gkj
8kqgi+QOxI/XJEMAi3gavgUE6vEePLB23M2Dl8p8xV3eBsG25KwM3HJaa0vuUl0=
=AtnI
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the dump bitmap process
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (4 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 5/8] BitmapLog: cancel mechanism for an already running dump bitmap process Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 12:28 ` Eric Blake
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters Sanidhya Kashyap
` (3 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Rectified the example mistake in qmp-commands.hx.
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
hmp-commands.hx | 15 +++++++++++++++
hmp.c | 12 ++++++++++++
hmp.h | 1 +
qapi-schema.json | 13 +++++++++++++
qmp-commands.hx | 24 ++++++++++++++++++++++++
savevm.c | 14 ++++++++++++++
6 files changed, 79 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 61eca66..9f940ab 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1818,6 +1818,21 @@ STEXI
Cancel the current bitmap dump process
ETEXI
+ {
+ .name = "ldbsf|log-dirty-bitmap-set-frequency",
+ .args_type = "frequency:i",
+ .params = "frequency",
+ .help = "set the frequency for bitmap dump process\n\t\t\t"
+ "frequency: the new frequency value to replace the existing",
+ .mhandler.cmd = hmp_log_dirty_bitmap_set_frequency,
+ },
+
+STEXI
+@item ldbsf or log-dirty-bitmap-set-frequency @var{frequency}
+@findex log-dirty-bitmap-set-frequency
+Set the frequency to @var{frequency} (int) for bitmap dump process.
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 782f788..3408709 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1339,6 +1339,18 @@ void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict)
qmp_log_dirty_bitmap_cancel(NULL);
}
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict)
+{
+ int64_t frequency = qdict_get_int(qdict, "frequency");
+ Error *err = NULL;
+ qmp_log_dirty_bitmap_set_frequency(frequency, &err);
+ if (err) {
+ monitor_printf(mon, "log-dirty-bitmap-set-frequency: %s\n",
+ error_get_pretty(err));
+ error_free(err);
+ }
+}
+
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 12691f9..33a2c64 100644
--- a/hmp.h
+++ b/hmp.h
@@ -96,6 +96,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 70e07e9..90977eb 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3511,3 +3511,16 @@
# Since 2.2
##
{ 'command': 'log-dirty-bitmap-cancel' }
+
+##
+# @log-dirty-bitmap-set-frequency
+#
+# sets the frequency of the dirty bitmap logging process
+# @frequency: the updated frequency value (in milliseconds).
+# The min and max values are 10 and 100000 respectively.
+#
+# Since 2.2
+##
+{ 'command': 'log-dirty-bitmap-set-frequency',
+ 'data': {'frequency': 'int' } }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 69d4a07..0a13b74 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3806,3 +3806,27 @@ Example:
<- { "return": {} }
EQMP
+
+ {
+ .name = "log-dirty-bitmap-set-frequency",
+ .args_type = "frequency:i",
+ .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_set_frequency,
+ },
+
+SQMP
+log-dirty-bitmap-set-frequency
+--------------------
+
+Update the frequency for the remaining epochs.
+
+Arguments:
+
+- "frequency": the updated frequency (json-int) (in milliseconds)
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-set-frequency", "arguments": { "frequency": 1024 } }
+<- { "return": {} }
+
+EQMP
+
diff --git a/savevm.c b/savevm.c
index b1b0421..d22771c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1481,6 +1481,20 @@ void qmp_log_dirty_bitmap_cancel(Error **errp)
logging_bitmap_cancel(logging_current_state());
}
+void qmp_log_dirty_bitmap_set_frequency(int64_t frequency, Error **errp)
+{
+ BitmapLogState *b = logging_current_state();
+ Error *local_err = NULL;
+ if (value_in_range(frequency, MIN_FREQUENCY_VALUE,
+ MAX_FREQUENCY_VALUE, "frequency", &local_err)) {
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ }
+ b->current_frequency = frequency;
+}
+
void qmp_xen_save_devices_state(const char *filename, Error **errp)
{
QEMUFile *f;
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the dump bitmap process
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the " Sanidhya Kashyap
@ 2014-07-18 12:28 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:28 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Rectified the example mistake in qmp-commands.hx.
Is this comment about a mistake that existed prior to your series, or is
it a changelog compared to v1-3 of your posts? If the former, it might
be nice to say which existing commit was buggy (so we know how long the
docs have been broken); if the latter, then please make it obvious that
it is a patch revision changelog by moving the comment out of the commit
message...
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
...and sticking it here. Remember, this is the location for comments
that aid review for someone that has looked at earlier revisions of your
series, but which add no value to the qemu.git log a year from now when
only the final version of your series is in place.
> hmp-commands.hx | 15 +++++++++++++++
> hmp.c | 12 ++++++++++++
> hmp.h | 1 +
> qapi-schema.json | 13 +++++++++++++
> qmp-commands.hx | 24 ++++++++++++++++++++++++
> savevm.c | 14 ++++++++++++++
> 6 files changed, 79 insertions(+)
Given that your patch is purely additive, I conclude that your commit
comment is in relation to your v1-3 postings of the series, and thus
misplaced.
> +++ b/qapi-schema.json
> @@ -3511,3 +3511,16 @@
> # Since 2.2
> ##
> { 'command': 'log-dirty-bitmap-cancel' }
> +
> +##
> +# @log-dirty-bitmap-set-frequency
> +#
> +# sets the frequency of the dirty bitmap logging process
> +# @frequency: the updated frequency value (in milliseconds).
> +# The min and max values are 10 and 100000 respectively.
> +#
> +# Since 2.2
> +##
> +{ 'command': 'log-dirty-bitmap-set-frequency',
> + 'data': {'frequency': 'int' } }
I hate write-only commands. Where is the counterpart query command that
I can use to learn if I am currently logging, and what the current
logging frequency is?
> +
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 69d4a07..0a13b74 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3806,3 +3806,27 @@ Example:
> <- { "return": {} }
>
> EQMP
> +
> + {
> + .name = "log-dirty-bitmap-set-frequency",
> + .args_type = "frequency:i",
> + .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_set_frequency,
> + },
> +
> +SQMP
> +log-dirty-bitmap-set-frequency
> +--------------------
Nit - the separator line is typically the same length as the command
name, for better visual appeal.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (5 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the " Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 12:35 ` Eric Blake
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 8/8] BitmapLog: python script for extracting bitmap from a binary file Sanidhya Kashyap
` (2 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Added the qmp interface to know about the information of the bitmap dump
process. When the command is executed, one can know about the current epoch
value in which the process is in, the total iterations value and the current
frequency value.
Currently, I do not think that that one needs to modify the other values
except frequency, so that is why I have not added the generic set-tuning
interface. If required, I will add it.
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
qapi-schema.json | 31 +++++++++++++++++++++++++++++++
qmp-commands.hx | 24 ++++++++++++++++++++++++
savevm.c | 26 ++++++++++++++++++++++++++
3 files changed, 81 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 90977eb..1b09235d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3487,6 +3487,24 @@
{ 'command': 'rtc-reset-reinjection' }
##
+# @BitmapLogStateInfo
+#
+# Provides information for the bitmap logging process
+#
+# @currepoch: provides the value of the running epoch value
+#
+# @epochs: provides the information about the actual epoch
+#
+# @frequency: the time difference in milliseconds between each epcoh
+#
+# Since 2.2
+##
+{ 'type': 'BitmapLogStateInfo',
+ 'data': { 'currepoch': 'int',
+ 'epochs': 'int',
+ 'frequency': 'int' } }
+
+##
# @log-dirty-bitmap
#
# dumps the dirty bitmap to a file by logging the
@@ -3524,3 +3542,16 @@
{ 'command': 'log-dirty-bitmap-set-frequency',
'data': {'frequency': 'int' } }
+##
+# @log-dirty-bitmap-get-tuning
+#
+# Get the current values of the parameters involved in bitmap logging process
+#
+# This command returns the following elements in the form of BitmapLogStateInfo:
+# - currepoch: current epoch value
+# - epochs: total epochs for which the bitmap dumping will continue
+# - frequently: the current sleep interval between each epoch
+#
+# Since 2.2
+##
+{ 'command': 'log-dirty-bitmap-get-tuning', 'returns': 'BitmapLogStateInfo' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0a13b74..3bd60e1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3830,3 +3830,27 @@ Example:
EQMP
+ {
+ .name = "log-dirty-bitmap-get-tuning",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_get_tuning,
+ },
+
+SQMP
+log-dirty-bitmap-get-tuning
+--------------------
+
+Get the parameters information
+
+- "currepoch": the current epoch going on
+- "epochs" the total number of assigned epochs
+- "frequency": the sleep interval between each epoch (in milliseconds)
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-get-tuning" }
+<- { "return": {
+ "currepoch": 3
+ "epochs": 100
+ "frequency": 100 } }
+EQMP
diff --git a/savevm.c b/savevm.c
index d22771c..260c919 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1495,6 +1495,32 @@ void qmp_log_dirty_bitmap_set_frequency(int64_t frequency, Error **errp)
b->current_frequency = frequency;
}
+BitmapLogStateInfo *qmp_log_dirty_bitmap_get_tuning(Error **errp)
+{
+ BitmapLogState *b = logging_current_state();
+ BitmapLogStateInfo *info = NULL;
+
+ if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
+ b->state != LOG_BITMAP_STATE_ACTIVE) {
+ return info;
+ }
+
+ info = g_malloc0(sizeof(BitmapLogStateInfo));
+ info->currepoch = b->current_epoch;
+ info->epochs = b->total_epochs;
+ info->frequency = b->current_frequency;
+
+ if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
+ b->state != LOG_BITMAP_STATE_ACTIVE) {
+ g_free(info);
+ info = NULL;
+ error_setg(errp, "Dirty bitmap dump process is not running\n");
+ return info;
+ };
+
+ return info;
+}
+
void qmp_xen_save_devices_state(const char *filename, Error **errp)
{
QEMUFile *f;
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters Sanidhya Kashyap
@ 2014-07-18 12:35 ` Eric Blake
2014-07-18 17:41 ` Sanidhya Kashyap
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:35 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 3915 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Added the qmp interface to know about the information of the bitmap dump
> process. When the command is executed, one can know about the current epoch
> value in which the process is in, the total iterations value and the current
> frequency value.
>
> Currently, I do not think that that one needs to modify the other values
> except frequency, so that is why I have not added the generic set-tuning
> interface. If required, I will add it.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
> qapi-schema.json | 31 +++++++++++++++++++++++++++++++
> qmp-commands.hx | 24 ++++++++++++++++++++++++
> savevm.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+)
Ah, this answers one of my questions in 6/8. I would reorder the series
and put this right after 3/8 (being able to query initial values is
still useful, whether or not we also add on-the-fly tuning commands).
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 90977eb..1b09235d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3487,6 +3487,24 @@
> { 'command': 'rtc-reset-reinjection' }
>
> ##
> +# @BitmapLogStateInfo
> +#
> +# Provides information for the bitmap logging process
> +#
> +# @currepoch: provides the value of the running epoch value
No need to abbreviate. And given my naming question in 3/8, would this
be better as: current-iteration
> +#
> +# @epochs: provides the information about the actual epoch
and iterations
Wait. Is this number a constant (the number requested when logging
started, no matter what currepoch is)...[1]
> +#
> +# @frequency: the time difference in milliseconds between each epcoh
s/epcoh/epoch/ (or iteration)
> +#
> +# Since 2.2
> +##
> +{ 'type': 'BitmapLogStateInfo',
> + 'data': { 'currepoch': 'int',
> + 'epochs': 'int',
> + 'frequency': 'int' } }
> +
> +##
> # @log-dirty-bitmap
> #
> # dumps the dirty bitmap to a file by logging the
> @@ -3524,3 +3542,16 @@
> { 'command': 'log-dirty-bitmap-set-frequency',
> 'data': {'frequency': 'int' } }
>
> +##
> +# @log-dirty-bitmap-get-tuning
Most query interfaces are in the query- namespace. Maybe this would be
better as query-log-dirty-bitmap?
> +#
> +# Get the current values of the parameters involved in bitmap logging process
Accidental double space.
> +#
> +# This command returns the following elements in the form of BitmapLogStateInfo:
> +# - currepoch: current epoch value
> +# - epochs: total epochs for which the bitmap dumping will continue
...[1]or is it the number of iterations remaining (that is,
currepoch+epochs is the original value, and both numbers are changing as
iterations progress)? Rather than duplicate the parameters in two
places (and with different information, which led to my question about
semantics), it might be nicer to just mention here that the information
is returned as a BitmapLogStateInfo and be done with it (the user can go
look up the documentation of that struct).
> +# - frequently: the current sleep interval between each epoch
s/frequently/frequency/
> +#
> +# Since 2.2
> +##
> +{ 'command': 'log-dirty-bitmap-get-tuning', 'returns': 'BitmapLogStateInfo' }
> +Get the parameters information
> +
> +- "currepoch": the current epoch going on
> +- "epochs" the total number of assigned epochs
> +- "frequency": the sleep interval between each epoch (in milliseconds)
> +
> +Example:
> +
> +-> { "execute": "log-dirty-bitmap-get-tuning" }
> +<- { "return": {
> + "currepoch": 3
> + "epochs": 100
> + "frequency": 100 } }
Invalid JSON. You are missing two commas.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters
2014-07-18 12:35 ` Eric Blake
@ 2014-07-18 17:41 ` Sanidhya Kashyap
0 siblings, 0 replies; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 17:41 UTC (permalink / raw)
To: Eric Blake, qemu list; +Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
>
> No need to abbreviate. And given my naming question in 3/8, would
> this be better as: current-iteration
>
Okay, I will surely modify it.
>> +# +# @epochs: provides the information about the actual epoch
>
> and iterations
>
> Wait. Is this number a constant (the number requested when
> logging started, no matter what currepoch is)...[1]
>
yup.
> Most query interfaces are in the query- namespace. Maybe this
> would be better as query-log-dirty-bitmap?
>
My bad, I didn't look others clearly. Since, you pointed out to use
get-tuning and set-tuning in one of my other patches; so, I used
get-tuning. Will make the relevant changes.
>> +# +# Get the current values of the parameters involved in
>> bitmap logging process
>
> Accidental double space.
>
>> +# +# This command returns the following elements in the form of
>> BitmapLogStateInfo: +# - currepoch: current epoch value +# -
>> epochs: total epochs for which the bitmap dumping will continue
>
> ...[1]or is it the number of iterations remaining (that is,
> currepoch+epochs is the original value, and both numbers are
> changing as iterations progress)? Rather than duplicate the
> parameters in two
No, currepoch changes while the epoch is a fix value. Need to change
the names :-/. They are confusing.
> places (and with different information, which led to my question
> about semantics), it might be nicer to just mention here that the
> information is returned as a BitmapLogStateInfo and be done with it
> (the user can go look up the documentation of that struct).
>
Will do that.
- --
- -----
Sanidhya Kashyap
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTyVwyAAoJEFt9RLmoahlnS1QH/36mLxnS3sblvYI6v7fJxAF0
bklZpcUKVmanLaOTY498FD8kZN2//Q5rPlJis2AiMAyZ7I1zz4JdPjW+hfjRI+Lb
KgewG4T6tP3zPzponD4EUcIrBOLlgTIuNwP7Rd+8Cpy37MkFm3MyUqU4kVwS98wS
MY99/UBEMkX/+IXLANAOjOmTIjwgg/HU2NyhMUFbR4tiYWdQCtozfr+/uBfz2tBg
AMTy3gkPTMvB/frXovBN+V2KfdJPn14oaBxdfms94SSwmsfOz1ot4j0nx1MuwoPW
XOL6t/P9W1w3V43av/NG+U2CUPo8NCb5S1MmZse5oqx1CPvbhoDIlYOySmp//Pg=
=IyAr
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 8/8] BitmapLog: python script for extracting bitmap from a binary file
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (6 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 7/8] BitmapLog: get the information about the parameters Sanidhya Kashyap
@ 2014-07-17 11:21 ` Sanidhya Kashyap
2014-07-18 11:17 ` Dr. David Alan Gilbert
2014-07-18 10:56 ` [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Dr. David Alan Gilbert
2014-07-18 12:39 ` Eric Blake
9 siblings, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-17 11:21 UTC (permalink / raw)
To: qemu list
Cc: Amit Shah, Sanidhya Kashyap, Dr. David Alan Gilbert,
Juan Quintela
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
| 97 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
create mode 100755 scripts/extract-bitmap.py
--git a/scripts/extract-bitmap.py b/scripts/extract-bitmap.py
new file mode 100755
index 0000000..5edc49d
--- /dev/null
+++ b/scripts/extract-bitmap.py
@@ -0,0 +1,97 @@
+#!/usr/bin/python
+# This python script helps in extracting the dirty bitmap present
+# in the file after executing the log-dirty-bitmap command either
+# from the qmp or hmp interface. This file only processes binary
+# file obtained via command.
+#
+# Copyright (C) 2014 Sanidhya Kashyap <sanidhya.iiith@gmail.com>
+#
+# Authors:
+# Sanidhya Kashyap
+#
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+
+import struct
+import argparse
+from functools import partial
+
+long_bytes = 8
+byte_size = 8
+int_bytes = 4
+string_bytes = 256
+complete_bitmap_list = []
+block_list = []
+
+def get_unsigned_long_integer(value):
+ return struct.unpack('<Q', value)[0]
+
+def get_long_integer(value):
+ return struct.unpack('<q', value)[0]
+
+def get_integer(value):
+ return struct.unpack('<i', value)[0]
+
+def get_char(value):
+ return struct.unpack('<c', value)[0]
+
+def get_string(value, length):
+ name = struct.unpack('<'+str(length)+'s', value)[0]
+ for i in range(len(name)):
+ if name[i] == '\x00':
+ return name[:i]
+
+def dump_ram_block_info(infile):
+ total_blocks = get_integer(infile.read(int_bytes))
+ for i in range(total_blocks):
+ block_name = get_string(infile.read(string_bytes), string_bytes)
+ block_offset = get_unsigned_long_integer(infile.read(long_bytes))
+ block_length = get_unsigned_long_integer(infile.read(long_bytes))
+ block_list.append(dict(name=block_name, offset=block_offset, length=block_length))
+ print block_list
+
+def dump_bitmap(infile, bitmap_length):
+ marker = 'M'
+ count = 0
+ value = ' '
+ while True:
+ if len(value) == 0 or marker != 'M':
+ print len(complete_bitmap_list)
+ print "issue with the dump"
+ return
+ bitmap_list = []
+ bitmap_raw_value = infile.read(long_bytes * bitmap_length)
+ if not bitmap_raw_value:
+ print len(bitmap_raw_value)
+ break
+ count+=1
+ for i in range(bitmap_length):
+ mark = i * long_bytes
+ bitmap_list.append(hex(get_unsigned_long_integer(bitmap_raw_value[mark:mark+long_bytes])))
+ complete_bitmap_list.append(bitmap_list)
+ value = infile.read(1)
+ marker = get_char(value)
+ print complete_bitmap_list
+
+def main():
+ extracter = argparse.ArgumentParser(description='Extract dirty bitmap from binary file.')
+ extracter.add_argument('infile', help='Input file to extract the bitmap')
+ args = extracter.parse_args()
+ print 'The filename is {}'.format(args.infile)
+
+ infile = open(format(args.infile), 'rb')
+
+ ram_bitmap_pages = get_long_integer(infile.read(long_bytes))
+ print ram_bitmap_pages
+ dump_ram_block_info(infile)
+ bitmap_length = ram_bitmap_pages / (long_bytes * byte_size)
+ if ram_bitmap_pages % (long_bytes * byte_size) != 0:
+ bitmap_length += 1
+ print bitmap_length
+
+ dump_bitmap(infile, bitmap_length);
+
+ infile.close()
+
+if __name__ == '__main__':
+ main()
--
1.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 8/8] BitmapLog: python script for extracting bitmap from a binary file
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 8/8] BitmapLog: python script for extracting bitmap from a binary file Sanidhya Kashyap
@ 2014-07-18 11:17 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 11:17 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
I'm not really a python-person, but one observation:
> +def get_string(value, length):
> + name = struct.unpack('<'+str(length)+'s', value)[0]
> + for i in range(len(name)):
> + if name[i] == '\x00':
> + return name[:i]
Would it have been easier for you to format the strings in the stream
the same way the migration does it; i.e. a length byte followed by the text
of the string?
Dave
> +
> +def dump_ram_block_info(infile):
> + total_blocks = get_integer(infile.read(int_bytes))
> + for i in range(total_blocks):
> + block_name = get_string(infile.read(string_bytes), string_bytes)
> + block_offset = get_unsigned_long_integer(infile.read(long_bytes))
> + block_length = get_unsigned_long_integer(infile.read(long_bytes))
> + block_list.append(dict(name=block_name, offset=block_offset, length=block_length))
> + print block_list
> +
> +def dump_bitmap(infile, bitmap_length):
> + marker = 'M'
> + count = 0
> + value = ' '
> + while True:
> + if len(value) == 0 or marker != 'M':
> + print len(complete_bitmap_list)
> + print "issue with the dump"
> + return
> + bitmap_list = []
> + bitmap_raw_value = infile.read(long_bytes * bitmap_length)
> + if not bitmap_raw_value:
> + print len(bitmap_raw_value)
> + break
> + count+=1
> + for i in range(bitmap_length):
> + mark = i * long_bytes
> + bitmap_list.append(hex(get_unsigned_long_integer(bitmap_raw_value[mark:mark+long_bytes])))
> + complete_bitmap_list.append(bitmap_list)
> + value = infile.read(1)
> + marker = get_char(value)
> + print complete_bitmap_list
> +
> +def main():
> + extracter = argparse.ArgumentParser(description='Extract dirty bitmap from binary file.')
> + extracter.add_argument('infile', help='Input file to extract the bitmap')
> + args = extracter.parse_args()
> + print 'The filename is {}'.format(args.infile)
> +
> + infile = open(format(args.infile), 'rb')
> +
> + ram_bitmap_pages = get_long_integer(infile.read(long_bytes))
> + print ram_bitmap_pages
> + dump_ram_block_info(infile)
> + bitmap_length = ram_bitmap_pages / (long_bytes * byte_size)
> + if ram_bitmap_pages % (long_bytes * byte_size) != 0:
> + bitmap_length += 1
> + print bitmap_length
> +
> + dump_bitmap(infile, bitmap_length);
> +
> + infile.close()
> +
> +if __name__ == '__main__':
> + main()
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (7 preceding siblings ...)
2014-07-17 11:21 ` [Qemu-devel] [PATCH v4 8/8] BitmapLog: python script for extracting bitmap from a binary file Sanidhya Kashyap
@ 2014-07-18 10:56 ` Dr. David Alan Gilbert
2014-07-18 13:42 ` Eric Blake
2014-07-18 17:28 ` Sanidhya Kashyap
2014-07-18 12:39 ` Eric Blake
9 siblings, 2 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 10:56 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Hi,
>
> The following patches introduce the support of the dirty bitmap logging and
> dumping to a specified file. This patch addresses the previous issues raised
> by David and Juan. Since, I have not received any comments on the runstates,
> I'll keep them in the patch series.
Hi Sanidhya,
I've finally got around to reading through your patches. Some general
thoughts first, and I'll put some more detail on the patches themselves:
1) Do you have some examples of output?
Can you see hot areas in the kernel or something else?
2) 'frequency' is probably the wrong name for the parameter you have; since
the parameter is actually the delay between epochs, where as frequency
would imply the number of epochs/second.
3) The change to runstates is interesting; up until now 'runstate' is really
mostly about the state of the CPU, but by adding migration/dumping to the
states you're trying to convey more in that single state variable; I'm not
quite sure if this is the right thing to do or not.
Dave
>
> v3 --> v4
> * Added new qmp interface for information extraction from the bitmap process
>
> v2 --> v3
> * Reformatted the code and removed some unnecessary parts.
> * Printing block info along with length and offset.
> * Changed the functions that were directly using RUN_STATE_RUNNING as state.
>
> v1 --> v2:
> * Added two new run states to avoid simultaneous execution of both migration and
> bitmap dump process.
> * Removed FILE pointer usage.
> * Dumping the data only in machine-readable format.
> * Tried to rectify mistakes of the previous version.
>
>
>
> Sanidhya Kashyap (8):
> enable sharing of the function between migration and bitmap dump
> RunState: added two new flags for bitmap dump and migration process
> BitmapLog: bitmap dump code via QAPI framework with runstates
> BitmapLog: hmp interface for dirty bitmap dump
> BitmapLog: cancel mechanism for an already running dump bitmap process
> BitmapLog: set the frequency of the dump bitmap process
> BitmapLog: get the information about the parameters
> BitmapLog: python script for extracting bitmap from a binary file
>
> arch_init.c | 19 ++-
> hmp-commands.hx | 45 ++++++
> hmp.c | 33 ++++
> hmp.h | 3 +
> hw/usb/hcd-ehci.c | 2 +-
> hw/usb/redirect.c | 6 +-
> include/exec/cpu-all.h | 4 +-
> include/exec/ram_addr.h | 4 +
> migration.c | 7 +
> qapi-schema.json | 77 +++++++++-
> qmp-commands.hx | 99 ++++++++++++
> savevm.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++
> scripts/extract-bitmap.py | 97 ++++++++++++
> vl.c | 29 +++-
> 14 files changed, 794 insertions(+), 15 deletions(-)
> create mode 100755 scripts/extract-bitmap.py
>
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging
2014-07-18 10:56 ` [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Dr. David Alan Gilbert
@ 2014-07-18 13:42 ` Eric Blake
2014-07-18 17:28 ` Sanidhya Kashyap
1 sibling, 0 replies; 30+ messages in thread
From: Eric Blake @ 2014-07-18 13:42 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Sanidhya Kashyap
Cc: Amit Shah, qemu list, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]
On 07/18/2014 04:56 AM, Dr. David Alan Gilbert wrote:
> * Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
>> Hi,
>>
>> The following patches introduce the support of the dirty bitmap logging and
>> dumping to a specified file. This patch addresses the previous issues raised
>> by David and Juan. Since, I have not received any comments on the runstates,
>> I'll keep them in the patch series.
>
> Hi Sanidhya,
> I've finally got around to reading through your patches. Some general
> thoughts first, and I'll put some more detail on the patches themselves:
>
> 1) Do you have some examples of output?
> Can you see hot areas in the kernel or something else?
>
> 2) 'frequency' is probably the wrong name for the parameter you have; since
> the parameter is actually the delay between epochs, where as frequency
> would imply the number of epochs/second.
Agreed, please s/frequency/period/ throughout.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging
2014-07-18 10:56 ` [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Dr. David Alan Gilbert
2014-07-18 13:42 ` Eric Blake
@ 2014-07-18 17:28 ` Sanidhya Kashyap
2014-07-18 17:42 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 30+ messages in thread
From: Sanidhya Kashyap @ 2014-07-18 17:28 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Amit Shah, qemu list, Juan Quintela
>
> 1) Do you have some examples of output?
> Can you see hot areas in the kernel or something else?
>
I looked at it sometimes back. I will try to provide some images (as
suggested by you earlier) to see any consistently used areas.
> 2) 'frequency' is probably the wrong name for the parameter you have; since
> the parameter is actually the delay between epochs, where as frequency
> would imply the number of epochs/second.
>
will surely change it to period. It is pointed out by Eric too.
> 3) The change to runstates is interesting; up until now 'runstate' is really
> mostly about the state of the CPU, but by adding migration/dumping to the
> states you're trying to convey more in that single state variable; I'm not
> quite sure if this is the right thing to do or not.
>
Is there any alternative that can allow us to execute only one of the
processes - either migration or bitmap dump. One very basic approach
that I have thought is about using a global variable but don't know
whether that is a good option or not.
Any other alternative is welcomed.
-----
Sanidhya Kashyap
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging
2014-07-18 17:28 ` Sanidhya Kashyap
@ 2014-07-18 17:42 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 30+ messages in thread
From: Dr. David Alan Gilbert @ 2014-07-18 17:42 UTC (permalink / raw)
To: Sanidhya Kashyap; +Cc: Amit Shah, qemu list, Juan Quintela
* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> >
> > 1) Do you have some examples of output?
> > Can you see hot areas in the kernel or something else?
> >
>
> I looked at it sometimes back. I will try to provide some images (as
> suggested by you earlier) to see any consistently used areas.
>
> > 2) 'frequency' is probably the wrong name for the parameter you have; since
> > the parameter is actually the delay between epochs, where as frequency
> > would imply the number of epochs/second.
> >
>
> will surely change it to period. It is pointed out by Eric too.
>
> > 3) The change to runstates is interesting; up until now 'runstate' is really
> > mostly about the state of the CPU, but by adding migration/dumping to the
> > states you're trying to convey more in that single state variable; I'm not
> > quite sure if this is the right thing to do or not.
> >
>
> Is there any alternative that can allow us to execute only one of the
> processes - either migration or bitmap dump. One very basic approach
> that I have thought is about using a global variable but don't know
> whether that is a good option or not.
>
> Any other alternative is welcomed.
I think a global variable is the right thing; something like 'dirty_bitmap_user'
being an enum between none, migration, dumping would make sense to me.
While adding global variables ia a bad thing, since the dirty bitmap already is
global all you're doing really is adding an extra bit of state to it.
If you have something like
char *dirty_bitmap_user_as_string()
then your test in your qml code would look something like:
if (dirty_bitmap_user != dbu_none) {
error_report("Can't do dirty dumping since %s is in use", dirty_bitmap_user_as_string());
}
and then if more things use it in the future the existing users don't
have to change.
Dave
>
> -----
>
> Sanidhya Kashyap
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging
2014-07-17 11:21 [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Sanidhya Kashyap
` (8 preceding siblings ...)
2014-07-18 10:56 ` [Qemu-devel] [PATCH v4 0/8] Obtain dirty bitmap via VM logging Dr. David Alan Gilbert
@ 2014-07-18 12:39 ` Eric Blake
9 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2014-07-18 12:39 UTC (permalink / raw)
To: Sanidhya Kashyap, qemu list
Cc: Amit Shah, Dr. David Alan Gilbert, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]
On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Hi,
>
> The following patches introduce the support of the dirty bitmap logging and
> dumping to a specified file. This patch addresses the previous issues raised
> by David and Juan. Since, I have not received any comments on the runstates,
> I'll keep them in the patch series.
I'm really not sure you need to be introducing a new runstate. I'm not
an expert in runstates, but it feels fishy to me - adding a new state
causes all sorts of complications (can I pause a domain that is logging
a bitmap? If I resume a paused domain, does it resume back to normal
running or back to logging?). We _don't_ have a new state for block
jobs, and this feels similar to a block job. Remember, it is still
possible to block migration for the duration of certain block jobs. If
that was your goal (prevent migration and logging at the same time,
because they stomp over each other's use of the dirty bitmap), then I
think it would be sufficient to have this behave more like a block job
that can inhibit migration, but not as an entirely new runstate.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread