[PULL,15/18] qapi: introduce x-query-ramblock QMP command

This is a counterpart to the HMP "info ramblock" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hmp-commands-info.hx   |  2 +-
 include/exec/ramlist.h |  2 +-
 monitor/hmp-cmds.c     |  6 ------
 monitor/qmp-cmds.c     |  8 ++++++++
 qapi/machine.json      | 12 ++++++++++++
 softmmu/physmem.c      | 19 +++++++++++--------
 6 files changed, 33 insertions(+), 16 deletions(-)

Hello all,

it would be really good to be able to rely on this command or something similar,
to be able to know the approximate size of a migration before starting it.

in QEMU ram_bytes_total() returns what I would like to have,
but there is currently no QMP way to get it without starting a migration,
which when trying to optimize it/size it is just about too late.

Do you think x-query-ramblock could be promoted to non-experimental?

Should another one be made available instead, like :
query-ram-bytes-total ?

Thanks,

Claudio


On 11/2/21 18:56, Daniel P. Berrangé wrote:
> This is a counterpart to the HMP "info ramblock" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> The existing HMP command is rewritten to call the QMP command.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hmp-commands-info.hx   |  2 +-
>  include/exec/ramlist.h |  2 +-
>  monitor/hmp-cmds.c     |  6 ------
>  monitor/qmp-cmds.c     |  8 ++++++++
>  qapi/machine.json      | 12 ++++++++++++
>  softmmu/physmem.c      | 19 +++++++++++--------
>  6 files changed, 33 insertions(+), 16 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9af216473..c2d7275bf5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -772,7 +772,7 @@ ERST
>          .args_type  = "",
>          .params     = "",
>          .help       = "Display system ramblock information",
> -        .cmd        = hmp_info_ramblock,
> +        .cmd_info_hrt = qmp_x_query_ramblock,
>      },
>  
>  SRST
> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> index ece6497ee2..2ad2a81acc 100644
> --- a/include/exec/ramlist.h
> +++ b/include/exec/ramlist.h
> @@ -80,6 +80,6 @@ void ram_block_notify_add(void *host, size_t size, size_t max_size);
>  void ram_block_notify_remove(void *host, size_t size, size_t max_size);
>  void ram_block_notify_resize(void *host, size_t old_size, size_t new_size);
>  
> -void ram_block_dump(Monitor *mon);
> +GString *ram_block_format(void);
>  
>  #endif /* RAMLIST_H */
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 9d221622d7..90f9a64573 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -52,7 +52,6 @@
>  #include "ui/console.h"
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
> -#include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
> @@ -2176,11 +2175,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
>      qapi_free_RockerOfDpaGroupList(list);
>  }
>  
> -void hmp_info_ramblock(Monitor *mon, const QDict *qdict)
> -{
> -    ram_block_dump(mon);
> -}
> -
>  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
>  {
>      Error *err = NULL;
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 0a9ba7595c..a9766fa38d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -38,6 +38,7 @@
>  #include "qapi/qapi-commands-ui.h"
>  #include "qapi/type-helpers.h"
>  #include "qapi/qmp/qerror.h"
> +#include "exec/ramlist.h"
>  #include "hw/mem/memory-device.h"
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "hw/rdma/rdma.h"
> @@ -414,3 +415,10 @@ HumanReadableText *qmp_x_query_rdma(Error **errp)
>  
>      return human_readable_text_from_str(buf);
>  }
> +
> +HumanReadableText *qmp_x_query_ramblock(Error **errp)
> +{
> +    g_autoptr(GString) buf = ram_block_format();
> +
> +    return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 1b2748c77a..be81170c2b 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1436,6 +1436,18 @@
>  { 'command': 'x-query-profile',
>    'returns': 'HumanReadableText' }
>  
> +##
> +# @x-query-ramblock:
> +#
> +# Query system ramblock information
> +#
> +# Returns: system ramblock information
> +#
> +# Since: 6.2
> +##
> +{ 'command': 'x-query-ramblock',
> +  'returns': 'HumanReadableText' }
> +
>  ##
>  # @x-query-rdma:
>  #
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index b9a8c1d1f4..314f8b439c 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1296,23 +1296,26 @@ void qemu_mutex_unlock_ramlist(void)
>      qemu_mutex_unlock(&ram_list.mutex);
>  }
>  
> -void ram_block_dump(Monitor *mon)
> +GString *ram_block_format(void)
>  {
>      RAMBlock *block;
>      char *psize;
> +    GString *buf = g_string_new("");
>  
>      RCU_READ_LOCK_GUARD();
> -    monitor_printf(mon, "%24s %8s  %18s %18s %18s\n",
> -                   "Block Name", "PSize", "Offset", "Used", "Total");
> +    g_string_append_printf(buf, "%24s %8s  %18s %18s %18s\n",
> +                           "Block Name", "PSize", "Offset", "Used", "Total");
>      RAMBLOCK_FOREACH(block) {
>          psize = size_to_str(block->page_size);
> -        monitor_printf(mon, "%24s %8s  0x%016" PRIx64 " 0x%016" PRIx64
> -                       " 0x%016" PRIx64 "\n", block->idstr, psize,
> -                       (uint64_t)block->offset,
> -                       (uint64_t)block->used_length,
> -                       (uint64_t)block->max_length);
> +        g_string_append_printf(buf, "%24s %8s  0x%016" PRIx64 " 0x%016" PRIx64
> +                               " 0x%016" PRIx64 "\n", block->idstr, psize,
> +                               (uint64_t)block->offset,
> +                               (uint64_t)block->used_length,
> +                               (uint64_t)block->max_length);
>          g_free(psize);
>      }
> +
> +    return buf;
>  }
>  
>  #ifdef __linux__

