Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
blockdev-hmp-cmds.c | 247 ++++++++++++++++++++++++++++++++++++++++++++
monitor/hmp-cmds.c | 245 -------------------------------------------
2 files changed, 247 insertions(+), 245 deletions(-)
diff --git a/blockdev-hmp-cmds.c b/blockdev-hmp-cmds.c
index 76951352b1..c943dccd03 100644
--- a/blockdev-hmp-cmds.c
+++ b/blockdev-hmp-cmds.c
@@ -33,6 +33,7 @@
#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
#include "block/block_int.h"
+#include "block/qapi.h"
#include "qapi/qapi-commands-block.h"
#include "qapi/qmp/qerror.h"
#include "monitor/hmp.h"
@@ -400,3 +401,249 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
qmp_block_set_io_throttle(&throttle, &err);
hmp_handle_error(mon, &err);
}
+
+static void print_block_info(Monitor *mon, BlockInfo *info,
+ BlockDeviceInfo *inserted, bool verbose)
+{
+ ImageInfo *image_info;
+
+ assert(!info || !info->has_inserted || info->inserted == inserted);
+
+ if (info && *info->device) {
+ monitor_printf(mon, "%s", info->device);
+ if (inserted && inserted->has_node_name) {
+ monitor_printf(mon, " (%s)", inserted->node_name);
+ }
+ } else {
+ assert(info || inserted);
+ monitor_printf(mon, "%s",
+ inserted && inserted->has_node_name ? inserted->node_name
+ : info && info->has_qdev ? info->qdev
+ : "<anonymous>");
+ }
+
+ if (inserted) {
+ monitor_printf(mon, ": %s (%s%s%s)\n",
+ inserted->file,
+ inserted->drv,
+ inserted->ro ? ", read-only" : "",
+ inserted->encrypted ? ", encrypted" : "");
+ } else {
+ monitor_printf(mon, ": [not inserted]\n");
+ }
+
+ if (info) {
+ if (info->has_qdev) {
+ monitor_printf(mon, " Attached to: %s\n", info->qdev);
+ }
+ if (info->has_io_status && info->io_status != BLOCK_DEVICE_IO_STATUS_OK) {
+ monitor_printf(mon, " I/O status: %s\n",
+ BlockDeviceIoStatus_str(info->io_status));
+ }
+
+ if (info->removable) {
+ monitor_printf(mon, " Removable device: %slocked, tray %s\n",
+ info->locked ? "" : "not ",
+ info->tray_open ? "open" : "closed");
+ }
+ }
+
+
+ if (!inserted) {
+ return;
+ }
+
+ monitor_printf(mon, " Cache mode: %s%s%s\n",
+ inserted->cache->writeback ? "writeback" : "writethrough",
+ inserted->cache->direct ? ", direct" : "",
+ inserted->cache->no_flush ? ", ignore flushes" : "");
+
+ if (inserted->has_backing_file) {
+ monitor_printf(mon,
+ " Backing file: %s "
+ "(chain depth: %" PRId64 ")\n",
+ inserted->backing_file,
+ inserted->backing_file_depth);
+ }
+
+ if (inserted->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF) {
+ monitor_printf(mon, " Detect zeroes: %s\n",
+ BlockdevDetectZeroesOptions_str(inserted->detect_zeroes));
+ }
+
+ if (inserted->bps || inserted->bps_rd || inserted->bps_wr ||
+ inserted->iops || inserted->iops_rd || inserted->iops_wr)
+ {
+ monitor_printf(mon, " I/O throttling: bps=%" PRId64
+ " bps_rd=%" PRId64 " bps_wr=%" PRId64
+ " bps_max=%" PRId64
+ " bps_rd_max=%" PRId64
+ " bps_wr_max=%" PRId64
+ " iops=%" PRId64 " iops_rd=%" PRId64
+ " iops_wr=%" PRId64
+ " iops_max=%" PRId64
+ " iops_rd_max=%" PRId64
+ " iops_wr_max=%" PRId64
+ " iops_size=%" PRId64
+ " group=%s\n",
+ inserted->bps,
+ inserted->bps_rd,
+ inserted->bps_wr,
+ inserted->bps_max,
+ inserted->bps_rd_max,
+ inserted->bps_wr_max,
+ inserted->iops,
+ inserted->iops_rd,
+ inserted->iops_wr,
+ inserted->iops_max,
+ inserted->iops_rd_max,
+ inserted->iops_wr_max,
+ inserted->iops_size,
+ inserted->group);
+ }
+
+ if (verbose) {
+ monitor_printf(mon, "\nImages:\n");
+ image_info = inserted->image;
+ while (1) {
+ bdrv_image_info_dump(image_info);
+ if (image_info->has_backing_image) {
+ image_info = image_info->backing_image;
+ } else {
+ break;
+ }
+ }
+ }
+}
+
+void hmp_info_block(Monitor *mon, const QDict *qdict)
+{
+ BlockInfoList *block_list, *info;
+ BlockDeviceInfoList *blockdev_list, *blockdev;
+ const char *device = qdict_get_try_str(qdict, "device");
+ bool verbose = qdict_get_try_bool(qdict, "verbose", false);
+ bool nodes = qdict_get_try_bool(qdict, "nodes", false);
+ bool printed = false;
+
+ /* Print BlockBackend information */
+ if (!nodes) {
+ block_list = qmp_query_block(NULL);
+ } else {
+ block_list = NULL;
+ }
+
+ for (info = block_list; info; info = info->next) {
+ if (device && strcmp(device, info->value->device)) {
+ continue;
+ }
+
+ if (info != block_list) {
+ monitor_printf(mon, "\n");
+ }
+
+ print_block_info(mon, info->value, info->value->has_inserted
+ ? info->value->inserted : NULL,
+ verbose);
+ printed = true;
+ }
+
+ qapi_free_BlockInfoList(block_list);
+
+ if ((!device && !nodes) || printed) {
+ return;
+ }
+
+ /* Print node information */
+ blockdev_list = qmp_query_named_block_nodes(NULL);
+ for (blockdev = blockdev_list; blockdev; blockdev = blockdev->next) {
+ assert(blockdev->value->has_node_name);
+ if (device && strcmp(device, blockdev->value->node_name)) {
+ continue;
+ }
+
+ if (blockdev != blockdev_list) {
+ monitor_printf(mon, "\n");
+ }
+
+ print_block_info(mon, NULL, blockdev->value, verbose);
+ }
+ qapi_free_BlockDeviceInfoList(blockdev_list);
+}
+
+void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
+{
+ BlockStatsList *stats_list, *stats;
+
+ stats_list = qmp_query_blockstats(false, false, NULL);
+
+ for (stats = stats_list; stats; stats = stats->next) {
+ if (!stats->value->has_device) {
+ continue;
+ }
+
+ monitor_printf(mon, "%s:", stats->value->device);
+ monitor_printf(mon, " rd_bytes=%" PRId64
+ " wr_bytes=%" PRId64
+ " rd_operations=%" PRId64
+ " wr_operations=%" PRId64
+ " flush_operations=%" PRId64
+ " wr_total_time_ns=%" PRId64
+ " rd_total_time_ns=%" PRId64
+ " flush_total_time_ns=%" PRId64
+ " rd_merged=%" PRId64
+ " wr_merged=%" PRId64
+ " idle_time_ns=%" PRId64
+ "\n",
+ stats->value->stats->rd_bytes,
+ stats->value->stats->wr_bytes,
+ stats->value->stats->rd_operations,
+ stats->value->stats->wr_operations,
+ stats->value->stats->flush_operations,
+ stats->value->stats->wr_total_time_ns,
+ stats->value->stats->rd_total_time_ns,
+ stats->value->stats->flush_total_time_ns,
+ stats->value->stats->rd_merged,
+ stats->value->stats->wr_merged,
+ stats->value->stats->idle_time_ns);
+ }
+
+ qapi_free_BlockStatsList(stats_list);
+}
+
+void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
+{
+ BlockJobInfoList *list;
+ Error *err = NULL;
+
+ list = qmp_query_block_jobs(&err);
+ assert(!err);
+
+ if (!list) {
+ monitor_printf(mon, "No active jobs\n");
+ return;
+ }
+
+ while (list) {
+ if (strcmp(list->value->type, "stream") == 0) {
+ monitor_printf(mon, "Streaming device %s: Completed %" PRId64
+ " of %" PRId64 " bytes, speed limit %" PRId64
+ " bytes/s\n",
+ list->value->device,
+ list->value->offset,
+ list->value->len,
+ list->value->speed);
+ } else {
+ monitor_printf(mon, "Type %s, device %s: Completed %" PRId64
+ " of %" PRId64 " bytes, speed limit %" PRId64
+ " bytes/s\n",
+ list->value->type,
+ list->value->device,
+ list->value->offset,
+ list->value->len,
+ list->value->speed);
+ }
+ list = list->next;
+ }
+
+ qapi_free_BlockJobInfoList(list);
+}
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8be48e0af6..1008902bc3 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -468,213 +468,6 @@ void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
qmp_query_migrate_cache_size(NULL) >> 10);
}
-static void print_block_info(Monitor *mon, BlockInfo *info,
- BlockDeviceInfo *inserted, bool verbose)
-{
- ImageInfo *image_info;
-
- assert(!info || !info->has_inserted || info->inserted == inserted);
-
- if (info && *info->device) {
- monitor_printf(mon, "%s", info->device);
- if (inserted && inserted->has_node_name) {
- monitor_printf(mon, " (%s)", inserted->node_name);
- }
- } else {
- assert(info || inserted);
- monitor_printf(mon, "%s",
- inserted && inserted->has_node_name ? inserted->node_name
- : info && info->has_qdev ? info->qdev
- : "<anonymous>");
- }
-
- if (inserted) {
- monitor_printf(mon, ": %s (%s%s%s)\n",
- inserted->file,
- inserted->drv,
- inserted->ro ? ", read-only" : "",
- inserted->encrypted ? ", encrypted" : "");
- } else {
- monitor_printf(mon, ": [not inserted]\n");
- }
-
- if (info) {
- if (info->has_qdev) {
- monitor_printf(mon, " Attached to: %s\n", info->qdev);
- }
- if (info->has_io_status && info->io_status != BLOCK_DEVICE_IO_STATUS_OK) {
- monitor_printf(mon, " I/O status: %s\n",
- BlockDeviceIoStatus_str(info->io_status));
- }
-
- if (info->removable) {
- monitor_printf(mon, " Removable device: %slocked, tray %s\n",
- info->locked ? "" : "not ",
- info->tray_open ? "open" : "closed");
- }
- }
-
-
- if (!inserted) {
- return;
- }
-
- monitor_printf(mon, " Cache mode: %s%s%s\n",
- inserted->cache->writeback ? "writeback" : "writethrough",
- inserted->cache->direct ? ", direct" : "",
- inserted->cache->no_flush ? ", ignore flushes" : "");
-
- if (inserted->has_backing_file) {
- monitor_printf(mon,
- " Backing file: %s "
- "(chain depth: %" PRId64 ")\n",
- inserted->backing_file,
- inserted->backing_file_depth);
- }
-
- if (inserted->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF) {
- monitor_printf(mon, " Detect zeroes: %s\n",
- BlockdevDetectZeroesOptions_str(inserted->detect_zeroes));
- }
-
- if (inserted->bps || inserted->bps_rd || inserted->bps_wr ||
- inserted->iops || inserted->iops_rd || inserted->iops_wr)
- {
- monitor_printf(mon, " I/O throttling: bps=%" PRId64
- " bps_rd=%" PRId64 " bps_wr=%" PRId64
- " bps_max=%" PRId64
- " bps_rd_max=%" PRId64
- " bps_wr_max=%" PRId64
- " iops=%" PRId64 " iops_rd=%" PRId64
- " iops_wr=%" PRId64
- " iops_max=%" PRId64
- " iops_rd_max=%" PRId64
- " iops_wr_max=%" PRId64
- " iops_size=%" PRId64
- " group=%s\n",
- inserted->bps,
- inserted->bps_rd,
- inserted->bps_wr,
- inserted->bps_max,
- inserted->bps_rd_max,
- inserted->bps_wr_max,
- inserted->iops,
- inserted->iops_rd,
- inserted->iops_wr,
- inserted->iops_max,
- inserted->iops_rd_max,
- inserted->iops_wr_max,
- inserted->iops_size,
- inserted->group);
- }
-
- if (verbose) {
- monitor_printf(mon, "\nImages:\n");
- image_info = inserted->image;
- while (1) {
- bdrv_image_info_dump(image_info);
- if (image_info->has_backing_image) {
- image_info = image_info->backing_image;
- } else {
- break;
- }
- }
- }
-}
-
-void hmp_info_block(Monitor *mon, const QDict *qdict)
-{
- BlockInfoList *block_list, *info;
- BlockDeviceInfoList *blockdev_list, *blockdev;
- const char *device = qdict_get_try_str(qdict, "device");
- bool verbose = qdict_get_try_bool(qdict, "verbose", false);
- bool nodes = qdict_get_try_bool(qdict, "nodes", false);
- bool printed = false;
-
- /* Print BlockBackend information */
- if (!nodes) {
- block_list = qmp_query_block(NULL);
- } else {
- block_list = NULL;
- }
-
- for (info = block_list; info; info = info->next) {
- if (device && strcmp(device, info->value->device)) {
- continue;
- }
-
- if (info != block_list) {
- monitor_printf(mon, "\n");
- }
-
- print_block_info(mon, info->value, info->value->has_inserted
- ? info->value->inserted : NULL,
- verbose);
- printed = true;
- }
-
- qapi_free_BlockInfoList(block_list);
-
- if ((!device && !nodes) || printed) {
- return;
- }
-
- /* Print node information */
- blockdev_list = qmp_query_named_block_nodes(NULL);
- for (blockdev = blockdev_list; blockdev; blockdev = blockdev->next) {
- assert(blockdev->value->has_node_name);
- if (device && strcmp(device, blockdev->value->node_name)) {
- continue;
- }
-
- if (blockdev != blockdev_list) {
- monitor_printf(mon, "\n");
- }
-
- print_block_info(mon, NULL, blockdev->value, verbose);
- }
- qapi_free_BlockDeviceInfoList(blockdev_list);
-}
-
-void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
-{
- BlockStatsList *stats_list, *stats;
-
- stats_list = qmp_query_blockstats(false, false, NULL);
-
- for (stats = stats_list; stats; stats = stats->next) {
- if (!stats->value->has_device) {
- continue;
- }
-
- monitor_printf(mon, "%s:", stats->value->device);
- monitor_printf(mon, " rd_bytes=%" PRId64
- " wr_bytes=%" PRId64
- " rd_operations=%" PRId64
- " wr_operations=%" PRId64
- " flush_operations=%" PRId64
- " wr_total_time_ns=%" PRId64
- " rd_total_time_ns=%" PRId64
- " flush_total_time_ns=%" PRId64
- " rd_merged=%" PRId64
- " wr_merged=%" PRId64
- " idle_time_ns=%" PRId64
- "\n",
- stats->value->stats->rd_bytes,
- stats->value->stats->wr_bytes,
- stats->value->stats->rd_operations,
- stats->value->stats->wr_operations,
- stats->value->stats->flush_operations,
- stats->value->stats->wr_total_time_ns,
- stats->value->stats->rd_total_time_ns,
- stats->value->stats->flush_total_time_ns,
- stats->value->stats->rd_merged,
- stats->value->stats->wr_merged,
- stats->value->stats->idle_time_ns);
- }
-
- qapi_free_BlockStatsList(stats_list);
-}
#ifdef CONFIG_VNC
/* Helper for hmp_info_vnc_clients, _servers */
@@ -1054,44 +847,6 @@ void hmp_info_pci(Monitor *mon, const QDict *qdict)
qapi_free_PciInfoList(info_list);
}
-void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
-{
- BlockJobInfoList *list;
- Error *err = NULL;
-
- list = qmp_query_block_jobs(&err);
- assert(!err);
-
- if (!list) {
- monitor_printf(mon, "No active jobs\n");
- return;
- }
-
- while (list) {
- if (strcmp(list->value->type, "stream") == 0) {
- monitor_printf(mon, "Streaming device %s: Completed %" PRId64
- " of %" PRId64 " bytes, speed limit %" PRId64
- " bytes/s\n",
- list->value->device,
- list->value->offset,
- list->value->len,
- list->value->speed);
- } else {
- monitor_printf(mon, "Type %s, device %s: Completed %" PRId64
- " of %" PRId64 " bytes, speed limit %" PRId64
- " bytes/s\n",
- list->value->type,
- list->value->device,
- list->value->offset,
- list->value->len,
- list->value->speed);
- }
- list = list->next;
- }
-
- qapi_free_BlockJobInfoList(list);
-}
-
void hmp_info_tpm(Monitor *mon, const QDict *qdict)
{
TPMInfoList *info_list, *info;
--
2.17.2
I think it makes sense to collect *all* block HMP stuff here. Left in monitor/hmp-cmds.c: hmp_eject(), hmp_nbd_server_start(), ... I guess hmp_change() has to stay there, because it's both block and ui. Left in blockdev.c: hmp_drive_add_node(). Quick grep for possible files to check: $ git-grep -l 'monitor[a-z_-]*.h' | xargs grep -l 'block[a-z_-]*\.h' MAINTAINERS blockdev-hmp-cmds.c blockdev.c cpus.c dump/dump.c hw/display/qxl.c hw/scsi/vhost-scsi.c hw/usb/dev-storage.c include/monitor/monitor.h migration/migration.c monitor/hmp-cmds.c monitor/hmp.c monitor/misc.c monitor/qmp-cmds.c qdev-monitor.c vl.c
On Wed, 2019-11-27 at 09:08 +0100, Markus Armbruster wrote: > I think it makes sense to collect *all* block HMP stuff here. > > Left in monitor/hmp-cmds.c: hmp_eject(), hmp_nbd_server_start(), ... > > I guess hmp_change() has to stay there, because it's both block and ui. > > Left in blockdev.c: hmp_drive_add_node(). Thank you very much. I added these and bunch more to my patchset. > > Quick grep for possible files to check: > > $ git-grep -l 'monitor[a-z_-]*.h' | xargs grep -l 'block[a-z_-]*\.h' > MAINTAINERS > blockdev-hmp-cmds.c > > blockdev.c hmp_drive_add_node is there and I moved it too. > cpus.c Nothing suspicious > dump/dump.c qmp_dump_guest_memory is only monitor reference there I think > hw/display/qxl.c No way that is related to the block layer > hw/scsi/vhost-scsi.c All right, the monitor_fd_param is an interesting thing. Not related to block though. > hw/usb/dev-storage.c All right, this for no reason includes monitor/monitor.h, added patch to remove this because why not. > include/monitor/monitor.h Nothing suspicious > migration/migration.c Nothing suspicious > monitor/hmp-cmds.c Added hmp_qemu_io Maybe I need to add hmp_delvm too? savevm/delvm do old style snapshots which are stored to the first block device > monitor/hmp.c There are some block references in monitor_find_completion, but I guess it is not worth it to move that > monitor/misc.c vm_completion for delvm/loadvm. > monitor/qmp-cmds.c Nothing hmp related at first glance. > qdev-monitor.c blk_by_qdev_id - used by both hmp and qmp code > vl.c Hopefully nothing hmp+block related, I searched the file for few things but I can't be fully sure. Out of the curiosity do you know why this file is called like that, since it hosts qemu main(), shouldn't it be called main.c ? Best regards and thanks for the detailed review! Maxim Levitsky
Maxim Levitsky <mlevitsk@redhat.com> writes:
> On Wed, 2019-11-27 at 09:08 +0100, Markus Armbruster wrote:
>> I think it makes sense to collect *all* block HMP stuff here.
>>
>> Left in monitor/hmp-cmds.c: hmp_eject(), hmp_nbd_server_start(), ...
>>
>> I guess hmp_change() has to stay there, because it's both block and ui.
>>
>> Left in blockdev.c: hmp_drive_add_node().
>
> Thank you very much. I added these and bunch more to my patchset.
>
>>
>> Quick grep for possible files to check:
>>
>> $ git-grep -l 'monitor[a-z_-]*.h' | xargs grep -l 'block[a-z_-]*\.h'
>> MAINTAINERS
>> blockdev-hmp-cmds.c
>>
>
>> blockdev.c
> hmp_drive_add_node is there and I moved it too.
>
>
>> cpus.c
> Nothing suspicious
>
>> dump/dump.c
> qmp_dump_guest_memory is only monitor reference there I think
>
>> hw/display/qxl.c
> No way that is related to the block layer
>
>> hw/scsi/vhost-scsi.c
> All right, the monitor_fd_param is an interesting thing.
> Not related to block though.
>
>> hw/usb/dev-storage.c
> All right, this for no reason includes monitor/monitor.h,
> added patch to remove this because why not.
>
>> include/monitor/monitor.h
> Nothing suspicious
>
>> migration/migration.c
> Nothing suspicious
>
>> monitor/hmp-cmds.c
> Added hmp_qemu_io
>
> Maybe I need to add hmp_delvm too?
> savevm/delvm do old style snapshots
> which are stored to the first block device
One foot in the block subsystem, the other foot in the migration
subsystem. I'm not sure where this should go. Kevin?
>> monitor/hmp.c
> There are some block references in monitor_find_completion,
> but I guess it is not worth it to move that
>
>> monitor/misc.c
> vm_completion for delvm/loadvm.
Having completion close to whatever it completes would be nice, I guess.
When in doubt, leave the savevm / delvm stuff alone.
>> monitor/qmp-cmds.c
> Nothing hmp related at first glance.
>
>> qdev-monitor.c
> blk_by_qdev_id - used by both hmp and qmp code
>
>> vl.c
> Hopefully nothing hmp+block related, I searched the file for
> few things but I can't be fully sure.
> Out of the curiosity do you know why this file is called like that,
> since it hosts qemu main(), shouldn't it be called main.c ?
Its first commit 0824d6fc67 "for hard core developpers only: a new user
mode linux project :-)" calls the executable "vl", and has
void help(void)
{
printf("Virtual Linux version " QEMU_VERSION ", Copyright (c) 2003 Fabrice Bellard\n"
"usage: vl [-h] bzImage initrd [kernel parameters...]\n"
"\n"
[...]
exit(1);
}
The executable was renamed soon after. I guess the source file name has
made people wonder ever since.
>
> Best regards and thanks for the detailed review!
> Maxim Levitsky
You're welcome!
On Mon, 2020-01-27 at 14:33 +0100, Markus Armbruster wrote:
> Maxim Levitsky <mlevitsk@redhat.com> writes:
>
> > On Wed, 2019-11-27 at 09:08 +0100, Markus Armbruster wrote:
> > > I think it makes sense to collect *all* block HMP stuff here.
> > >
> > > Left in monitor/hmp-cmds.c: hmp_eject(), hmp_nbd_server_start(), ...
> > >
> > > I guess hmp_change() has to stay there, because it's both block and ui.
> > >
> > > Left in blockdev.c: hmp_drive_add_node().
> >
> > Thank you very much. I added these and bunch more to my patchset.
> >
> > >
> > > Quick grep for possible files to check:
> > >
> > > $ git-grep -l 'monitor[a-z_-]*.h' | xargs grep -l 'block[a-z_-]*\.h'
> > > MAINTAINERS
> > > blockdev-hmp-cmds.c
> > >
> > > blockdev.c
> >
> > hmp_drive_add_node is there and I moved it too.
> >
> >
> > > cpus.c
> >
> > Nothing suspicious
> >
> > > dump/dump.c
> >
> > qmp_dump_guest_memory is only monitor reference there I think
> >
> > > hw/display/qxl.c
> >
> > No way that is related to the block layer
> >
> > > hw/scsi/vhost-scsi.c
> >
> > All right, the monitor_fd_param is an interesting thing.
> > Not related to block though.
> >
> > > hw/usb/dev-storage.c
> >
> > All right, this for no reason includes monitor/monitor.h,
> > added patch to remove this because why not.
> >
> > > include/monitor/monitor.h
> >
> > Nothing suspicious
> >
> > > migration/migration.c
> >
> > Nothing suspicious
> >
> > > monitor/hmp-cmds.c
> >
> > Added hmp_qemu_io
> >
> > Maybe I need to add hmp_delvm too?
> > savevm/delvm do old style snapshots
> > which are stored to the first block device
>
> One foot in the block subsystem, the other foot in the migration
> subsystem. I'm not sure where this should go. Kevin?
>
> > > monitor/hmp.c
> >
> > There are some block references in monitor_find_completion,
> > but I guess it is not worth it to move that
> >
> > > monitor/misc.c
> >
> > vm_completion for delvm/loadvm.
>
> Having completion close to whatever it completes would be nice, I guess.
>
> When in doubt, leave the savevm / delvm stuff alone.
Yep.
>
> > > monitor/qmp-cmds.c
> >
> > Nothing hmp related at first glance.
> >
> > > qdev-monitor.c
> >
> > blk_by_qdev_id - used by both hmp and qmp code
> >
> > > vl.c
> >
> > Hopefully nothing hmp+block related, I searched the file for
> > few things but I can't be fully sure.
> > Out of the curiosity do you know why this file is called like that,
> > since it hosts qemu main(), shouldn't it be called main.c ?
>
> Its first commit 0824d6fc67 "for hard core developpers only: a new user
> mode linux project :-)" calls the executable "vl", and has
>
> void help(void)
> {
> printf("Virtual Linux version " QEMU_VERSION ", Copyright (c) 2003 Fabrice Bellard\n"
> "usage: vl [-h] bzImage initrd [kernel parameters...]\n"
> "\n"
> [...]
> exit(1);
> }
>
> The executable was renamed soon after. I guess the source file name has
> made people wonder ever since.
Nice :-)
>
> >
> > Best regards and thanks for the detailed review!
> > Maxim Levitsky
>
> You're welcome!
I hope we can move forward with this patch series as well.
Best regards,
Maxim Levitsky
Am 27.01.2020 um 14:33 hat Markus Armbruster geschrieben: > Maxim Levitsky <mlevitsk@redhat.com> writes: > > > On Wed, 2019-11-27 at 09:08 +0100, Markus Armbruster wrote: > >> I think it makes sense to collect *all* block HMP stuff here. > >> > >> Left in monitor/hmp-cmds.c: hmp_eject(), hmp_nbd_server_start(), ... > >> > >> I guess hmp_change() has to stay there, because it's both block and ui. > >> > >> Left in blockdev.c: hmp_drive_add_node(). > > > > Thank you very much. I added these and bunch more to my patchset. > > > >> > >> Quick grep for possible files to check: > >> > >> $ git-grep -l 'monitor[a-z_-]*.h' | xargs grep -l 'block[a-z_-]*\.h' > >> MAINTAINERS > >> blockdev-hmp-cmds.c > >> > > > >> blockdev.c > > hmp_drive_add_node is there and I moved it too. > > > > > >> cpus.c > > Nothing suspicious > > > >> dump/dump.c > > qmp_dump_guest_memory is only monitor reference there I think > > > >> hw/display/qxl.c > > No way that is related to the block layer > > > >> hw/scsi/vhost-scsi.c > > All right, the monitor_fd_param is an interesting thing. > > Not related to block though. > > > >> hw/usb/dev-storage.c > > All right, this for no reason includes monitor/monitor.h, > > added patch to remove this because why not. > > > >> include/monitor/monitor.h > > Nothing suspicious > > > >> migration/migration.c > > Nothing suspicious > > > >> monitor/hmp-cmds.c > > Added hmp_qemu_io > > > > Maybe I need to add hmp_delvm too? > > savevm/delvm do old style snapshots > > which are stored to the first block device > > One foot in the block subsystem, the other foot in the migration > subsystem. I'm not sure where this should go. Kevin? Moving it to block is not an obvious improvement, so I think I'd leave it alone. Kevin
© 2016 - 2026 Red Hat, Inc.