Make qemu-io commands available in the monitor
cover Make qemu-io commands available in the monitor
This is a prerequisite for some kind of tests. It involves reorganising the qemu-io code so that the command part can be separated and doesn't pollute the global namespace any more, so we can link it with qemu.
Kevin Wolf (16):
qemu-io: Remove unused args_command
cutils: Support 'P' and 'E' suffixes in strtosz()
qemu-io: Make cvtnum() a wrapper around strtosz_suffix()
qemu-io: Handle cvtnum() errors in 'alloc'
qemu-io: Don't use global bs in command implementations
qemu-io: Split off commands to qemu-io-cmds.c
qemu-io: Factor out qemuio_command
qemu-io: Move 'help' function
qemu-io: Move 'quit' function
qemu-io: Move qemu_strsep() to cutils.c
qemu-io: Move functions for registering and running commands
qemu-io: Move command_loop() and friends
qemu-io: Move remaining helpers from cmd.c
qemu-io: Interface cleanup
qemu-io: Use the qemu version for -V
Make qemu-io commands available in the monitor
Makefile | 2 +- Makefile.objs | 1 + blockdev.c | 15 + cmd.c | 612 ------------- cmd.h | 79 -- hmp-commands.hx | 16 + hmp.c | 10 + hmp.h | 1 + include/qemu-common.h | 3 + include/qemu-io.h | 46 + monitor.c | 8 +- qapi-schema.json | 16 + qemu-img.c | 10 +- qemu-io-cmds.c | 2118 ++++++++++++++++++++++++++++++++++++++++++++ qemu-io.c | 1988 ++++------------------------------------- qmp-commands.hx | 28 + tests/qemu-iotests/049.out | 8 +- util/cutils.c | 25 + 18 files changed, 2466 insertions(+), 2520 deletions(-) delete mode 100644 cmd.c delete mode 100644 cmd.h create mode 100644 include/qemu-io.h create mode 100644 qemu-io-cmds.c
Signature · 1 line
1.8.1.4
PATCH 1/16 qemu-io: Remove unused args_command
The original intention seems to be something with handling multiple images at once, but this has never been implemented and the only function ever registered is implemented to make everything behave like a "global" command. Just do that unconditionally now.
--- cmd.c | 28 ++-------------------------- cmd.h | 2 -- qemu-io.c | 10 ---------- 3 files changed, 2 insertions(+), 38 deletions(-)
cmd.c +2 -26
@@ -34,7 +34,6 @@cmdinfo_t *cmdtab;int ncmds; -static argsfunc_t args_func;static checkfunc_t check_func;static int ncmdline;static char **cmdline;
@@ -127,22 +126,6 @@ void add_user_command(char *optarg) cmdline[ncmdline-1 ] = optarg; } -static int -args_command ( -int index) -{ -if (args_func) -return args_func (index); -return 0 ; -} - -void -add_args_command ( -argsfunc_t af) -{ - args_func = af; -} -static void prep_fetchline (void *opaque) {int *fetchable = opaque;
@@ -155,7 +138,7 @@ static char *get_prompt(void);void command_loop (void ) { -int c, i, j =0 , done =0 , fetchable =0 , prompted =0 ; +int c, i, done =0 , fetchable =0 , prompted =0 ;char *input;char **v;const cmdinfo_t *ct;
@@ -171,14 +154,7 @@ void command_loop(void)if (c) { ct =find_command (v[0 ]);if (ct) { -if (ct->flags &CMD_FLAG_GLOBAL ) { - done =command (ct, c, v); - }else { - j =0 ; -while (!done && (j =args_command (j))) { - done =command (ct, c, v); - } - } + done =command (ct, c, v); }else {fprintf (stderr,_ ("command \"%s\" not found\n" ), v[0 ]); }
cmd.h +0 -2
@@ -41,12 +41,10 @@ extern int ncmds;void help_init (void );void quit_init (void ); -typedef int (*argsfunc_t )(int index);typedef int (*checkfunc_t )(const cmdinfo_t *ci);void add_command (const cmdinfo_t *ci);void add_user_command (char *optarg); -void add_args_command (argsfunc_t af);void add_check_command (checkfunc_t cf);const cmdinfo_t *find_command (const char *cmd);
qemu-io.c +0 -10
@@ -1888,15 +1888,6 @@ static int open_f(int argc, char **argv)return openfile (argv[optind], flags, growable); } -static int init_args_command (int index) -{ -/* only one device allowed so far */ -if (index >=1 ) { -return 0 ; - } -return ++index; -} -static int init_check_command (const cmdinfo_t *ct) {if (ct->flags &CMD_FLAG_GLOBAL ) {
@@ -2043,7 +2034,6 @@ int main(int argc, char **argv)add_command (&wait_break_cmd);add_command (&abort_cmd); -add_args_command (init_args_command);add_check_command (init_check_command);/* open the device */
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 01/16] qemu-io: Remove unused args_command
Quoted 11 lines · Kevin Wolf
The original intention seems to be something with handling multiple images at once, but this has never been implemented and the only function ever registered is implemented to make everything behave like a "global" command. Just do that unconditionally now.
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 28 ++-------------------------- cmd.h | 2 -- qemu-io.c | 10 ---------- 3 files changed, 2 insertions(+), 38 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 2/16 cutils: Support 'P' and 'E' suffixes in strtosz()
In reply to Kevin Wolf--- include/qemu-common.h | 2 ++ monitor.c | 8 ++++---- qemu-img.c | 10 ++++++---- tests/qemu-iotests/049.out | 8 ++++---- util/cutils.c | 4 ++++ 5 files changed, 20 insertions(+), 12 deletions(-)
include/qemu-common.h +2 -0
@@ -191,6 +191,8 @@ int parse_uint_full(const char *s, unsigned long long *value, int base);* A-Z, as strtosz() will use qemu_toupper() on the given argument * prior to comparison. */ +#define STRTOSZ_DEFSUFFIX_EB 'E' +#define STRTOSZ_DEFSUFFIX_PB 'P'#define STRTOSZ_DEFSUFFIX_TB 'T'#define STRTOSZ_DEFSUFFIX_GB 'G'#define STRTOSZ_DEFSUFFIX_MB 'M'
monitor.c +4 -4
@@ -93,10 +93,10 @@* 'M' Non-negative target long (32 or 64 bit), in user mode the * value is multiplied by 2^20 (think Mebibyte) * 'o' octets (aka bytes) -* user mode accepts an optional T, t, G, g, M, m, K, k -* suffix, which multiplies the value by 2^40 for -* suffixes T and t, 2^30 for suffixes G and g, 2^20 for -* M and m, 2^10 for K and k +* user mode accepts an optional E, e, P, p, T, t, G, g, M, m, +* K, k suffix, which multiplies the value by 2^60 for suffixes E +* and e, 2^50 for suffixes P and p, 2^40 for suffixes T and t, +* 2^30 for suffixes G and g, 2^20 for M and m, 2^10 for K and k * 'T' double * user mode accepts an optional ms, us, ns suffix, * which divides the value by 1e3, 1e6, 1e9, respectively
qemu-img.c +6 -4
@@ -84,8 +84,9 @@ static void help(void)" options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" " 'directsync' and 'unsafe' (default for convert)\n" " 'size' is the disk image size in bytes. Optional suffixes\n" -" 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M)\n" -" and T (terabyte, 1024G) are supported. 'b' is ignored.\n" +" 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n" +" 'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P) are\n" +" supported. 'b' is ignored.\n" " 'output_filename' is the destination disk image filename\n" " 'output_fmt' is the destination format\n" " 'options' is a comma separated list of format specific options in a\n"
@@ -386,8 +387,9 @@ static int img_create(int argc, char **argv)error_report ("Image size must be less than 8 EiB!" ); }else {error_report ("Invalid image size specified! You may use k, M, " -"G or T suffixes for " ); -error_report ("kilobytes, megabytes, gigabytes and terabytes." ); +"G, T, P or E suffixes for " ); +error_report ("kilobytes, megabytes, gigabytes, terabytes, " +"petabytes and exabytes." ); }return 1 ; }
tests/qemu-iotests/049.out +4 -4
@@ -108,15 +108,15 @@ qemu-img: Formatting or formatting option not supported for file format 'qcow2' Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte -qemu-img: Invalid image size specified! You may use k, M, G or T suffixes for -qemu-img: kilobytes, megabytes, gigabytes and terabytes. +qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for +qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar -qemu-img: Invalid image size specified! You may use k, M, G or T suffixes for -qemu-img: kilobytes, megabytes, gigabytes and terabytes. +qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for +qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2 qemu-img: Parameter 'size' expects a size
util/cutils.c +4 -0
@@ -267,6 +267,10 @@ static int64_t suffix_mul(char suffix, int64_t unit)return unit * unit * unit;case STRTOSZ_DEFSUFFIX_TB :return unit * unit * unit * unit; +case STRTOSZ_DEFSUFFIX_PB : +return unit * unit * unit * unit * unit; +case STRTOSZ_DEFSUFFIX_EB : +return unit * unit * unit * unit * unit * unit; }return -1 ; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 02/16] cutils: Support 'P' and 'E' suffixes in strtosz()
Quoted 9 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- include/qemu-common.h | 2 ++ monitor.c | 8 ++++---- qemu-img.c | 10 ++++++---- tests/qemu-iotests/049.out | 8 ++++---- util/cutils.c | 4 ++++ 5 files changed, 20 insertions(+), 12 deletions(-)
qemu-img.c static void help(void) · line 88
Quoted 9 lines
+++
b/qemu-img.cqemu-img.c +2
-2@@ -84,8 +84,9 @@ static void help(void)" options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" " 'directsync' and 'unsafe' (default for convert)\n" " 'size' is the disk image size in bytes. Optional suffixes\n" -" 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M)\n" -" and T (terabyte, 1024G) are supported. 'b' is ignored.\n" +" 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n" +" 'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P) are\n"
Pre-existing, but should we be favoring the technically correct kibibyte, mebibyte, gibibyte, ... terms, since we are treating it as powers of 1024, rather than the potentially confusing kilobyte term (since disk vendors favor powers of 1000)?
util/cutils.c static int64_t suffix_mul(char suffix, int64_t unit) · line 273
Quoted 9 lines
util/cutils.c +5
-0+++ b/util/cutils.c@@ -267,6 +267,10 @@ static int64_t suffix_mul(char suffix, int64_t unit)return unit * unit * unit;case STRTOSZ_DEFSUFFIX_TB :return unit * unit * unit * unit; +case STRTOSZ_DEFSUFFIX_PB : +return unit * unit * unit * unit * unit; +case STRTOSZ_DEFSUFFIX_EB : +return unit * unit * unit * unit * unit * unit;
Pre-existing, but this code is not robust against overflow. And now that you are adding E, it's 6 fewer decimal characters that I have to type to pass in a value that will exceed even int64_t.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 3/16 qemu-io: Make cvtnum() a wrapper around strtosz_suffix()
In reply to Kevin WolfNo reason to implement the same thing multiple times. A nice side effect is that fractional numbers like 0.5M can be used in qemu-io now.
--- cmd.c | 37 ------------------------------------- cmd.h | 1 - qemu-io.c | 6 ++++++ 3 files changed, 6 insertions(+), 38 deletions(-)
cmd.c +0 -37
@@ -344,43 +344,6 @@ doneline(#define MEGABYTES (x) ((long long)(x) << 20)#define KILOBYTES (x) ((long long)(x) << 10) -long long -cvtnum ( -char *s) -{ -long long i; -char *sp; -int c; - - i =strtoll (s, &sp,0 ); -if (i ==0 && sp == s) -return -1LL ; -if (*sp =='\0' ) -return i; - -if (sp[1 ] !='\0' ) -return -1LL ; - - c =qemu_tolower (*sp); -switch (c) { -default : -return i; -case 'k' : -return KILOBYTES (i); -case 'm' : -return MEGABYTES (i); -case 'g' : -return GIGABYTES (i); -case 't' : -return TERABYTES (i); -case 'p' : -return PETABYTES (i); -case 'e' : -return EXABYTES (i); - } -return -1LL ; -} -#define TO_EXABYTES (x) ((x) / EXABYTES(1))#define TO_PETABYTES (x) ((x) / PETABYTES(1))#define TO_TERABYTES (x) ((x) / TERABYTES(1))
cmd.h +0 -1
@@ -58,7 +58,6 @@ char **breakline(char *input, int *count);void doneline (char *input,char **vec);char *fetchline (void ); -long long cvtnum (char *s);void cvtstr (double value,char *str,size_t sz);struct timeval tsub (struct timeval t1,struct timeval t2);
qemu-io.c +6 -0
@@ -29,6 +29,12 @@ static BlockDriverState *bs;static int misalign; +static int64_t cvtnum (const char *s) +{ +char *end; +return strtosz_suffix (s, &end,STRTOSZ_DEFSUFFIX_B ); +} +/* * Parse the pattern argument to various sub-commands. *
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 03/16] qemu-io: Make cvtnum() a wrapper around strtosz_suffix()
Quoted 10 lines · Kevin Wolf
No reason to implement the same thing multiple times. A nice side effect is that fractional numbers like 0.5M can be used in qemu-io now.
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 37 ------------------------------------- cmd.h | 1 - qemu-io.c | 6 ++++++ 3 files changed, 6 insertions(+), 38 deletions(-)
cmd.c doneline( · line 359
Quoted 6 lines
cmd.c +0
-6- - i =strtoll (s, &sp,0 ); -if (i ==0 && sp == s) -return -1LL ; -if (*sp =='\0' ) -return i;
Another nice side effect: you are also getting rid of poor handling that fails to check for errors after strtoll() (such as clamping at INT64_MAX on EOVERFLOW). On the other hand, strtosz_suffix uses strtod() which may inadvertently cause rounding of low-order bits of large input numbers (that is, parsing with only 53 significant bits instead of 64 may cause us to round a value that was previously exact); but I'd rather deal with consistent behavior even if it rounds some values, than question which of multiple slightly different implementations will be parsing my values (we can also argue that perhaps strtosz_suffix could be improved to guarantee 64 bits of precision if there is no '.' in the value being parsed, while still providing the flexibility of 0.5G parsing when precision isn't important).
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 4/16 qemu-io: Handle cvtnum() errors in 'alloc'
In reply to Kevin Wolf--- qemu-io.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
qemu-io.c +8 -1
@@ -1596,7 +1596,10 @@ static int alloc_f(int argc, char **argv)int ret; offset =cvtnum (argv[1 ]); -if (offset &0x1ff ) { +if (offset <0 ) { +printf ("non-numeric offset argument -- %s\n" , argv[1 ]); +return 0 ; + }else if (offset &0x1ff ) {printf ("offset %" PRId64" is not sector aligned\n" , offset);return 0 ;
@@ -1604,6 +1607,10 @@ static int alloc_f(int argc, char **argv)if (argc ==3 ) { nb_sectors =cvtnum (argv[2 ]); +if (nb_sectors <0 ) { +printf ("non-numeric length argument -- %s\n" , argv[2 ]); +return 0 ; + } }else { nb_sectors =1 ; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 04/16] qemu-io: Handle cvtnum() errors in 'alloc'
Quoted 5 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- qemu-io.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 5/16 qemu-io: Don't use global bs in command implementations
In reply to Kevin WolfPass in the BlockDriverState to the command handlers instead of using the global variable. This is an important step to make the commands usable outside of qemu-io.
--- cmd.c | 6 ++- cmd.h | 8 ++- qemu-io.c | 165 ++++++++++++++++++++++++++++++++++---------------------------- 3 files changed, 100 insertions(+), 79 deletions(-)
cmd.c +4 -2
@@ -57,7 +57,7 @@ check_command( const cmdinfo_t *ci) {if (check_func) -return check_func (ci); +return check_func (qemuio_bs, ci);return 1 ; }
@@ -103,7 +103,7 @@ command(return 0 ; } optind =0 ; -return ct->cfunc (argc, argv); +return ct->cfunc (qemuio_bs, argc, argv); } const cmdinfo_t *
@@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;/* ARGSUSED */ static int quit_f ( +BlockDriverState *bs,int argc,char **argv) {
@@ -490,6 +491,7 @@ help_all(void)static int help_f ( +BlockDriverState *bs,int argc,char **argv) {
cmd.h +6 -2
@@ -17,9 +17,13 @@ #ifndef __COMMAND_H__ #define __COMMAND_H__ +#include "qemu-common.h" + #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */ -typedef int (*cfunc_t)(int argc, char **argv); +extern BlockDriverState *qemuio_bs; + +typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv); typedef void (*helpfunc_t)(void); typedef struct cmdinfo {
@@ -41,7 +45,7 @@ extern int ncmds;void help_init (void );void quit_init (void ); -typedef int (*checkfunc_t )(const cmdinfo_t *ci); +typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci);void add_command (const cmdinfo_t *ci);void add_user_command (char *optarg);
qemu-io.c +90 -75
563 lines not on this page · raw message
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations
Am 28.05.2013 um 17:27 hat Kevin Wolf geschrieben:
Quoted 7 lines
Pass in the BlockDriverState to the command handlers instead of using the global variable. This is an important step to make the commands usable outside of qemu-io.
Signed-off-by Kevin Wolf <[email protected]>
Review static const cmdinfo_t abort_cmd = { · line 1814
Quoted 8 lines
(quoted diff) +1
-1@@ -1793,7 +1808,7 @@ static const cmdinfo_t abort_cmd = { .oneline = "simulate a program crash using abort(3)", }; -static int close_f(int argc, char **argv) +static int close_f(BlockDriverState *bs, int argc, char **argv) { bdrv_delete(bs); bs = NULL;
Here I missed the following hunk that I've applied locally now:
qemu-io.c +1 -1
@@ -1811,7 +1811,7 @@ static const cmdinfo_t abort_cmd = {static int close_f (BlockDriverState *bs,int argc,char **argv) {bdrv_delete (bs); - bs =NULL ; + qemuio_bs =NULL ;return 0 ; }
Eric Blake
Reviewed-byRe: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations
Quoted 22 lines · Kevin Wolf
Am 28.05.2013 um 17:27 hat Kevin Wolf geschrieben:
Quoted 7 lines
Pass in the BlockDriverState to the command handlers instead of using the global variable. This is an important step to make the commands usable outside of qemu-io.
Signed-off-by Kevin Wolf <[email protected]>Review static const cmdinfo_t abort_cmd = { · line 1814
Quoted 8 lines
(quoted diff) +1
-1@@ -1793,7 +1808,7 @@ static const cmdinfo_t abort_cmd = { .oneline = "simulate a program crash using abort(3)", }; -static int close_f(int argc, char **argv) +static int close_f(BlockDriverState *bs, int argc, char **argv) { bdrv_delete(bs); bs = NULL;Here I missed the following hunk that I've applied locally now:
qemu-io.c static const cmdinfo_t abort_cmd = { · line 1814
Quoted 5 lines
qemu-io.c +1
-1@@ -1811,7 +1811,7 @@ static const cmdinfo_t abort_cmd = {static int close_f (BlockDriverState *bs,int argc,char **argv) {bdrv_delete (bs); - bs =NULL ; + qemuio_bs =NULL ;
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations
In reply to Kevin WolfQuoted 34 lines · Kevin Wolf
Pass in the BlockDriverState to the command handlers instead of using the global variable. This is an important step to make the commands usable outside of qemu-io.
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 6 ++- cmd.h | 8 ++- qemu-io.c | 165 ++++++++++++++++++++++++++++++++++---------------------------- 3 files changed, 100 insertions(+), 79 deletions(-)cmd.c +2
-2@@ -57,7 +57,7 @@ check_command( const cmdinfo_t *ci) {if (check_func) -return check_func (ci); +return check_func (qemuio_bs, ci);return 1 ; }@@ -103,7 +103,7 @@ command(return 0 ; } optind =0 ; -return ct->cfunc (argc, argv); +return ct->cfunc (qemuio_bs, argc, argv); } const cmdinfo_t *
cmd.c static cmdinfo_t quit_cmd; · line 456
Quoted 5 lines
cmd.c +1
-0@@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;/* ARGSUSED */ static int quit_f( + BlockDriverState *bs, int argc,
tabs vs spaces. I try to keep the existing style unless I decide to reformat the entire section of code.
Not trying to start a flamewar but this file appears to use tabs and IMO you should stick to that instead of mixing spaces :-).
Quoted 30 lines
cmd.c +1
-0char **argv) {@@ -490,6 +491,7 @@ help_all(void)static int help_f ( +BlockDriverState *bs,int argc,char **argv) {cmd.h +5
-1@@ -17,9 +17,13 @@ #ifndef __COMMAND_H__ #define __COMMAND_H__ +#include "qemu-common.h" + #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */ -typedef int (*cfunc_t)(int argc, char **argv); +extern BlockDriverState *qemuio_bs; + +typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv); typedef void (*helpfunc_t)(void); typedef struct cmdinfo {
cmd.h extern int ncmds; · line 51
Quoted 8 lines
cmd.h +1
-1@@ -41,7 +45,7 @@ extern int ncmds;void help_init (void );void quit_init (void ); -typedef int (*checkfunc_t )(const cmdinfo_t *ci); +typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci);void add_command (const cmdinfo_t *ci);void add_user_command (char *optarg);
cmd.h does not know about the block layer. I would use void *opaque instead of BlockDriverState *bs. That way the file stays generic and can be used in other command-line tools.
Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations
Am 05.06.2013 um 14:28 hat Stefan Hajnoczi geschrieben:
Quoted 44 lines · Kevin Wolf
Quoted 34 lines
Pass in the BlockDriverState to the command handlers instead of using the global variable. This is an important step to make the commands usable outside of qemu-io.
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 6 ++- cmd.h | 8 ++- qemu-io.c | 165 ++++++++++++++++++++++++++++++++++---------------------------- 3 files changed, 100 insertions(+), 79 deletions(-)cmd.c +2
-2@@ -57,7 +57,7 @@ check_command( const cmdinfo_t *ci) {if (check_func) -return check_func (ci); +return check_func (qemuio_bs, ci);return 1 ; }@@ -103,7 +103,7 @@ command(return 0 ; } optind =0 ; -return ct->cfunc (argc, argv); +return ct->cfunc (qemuio_bs, argc, argv); } const cmdinfo_t *cmd.c static cmdinfo_t quit_cmd; · line 456
Quoted 5 lines
cmd.c +1
-0@@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;/* ARGSUSED */ static int quit_f( + BlockDriverState *bs, int argc,tabs vs spaces. I try to keep the existing style unless I decide to reformat the entire section of code.
Quoted 2 lines
Not trying to start a flamewar but this file appears to use tabs and IMO you should stick to that instead of mixing spaces :-).
Ah yes, didn't notice that.
Doesn't really matter though, at the end of the series cmd.c is gone.
Quoted 24 lines
Quoted 24 lines
cmd.h +6
-2@@ -17,9 +17,13 @@ #ifndef __COMMAND_H__ #define __COMMAND_H__ +#include "qemu-common.h" + #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */ -typedef int (*cfunc_t)(int argc, char **argv); +extern BlockDriverState *qemuio_bs; + +typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv); typedef void (*helpfunc_t)(void); typedef struct cmdinfo {@@ -41,7 +45,7 @@ extern int ncmds;void help_init (void );void quit_init (void ); -typedef int (*checkfunc_t )(const cmdinfo_t *ci); +typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci);
Quoted 6 lines
cmd.h extern int ncmds; · line 51
Quoted 2 lines
cmd.h +0
-0void add_command (const cmdinfo_t *ci);void add_user_command (char *optarg);
cmd.hdoes not know about the block layer. I would use void *opaque instead of BlockDriverState *bs. That way the file stays generic and can be used in other command-line tools.
Do you plan to use this in different context? Because this series is exactly the opposite of keeping it generic. It moves everything directly into qemu-io.
Kevin
PATCH 6/16 qemu-io: Split off commands to qemu-io-cmds.c
In reply to Kevin WolfThis is the implementation of all qemu-io commands that make sense to be called from the qemu monitor, i.e. everything except open, close and quit.
--- Makefile | 2 +- qemu-io-cmds.c | 1835 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-io.c | 1817 +------------------------------------------------------ 3 files changed, 1838 insertions(+), 1816 deletions(-) create mode 100644 qemu-io-cmds.c
Makefile +1 -1
@@ -186,7 +186,7 @@ qemu-img.o: qemu-img-cmds.h qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a -qemu-io$(EXESUF): qemu-io.o cmd.o $(block-obj-y) libqemuutil.a libqemustub.a +qemu-io$(EXESUF): qemu-io.o qemu-io-cmds.o cmd.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
qemu-io-cmds.c new file +1835 -0
1,835 lines not on this page · raw message
qemu-io.c +2 -1815
1,157 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
1.8.1.4
Part of the diff is not shown here. View the raw message.
Re: [Qemu-devel] [PATCH 06/16] qemu-io: Split off commands to qemu-io-cmds.c
Quoted 11 lines · Kevin Wolf
This is the implementation of all qemu-io commands that make sense to be called from the qemu monitor, i.e. everything except open, close and quit.
Signed-off-by Kevin Wolf <[email protected]>--- Makefile | 2 +- qemu-io-cmds.c | 1835 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-io.c | 1817 +------------------------------------------------------ 3 files changed, 1838 insertions(+), 1816 deletions(-) create mode 100644 qemu-io-cmds.c
I checked the bulk of this patch via:
$ diff -u <(sed -n '/^-/ s///p' file) <(sed -n '/^\+/ s///p' file)
The bulk of the patch is blind code motion plus a bit of touchup; I did however spot one case where you were too eager in your search-and-replace:
* Memory allocation helpers.
*
- * Make sure memory is aligned by default, or purposefully misaligned if
+ * Make sure memory is aligned by default, or purposefully qemuio_misaligned if
* that is specified on the command line.
Review
Quoted 2 lines
(quoted diff) +2
-0+ +int qemuio_misalign;
Should this variable be typed 'bool'?
Other than that,
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 06/16] qemu-io: Split off commands to qemu-io-cmds.c
Am 29.05.2013 um 22:29 hat Eric Blake geschrieben:
Quoted 20 lines · Kevin Wolf
Quoted 11 lines
This is the implementation of all qemu-io commands that make sense to be called from the qemu monitor, i.e. everything except open, close and quit.
Signed-off-by Kevin Wolf <[email protected]>--- Makefile | 2 +- qemu-io-cmds.c | 1835 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-io.c | 1817 +------------------------------------------------------ 3 files changed, 1838 insertions(+), 1816 deletions(-) create mode 100644 qemu-io-cmds.cI checked the bulk of this patch via:
$ diff -u <(sed -n '/^-/ s///p' file) <(sed -n '/^\+/ s///p' file)The bulk of the patch is blind code motion plus a bit of touchup; I did however spot one case where you were too eager in your search-and-replace:
Quoted 6 lines
* Memory allocation helpers.
*
- * Make sure memory is aligned by default, or purposefully misaligned if
+ * Make sure memory is aligned by default, or purposefullyqemuio_misalignedif
* that is specified on the command line.
Whoops, thanks for spotting this. :-)
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ +int qemuio_misalign;Should this variable be typed 'bool'?
It should, but this is merely code motion, so I won't fix it here.
Kevin
PATCH 7/16 qemu-io: Factor out qemuio_command
In reply to Kevin WolfIt's duplicated code. Move it to qemu-io-cmds.c because it's not dependent on any static data of the qemu-io tool.
--- cmd.c | 43 +++++-------------------------------------- cmd.h | 3 ++- qemu-io-cmds.c | 24 ++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 39 deletions(-)
cmd.c +5 -38
@@ -138,28 +138,11 @@ static char *get_prompt(void);void command_loop (void ) { -int c, i, done =0 , fetchable =0 , prompted =0 ; +int i, done =0 , fetchable =0 , prompted =0 ;char *input; -char **v; -const cmdinfo_t *ct;for (i =0 ; !done && i < ncmdline; i++) { - input =strdup (cmdline[i]); -if (!input) { -fprintf (stderr,_ ("cannot strdup command '%s': %s\n" ), - cmdline[i],strerror (errno)); -exit (1 ); - } - v =breakline (input, &c); -if (c) { - ct =find_command (v[0 ]); -if (ct) { - done =command (ct, c, v); - }else { -fprintf (stderr,_ ("command \"%s\" not found\n" ), v[0 ]); - } - } -doneline (input, v); + done =qemuio_command (cmdline[i]); }if (cmdline) {g_free (cmdline);
@@ -179,20 +162,13 @@ void command_loop(void)if (!fetchable) {continue ; } + input =fetchline ();if (input ==NULL ) {break ; } - v =breakline (input, &c); -if (c) { - ct =find_command (v[0 ]); -if (ct) { - done =command (ct, c, v); - }else { -fprintf (stderr,_ ("command \"%s\" not found\n" ), v[0 ]); - } - } -doneline (input, v); + done =qemuio_command (input); +free (input); prompted =0 ; fetchable =0 ;
@@ -328,15 +304,6 @@ char **breakline(char *input, int *count)return rval; } -void -doneline ( -char *input, -char **vec) -{ -free (input); -free (vec); -} -#define EXABYTES (x) ((long long)(x) << 60)#define PETABYTES (x) ((long long)(x) << 50)#define TERABYTES (x) ((long long)(x) << 40)
cmd.h +2 -1
@@ -59,7 +59,6 @@ int command(const cmdinfo_t *ci, int argc, char **argv);/* from input.h */ char **breakline (char *input,int *count); -void doneline (char *input,char **vec);char *fetchline (void );void cvtstr (double value,char *str,size_t sz);
@@ -77,4 +76,6 @@ void timestr(struct timeval *tv, char *str, size_t sz, int flags);extern char *progname; +bool qemuio_command (const char *cmd); +#endif /* __COMMAND_H__ */
qemu-io-cmds.c +24 -0
@@ -1807,6 +1807,30 @@ static int init_check_command(BlockDriverState *bs, const cmdinfo_t *ct)return 1 ; } +bool qemuio_command (const char *cmd) +{ +char *input; +const cmdinfo_t *ct; +char **v; +int c; +bool done = false; + + input =g_strdup (cmd); + v =breakline (input, &c); +if (c) { + ct =find_command (v[0 ]); +if (ct) { + done =command (ct, c, v); + }else { +fprintf (stderr,"command \"%s\" not found\n" , v[0 ]); + } + } +g_free (input); +g_free (v); + +return done; +} +static void __attribute((constructor))init_qemuio_commands (void ) {/* initialize commands */
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 07/16] qemu-io: Factor out qemuio_command
Quoted 10 lines · Kevin Wolf
It's duplicated code. Move it to
qemu-io-cmds.cbecause it's not dependent on any static data of the qemu-io tool.Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 43 +++++-------------------------------------- cmd.h | 3 ++- qemu-io-cmds.c | 24 ++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 39 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 07/16] qemu-io: Factor out qemuio_command
In reply to Kevin WolfQuoted 13 lines · Kevin Wolf
cmd.h +0
-1@@ -59,7 +59,6 @@ int command(const cmdinfo_t *ci, int argc, char **argv);/* from input.h */ char **breakline (char *input,int *count); -void doneline (char *input,char **vec);char *fetchline (void );void cvtstr (double value,char *str,size_t sz);
cmd.h void timestr(struct timeval *tv, char *str, size_t sz, int flags); · line 79
Quoted 4 lines
cmd.h +1
-0@@ -77,4 +76,6 @@ void timestr(struct timeval *tv, char *str, size_t sz, int flags);extern char *progname; +bool qemuio_command (const char *cmd);
If we keep cmd.h generic then qemuio_command() should go into another header file.
PATCH 8/16 qemu-io: Move 'help' function
In reply to Kevin WolfNo reason to treat it different from other commands. Move it to qemu-io-cmds.c, adapt the coding style and register it like any other command.
--- cmd.c | 79 ---------------------------------------------------------- cmd.h | 1 - qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 81 deletions(-)
cmd.c +0 -79
@@ -439,82 +439,3 @@ quit_init(void)add_command (&quit_cmd); } - -/* from libxcmd/help.c */ - -static cmdinfo_t help_cmd; -static void help_onecmd (const char *cmd,const cmdinfo_t *ct); -static void help_oneline (const char *cmd,const cmdinfo_t *ct); - -static void -help_all (void ) -{ -const cmdinfo_t *ct; - -for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++) -help_oneline (ct->name, ct); -printf (_ ("\nUse 'help commandname' for extended help.\n" )); -} - -static int -help_f ( -BlockDriverState *bs, -int argc, -char **argv) -{ -const cmdinfo_t *ct; - -if (argc ==1 ) { -help_all (); -return 0 ; - } - ct =find_command (argv[1 ]); -if (ct ==NULL ) { -printf (_ ("command %s not found\n" ), argv[1 ]); -return 0 ; - } -help_onecmd (argv[1 ], ct); -return 0 ; -} - -static void -help_onecmd ( -const char *cmd, -const cmdinfo_t *ct) -{ -help_oneline (cmd, ct); -if (ct->help) - ct->help (); -} - -static void -help_oneline ( -const char *cmd, -const cmdinfo_t *ct) -{ -if (cmd) -printf ("%s " , cmd); -else { -printf ("%s " , ct->name); -if (ct->altname) -printf ("(or %s) " , ct->altname); - } -if (ct->args) -printf ("%s " , ct->args); -printf ("-- %s\n" , ct->oneline); -} - -void -help_init (void ) -{ - help_cmd.name =_ ("help" ); - help_cmd.altname =_ ("?" ); - help_cmd.cfunc = help_f; - help_cmd.argmin =0 ; - help_cmd.argmax =1 ; - help_cmd.flags =CMD_FLAG_GLOBAL ; - help_cmd.args =_ ("[command]" ); - help_cmd.oneline =_ ("help for one or all commands" ); - -add_command (&help_cmd); -}
cmd.h +0 -1
@@ -42,7 +42,6 @@ typedef struct cmdinfo {extern cmdinfo_t *cmdtab;extern int ncmds; -void help_init (void );void quit_init (void );typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci);
qemu-io-cmds.c +66 -1
79 lines not on this page · raw message
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 08/16] qemu-io: Move 'help' function
Quoted 12 lines · Kevin Wolf
No reason to treat it different from other commands. Move it to
qemu-io-cmds.c, adapt the coding style and register it like any other command.Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 79 ---------------------------------------------------------- cmd.h | 1 - qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 81 deletions(-)cmd.c +0
-1-void
cmd.c quit_init(void) · line 517
Quoted 10 lines
cmd.c +0
-10-help_init (void) -{ - help_cmd.name =_ ("help" ); - help_cmd.altname =_ ("?" ); - help_cmd.cfunc = help_f; - help_cmd.argmin =0 ; - help_cmd.argmax =1 ; - help_cmd.flags =CMD_FLAG_GLOBAL ; - help_cmd.args =_ ("[command]" ); - help_cmd.oneline =_ ("help for one or all commands" );
The old code marked strings for translation with _(), ...
qemu-io-cmds.c static const cmdinfo_t abort_cmd = { · line 1860
Quoted 9 lines
qemu-io-cmds.c +9
-0+static const cmdinfo_t help_cmd = { + .name ="help" , + .altname ="?" , + .cfunc = help_f, + .argmin =0 , + .argmax =1 , + .flags =CMD_FLAG_GLOBAL , + .args ="[command]" , + .oneline ="help for one or all commands" ,
...whereas the new code uses fixed literals. Then again, po/messages.po doesn't contain any mention of cmd.c, so it's not like we were actually translating the strings, even if they were marked for translation. So no real change.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 08/16] qemu-io: Move 'help' function
Am 29.05.2013 um 23:25 hat Eric Blake geschrieben:
Quoted 36 lines · Kevin Wolf
Quoted 12 lines
No reason to treat it different from other commands. Move it to
qemu-io-cmds.c, adapt the coding style and register it like any other command.Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 79 ---------------------------------------------------------- cmd.h | 1 - qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 81 deletions(-)cmd.c +0
-1-void cmd.c quit_init(void) · line 517
Quoted 10 lines
cmd.c +0
-10-help_init (void) -{ - help_cmd.name =_ ("help" ); - help_cmd.altname =_ ("?" ); - help_cmd.cfunc = help_f; - help_cmd.argmin =0 ; - help_cmd.argmax =1 ; - help_cmd.flags =CMD_FLAG_GLOBAL ; - help_cmd.args =_ ("[command]" ); - help_cmd.oneline =_ ("help for one or all commands" );The old code marked strings for translation with
_(), ...qemu-io-cmds.c static const cmdinfo_t abort_cmd = { · line 1860
Quoted 9 lines
qemu-io-cmds.c +9
-0+static const cmdinfo_t help_cmd = { + .name ="help" , + .altname ="?" , + .cfunc = help_f, + .argmin =0 , + .argmax =1 , + .flags =CMD_FLAG_GLOBAL , + .args ="[command]" , + .oneline ="help for one or all commands" ,
Quoted 4 lines
...whereas the new code uses fixed literals. Then again, po/messages.po doesn't contain any mention of
cmd.c, so it's not like we were actually translating the strings, even if they were marked for translation. So no real change.
In fact, the real reason is that cmd.c has this line:
#define _(x) x /* not gettext support yet */
Apart from that, translating command names has never been a great idea anyway, so at least that part wouldn't a big loss even if gettext was actually used...
Kevin
Re: [Qemu-devel] [PATCH 08/16] qemu-io: Move 'help' function
In reply to Kevin Wolfcmd.c
Quoted 15 lines · Kevin Wolf
No reason to treat it different from other commands. Move it to
qemu-io-cmds.c, adapt the coding style and register it like any other command.Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 79 ---------------------------------------------------------- cmd.h | 1 - qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 81 deletions(-)cmd.c +0
-0
'help' is a generic command, it's not specific to qemu-io. IMO cmd.c is the right place for it since other command-line tools might also need 'help'.
PATCH 9/16 qemu-io: Move 'quit' function
In reply to Kevin WolfThis one only makes sense in the context of the qemu-io tool, so move it to qemu-io.c. Adapt coding style and register it like other commands.
--- cmd.c | 29 ----------------------------- cmd.h | 2 -- qemu-io.c | 17 ++++++++++++++++- 3 files changed, 16 insertions(+), 32 deletions(-)
cmd.c +0 -29
@@ -410,32 +410,3 @@ timestr(snprintf (ts, size,"0.%04u sec" , (unsigned int ) (usec *10000 )); } } - - -/* from libxcmd/quit.c */ - -static cmdinfo_t quit_cmd; - -/* ARGSUSED */ -static int -quit_f ( -BlockDriverState *bs, -int argc, -char **argv) -{ -return 1 ; -} - -void -quit_init (void ) -{ - quit_cmd.name =_ ("quit" ); - quit_cmd.altname =_ ("q" ); - quit_cmd.cfunc = quit_f; - quit_cmd.argmin =-1 ; - quit_cmd.argmax =-1 ; - quit_cmd.flags =CMD_FLAG_GLOBAL ; - quit_cmd.oneline =_ ("exit the program" ); - -add_command (&quit_cmd); -}
cmd.h +0 -2
@@ -42,8 +42,6 @@ typedef struct cmdinfo {extern cmdinfo_t *cmdtab;extern int ncmds; -void quit_init (void ); -typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci);void add_command (const cmdinfo_t *ci);
qemu-io.c +16 -1
@@ -137,6 +137,21 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)return openfile (argv[optind], flags, growable); } +static int quit_f (BlockDriverState *bs,int argc,char **argv) +{ +return 1 ; +} + +static const cmdinfo_t quit_cmd = { + .name ="quit" , + .altname ="q" , + .cfunc = quit_f, + .argmin =-1 , + .argmax =-1 , + .flags =CMD_FLAG_GLOBAL , + .oneline ="exit the program" , +}; +static void usage (const char *name) { printf(
@@ -247,7 +262,7 @@ int main(int argc, char **argv)bdrv_init ();/* initialize commands */ -quit_init (); +add_command (&quit_cmd);add_command (&open_cmd);add_command (&close_cmd);
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 09/16] qemu-io: Move 'quit' function
Quoted 10 lines · Kevin Wolf
This one only makes sense in the context of the qemu-io tool, so move it to
qemu-io.c. Adapt coding style and register it like other commands.Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 29 ----------------------------- cmd.h | 2 -- qemu-io.c | 17 ++++++++++++++++- 3 files changed, 16 insertions(+), 32 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 10/16 qemu-io: Move qemu_strsep() to cutils.c
In reply to Kevin Wolf--- cmd.c | 21 --------------------- include/qemu-common.h | 1 + util/cutils.c | 21 +++++++++++++++++++++ 3 files changed, 22 insertions(+), 21 deletions(-)
cmd.c +0 -21
@@ -255,27 +255,6 @@ fetchline(void) }#endif -static char *qemu_strsep (char **input,const char *delim) -{ -char *result = *input; -if (result !=NULL ) { -char *p; - -for (p = result; *p !='\0' ; p++) { -if (strchr (delim, *p)) { -break ; - } - } -if (*p =='\0' ) { - *input =NULL ; - }else { - *p ='\0' ; - *input = p +1 ; - } - } -return result; -} -char **breakline (char *input,int *count) {int c =0 ;
include/qemu-common.h +1 -0
@@ -174,6 +174,7 @@ char *pstrcat(char *buf, int buf_size, const char *s);int strstart (const char *str,const char *val,const char **ptr);int stristart (const char *str,const char *val,const char **ptr);int qemu_strnlen (const char *s,int max_len); +char *qemu_strsep (char **input,const char *delim);time_t mktimegm (struct tm *tm);int qemu_fls (int i);int qemu_fdatasync (int fd);
util/cutils.c +21 -0
@@ -107,6 +107,27 @@ int qemu_strnlen(const char *s, int max_len)return i; } +char *qemu_strsep (char **input,const char *delim) +{ +char *result = *input; +if (result !=NULL ) { +char *p; + +for (p = result; *p !='\0' ; p++) { +if (strchr (delim, *p)) { +break ; + } + } +if (*p =='\0' ) { + *input =NULL ; + }else { + *p ='\0' ; + *input = p +1 ; + } + } +return result; +} +time_t mktimegm (struct tm *tm) {time_t t;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 10/16] qemu-io: Move qemu_strsep() to cutils.c
Quoted 6 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 21 --------------------- include/qemu-common.h | 1 + util/cutils.c | 21 +++++++++++++++++++++ 3 files changed, 22 insertions(+), 21 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 11/16 qemu-io: Move functions for registering and running commands
In reply to Kevin Wolf--- cmd.c | 113 --------------------------------- cmd.h | 11 +--- qemu-io-cmds.c | 192 ++++++++++++++++++++++++++++++++++++++++++--------------- qemu-io.c | 10 +-- 4 files changed, 148 insertions(+), 178 deletions(-)
cmd.c +0 -113
@@ -31,94 +31,9 @@/* from libxcmd/command.c */ -cmdinfo_t *cmdtab; -int ncmds; - -static checkfunc_t check_func;static int ncmdline;static char **cmdline; -static int -compare (const void *a,const void *b) -{ -return strcmp (((const cmdinfo_t *)a)->name, - ((const cmdinfo_t *)b)->name); -} - -void add_command (const cmdinfo_t *ci) -{ - cmdtab =g_realloc ((void *)cmdtab, ++ncmds *sizeof (*cmdtab)); - cmdtab[ncmds -1 ] = *ci; -qsort (cmdtab, ncmds,sizeof (*cmdtab), compare); -} - -static int -check_command ( -const cmdinfo_t *ci) -{ -if (check_func) -return check_func (qemuio_bs, ci); -return 1 ; -} - -void -add_check_command ( -checkfunc_t cf) -{ - check_func = cf; -} - -int -command_usage ( -const cmdinfo_t *ci) -{ -printf ("%s %s -- %s\n" , ci->name, ci->args, ci->oneline); -return 0 ; -} - -int -command ( -const cmdinfo_t *ct, -int argc, -char **argv) -{ -char *cmd = argv[0 ]; - -if (!check_command (ct)) -return 0 ; - -if (argc-1 < ct->argmin || (ct->argmax !=-1 && argc-1 > ct->argmax)) { -if (ct->argmax ==-1 ) -fprintf (stderr, -_ ("bad argument count %d to %s, expected at least %d arguments\n" ), - argc-1 , cmd, ct->argmin); -else if (ct->argmin == ct->argmax) -fprintf (stderr, -_ ("bad argument count %d to %s, expected %d arguments\n" ), - argc-1 , cmd, ct->argmin); -else -fprintf (stderr, -_ ("bad argument count %d to %s, expected between %d and %d arguments\n" ), - argc-1 , cmd, ct->argmin, ct->argmax); -return 0 ; - } - optind =0 ; -return ct->cfunc (qemuio_bs, argc, argv); -} - -const cmdinfo_t * -find_command ( -const char *cmd) -{ -cmdinfo_t *ct; - -for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++) { -if (strcmp (ct->name, cmd) ==0 || - (ct->altname &&strcmp (ct->altname, cmd) ==0 )) -return (const cmdinfo_t *)ct; - } -return NULL ; -}void add_user_command (char *optarg) {
@@ -255,34 +170,6 @@ fetchline(void) }#endif -char **breakline (char *input,int *count) -{ -int c =0 ; -char *p; -char **rval =calloc (sizeof (char *),1 ); -char **tmp; - -while (rval && (p =qemu_strsep (&input," " )) !=NULL ) { -if (!*p) { -continue ; - } - c++; - tmp =realloc (rval,sizeof (*rval) * (c +1 )); -if (!tmp) { -free (rval); - rval =NULL ; - c =0 ; -break ; - }else { - rval = tmp; - } - rval[c -1 ] = p; - rval[c] =NULL ; - } - *count = c; -return rval; -} -#define EXABYTES (x) ((long long)(x) << 60)#define PETABYTES (x) ((long long)(x) << 50)#define TERABYTES (x) ((long long)(x) << 40)
cmd.h +2 -9
@@ -39,23 +39,16 @@ typedef struct cmdinfo {helpfunc_t help; } cmdinfo_t; -extern cmdinfo_t *cmdtab; -extern int ncmds; -typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci); -void add_command (const cmdinfo_t *ci); +void qemuio_add_command (const cmdinfo_t *ci);void add_user_command (char *optarg);void add_check_command (checkfunc_t cf); -const cmdinfo_t *find_command (const char *cmd); -void command_loop (void ); -int command_usage (const cmdinfo_t *ci); -int command (const cmdinfo_t *ci,int argc,char **argv); +int qemuio_command_usage (const cmdinfo_t *ci);/* from input.h */ -char **breakline (char *input,int *count);char *fetchline (void );void cvtstr (double value,char *str,size_t sz);
qemu-io-cmds.c +141 -51
296 lines not on this page · raw message
qemu-io.c +5 -5
@@ -122,7 +122,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) growable =1 ;break ;default : -return command_usage (&open_cmd); +return qemuio_command_usage (&open_cmd); } }
@@ -131,7 +131,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) }if (optind != argc -1 ) { -return command_usage (&open_cmd); +return qemuio_command_usage (&open_cmd); }return openfile (argv[optind], flags, growable);
@@ -262,9 +262,9 @@ int main(int argc, char **argv)bdrv_init ();/* initialize commands */ -add_command (&quit_cmd); -add_command (&open_cmd); -add_command (&close_cmd); +qemuio_add_command (&quit_cmd); +qemuio_add_command (&open_cmd); +qemuio_add_command (&close_cmd);/* open the device */ if (!readonly) {
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 11/16] qemu-io: Move functions for registering and running commands
Quoted 7 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 113 --------------------------------- cmd.h | 11 +--- qemu-io-cmds.c | 192 ++++++++++++++++++++++++++++++++++++++++++--------------- qemu-io.c | 10 +-- 4 files changed, 148 insertions(+), 178 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 11/16] qemu-io: Move functions for registering and running commands
In reply to Kevin WolfQuoted 7 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- cmd.c | 113 --------------------------------- cmd.h | 11 +--- qemu-io-cmds.c | 192 ++++++++++++++++++++++++++++++++++++++++++--------------- qemu-io.c | 10 +-- 4 files changed, 148 insertions(+), 178 deletions(-)
I haven't figured this out yet. These patches move a bunch of generic code and make it qemu-io-specific. That seems the wrong direction to go.
PATCH 12/16 qemu-io: Move command_loop() and friends
In reply to Kevin Wolf--- cmd.c | 139 -------------------------------------------------------------- cmd.h | 9 ---- qemu-io.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 148 deletions(-)
cmd.c +0 -139
@@ -31,145 +31,6 @@/* from libxcmd/command.c */ -static int ncmdline; -static char **cmdline; - - -void add_user_command (char *optarg) -{ - cmdline =g_realloc (cmdline, ++ncmdline *sizeof (char *)); - cmdline[ncmdline-1 ] = optarg; -} - -static void prep_fetchline (void *opaque) -{ -int *fetchable = opaque; - -qemu_set_fd_handler (STDIN_FILENO ,NULL ,NULL ,NULL ); - *fetchable=1 ; -} - -static char *get_prompt (void ); - -void command_loop (void ) -{ -int i, done =0 , fetchable =0 , prompted =0 ; -char *input; - -for (i =0 ; !done && i < ncmdline; i++) { - done =qemuio_command (cmdline[i]); - } -if (cmdline) { -g_free (cmdline); -return ; - } - -while (!done) { -if (!prompted) { -printf ("%s" ,get_prompt ()); -fflush (stdout); -qemu_set_fd_handler (STDIN_FILENO , prep_fetchline,NULL , &fetchable); - prompted =1 ; - } - -main_loop_wait (false); - -if (!fetchable) { -continue ; - } - - input =fetchline (); -if (input ==NULL ) { -break ; - } - done =qemuio_command (input); -free (input); - - prompted =0 ; - fetchable =0 ; - } -qemu_set_fd_handler (STDIN_FILENO ,NULL ,NULL ,NULL ); -} - -/* from libxcmd/input.c */ - -#if defined(ENABLE_READLINE ) -# include <readline/history.h> -# include <readline/readline.h> -#elif defined(ENABLE_EDITLINE ) -# include <histedit.h> -#endif - -static char * -get_prompt (void ) -{ -static char prompt[FILENAME_MAX +2 /*"> "*/ +1 /*"\0"*/ ]; - -if (!prompt[0 ]) -snprintf (prompt,sizeof (prompt),"%s> " , progname); -return prompt; -} - -#if defined(ENABLE_READLINE ) -char * -fetchline (void ) -{ -char *line; - - line =readline (get_prompt ()); -if (line && *line) -add_history (line); -return line; -} -#elif defined(ENABLE_EDITLINE ) -static char *el_get_prompt (EditLine *e) {return get_prompt (); } -char * -fetchline (void ) -{ -static EditLine *el; -static History *hist; -HistEvent hevent; -char *line; -int count; - -if (!el) { - hist =history_init (); -history (hist, &hevent,H_SETSIZE ,100 ); - el =el_init (progname, stdin, stdout, stderr); -el_source (el,NULL ); -el_set (el,EL_SIGNAL ,1 ); -el_set (el,EL_PROMPT , el_get_prompt); -el_set (el,EL_HIST , history, (const char *)hist); - } - line =strdup (el_gets (el, &count)); -if (line) { -if (count >0 ) - line[count-1 ] ='\0' ; -if (*line) -history (hist, &hevent,H_ENTER , line); - } -return line; -} -#else -# define MAXREADLINESZ 1024 -char * -fetchline (void ) -{ -char *p, *line =malloc (MAXREADLINESZ ); - -if (!line) -return NULL ; -if (!fgets (line,MAXREADLINESZ , stdin)) { -free (line); -return NULL ; - } - p = line +strlen (line); -if (p != line && p[-1 ] =='\n' ) - p[-1 ] ='\0' ; -return line; -} -#endif -#define EXABYTES (x) ((long long)(x) << 60)#define PETABYTES (x) ((long long)(x) << 50)#define TERABYTES (x) ((long long)(x) << 40)
cmd.h +0 -9
@@ -39,18 +39,11 @@ typedef struct cmdinfo {helpfunc_t help; } cmdinfo_t; -typedef int (*checkfunc_t )(BlockDriverState *bs,const cmdinfo_t *ci); -void qemuio_add_command (const cmdinfo_t *ci); -void add_user_command (char *optarg); -void add_check_command (checkfunc_t cf); -void command_loop (void );int qemuio_command_usage (const cmdinfo_t *ci);/* from input.h */ -char *fetchline (void ); -void cvtstr (double value,char *str,size_t sz);struct timeval tsub (struct timeval t1,struct timeval t2);
@@ -64,8 +57,6 @@ enum {void timestr (struct timeval *tv,char *str,size_t sz,int flags); -extern char *progname; -bool qemuio_command (const char *cmd);#endif /* __COMMAND_H__ */
qemu-io.c +139 -0
151 lines not on this page · raw message
Signature · 1 line
1.8.1.4
PATCH 13/16 qemu-io: Move remaining helpers from cmd.c
In reply to Kevin Wolf--- Makefile | 2 +- cmd.c | 139 --------------------------------------------------------- cmd.h | 14 ------ qemu-io-cmds.c | 104 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 154 deletions(-) delete mode 100644 cmd.c
Makefile +1 -1
@@ -186,7 +186,7 @@ qemu-img.o: qemu-img-cmds.h qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a -qemu-io$(EXESUF): qemu-io.o qemu-io-cmds.o cmd.o $(block-obj-y) libqemuutil.a libqemustub.a +qemu-io$(EXESUF): qemu-io.o qemu-io-cmds.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
cmd.c deleted +0 -139
139 lines not on this page · raw message
cmd.h +0 -14
@@ -43,20 +43,6 @@ void qemuio_add_command(const cmdinfo_t *ci);int qemuio_command_usage (const cmdinfo_t *ci); -/* from input.h */ -void cvtstr (double value,char *str,size_t sz); - -struct timeval tsub (struct timeval t1,struct timeval t2); -double tdiv (double value,struct timeval tv); - -enum { -DEFAULT_TIME =0x0 , -TERSE_FIXED_TIME =0x1 , -VERBOSE_FIXED_TIME =0x2 -}; - -void timestr (struct timeval *tv,char *str,size_t sz,int flags); -bool qemuio_command (const char *cmd);#endif /* __COMMAND_H__ */
qemu-io-cmds.c +104 -0
110 lines not on this page · raw message
Signature · 1 line
1.8.1.4
PATCH 14/16 qemu-io: Interface cleanup
In reply to Kevin Wolf--- cmd.h | 48 ------------------------------------------------ include/qemu-io.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ qemu-io-cmds.c | 14 +++++++------- qemu-io.c | 7 +++---- 4 files changed, 56 insertions(+), 59 deletions(-) delete mode 100644 cmd.h create mode 100644 include/qemu-io.h
cmd.h deleted +0 -48
@@ -1,48 +0,0 @@ -/* -* Copyright (c) 2000-2005 Silicon Graphics, Inc. -* All Rights Reserved. -* -* This program is free software; you can redistribute it and/or -* modify it under the terms of the GNU General Public License as -* published by the Free Software Foundation. -* -* This program is distributed in the hope that it would be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program; if not, see <http://www.gnu.org/licenses/>. -*/ -#ifndef __COMMAND_H__ -#define __COMMAND_H__ - -#include "qemu-common.h" - -#define CMD_FLAG_GLOBAL ((int)0x80000000)/* don't iterate "args" */ - -extern BlockDriverState *qemuio_bs; - -typedef int (*cfunc_t )(BlockDriverState *bs,int argc,char **argv); -typedef void (*helpfunc_t )(void ); - -typedef struct cmdinfo { -const char *name; -const char *altname; -cfunc_t cfunc; -int argmin; -int argmax; -int canpush; -int flags; -const char *args; -const char *oneline; -helpfunc_t help; -}cmdinfo_t ; - -void qemuio_add_command (const cmdinfo_t *ci); - -int qemuio_command_usage (const cmdinfo_t *ci); - -bool qemuio_command (const char *cmd); - -#endif /* __COMMAND_H__ */
include/qemu-io.h new file +46 -0
46 lines not on this page · raw message
qemu-io-cmds.c +7 -7
@@ -8,9 +8,8 @@* See the COPYING file in the top-level directory. */ -#include "qemu-common.h" +#include "qemu-io.h" #include "block/block_int.h" -#include "cmd.h" #define CMD_NOFILE_OK 0x01
@@ -50,11 +49,12 @@ static int init_check_command(BlockDriverState *bs, const cmdinfo_t *ct)return 1 ; } -static int command (const cmdinfo_t *ct,int argc,char **argv) +static int command (BlockDriverState *bs,const cmdinfo_t *ct,int argc, +char **argv) {char *cmd = argv[0 ]; -if (!init_check_command (qemuio_bs, ct)) { +if (!init_check_command (bs, ct)) {return 0 ; }
@@ -75,7 +75,7 @@ static int command(const cmdinfo_t *ct, int argc, char **argv)return 0 ; } optind =0 ; -return ct->cfunc (qemuio_bs, argc, argv); +return ct->cfunc (bs, argc, argv); }static const cmdinfo_t *find_command (const char *cmd)
@@ -2068,7 +2068,7 @@ static const cmdinfo_t help_cmd = { .oneline = "help for one or all commands", }; -bool qemuio_command (const char *cmd) +bool qemuio_command (BlockDriverState *bs,const char *cmd) {char *input;const cmdinfo_t *ct;
@@ -2081,7 +2081,7 @@ bool qemuio_command(const char *cmd)if (c) { ct =find_command (v[0 ]);if (ct) { - done =command (ct, c, v); + done =command (bs, ct, c, v); }else {fprintf (stderr,"command \"%s\" not found\n" , v[0 ]); }
qemu-io.c +3 -4
@@ -14,10 +14,9 @@#include <getopt.h> #include <libgen.h> -#include "qemu-common.h" +#include "qemu-io.h" #include "qemu/main-loop.h" #include "block/block_int.h" -#include "cmd.h" #include "trace/control.h" #define VERSION "0.0.1"
@@ -273,7 +272,7 @@ static void command_loop(void)char *input;for (i =0 ; !done && i < ncmdline; i++) { - done =qemuio_command (cmdline[i]); + done =qemuio_command (qemuio_bs, cmdline[i]); }if (cmdline) {g_free (cmdline);
@@ -298,7 +297,7 @@ static void command_loop(void)if (input ==NULL ) {break ; } - done =qemuio_command (input); + done =qemuio_command (qemuio_bs, input);g_free (input); prompted =0 ;
Signature · 1 line
1.8.1.4
PATCH 15/16 qemu-io: Use the qemu version for -V
In reply to Kevin WolfAlways printing 0.0.1 and never updating the version number wasn't very useful. qemu-io is released with qemu, so using the same version number makes most sense.
--- qemu-io.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
qemu-io.c +1 -3
@@ -19,8 +19,6 @@#include "block/block_int.h" #include "trace/control.h" -#define VERSION "0.0.1" -#define CMD_NOFILE_OK 0x01char *progname;
@@ -380,7 +378,7 @@ int main(int argc, char **argv) }break ;case 'V' : -printf ("%s version %s\n" , progname,VERSION ); +printf ("%s version %s\n" , progname,QEMU_VERSION );exit (0 );case 'h' :usage (progname);
Signature · 1 line
1.8.1.4
PATCH 16/16 Make qemu-io commands available in the monitor
In reply to Kevin WolfThe QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
The HMP version is simply called 'qemu-io' for convenience.
--- Makefile | 2 +- Makefile.objs | 1 + blockdev.c | 15 +++++++++++++++ hmp-commands.hx | 16 ++++++++++++++++ hmp.c | 10 ++++++++++ hmp.h | 1 + qapi-schema.json | 16 ++++++++++++++++ qmp-commands.hx | 28 ++++++++++++++++++++++++++++ 8 files changed, 88 insertions(+), 1 deletion(-)
Makefile +1 -1
@@ -186,7 +186,7 @@ qemu-img.o: qemu-img-cmds.h qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a -qemu-io$(EXESUF): qemu-io.o qemu-io-cmds.o $(block-obj-y) libqemuutil.a libqemustub.a +qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
Makefile.objs +1 -0
@@ -13,6 +13,7 @@ block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o block-obj-y += block/ block-obj-y += qapi-types.o qapi-visit.o +block-obj-y += qemu-io-cmds.o block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o block-obj-y += qemu-coroutine-sleep.o
blockdev.c +15 -0
@@ -43,6 +43,7 @@#include "qmp-commands.h" #include "trace.h" #include "sysemu/arch_init.h" +#include "qemu-io.h" static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);extern QemuOptsList qemu_common_drive_opts;
@@ -1604,6 +1605,20 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)return dummy.next; } +void qmp___org_qemu_debug_qemu_io_command (const char *device,const char *cmd, +Error **errp) +{ +BlockDriverState *bs; + + bs =bdrv_find (device); +if (!bs) { +error_set (errp,QERR_DEVICE_NOT_FOUND , device); +return ; + } + +qemuio_command (bs, cmd); +} +QemuOptsList qemu_common_drive_opts = { .name ="drive" , .head =QTAILQ_HEAD_INITIALIZER (qemu_common_drive_opts.head),
hmp-commands.hx +16 -0
@@ -1551,6 +1551,22 @@ Removes the chardev @var{id}. ETEXI { + .name = "qemu-io", + .args_type = "device:B,command:s", + .params = "[device] \"[command]\"", + .help = "run a qemu-io command on a block device", + .mhandler.cmd = hmp_qemu_io, + }, + +STEXI +@item chardev_remove id +@findex chardev_remove + +Removes the chardev @var{id}. + +ETEXI + + { .name = "info", .args_type = "item:s?", .params = "[subcommand]",
hmp.c +10 -0
@@ -1425,3 +1425,13 @@ void hmp_chardev_remove(Monitor *mon, const QDict *qdict)qmp_chardev_remove (qdict_get_str (qdict,"id" ), &local_err);hmp_handle_error (mon, &local_err); } + +void hmp_qemu_io (Monitor *mon,const QDict *qdict) +{ +Error *local_err =NULL ; +const char * device =qdict_get_str (qdict,"device" ); +const char * command =qdict_get_str (qdict,"command" ); + +qmp___org_qemu_debug_qemu_io_command (device, command, &local_err); +hmp_handle_error (mon, &local_err); +}
hmp.h +1 -0
@@ -85,5 +85,6 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);void hmp_nbd_server_stop (Monitor *mon,const QDict *qdict);void hmp_chardev_add (Monitor *mon,const QDict *qdict);void hmp_chardev_remove (Monitor *mon,const QDict *qdict); +void hmp_qemu_io (Monitor *mon,const QDict *qdict);#endif
qapi-schema.json +16 -0
@@ -3618,3 +3618,19 @@ '*cpuid-input-ecx': 'int', 'cpuid-register': 'X86CPURegister32', 'features': 'int' } } + +## +# @__org.qemu.debug-qemu-io-command +# +# Execute a qemu-io command +# +# @device: The block device on which the command should be executed +# +# @command: The command to execute +# +# Returns: Nothing on success +# +# Since: 1.6 (testing and debugging use only, no API stability) +## +{ 'command': '__org.qemu.debug-qemu-io-command', + 'data': {'device': 'str', 'command': 'str'} }
qmp-commands.hx +28 -0
@@ -2932,3 +2932,31 @@ Example: <- { "return": {} } EQMP + + { + .name = "__org.qemu.debug-qemu-io-command", + .args_type = "device:s,command:s", + .mhandler.cmd_new = qmp_marshal_input___org_qemu_debug_qemu_io_command, + }, + + +SQMP +__org.qemu.debug-qemu-io-command +-------------------------------- + +Execute a qemu-io command. Debugging and testing use only. This API is not +stable. + +Arguments: + +- "device": The block device on which the command should be executed + (json-string) +- "command": The command to execute (json-string) + +Example: + +-> { "execute": "__org.qemu.debug-qemu-io-command", + "arguments": { "device" : "ide0-hd0", "command" : "flush" } } +<- { "return": {} } + +EQMP
Signature · 1 line
1.8.1.4
Quoted 3 lines · Kevin Wolf
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 13 lines
The HMP version is simply called 'qemu-io' for convenience.
Signed-off-by Kevin Wolf <[email protected]>---
+++ b/hmp-commands.hx
hmp-commands.hx +2
-0@@ -1551,6 +1551,22 @@ Removes the chardev @var{id}. ETEXI { + .name = "qemu-io", + .args_type = "device:B,command:s",
hmp-commands.hx Removes the chardev @var{id}. · line 1565
Quoted 10 lines
hmp-commands.hx +10
-0+ .params = "[device] \"[command]\"", + .help = "run a qemu-io command on a block device", + .mhandler.cmd = hmp_qemu_io, + }, + +STEXI +@item chardev_remove id +@findex chardev_remove + +Removes the chardev @var{id}.
Is this the right documentation?
Quoted 11 lines
qapi-schema.json +7
-0+++ b/qapi-schema.json@@ -3618,3 +3618,19 @@ '*cpuid-input-ecx': 'int', 'cpuid-register': 'X86CPURegister32', 'features': 'int' } } + +## +# @__org.qemu.debug-qemu-io-command +# +# Execute a qemu-io command +#
qapi-schema.json line 3636
Quoted 10 lines
qapi-schema.json +10
-0+# @device: The block device on which the command should be executed +# +# @command: The command to execute +# +# Returns: Nothing on success +# +# Since: 1.6 (testing and debugging use only, no API stability) +## +{ 'command': '__org.qemu.debug-qemu-io-command', + 'data': {'device': 'str', 'command': 'str'} }
I would have asked that 'command' be an enum, except that we promise no API stability so there's no need to lock us into an enum that could be introspected :) This command looks fine as-is.
Quoted 9 lines
qmp-commands.hx +1
-0@@ -2932,3 +2932,31 @@ Example: <- { "return": {} } EQMP +
qmp-commands.hx Example: · line 2945
Quoted 10 lines
qmp-commands.hx +10
-0+ { + .name = "__org.qemu.debug-qemu-io-command", + .args_type = "device:s,command:s", + .mhandler.cmd_new = qmp_marshal_input___org_qemu_debug_qemu_io_command, + }, + + +SQMP +__org.qemu.debug-qemu-io-command +--------------------------------
Do we really need to have SQMP documentation of the command, or is listing of its handler sufficient? In other words, if this is debug-only, can we just leave it undocumented instead of listing an example only to declare the example non-portable?
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
Quoted 10 lines
Quoted 10 lines
The HMP version is simply called 'qemu-io' for convenience.
Signed-off-by Kevin Wolf <[email protected]>---
+++ b/hmp-commands.hx
(quoted diff) +0
-0@@ -1551,6 +1551,22 @@ Removes the chardev @var{id}. ETEXI
Quoted 15 lines
hmp-commands.hx Removes the chardev @var{id}. · line 1565
Quoted 13 lines
hmp-commands.hx +12
-0{ + .name = "qemu-io", + .args_type = "device:B,command:s", + .params = "[device] \"[command]\"", + .help = "run a qemu-io command on a block device", + .mhandler.cmd = hmp_qemu_io, + }, + +STEXI +@item chardev_remove id +@findex chardev_remove + +Removes the chardev @var{id}.Is this the right documentation?
Obviously not. I find these files where you declare monitor commands horribly confusing, and this is the result...
Quoted 17 lines
Quoted 17 lines
qapi-schema.json +13
-0+++ b/qapi-schema.json@@ -3618,3 +3618,19 @@ '*cpuid-input-ecx': 'int', 'cpuid-register': 'X86CPURegister32', 'features': 'int' } } + +## +# @__org.qemu.debug-qemu-io-command +# +# Execute a qemu-io command +# +# @device: The block device on which the command should be executed +# +# @command: The command to execute +# +# Returns: Nothing on success +#
Quoted 8 lines
qapi-schema.json line 3636
Quoted 4 lines
qapi-schema.json +4
-0+# Since: 1.6 (testing and debugging use only, no API stability) +## +{ 'command': '__org.qemu.debug-qemu-io-command', + 'data': {'device': 'str', 'command': 'str'} }I would have asked that 'command' be an enum, except that we promise no API stability so there's no need to lock us into an enum that could be introspected :) This command looks fine as-is.
Wouldn't even work because the command includes parameters and things. This is really what you would type on the qemu-io command line. So a typical example might look like:
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }There's nothing structured about it, but for testing and debugging this should be good enough.
Quoted 16 lines
Quoted 16 lines
qmp-commands.hx +8
-0@@ -2932,3 +2932,31 @@ Example: <- { "return": {} } EQMP + + { + .name = "__org.qemu.debug-qemu-io-command", + .args_type = "device:s,command:s", + .mhandler.cmd_new = qmp_marshal_input___org_qemu_debug_qemu_io_command, + }, + +
Quoted 8 lines
qmp-commands.hx Example: · line 2945
Quoted 3 lines
qmp-commands.hx +3
-0+SQMP +__org.qemu.debug-qemu-io-command +--------------------------------Do we really need to have SQMP documentation of the command, or is listing of its handler sufficient? In other words, if this is debug-only, can we just leave it undocumented instead of listing an example only to declare the example non-portable?
I don't mind either way. Luiz?
Kevin
Quoted 13 lines · Kevin Wolf
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
qom-get/qom-set is somewhat of an unstable API (not that the parameters of the call will change, but that the underlying qom model is not locked down, and therefore you cannot blindly rely on a qom command that worked in one version to continue working in future versions). [Hmm, do we have a good way to introspect the qom model?]
But yes, this is indeed a different class of unstable interface, and worth agreeing that we have best practice in mind before committing to it.
Quoted 11 lines
Quoted 6 lines
qapi-schema.json line 3636
Quoted 2 lines
qapi-schema.json +2
-0+{ 'command': '__org.qemu.debug-qemu-io-command', + 'data': {'device': 'str', 'command': 'str'} }I would have asked that 'command' be an enum, except that we promise no API stability so there's no need to lock us into an enum that could be introspected :) This command looks fine as-is.
Wouldn't even work because the command includes parameters and things. This is really what you would type on the qemu-io command line. So a typical example might look like:
Quoted 5 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }There's nothing structured about it, but for testing and debugging this should be good enough.
I would suggest that you consider using 'command':['str'], so that at least you have structure that allows you to pass separate arguments (and thus pass an argument with spaces in the future, should the need arise):
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0",
"command": [ "write", "-P", "0x12", "4M", "512k" ] } }Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 13 lines · On Wed, 29 May 2013 10:13:42 +0200 Kevin Wolf <[email protected]> wrote:
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
I think that should be fine. However, it's not a perfect match for QMP as you don't expect mngt to use it anytime soon and that the command's syntax is not QMP friendly:
Quoted 2 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }
What about adding a HMP-only command (the good old way) and use it through human-monitor-command?
IMO, this matches better your current use-case and the API instability of the command.
Am 29.05.2013 um 19:51 hat Luiz Capitulino geschrieben:
Quoted 26 lines
Quoted 13 lines · On Wed, 29 May 2013 10:13:42 +0200 Kevin Wolf <[email protected]> wrote:
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
I think that should be fine. However, it's not a perfect match for QMP as you don't expect mngt to use it anytime soon and that the command's syntax is not QMP friendly:
Quoted 8 lines
Quoted 2 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }What about adding a HMP-only command (the good old way) and use it through human-monitor-command?
IMO, this matches better your current use-case and the API instability of the command.
Works for me, but wasn't the plan to make HMP purely a wrapper around QMP? Then adding HMP-only commands would be counterproductive. So I assumed that QMP is a must. I didn't even know that the code still allows you to have HMP-only commands. :-)
So you prefer a respin with the QMP part dropped?
Kevin
Quoted 33 lines · On Tue, 4 Jun 2013 12:08:23 +0200 Kevin Wolf <[email protected]> wrote:
Am 29.05.2013 um 19:51 hat Luiz Capitulino geschrieben:
Quoted 32 lines
Quoted 13 lines · On Wed, 29 May 2013 10:13:42 +0200 Kevin Wolf <[email protected]> wrote:
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
I think that should be fine. However, it's not a perfect match for QMP as you don't expect mngt to use it anytime soon and that the command's syntax is not QMP friendly:
Quoted 2 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }What about adding a HMP-only command (the good old way) and use it through human-monitor-command?
Quoted 7 lines
Quoted 2 lines
IMO, this matches better your current use-case and the API instability of the command.
Works for me, but wasn't the plan to make HMP purely a wrapper around QMP? Then adding HMP-only commands would be counterproductive. So I assumed that QMP is a must. I didn't even know that the code still allows you to have HMP-only commands. :-)
Yes, the long term plan is to have all HMP commands calling QMP counterparts. But the command you're adding doesn't fit QMP's design very well.
I suggested adding it as HMP-only for now because it's the simplest thing to do for this very specific case.
If more test-only non-QMP-friendly commands appear, then we'll need to think of a more general solution.
Now, something has just occurred to me. Why isn't it a good idea having this command as a stable API? Wouldn't it be a good idea to allow out-of-tree test tools like autotest to use it?
Quoted 1 line
So you prefer a respin with the QMP part dropped?
Yes.
Am 04.06.2013 um 14:40 hat Luiz Capitulino geschrieben:
Quoted 39 lines
Quoted 36 lines · On Tue, 4 Jun 2013 12:08:23 +0200 Kevin Wolf <[email protected]> wrote:
Am 29.05.2013 um 19:51 hat Luiz Capitulino geschrieben:
Quoted 34 lines
Quoted 13 lines · On Wed, 29 May 2013 10:13:42 +0200 Kevin Wolf <[email protected]> wrote:
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
I think that should be fine. However, it's not a perfect match for QMP as you don't expect mngt to use it anytime soon and that the command's syntax is not QMP friendly:
Quoted 2 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }What about adding a HMP-only command (the good old way) and use it through human-monitor-command?
IMO, this matches better your current use-case and the API instability of the command.
Quoted 8 lines
Quoted 4 lines
Works for me, but wasn't the plan to make HMP purely a wrapper around QMP? Then adding HMP-only commands would be counterproductive. So I assumed that QMP is a must. I didn't even know that the code still allows you to have HMP-only commands. :-)
Yes, the long term plan is to have all HMP commands calling QMP counterparts. But the command you're adding doesn't fit QMP's design very well.
It fits the design about as well as human-monitor-command. Both are passthrough commands, one to HMP, the other one to qemu-io.
Quoted 5 lines
I suggested adding it as HMP-only for now because it's the simplest thing to do for this very specific case.
If more test-only non-QMP-friendly commands appear, then we'll need to think of a more general solution.
What does "for now" mean, and what will the proper long-term solution look like?
Quoted 3 lines
Now, something has just occurred to me. Why isn't it a good idea having this command as a stable API? Wouldn't it be a good idea to allow out-of-tree test tools like autotest to use it?
Because qemu-io exposes internals of the block layer that we don't want to make a stable API. Autotest can have test cases using this, but they'd have to be against a specific qemu version.
Quoted 3 lines
Quoted 1 line
So you prefer a respin with the QMP part dropped?
Yes.
Okay, I'll do that.
Kevin
Quoted 45 lines · On Tue, 4 Jun 2013 14:49:55 +0200 Kevin Wolf <[email protected]> wrote:
Am 04.06.2013 um 14:40 hat Luiz Capitulino geschrieben:
Quoted 44 lines
Quoted 40 lines · On Tue, 4 Jun 2013 12:08:23 +0200 Kevin Wolf <[email protected]> wrote:
Am 29.05.2013 um 19:51 hat Luiz Capitulino geschrieben:
Quoted 26 lines
Quoted 13 lines · On Wed, 29 May 2013 10:13:42 +0200 Kevin Wolf <[email protected]> wrote:
Am 28.05.2013 um 18:07 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 3 lines
The QMP version is flagged with a __org.qemu.debug- prefix in order to reinforce the statement that qemu-io is for testing and debugging only, with no API guarantees.
Correct use of naming conventions.
Hmm, I wonder if the recent addition of an 'abort' action to 'transaction' should be renamed __org.qemu.debug-abort, to make it obvious that it is another case of a QMP command useful mainly for testing, and not real-life use.
Quoted 5 lines
Makes sense to me.
But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability.
I think that should be fine. However, it's not a perfect match for QMP as you don't expect mngt to use it anytime soon and that the command's syntax is not QMP friendly:
Quoted 8 lines
Quoted 2 lines
{ "execute": "__org.qemu.debug-qemu-io-command", "arguments":
{ "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } }What about adding a HMP-only command (the good old way) and use it through human-monitor-command?
IMO, this matches better your current use-case and the API instability of the command.
Works for me, but wasn't the plan to make HMP purely a wrapper around QMP? Then adding HMP-only commands would be counterproductive. So I assumed that QMP is a must. I didn't even know that the code still allows you to have HMP-only commands. :-)
Quoted 6 lines
Quoted 3 lines
Yes, the long term plan is to have all HMP commands calling QMP counterparts. But the command you're adding doesn't fit QMP's design very well.
It fits the design about as well as human-monitor-command. Both are passthrough commands, one to HMP, the other one to qemu-io.
Yes, but human-monitor-command is a stop gap command meant to be an exception. It's usage should decrease over time. I'd expect the opposite for qemu-io-command.
Quoted 8 lines
Quoted 5 lines
I suggested adding it as HMP-only for now because it's the simplest thing to do for this very specific case.
If more test-only non-QMP-friendly commands appear, then we'll need to think of a more general solution.
What does "for now" mean, and what will the proper long-term solution look like?
For now means: until we know what the proper solution should be. I Honestly don't know what's the best/proper solution here, that's why I suggested having this in HMP first, so that we don't commit ourselves to anything prematurely.
Quoted 6 lines
Quoted 3 lines
Now, something has just occurred to me. Why isn't it a good idea having this command as a stable API? Wouldn't it be a good idea to allow out-of-tree test tools like autotest to use it?
Because qemu-io exposes internals of the block layer that we don't want to make a stable API.
What kind of internals? In the example you posted I see just a generic enough write command.
Quoted 11 lines
Autotest can have test cases using this, but they'd have to be against a specific qemu version.
Quoted 3 lines
Quoted 1 line
So you prefer a respin with the QMP part dropped?
Yes.
Okay, I'll do that.
Kevin
Quoted 37 lines · Kevin Wolf
This is a prerequisite for some kind of tests. It involves reorganising the qemu-io code so that the command part can be separated and doesn't pollute the global namespace any more, so we can link it with qemu.
Kevin Wolf (16):
qemu-io: Remove unusedargs_command
cutils: Support 'P' and 'E' suffixes instrtosz()
qemu-io: Makecvtnum()a wrapper aroundstrtosz_suffix()
qemu-io: Handlecvtnum()errors in 'alloc'
qemu-io: Don't use global bs in command implementations
qemu-io: Split off commands toqemu-io-cmds.c
qemu-io: Factor outqemuio_command
qemu-io: Move 'help' function
qemu-io: Move 'quit' function
qemu-io: Moveqemu_strsep()tocutils.c
qemu-io: Move functions for registering and running commands
qemu-io: Movecommand_loop()and friends
qemu-io: Move remaining helpers fromcmd.c
qemu-io: Interface cleanup
qemu-io: Use the qemu version for -V
Make qemu-io commands available in the monitorMakefile | 2 +- Makefile.objs | 1 + blockdev.c | 15 + cmd.c | 612 ------------- cmd.h | 79 -- hmp-commands.hx | 16 + hmp.c | 10 + hmp.h | 1 + include/qemu-common.h | 3 + include/qemu-io.h | 46 + monitor.c | 8 +- qapi-schema.json | 16 + qemu-img.c | 10 +- qemu-io-cmds.c | 2118 ++++++++++++++++++++++++++++++++++++++++++++ qemu-io.c | 1988 ++++-------------------------------------
Quoted 8 lines
qmp-commands.hx | 28 + tests/qemu-iotests/049.out | 8 +- util/cutils.c | 25 + 18 files changed, 2466 insertions(+), 2520 deletions(-) delete mode 100644 cmd.c delete mode 100644 cmd.h create mode 100644 include/qemu-io.h create mode 100644 qemu-io-cmds.c
Mostly good but I think cmd.c should be preserved. It's a generic command-line dispatcher and shouldn't be squashed into qemu-io-cmds.c.