[4/4] trace: enable trace events in qemu-img

Commit Message

Denis V. Lunev May 31, 2016, 10:07 a.m. UTC

The command will work this way:
    qemu-img create --trace qcow2* -f qcow2 1.img 64G

Signed-off-by: Denis V. Lunev <[e-mail]>
Suggested by: Daniel P. Berrange <[e-mail]>
CC: Eric Blake <[e-mail]>
CC: Paolo Bonzini <[e-mail]>
CC: Stefan Hajnoczi <[e-mail]>
CC: Kevin Wolf <[e-mail]>
---
 qemu-img.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

Comments

Eric Blake May 31, 2016, 12:50 p.m. UTC | #1

On 05/31/2016 04:07 AM, Denis V. Lunev wrote:
> The command will work this way:
>     qemu-img create --trace qcow2* -f qcow2 1.img 64G
> 
> Signed-off-by: Denis V. Lunev <[e-mail]>
> Suggested by: Daniel P. Berrange <[e-mail]>
> CC: Eric Blake <[e-mail]>
> CC: Paolo Bonzini <[e-mail]>
> CC: Stefan Hajnoczi <[e-mail]>
> CC: Kevin Wolf <[e-mail]>
> ---
>  qemu-img.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)

Missing man page additions.


> @@ -155,7 +157,12 @@ static void QEMU_NORETURN help(void)
>             "Parameters to compare subcommand:\n"
>             "  '-f' first image format\n"
>             "  '-F' second image format\n"
> -           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
> +           "  '-s' run in Strict mode - fail on different image size or sector allocation\n"
> +           "\n"
> +           "General purpose options:\n"
> +           "  -R, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
> +           "       specify tracing options\n"
> +           "       see qemu(1) man page for full description\n";

Why -R? Because -T is already in use by rebase and compare?  I'd almost
rather have _just_ --trace with no short option, than to pick yet
another letter, but you might want to wait for other opinions first.

I'd list general parameters before sub-command parameters.  I'd also
update the synopsis:

qemu-img [general options] command [command options]

Denis V. Lunev May 31, 2016, 12:55 p.m. UTC | #2

On 05/31/2016 03:50 PM, Eric Blake wrote:
> On 05/31/2016 04:07 AM, Denis V. Lunev wrote:
>> The command will work this way:
>>      qemu-img create --trace qcow2* -f qcow2 1.img 64G
>>
>> Signed-off-by: Denis V. Lunev <[e-mail]>
>> Suggested by: Daniel P. Berrange <[e-mail]>
>> CC: Eric Blake <[e-mail]>
>> CC: Paolo Bonzini <[e-mail]>
>> CC: Stefan Hajnoczi <[e-mail]>
>> CC: Kevin Wolf <[e-mail]>
>> ---
>>   qemu-img.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 96 insertions(+), 1 deletion(-)
> Missing man page additions.
>
>
>> @@ -155,7 +157,12 @@ static void QEMU_NORETURN help(void)
>>              "Parameters to compare subcommand:\n"
>>              "  '-f' first image format\n"
>>              "  '-F' second image format\n"
>> -           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
>> +           "  '-s' run in Strict mode - fail on different image size or sector allocation\n"
>> +           "\n"
>> +           "General purpose options:\n"
>> +           "  -R, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
>> +           "       specify tracing options\n"
>> +           "       see qemu(1) man page for full description\n";
> Why -R? Because -T is already in use by rebase and compare?  I'd almost
> rather have _just_ --trace with no short option, than to pick yet
> another letter, but you might want to wait for other opinions first.
>
> I'd list general parameters before sub-command parameters.  I'd also
> update the synopsis:
>
> qemu-img [general options] command [command options]
>
Actually me too. I have though about this before submission but stick to
current approach following Fam's locking games. This would make the code 
MUCH better.

Fam, may be this also makes sense for your games with locking...

Den

Daniel P. Berrangé May 31, 2016, 1:23 p.m. UTC | #3

