mbox series

[v4,00/11] RFC: [for 5.0]: HMP monitor handlers refactoring

Message ID 20200130123448.21093-1-mlevitsk@redhat.com (mailing list archive)
Headers show
Series RFC: [for 5.0]: HMP monitor handlers refactoring | expand

Message

Maxim Levitsky Jan. 30, 2020, 12:34 p.m. UTC
This patch series is bunch of cleanups to the hmp monitor code.
It mostly moves the blockdev related hmp handlers to its own file,
and does some minor refactoring.

No functional changes expected.

Changes from V1:
   * move the handlers to block/monitor/block-hmp-cmds.c
   * tiny cleanup for the commit messages

Changes from V2:
   * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
   * Set the license of blockdev-hmp-cmds.c to GPLv2+
   * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
   * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
     (this change needed some new exports, thus in separate new patch)
   * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
   * Added 'error:' prefix to vreport, and updated the iotests
     This is invasive change, but really feels like the right one
   * Added minor refactoring patch that drops an unused #include

Changes from V3:
   * Dropped the error prefix patches for now due to fact that it seems
     that libvirt doesn't need that after all. Oh well...
     I'll send them in a separate series.

   * Hopefully correctly merged the copyright info the new files
     Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)

   * Addressed review feedback
   * Renamed the added header to block-hmp-cmds.h

   * Got rid of checkpatch.pl warnings in the moved code
     (cosmetic code changes only)

   * I kept the reviewed-by tags, since the changes I did are minor.
     I hope that this is right thing to do.

Best regards,
	Maxim Levitsky

Maxim Levitsky (11):
  usb/dev-storage: remove unused include
  monitor/hmp: uninline add_init_drive
  monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
  monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
  monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
    block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
    GPLv2+
  monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
  monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
    hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
    have to change the licence to GPLv2
  monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
  monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
  monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
  monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c

 MAINTAINERS                    |    1 +
 Makefile.objs                  |    2 +-
 block/Makefile.objs            |    1 +
 block/monitor/Makefile.objs    |    1 +
 block/monitor/block-hmp-cmds.c | 1002 ++++++++++++++++++++++++++++++++
 blockdev.c                     |  137 +----
 device-hotplug.c               |   91 ---
 hw/usb/dev-storage.c           |    1 -
 include/block/block-hmp-cmds.h |   54 ++
 include/block/block_int.h      |    5 +-
 include/monitor/hmp.h          |   24 -
 include/sysemu/blockdev.h      |    4 -
 include/sysemu/sysemu.h        |    3 -
 monitor/hmp-cmds.c             |  769 ------------------------
 monitor/misc.c                 |    1 +
 15 files changed, 1072 insertions(+), 1024 deletions(-)
 create mode 100644 block/monitor/Makefile.objs
 create mode 100644 block/monitor/block-hmp-cmds.c
 delete mode 100644 device-hotplug.c
 create mode 100644 include/block/block-hmp-cmds.h

Comments

Dr. David Alan Gilbert Feb. 3, 2020, 7:57 p.m. UTC | #1
* Maxim Levitsky (mlevitsk@redhat.com) wrote:
> This patch series is bunch of cleanups to the hmp monitor code.
> It mostly moves the blockdev related hmp handlers to its own file,
> and does some minor refactoring.
> 
> No functional changes expected.

You've still got the title marked as RFC - are you actually ready for
this log?

Dave

