Maxim Levitsky <mlevi...@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!