On Tue, May 31, 2016 at 06:50:04AM -0600, Eric Blake wrote:
> On 05/31/2016 04:07 AM, Denis V. Lunev wrote:
> > The command will work this way:
> >     qemu-img create --trace qcow2* -f qcow2 1.img 64G
> > 
> > Signed-off-by: Denis V. Lunev <[e-mail]>
> > Suggested by: Daniel P. Berrange <[e-mail]>
> > CC: Eric Blake <[e-mail]>
> > CC: Paolo Bonzini <[e-mail]>
> > CC: Stefan Hajnoczi <[e-mail]>
> > CC: Kevin Wolf <[e-mail]>
> > ---
> >  qemu-img.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 96 insertions(+), 1 deletion(-)
> 
> Missing man page additions.
> 
> 
> > @@ -155,7 +157,12 @@ static void QEMU_NORETURN help(void)
> >             "Parameters to compare subcommand:\n"
> >             "  '-f' first image format\n"
> >             "  '-F' second image format\n"
> > -           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
> > +           "  '-s' run in Strict mode - fail on different image size or sector allocation\n"
> > +           "\n"
> > +           "General purpose options:\n"
> > +           "  -R, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
> > +           "       specify tracing options\n"
> > +           "       see qemu(1) man page for full description\n";
> 
> Why -R? Because -T is already in use by rebase and compare?  I'd almost
> rather have _just_ --trace with no short option, than to pick yet
> another letter, but you might want to wait for other opinions first.

Agreed, I think it'd be fine to use --trace and skip the short option

Regards,
Daniel

Patch

diff --git a/qemu-img.c b/qemu-img.c
index 4b56ad3..9dc4efc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -31,6 +31,7 @@ 
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
+#include "qemu/log.h"
 #include "qom/object_interfaces.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
@@ -38,6 +39,7 @@ 
 #include "block/blockjob.h"
 #include "block/qapi.h"
 #include "crypto/init.h"
+#include "trace/control.h"
 #include <getopt.h>
 
 #define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION QEMU_PKGVERSION \
@@ -155,7 +157,12 @@  static void QEMU_NORETURN help(void)
            "Parameters to compare subcommand:\n"
            "  '-f' first image format\n"
            "  '-F' second image format\n"
-           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
+           "  '-s' run in Strict mode - fail on different image size or sector allocation\n"
+           "\n"
+           "General purpose options:\n"
+           "  -R, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
+           "       specify tracing options\n"
+           "       see qemu(1) man page for full description\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
@@ -194,6 +201,15 @@  static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
 }
 
 