> 
> Changes from V1:
>    * move the handlers to block/monitor/block-hmp-cmds.c
>    * tiny cleanup for the commit messages
> 
> Changes from V2:
>    * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
>    * Set the license of blockdev-hmp-cmds.c to GPLv2+
>    * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
>    * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
>      (this change needed some new exports, thus in separate new patch)
>    * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
>    * Added 'error:' prefix to vreport, and updated the iotests
>      This is invasive change, but really feels like the right one
>    * Added minor refactoring patch that drops an unused #include
> 
> Changes from V3:
>    * Dropped the error prefix patches for now due to fact that it seems
>      that libvirt doesn't need that after all. Oh well...
>      I'll send them in a separate series.
> 
>    * Hopefully correctly merged the copyright info the new files
>      Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)
> 
>    * Addressed review feedback
>    * Renamed the added header to block-hmp-cmds.h
> 
>    * Got rid of checkpatch.pl warnings in the moved code
>      (cosmetic code changes only)
> 
>    * I kept the reviewed-by tags, since the changes I did are minor.
>      I hope that this is right thing to do.
> 
> Best regards,
> 	Maxim Levitsky
> 
> Maxim Levitsky (11):
>   usb/dev-storage: remove unused include
>   monitor/hmp: uninline add_init_drive
>   monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
>   monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
>   monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
>     block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
>     GPLv2+
>   monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
>   monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
>     hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
>     have to change the licence to GPLv2
>   monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
>   monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
>   monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
>   monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c
> 
>  MAINTAINERS                    |    1 +
>  Makefile.objs                  |    2 +-
>  block/Makefile.objs            |    1 +
>  block/monitor/Makefile.objs    |    1 +
>  block/monitor/block-hmp-cmds.c | 1002 ++++++++++++++++++++++++++++++++
>  blockdev.c                     |  137 +----
>  device-hotplug.c               |   91 ---
>  hw/usb/dev-storage.c           |    1 -
>  include/block/block-hmp-cmds.h |   54 ++
>  include/block/block_int.h      |    5 +-
>  include/monitor/hmp.h          |   24 -
>  include/sysemu/blockdev.h      |    4 -
>  include/sysemu/sysemu.h        |    3 -
>  monitor/hmp-cmds.c             |  769 ------------------------
>  monitor/misc.c                 |    1 +
>  15 files changed, 1072 insertions(+), 1024 deletions(-)
>  create mode 100644 block/monitor/Makefile.objs
>  create mode 100644 block/monitor/block-hmp-cmds.c
>  delete mode 100644 device-hotplug.c
>  create mode 100644 include/block/block-hmp-cmds.h
> 
> -- 
> 2.17.2
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Maxim Levitsky Feb. 4, 2020, 8:15 a.m. UTC | #2
On Mon, 2020-02-03 at 19:57 +0000, Dr. David Alan Gilbert wrote:
> * Maxim Levitsky (mlevitsk@redhat.com) wrote:
> > This patch series is bunch of cleanups to the hmp monitor code.
> > It mostly moves the blockdev related hmp handlers to its own file,
> > and does some minor refactoring.
> > 
> > No functional changes expected.
> 
> You've still got the title marked as RFC - are you actually ready for
> this log?

I forgot to update this to be honest, I don't consider this an RFC,
especially since I dropped for now the patches that might cause
issues. This is now just a nice refactoring.

Best regards,
	Maxim Levitsky

> 
> Dave
> 
> > 
> > Changes from V1:
> >    * move the handlers to block/monitor/block-hmp-cmds.c
> >    * tiny cleanup for the commit messages
> > 
> > Changes from V2:
> >    * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
> >    * Set the license of blockdev-hmp-cmds.c to GPLv2+
> >    * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
> >    * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
> >      (this change needed some new exports, thus in separate new patch)
> >    * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
> >    * Added 'error:' prefix to vreport, and updated the iotests
> >      This is invasive change, but really feels like the right one
> >    * Added minor refactoring patch that drops an unused #include
> > 
> > Changes from V3:
> >    * Dropped the error prefix patches for now due to fact that it seems
> >      that libvirt doesn't need that after all. Oh well...
> >      I'll send them in a separate series.
> > 
> >    * Hopefully correctly merged the copyright info the new files
> >      Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)
> > 
> >    * Addressed review feedback
> >    * Renamed the added header to block-hmp-cmds.h
> > 
> >    * Got rid of checkpatch.pl warnings in the moved code
> >      (cosmetic code changes only)
> > 
> >    * I kept the reviewed-by tags, since the changes I did are minor.
> >      I hope that this is right thing to do.
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > Maxim Levitsky (11):
> >   usb/dev-storage: remove unused include
> >   monitor/hmp: uninline add_init_drive
> >   monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
> >   monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
> >   monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
> >     block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
> >     GPLv2+
> >   monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
> >   monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
> >     hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
> >     have to change the licence to GPLv2
> >   monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
> >   monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
> >   monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
> >   monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c
> > 
> >  MAINTAINERS                    |    1 +
> >  Makefile.objs                  |    2 +-
> >  block/Makefile.objs            |    1 +
> >  block/monitor/Makefile.objs    |    1 +
> >  block/monitor/block-hmp-cmds.c | 1002 ++++++++++++++++++++++++++++++++
> >  blockdev.c                     |  137 +----
> >  device-hotplug.c               |   91 ---
> >  hw/usb/dev-storage.c           |    1 -
> >  include/block/block-hmp-cmds.h |   54 ++
> >  include/block/block_int.h      |    5 +-
> >  include/monitor/hmp.h          |   24 -
> >  include/sysemu/blockdev.h      |    4 -
> >  include/sysemu/sysemu.h        |    3 -
> >  monitor/hmp-cmds.c             |  769 ------------------------
> >  monitor/misc.c                 |    1 +
> >  15 files changed, 1072 insertions(+), 1024 deletions(-)
> >  create mode 100644 block/monitor/Makefile.objs
> >  create mode 100644 block/monitor/block-hmp-cmds.c
> >  delete mode 100644 device-hotplug.c
> >  create mode 100644 include/block/block-hmp-cmds.h
> > 
> > -- 
> > 2.17.2
> > 
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert Feb. 7, 2020, 6:28 p.m. UTC | #3
* Maxim Levitsky (mlevitsk@redhat.com) wrote:
> On Mon, 2020-02-03 at 19:57 +0000, Dr. David Alan Gilbert wrote:
> > * Maxim Levitsky (mlevitsk@redhat.com) wrote:
> > > This patch series is bunch of cleanups to the hmp monitor code.
> > > It mostly moves the blockdev related hmp handlers to its own file,
> > > and does some minor refactoring.
> > > 
> > > No functional changes expected.
> > 
> > You've still got the title marked as RFC - are you actually ready for
> > this log?
> 
> I forgot to update this to be honest, I don't consider this an RFC,
> especially since I dropped for now the patches that might cause
> issues. This is now just a nice refactoring.

