On 1/31/19 2:08 PM, Yuval Shaia wrote: > On Thu, Jan 31, 2019 at 07:17:16AM -0600, Eric Blake wrote: >> On 1/31/19 7:08 AM, Yuval Shaia wrote: >>> Signed-off-by: Yuval Shaia >>> --- >>> hmp-commands-info.hx | 14 ++++++++++++++ >>> monitor.c | 6 ++++++ >>> 2 files changed, 20 insertions(+) > > Hi Eric, > >> >> Commit message should state WHY this is being added as an HMP-only >> command, and does not have a QMP counterpart. It may be okay if the >> interface is only designed to be useful to developers, but having that >> justification in the git log is important. > > Thanks for your review. > > See, i need this interface mainly for development/debug purposes, to help > troubleshot problems and to give insights to what device "is doing". > > Trace points are great but not effective in high load. > QMP as i see it, and correct me if i'm wrong, is used to report management > events etc and also here, is not effective in high load. > > I choose this interface as it is interactive, i.e. whenever i need the info > i trigger 'info pvrdmastats' command from the monitor console. > > During my research i notice that some devices (or families) have nice user > interface via virsh (blkstat, ifstat, memstat etc). Is it the preferred way > for non-devel/debug purposes? Using existing HMP-only debug interfaces as the design you copied is indeed acceptable justification for making yours HMP-only as well. So now you just need to copy the rationale from this email into your commit message, so it doesn't get lost. > > If this is the correct method for this purpose then let me know and i'll > update the git log message accordingly. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org