+static void trace_setup(const char *trace_file)
+{
+    if (!trace_init_backends()) {
+        exit(1);
+    }
+    trace_init_file(trace_file);
+    qemu_set_log(LOG_TRACE);
+}
+
 static int print_block_option_help(const char *filename, const char *fmt)
 {
     BlockDriver *drv, *proto_drv;
@@ -360,11 +376,13 @@  static int img_create(int argc, char **argv)
     char *options = NULL;
     Error *local_err = NULL;
     bool quiet = false;
+    char *trace_file = NULL;
 
     for(;;) {
         static const struct option long_options[] = {
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "F:b:f:he6o:q",
@@ -410,6 +428,9 @@  static int img_create(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -421,6 +442,8 @@  static int img_create(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     /* Get the filename */
     filename = (optind < argc) ? argv[optind] : NULL;
     if (options && has_help_option(options)) {
@@ -598,6 +621,7 @@  static int img_check(int argc, char **argv)
     ImageCheck *check;
     bool quiet = false;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     fmt = NULL;
     output = NULL;
@@ -612,6 +636,7 @@  static int img_check(int argc, char **argv)
             {"output", required_argument, 0, OPTION_OUTPUT},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "hf:r:T:q",
@@ -648,6 +673,9 @@  static int img_check(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -661,6 +689,9 @@  static int img_check(int argc, char **argv)
             break;
         }
     }
+
+    trace_setup(trace_file);
+
     if (optind != argc - 1) {
         error_exit("Expecting one image file name");
     }
@@ -801,6 +832,7 @@  static int img_commit(int argc, char **argv)
     Error *local_err = NULL;
     CommonBlockJobCBInfo cbi;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
@@ -810,6 +842,7 @@  static int img_commit(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "f:ht:b:dpq",
@@ -842,6 +875,9 @@  static int img_commit(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -856,6 +892,8 @@  static int img_commit(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     /* Progress is not shown in Quiet mode */
     if (quiet) {
         progress = false;
@@ -1131,6 +1169,7 @@  static int img_compare(int argc, char **argv)
     int c, pnum;
     uint64_t progress_base;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     cache = BDRV_DEFAULT_CACHE;
     for (;;) {
@@ -1138,6 +1177,7 @@  static int img_compare(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "hf:F:T:pqs",
@@ -1168,6 +1208,9 @@  static int img_compare(int argc, char **argv)
         case 's':
             strict = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -1183,6 +1226,8 @@  static int img_compare(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     /* Progress is not shown in Quiet mode */
     if (quiet) {
         progress = false;
@@ -1754,6 +1799,7 @@  static int img_convert(int argc, char **argv)
     QemuOpts *sn_opts = NULL;
     ImgConvertState state;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     fmt = NULL;
     out_fmt = "raw";
@@ -1767,6 +1813,7 @@  static int img_convert(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn",
@@ -1861,6 +1908,9 @@  static int img_convert(int argc, char **argv)
         case 'n':
             skip_create = 1;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT:
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
                                            optarg, true);
@@ -1874,6 +1924,8 @@  static int img_convert(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
                           NULL, NULL)) {
@@ -2311,6 +2363,7 @@  static int img_info(int argc, char **argv)
     const char *filename, *fmt, *output;
     ImageInfoList *list;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     fmt = NULL;
     output = NULL;
@@ -2323,6 +2376,7 @@  static int img_info(int argc, char **argv)
             {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "f:h",
@@ -2344,6 +2398,9 @@  static int img_info(int argc, char **argv)
         case OPTION_BACKING_CHAIN:
             chain = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2357,6 +2414,9 @@  static int img_info(int argc, char **argv)
             break;
         }
     }
+
+    trace_setup(trace_file);
+
     if (optind != argc - 1) {
         error_exit("Expecting one image file name");
     }
@@ -2523,6 +2583,7 @@  static int img_map(int argc, char **argv)
     MapEntry curr = { .length = 0 }, next;
     int ret = 0;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     fmt = NULL;
     output = NULL;
@@ -2534,6 +2595,7 @@  static int img_map(int argc, char **argv)
             {"output", required_argument, 0, OPTION_OUTPUT},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "f:h",
@@ -2552,6 +2614,9 @@  static int img_map(int argc, char **argv)
         case OPTION_OUTPUT:
             output = optarg;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2648,6 +2713,7 @@  static int img_snapshot(int argc, char **argv)
     bool quiet = false;
     Error *err = NULL;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     bdrv_oflags = BDRV_O_RDWR;
     /* Parse commandline parameters */
@@ -2656,6 +2722,7 @@  static int img_snapshot(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "la:c:d:hq",
@@ -2703,6 +2770,9 @@  static int img_snapshot(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2717,6 +2787,8 @@  static int img_snapshot(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     if (optind != argc - 1) {
         error_exit("Expecting one image file name");
     }
@@ -2797,6 +2869,7 @@  static int img_rebase(int argc, char **argv)
     bool quiet = false;
     Error *local_err = NULL;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -2809,6 +2882,7 @@  static int img_rebase(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "hf:F:b:upt:T:q",
@@ -2845,6 +2919,9 @@  static int img_rebase(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2859,6 +2936,8 @@  static int img_rebase(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     if (quiet) {
         progress = 0;
     }
@@ -3157,6 +3236,7 @@  static int img_resize(int argc, char **argv)
         },
     };
     bool image_opts = false;
+    char *trace_file = NULL;
 
     /* Remove size from argv manually so that negative numbers are not treated
      * as options by getopt. */
@@ -3174,6 +3254,7 @@  static int img_resize(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "f:hq",
@@ -3192,6 +3273,9 @@  static int img_resize(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'R':
+            trace_file = trace_opt_parse(optarg, trace_file);
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -3205,6 +3289,9 @@  static int img_resize(int argc, char **argv)
             break;
         }
     }
+
+    trace_setup(trace_file);
+
     if (optind != argc - 1) {
         error_exit("Expecting one image file name");
     }
@@ -3305,6 +3392,7 @@  static int img_amend(int argc, char **argv)
     BlockBackend *blk = NULL;
     BlockDriverState *bs = NULL;
     bool image_opts = false;
+    char *trace_file = NULL;
 
     cache = BDRV_DEFAULT_CACHE;
     for (;;) {
@@ -3312,6 +3400,7 @@  static int img_amend(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"trace", required_argument, NULL, 'R'},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "ho:f:t:pq",
@@ -3351,6 +3440,9 @@  static int img_amend(int argc, char **argv)
             case 'q':
                 quiet = true;
                 break;
+            case 'R':
+                trace_file = trace_opt_parse(optarg, trace_file);
+                break;
             case OPTION_OBJECT:
                 opts = qemu_opts_parse_noisily(&qemu_object_opts,
                                                optarg, true);
@@ -3365,6 +3457,8 @@  static int img_amend(int argc, char **argv)
         }
     }
 
+    trace_setup(trace_file);
+
     if (!options) {
         error_exit("Must specify options (-o)");
     }
@@ -3503,6 +3597,7 @@  int main(int argc, char **argv)
 
     qemu_add_opts(&qemu_object_opts);
     qemu_add_opts(&qemu_source_opts);
+    qemu_add_opts(&qemu_trace_opts);
 
     /* find the command */
     for (cmd = img_cmds; cmd->name != NULL; cmd++) {