On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> Hello all,
> 
> it would be really good to be able to rely on this command or something similar,
> to be able to know the approximate size of a migration before starting it.
> 
> in QEMU ram_bytes_total() returns what I would like to have,
> but there is currently no QMP way to get it without starting a migration,
> which when trying to optimize it/size it is just about too late.

Aside from the main VM RAM, what other RAM blocks are likely to have
a size large enough to be of consequence to the live migration
data copy, and whose size is not already known to the mgmt app from
the guest config choices it made ? VGA RAM could be a few 100MB I
guess, but the mgmt app knows about that. I've always assumed everything
else is just noise in comparison to the main RAM region.

Still I wonder how useful this is as its just a static figure, and the
problems with migration transfer are the bulking up of data when the
VM is repeatedly dirtying stuff at a high rate.

> Do you think x-query-ramblock could be promoted to non-experimental?

It would have to be re-written, as this current impl is just emitting
a huge printf formatted string. To be considered supportable, the data
would have to be formally modelled in QAPI instead.

IOW, it would be a case of introducing a new command that emits formal
data, convertintg 'info ramblock' to use that, and then deprecating this 
x-query-ramblock.

> Should another one be made available instead, like :
> query-ram-bytes-total ?

That would be simpler if you're just wanting it to give a single
figure.


With regards,
Daniel

On 09.06.22 12:19, Daniel P. Berrangé wrote:
> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>> Hello all,
>>
>> it would be really good to be able to rely on this command or something similar,
>> to be able to know the approximate size of a migration before starting it.
>>
>> in QEMU ram_bytes_total() returns what I would like to have,
>> but there is currently no QMP way to get it without starting a migration,
>> which when trying to optimize it/size it is just about too late.
> 
> Aside from the main VM RAM, what other RAM blocks are likely to have
> a size large enough to be of consequence to the live migration
> data copy, and whose size is not already known to the mgmt app from
> the guest config choices it made ? VGA RAM could be a few 100MB I
> guess, but the mgmt app knows about that. I've always assumed everything
> else is just noise in comparison to the main RAM region.
> 
> Still I wonder how useful this is as its just a static figure, and the
> problems with migration transfer are the bulking up of data when the
> VM is repeatedly dirtying stuff at a high rate.
> 
>> Do you think x-query-ramblock could be promoted to non-experimental?
> 
> It would have to be re-written, as this current impl is just emitting
> a huge printf formatted string. To be considered supportable, the data
> would have to be formally modelled in QAPI instead.
> 
> IOW, it would be a case of introducing a new command that emits formal
> data, convertintg 'info ramblock' to use that, and then deprecating this 
> x-query-ramblock.
> 
>> Should another one be made available instead, like :
>> query-ram-bytes-total ?

With virtio-balloon free page hinting and virtio-mem, that number does
not reflect reality (IOW, with sparse ramblocks the total size of
ramblocks is not expressive; it's rather a "worst case").

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> > Hello all,
> > 
> > it would be really good to be able to rely on this command or something similar,
> > to be able to know the approximate size of a migration before starting it.
> > 
> > in QEMU ram_bytes_total() returns what I would like to have,
> > but there is currently no QMP way to get it without starting a migration,
> > which when trying to optimize it/size it is just about too late.
> 
> Aside from the main VM RAM, what other RAM blocks are likely to have
> a size large enough to be of consequence to the live migration
> data copy, and whose size is not already known to the mgmt app from
> the guest config choices it made ? VGA RAM could be a few 100MB I
> guess, but the mgmt app knows about that. I've always assumed everything
> else is just noise in comparison to the main RAM region.
> 
> Still I wonder how useful this is as its just a static figure, and the
> problems with migration transfer are the bulking up of data when the
> VM is repeatedly dirtying stuff at a high rate.
> 
> > Do you think x-query-ramblock could be promoted to non-experimental?
> 
> It would have to be re-written, as this current impl is just emitting
> a huge printf formatted string. To be considered supportable, the data
> would have to be formally modelled in QAPI instead.
> 
> IOW, it would be a case of introducing a new command that emits formal
> data, convertintg 'info ramblock' to use that, and then deprecating this 
> x-query-ramblock.
> 
> > Should another one be made available instead, like :
> > query-ram-bytes-total ?
> 
> That would be simpler if you're just wanting it to give a single
> figure.

Is this what qmp_query_memory_size_summary does?

Dave

> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>>> Hello all,
>>>
>>> it would be really good to be able to rely on this command or something similar,
>>> to be able to know the approximate size of a migration before starting it.
>>>
>>> in QEMU ram_bytes_total() returns what I would like to have,
>>> but there is currently no QMP way to get it without starting a migration,
>>> which when trying to optimize it/size it is just about too late.
>>
>> Aside from the main VM RAM, what other RAM blocks are likely to have
>> a size large enough to be of consequence to the live migration
>> data copy, and whose size is not already known to the mgmt app from
>> the guest config choices it made ? VGA RAM could be a few 100MB I
>> guess, but the mgmt app knows about that. I've always assumed everything
>> else is just noise in comparison to the main RAM region.
>>
>> Still I wonder how useful this is as its just a static figure, and the
>> problems with migration transfer are the bulking up of data when the
>> VM is repeatedly dirtying stuff at a high rate.
>>
>>> Do you think x-query-ramblock could be promoted to non-experimental?
>>
>> It would have to be re-written, as this current impl is just emitting
>> a huge printf formatted string. To be considered supportable, the data
>> would have to be formally modelled in QAPI instead.
>>
>> IOW, it would be a case of introducing a new command that emits formal
>> data, convertintg 'info ramblock' to use that, and then deprecating this 
>> x-query-ramblock.
>>
>>> Should another one be made available instead, like :
>>> query-ram-bytes-total ?
>>
>> That would be simpler if you're just wanting it to give a single
>> figure.
> 
> Is this what qmp_query_memory_size_summary does?

No, I am not looking at something returning the machine->ram_size,
but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.

This would be the return value of something like migration/ram.c::ram_bytes_total().

The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.

If there was such a feature that just returns ram_bytes_total via QMP,
by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:

https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/

The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
but still incurs in a 4-5% performance penalty compared with the multiple files approach,
that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).

Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.

Feedback on the interleaved format I posted there is welcome as well,

Thanks,

Claudio



> 
> Dave
> 
>>
>> With regards,
>> Daniel
>> -- 
>> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>>

On Thu, Jun 30, 2022 at 12:14:36PM +0200, Claudio Fontana wrote:
> On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> >> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> >>> Hello all,
> >>>
> >>> it would be really good to be able to rely on this command or something similar,
> >>> to be able to know the approximate size of a migration before starting it.
> >>>
> >>> in QEMU ram_bytes_total() returns what I would like to have,
> >>> but there is currently no QMP way to get it without starting a migration,
> >>> which when trying to optimize it/size it is just about too late.
> >>
> >> Aside from the main VM RAM, what other RAM blocks are likely to have
> >> a size large enough to be of consequence to the live migration
> >> data copy, and whose size is not already known to the mgmt app from
> >> the guest config choices it made ? VGA RAM could be a few 100MB I
> >> guess, but the mgmt app knows about that. I've always assumed everything
> >> else is just noise in comparison to the main RAM region.
> >>
> >> Still I wonder how useful this is as its just a static figure, and the
> >> problems with migration transfer are the bulking up of data when the
> >> VM is repeatedly dirtying stuff at a high rate.
> >>
> >>> Do you think x-query-ramblock could be promoted to non-experimental?
> >>
> >> It would have to be re-written, as this current impl is just emitting
> >> a huge printf formatted string. To be considered supportable, the data
> >> would have to be formally modelled in QAPI instead.
> >>
> >> IOW, it would be a case of introducing a new command that emits formal
> >> data, convertintg 'info ramblock' to use that, and then deprecating this 
> >> x-query-ramblock.
> >>
> >>> Should another one be made available instead, like :
> >>> query-ram-bytes-total ?
> >>
> >> That would be simpler if you're just wanting it to give a single
> >> figure.
> > 
> > Is this what qmp_query_memory_size_summary does?
> 
> No, I am not looking at something returning the machine->ram_size,
> but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.
> 
> This would be the return value of something like migration/ram.c::ram_bytes_total().
> 
> The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.
> 
> If there was such a feature that just returns ram_bytes_total via QMP,
> by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:
> 
> https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/
> 
> The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
> but still incurs in a 4-5% performance penalty compared with the multiple files approach,
> that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).
> 
> Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
> but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.
> 
> Feedback on the interleaved format I posted there is welcome as well,