OK, so if we can get some block people to say they're happy, then
I'd be happy to take this through HMP or they can take it through block.

Dave

> Best regards,
> 	Maxim Levitsky
> 
> > 
> > Dave
> > 
> > > 
> > > Changes from V1:
> > >    * move the handlers to block/monitor/block-hmp-cmds.c
> > >    * tiny cleanup for the commit messages
> > > 
> > > Changes from V2:
> > >    * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
> > >    * Set the license of blockdev-hmp-cmds.c to GPLv2+
> > >    * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
> > >    * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
> > >      (this change needed some new exports, thus in separate new patch)
> > >    * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
> > >    * Added 'error:' prefix to vreport, and updated the iotests
> > >      This is invasive change, but really feels like the right one
> > >    * Added minor refactoring patch that drops an unused #include
> > > 
> > > Changes from V3:
> > >    * Dropped the error prefix patches for now due to fact that it seems
> > >      that libvirt doesn't need that after all. Oh well...
> > >      I'll send them in a separate series.
> > > 
> > >    * Hopefully correctly merged the copyright info the new files
> > >      Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)
> > > 
> > >    * Addressed review feedback
> > >    * Renamed the added header to block-hmp-cmds.h
> > > 
> > >    * Got rid of checkpatch.pl warnings in the moved code
> > >      (cosmetic code changes only)
> > > 
> > >    * I kept the reviewed-by tags, since the changes I did are minor.
> > >      I hope that this is right thing to do.
> > > 
> > > Best regards,
> > > 	Maxim Levitsky
> > > 
> > > Maxim Levitsky (11):
> > >   usb/dev-storage: remove unused include
> > >   monitor/hmp: uninline add_init_drive
> > >   monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
> > >   monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
> > >   monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
> > >     block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
> > >     GPLv2+
> > >   monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
> > >   monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
> > >     hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
> > >     have to change the licence to GPLv2
> > >   monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
> > >   monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
> > >   monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
> > >   monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c
> > > 
> > >  MAINTAINERS                    |    1 +
> > >  Makefile.objs                  |    2 +-
> > >  block/Makefile.objs            |    1 +
> > >  block/monitor/Makefile.objs    |    1 +
> > >  block/monitor/block-hmp-cmds.c | 1002 ++++++++++++++++++++++++++++++++
> > >  blockdev.c                     |  137 +----
> > >  device-hotplug.c               |   91 ---
> > >  hw/usb/dev-storage.c           |    1 -
> > >  include/block/block-hmp-cmds.h |   54 ++
> > >  include/block/block_int.h      |    5 +-
> > >  include/monitor/hmp.h          |   24 -
> > >  include/sysemu/blockdev.h      |    4 -
> > >  include/sysemu/sysemu.h        |    3 -
> > >  monitor/hmp-cmds.c             |  769 ------------------------
> > >  monitor/misc.c                 |    1 +
> > >  15 files changed, 1072 insertions(+), 1024 deletions(-)
> > >  create mode 100644 block/monitor/Makefile.objs
> > >  create mode 100644 block/monitor/block-hmp-cmds.c
> > >  delete mode 100644 device-hotplug.c
> > >  create mode 100644 include/block/block-hmp-cmds.h
> > > 
> > > -- 
> > > 2.17.2
> > > 
> > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Maxim Levitsky Feb. 20, 2020, 5:06 p.m. UTC | #4
On Fri, 2020-02-07 at 18:28 +0000, Dr. David Alan Gilbert wrote:
> * Maxim Levitsky (mlevitsk@redhat.com) wrote:
> > On Mon, 2020-02-03 at 19:57 +0000, Dr. David Alan Gilbert wrote:
> > > * Maxim Levitsky (mlevitsk@redhat.com) wrote:
> > > > This patch series is bunch of cleanups to the hmp monitor code.
> > > > It mostly moves the blockdev related hmp handlers to its own file,
> > > > and does some minor refactoring.
> > > > 
> > > > No functional changes expected.
> > > 
> > > You've still got the title marked as RFC - are you actually ready for
> > > this log?
> > 
> > I forgot to update this to be honest, I don't consider this an RFC,
> > especially since I dropped for now the patches that might cause
> > issues. This is now just a nice refactoring.
> 
> OK, so if we can get some block people to say they're happy, then
> I'd be happy to take this through HMP or they can take it through block.


