[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 8/9] monitor: move hmp_info_block* to blockdev-hmp-cmds.c

From: Markus Armbruster
Subject: Re: [PATCH 8/9] monitor: move hmp_info_block* to blockdev-hmp-cmds.c
Date: Mon, 27 Jan 2020 14:33:11 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Maxim Levitsky <address@hidden> 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'
>> 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"

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!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]