I still believe that libvirt is the wrong place to be implementing any
of this logic. It all belongs in QEMU, because QEMU is the place which
holds all the information needed to do an optimal job, and libvirt does
not, as this request for extra QMP features shows.

With regards,
Daniel

On 6/30/22 12:20, Daniel P. Berrangé wrote:
> On Thu, Jun 30, 2022 at 12:14:36PM +0200, Claudio Fontana wrote:
>> On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
>>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>>>> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>>>>> Hello all,
>>>>>
>>>>> it would be really good to be able to rely on this command or something similar,
>>>>> to be able to know the approximate size of a migration before starting it.
>>>>>
>>>>> in QEMU ram_bytes_total() returns what I would like to have,
>>>>> but there is currently no QMP way to get it without starting a migration,
>>>>> which when trying to optimize it/size it is just about too late.
>>>>
>>>> Aside from the main VM RAM, what other RAM blocks are likely to have
>>>> a size large enough to be of consequence to the live migration
>>>> data copy, and whose size is not already known to the mgmt app from
>>>> the guest config choices it made ? VGA RAM could be a few 100MB I
>>>> guess, but the mgmt app knows about that. I've always assumed everything
>>>> else is just noise in comparison to the main RAM region.
>>>>
>>>> Still I wonder how useful this is as its just a static figure, and the
>>>> problems with migration transfer are the bulking up of data when the
>>>> VM is repeatedly dirtying stuff at a high rate.
>>>>
>>>>> Do you think x-query-ramblock could be promoted to non-experimental?
>>>>
>>>> It would have to be re-written, as this current impl is just emitting
>>>> a huge printf formatted string. To be considered supportable, the data
>>>> would have to be formally modelled in QAPI instead.
>>>>
>>>> IOW, it would be a case of introducing a new command that emits formal
>>>> data, convertintg 'info ramblock' to use that, and then deprecating this 
>>>> x-query-ramblock.
>>>>
>>>>> Should another one be made available instead, like :
>>>>> query-ram-bytes-total ?
>>>>
>>>> That would be simpler if you're just wanting it to give a single
>>>> figure.
>>>
>>> Is this what qmp_query_memory_size_summary does?
>>
>> No, I am not looking at something returning the machine->ram_size,
>> but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.
>>
>> This would be the return value of something like migration/ram.c::ram_bytes_total().
>>
>> The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.
>>
>> If there was such a feature that just returns ram_bytes_total via QMP,
>> by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:
>>
>> https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/
>>
>> The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
>> but still incurs in a 4-5% performance penalty compared with the multiple files approach,
>> that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).
>>
>> Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
>> but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.
>>
>> Feedback on the interleaved format I posted there is welcome as well,
> 
> I still believe that libvirt is the wrong place to be implementing any
> of this logic. It all belongs in QEMU, because QEMU is the place which
> holds all the information needed to do an optimal job, and libvirt does
> not, as this request for extra QMP features shows.
> 
> With regards,
> Daniel

Hi Daniel,

I know your position about the implementation in libvirt vs a potential (non-existing for now) QEMU implementation.

The implementation in QEMU seems to me to require more investment due to the need of a new migration target protocol to be defined carefully (possibly "disk://")
and the need to alter and test all migrated devices participating in the creating the migration stream.

I don't think this request for an QMP feature shows anything really.

Knowing the _actual_ size of a migration stream before deciding to migrate is I think a pretty useful feature in itself I would think,
including for libvirt and higher level components in the stack. The lack of the feature just shows, well, the lack of this feature.

Regarding my prototype I pointed at that happens to use libvirt, I wonder if you or anyone have any feedback on the actual format of the VM saved in parallel to disk,
regardless of which component writes it, libvirt or QEMU, and I wonder also if there is any feedback on the O_DIRECT -friendly I/O API, which are both things we can make progress on also regardless of the libvirt vs QEMU implementation of the parallel migration to disk.

Thanks,

Claudio