Any update?

Best regards,
	Maxim Levitsky

> 
> Dave
> 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > > 
> > > Dave
> > > 
> > > > 
> > > > Changes from V1:
> > > >    * move the handlers to block/monitor/block-hmp-cmds.c
> > > >    * tiny cleanup for the commit messages
> > > > 
> > > > Changes from V2:
> > > >    * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
> > > >    * Set the license of blockdev-hmp-cmds.c to GPLv2+
> > > >    * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
> > > >    * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
> > > >      (this change needed some new exports, thus in separate new patch)
> > > >    * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
> > > >    * Added 'error:' prefix to vreport, and updated the iotests
> > > >      This is invasive change, but really feels like the right one
> > > >    * Added minor refactoring patch that drops an unused #include
> > > > 
> > > > Changes from V3:
> > > >    * Dropped the error prefix patches for now due to fact that it seems
> > > >      that libvirt doesn't need that after all. Oh well...
> > > >      I'll send them in a separate series.
> > > > 
> > > >    * Hopefully correctly merged the copyright info the new files
> > > >      Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)
> > > > 
> > > >    * Addressed review feedback
> > > >    * Renamed the added header to block-hmp-cmds.h
> > > > 
> > > >    * Got rid of checkpatch.pl warnings in the moved code
> > > >      (cosmetic code changes only)
> > > > 
> > > >    * I kept the reviewed-by tags, since the changes I did are minor.
> > > >      I hope that this is right thing to do.
> > > > 
> > > > Best regards,
> > > > 	Maxim Levitsky
> > > > 
> > > > Maxim Levitsky (11):
> > > >   usb/dev-storage: remove unused include
> > > >   monitor/hmp: uninline add_init_drive
> > > >   monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
> > > >   monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
> > > >   monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
> > > >     block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
> > > >     GPLv2+
> > > >   monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
> > > >   monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
> > > >     hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
> > > >     have to change the licence to GPLv2
> > > >   monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
> > > >   monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
> > > >   monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
> > > >   monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c
> > > > 
> > > >  MAINTAINERS                    |    1 +
> > > >  Makefile.objs                  |    2 +-
> > > >  block/Makefile.objs            |    1 +
> > > >  block/monitor/Makefile.objs    |    1 +
> > > >  block/monitor/block-hmp-cmds.c | 1002 ++++++++++++++++++++++++++++++++
> > > >  blockdev.c                     |  137 +----
> > > >  device-hotplug.c               |   91 ---
> > > >  hw/usb/dev-storage.c           |    1 -
> > > >  include/block/block-hmp-cmds.h |   54 ++
> > > >  include/block/block_int.h      |    5 +-
> > > >  include/monitor/hmp.h          |   24 -
> > > >  include/sysemu/blockdev.h      |    4 -
> > > >  include/sysemu/sysemu.h        |    3 -
> > > >  monitor/hmp-cmds.c             |  769 ------------------------
> > > >  monitor/misc.c                 |    1 +
> > > >  15 files changed, 1072 insertions(+), 1024 deletions(-)
> > > >  create mode 100644 block/monitor/Makefile.objs
> > > >  create mode 100644 block/monitor/block-hmp-cmds.c
> > > >  delete mode 100644 device-hotplug.c
> > > >  create mode 100644 include/block/block-hmp-cmds.h
> > > > 
> > > > -- 
> > > > 2.17.2
> > > > 
> > > 
> > > --
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK