| Message ID | 20191017130204.16131-2-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add qemu-storage-daemon | expand |
On 10/17/19 8:01 AM, Kevin Wolf wrote: > This adds a new binary qemu-storage-daemon that doesn't yet do more than > some typical initialisation for tools and parsing the basic command > options --version, --help and --trace. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > configure | 2 +- > qemu-storage-daemon.c | 141 ++++++++++++++++++++++++++++++++++++++++++ > Makefile | 1 + > 3 files changed, 143 insertions(+), 1 deletion(-) > create mode 100644 qemu-storage-daemon.c > > diff --git a/configure b/configure > +++ b/qemu-storage-daemon.c > @@ -0,0 +1,141 @@ > +/* > + * QEMU storage daemon > + * > + * Copyright (c) 2019 Kevin Wolf <kwolf@redhat.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. Is there an intent to license this binary as BSD (by restricting sources that can be linked in), or is it going to end up as GPLv2+ for other reasons? If the latter, would it be better to license this file GPLv2+? > + */ > + > +#include "qemu/osdep.h" > + > +#include "block/block.h" > +#include "crypto/init.h" > + > +#include "qapi/error.h" > +#include "qemu-common.h" > +#include "qemu-version.h" > +#include "qemu/config-file.h" > +#include "qemu/error-report.h" > +#include "qemu/log.h" > +#include "qemu/main-loop.h" > +#include "qemu/module.h" > + > +#include "trace/control.h" > + > +#include <getopt.h> Shouldn't system headers appear right after qemu/osdep.h? > + > +static void help(void) > +{ > + printf( > +"Usage: %s [options]\n" > +"QEMU storage daemon\n" > +"\n" > +" -h, --help display this help and exit\n" > +" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" > +" specify tracing options\n" > +" -V, --version output version information and exit\n" > +"\n" > +QEMU_HELP_BOTTOM "\n", > + error_get_progname()); > +} > + > +static int process_options(int argc, char *argv[], Error **errp) > +{ > + int c; > + char *trace_file = NULL; > + int ret = -EINVAL; > + > + static const struct option long_options[] = { > + {"help", no_argument, 0, 'h'}, > + {"version", no_argument, 0, 'V'}, > + {"trace", required_argument, NULL, 'T'}, I find it harder to maintain lists of options (which will get longer over time) when the order of the struct... > + {0, 0, 0, 0} > + }; > + > + while ((c = getopt_long(argc, argv, ":hT:V", long_options, NULL)) != -1) { ...the order of the short options... > + switch (c) { > + case '?': > + error_setg(errp, "Unknown option '%s'", argv[optind - 1]); > + goto out; > + case ':': > + error_setg(errp, "Missing option argument for '%s'", > + argv[optind - 1]); > + goto out; > + case 'h': > + help(); > + exit(EXIT_SUCCESS); > + case 'V': > + printf("qemu-storage-daemon version " > + QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n"); > + exit(EXIT_SUCCESS); > + case 'T': > + g_free(trace_file); > + trace_file = trace_opt_parse(optarg); > + break; ...and the order of the case statements are not identical. Case-insensitive alphabetical may be easiest (matching your shortopt ordering of ":hT:V"). > + } > + } > + if (optind != argc) { > + error_setg(errp, "Unexpected argument: %s", argv[optind]); > + goto out; > + } > + > + if (!trace_init_backends()) { > + error_setg(errp, "Could not initialize trace backends"); > + goto out; > + } > + trace_init_file(trace_file); > + qemu_set_log(LOG_TRACE); > + > + ret = 0; > +out: > + g_free(trace_file); Mark trace_file as g_auto, and you can avoid the out: label altogether. > + return ret; > +} > + > +int main(int argc, char *argv[]) > +{ > + Error *local_err = NULL; > + int ret; > + > +#ifdef CONFIG_POSIX > + signal(SIGPIPE, SIG_IGN); > +#endif > + > + error_init(argv[0]); > + qemu_init_exec_dir(argv[0]); > + > + module_call_init(MODULE_INIT_QOM); > + module_call_init(MODULE_INIT_TRACE); > + qemu_add_opts(&qemu_trace_opts); > + qcrypto_init(&error_fatal); > + bdrv_init(); > + > + if (qemu_init_main_loop(&local_err)) { > + error_report_err(local_err); > + return EXIT_FAILURE; > + } > + > + ret = process_options(argc, argv, &local_err); > + if (ret < 0) { > + error_report_err(local_err); > + return EXIT_FAILURE; > + } > + > + return EXIT_SUCCESS; > +} Quite a trivial shell for now, but looks interesting. Sadly, I don't have much time to review the rest of the series until after KVM Forum, which means getting this in (even as experimental) for 4.2 is at risk.
On 17.10.19 15:01, Kevin Wolf wrote: > This adds a new binary qemu-storage-daemon that doesn't yet do more than > some typical initialisation for tools and parsing the basic command > options --version, --help and --trace. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > configure | 2 +- > qemu-storage-daemon.c | 141 ++++++++++++++++++++++++++++++++++++++++++ > Makefile | 1 + > 3 files changed, 143 insertions(+), 1 deletion(-) > create mode 100644 qemu-storage-daemon.c > > diff --git a/configure b/configure > index 08ca4bcb46..bb3d55fb25 100755 > --- a/configure > +++ b/configure > @@ -6034,7 +6034,7 @@ tools="" > if test "$want_tools" = "yes" ; then > tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-edid\$(EXESUF) $tools" > if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then > - tools="qemu-nbd\$(EXESUF) $tools" > + tools="qemu-nbd\$(EXESUF) qemu-storage-daemon\$(EXESUF) $tools" > fi > if [ "$ivshmem" = "yes" ]; then > tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools" > diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c > new file mode 100644 > index 0000000000..a251dc255c > --- /dev/null > +++ b/qemu-storage-daemon.c > @@ -0,0 +1,141 @@ > +/* > + * QEMU storage daemon > + * > + * Copyright (c) 2019 Kevin Wolf <kwolf@redhat.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#include "qemu/osdep.h" > + > +#include "block/block.h" > +#include "crypto/init.h" > + > +#include "qapi/error.h" > +#include "qemu-common.h" > +#include "qemu-version.h" > +#include "qemu/config-file.h" > +#include "qemu/error-report.h" > +#include "qemu/log.h" > +#include "qemu/main-loop.h" > +#include "qemu/module.h" > + > +#include "trace/control.h" > + > +#include <getopt.h> > + > +static void help(void) > +{ > + printf( > +"Usage: %s [options]\n" > +"QEMU storage daemon\n" > +"\n" > +" -h, --help display this help and exit\n" > +" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" > +" specify tracing options\n" > +" -V, --version output version information and exit\n" > +"\n" > +QEMU_HELP_BOTTOM "\n", > + error_get_progname()); > +} > + > +static int process_options(int argc, char *argv[], Error **errp) > +{ > + int c; > + char *trace_file = NULL; > + int ret = -EINVAL; > + > + static const struct option long_options[] = { > + {"help", no_argument, 0, 'h'}, > + {"version", no_argument, 0, 'V'}, > + {"trace", required_argument, NULL, 'T'}, > + {0, 0, 0, 0} > + }; > + > + while ((c = getopt_long(argc, argv, ":hT:V", long_options, NULL)) != -1) { > + switch (c) { > + case '?': > + error_setg(errp, "Unknown option '%s'", argv[optind - 1]); > + goto out; Am I doing something wrong or is optind really not updated when '?' is returned? Because I’m getting this: $ ./qemu-storage-daemon -42 qemu-storage-daemon: Unknown option './qemu-storage-daemon' But OTOH I also get: $ ./qemu-img create -42 qemu-img: unrecognized option 'create' Try 'qemu-img --help' for more information So, uh, well. Max
In addition to Eric's review: Kevin Wolf <kwolf@redhat.com> writes: > This adds a new binary qemu-storage-daemon that doesn't yet do more than > some typical initialisation for tools and parsing the basic command > options --version, --help and --trace. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > configure | 2 +- > qemu-storage-daemon.c | 141 ++++++++++++++++++++++++++++++++++++++++++ > Makefile | 1 + > 3 files changed, 143 insertions(+), 1 deletion(-) > create mode 100644 qemu-storage-daemon.c > > diff --git a/configure b/configure > index 08ca4bcb46..bb3d55fb25 100755 > --- a/configure > +++ b/configure > @@ -6034,7 +6034,7 @@ tools="" > if test "$want_tools" = "yes" ; then > tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-edid\$(EXESUF) $tools" > if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then > - tools="qemu-nbd\$(EXESUF) $tools" > + tools="qemu-nbd\$(EXESUF) qemu-storage-daemon\$(EXESUF) $tools" > fi > if [ "$ivshmem" = "yes" ]; then > tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools" > diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c > new file mode 100644 > index 0000000000..a251dc255c > --- /dev/null > +++ b/qemu-storage-daemon.c > @@ -0,0 +1,141 @@ > +/* > + * QEMU storage daemon > + * > + * Copyright (c) 2019 Kevin Wolf <kwolf@redhat.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ Standard request for new code: please make this GPLv2+, or tell us why it has to be something else. > + > +#include "qemu/osdep.h" > + > +#include "block/block.h" > +#include "crypto/init.h" > + > +#include "qapi/error.h" > +#include "qemu-common.h" > +#include "qemu-version.h" > +#include "qemu/config-file.h" > +#include "qemu/error-report.h" > +#include "qemu/log.h" > +#include "qemu/main-loop.h" > +#include "qemu/module.h" > + > +#include "trace/control.h" > + > +#include <getopt.h> > + > +static void help(void) > +{ > + printf( > +"Usage: %s [options]\n" > +"QEMU storage daemon\n" > +"\n" > +" -h, --help display this help and exit\n" > +" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" > +" specify tracing options\n" > +" -V, --version output version information and exit\n" > +"\n" > +QEMU_HELP_BOTTOM "\n", > + error_get_progname()); > +} > + > +static int process_options(int argc, char *argv[], Error **errp) > +{ > + int c; > + char *trace_file = NULL; > + int ret = -EINVAL; > + > + static const struct option long_options[] = { > + {"help", no_argument, 0, 'h'}, You initialize member int *flag with 0 here, .... > + {"version", no_argument, 0, 'V'}, > + {"trace", required_argument, NULL, 'T'}, ... and with NULL here. Recommend to pick one and stick to it. > + {0, 0, 0, 0} {0} or {} would do. > + }; > + > + while ((c = getopt_long(argc, argv, ":hT:V", long_options, NULL)) != -1) { > + switch (c) { > + case '?': > + error_setg(errp, "Unknown option '%s'", argv[optind - 1]); > + goto out; > + case ':': > + error_setg(errp, "Missing option argument for '%s'", > + argv[optind - 1]); > + goto out; > + case 'h': > + help(); > + exit(EXIT_SUCCESS); > + case 'V': > + printf("qemu-storage-daemon version " > + QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n"); > + exit(EXIT_SUCCESS); > + case 'T': > + g_free(trace_file); > + trace_file = trace_opt_parse(optarg); This is QemuOpts below the hood. Fact, not criticism :) > + break; > + } Suggest (your preferred variation of) default: assert(0) to catch omissions. > + } > + if (optind != argc) { > + error_setg(errp, "Unexpected argument: %s", argv[optind]); > + goto out; > + } > + > + if (!trace_init_backends()) { > + error_setg(errp, "Could not initialize trace backends"); > + goto out; > + } > + trace_init_file(trace_file); > + qemu_set_log(LOG_TRACE); I suspect the only reason for hiding trace initialization within process_options() is avoiding a global variable @trace_file. I'd prefer the global variable over the hiding. > + > + ret = 0; > +out: > + g_free(trace_file); > + return ret; > +} Since the function exit(0)s on -h and -V anyway, let's exit(1) on error instead of mucking around with error_setg(). You can then leave reporting unknown options and missing option arguments to getopt_long(). Saves you the trouble of fixing the bug Max pointed out. > + > +int main(int argc, char *argv[]) > +{ > + Error *local_err = NULL; > + int ret; > + > +#ifdef CONFIG_POSIX > + signal(SIGPIPE, SIG_IGN); > +#endif > + > + error_init(argv[0]); > + qemu_init_exec_dir(argv[0]); > + > + module_call_init(MODULE_INIT_QOM); > + module_call_init(MODULE_INIT_TRACE); > + qemu_add_opts(&qemu_trace_opts); > + qcrypto_init(&error_fatal); > + bdrv_init(); Out of curiosity: how did you come up with this set of initializations? > + > + if (qemu_init_main_loop(&local_err)) { > + error_report_err(local_err); > + return EXIT_FAILURE; What's wrong with qemu_init_main_loop(&error_fatal) ? > + } > + > + ret = process_options(argc, argv, &local_err); > + if (ret < 0) { > + error_report_err(local_err); > + return EXIT_FAILURE; > + } > + > + return EXIT_SUCCESS; > +} > diff --git a/Makefile b/Makefile > index 30f0abfb42..76338d0ab4 100644 > --- a/Makefile > +++ b/Makefile > @@ -558,6 +558,7 @@ qemu-img.o: qemu-img-cmds.h > qemu-img$(EXESUF): qemu-img.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) > qemu-nbd$(EXESUF): qemu-nbd.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) > qemu-io$(EXESUF): qemu-io.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) > +qemu-storage-daemon$(EXESUF): qemu-storage-daemon.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) > > qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
Am 24.10.2019 um 15:50 hat Eric Blake geschrieben: > On 10/17/19 8:01 AM, Kevin Wolf wrote: > > This adds a new binary qemu-storage-daemon that doesn't yet do more than > > some typical initialisation for tools and parsing the basic command > > options --version, --help and --trace. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > configure | 2 +- > > qemu-storage-daemon.c | 141 ++++++++++++++++++++++++++++++++++++++++++ > > Makefile | 1 + > > 3 files changed, 143 insertions(+), 1 deletion(-) > > create mode 100644 qemu-storage-daemon.c > > > > diff --git a/configure b/configure > > > +++ b/qemu-storage-daemon.c > > @@ -0,0 +1,141 @@ > > +/* > > + * QEMU storage daemon > > + * > > + * Copyright (c) 2019 Kevin Wolf <kwolf@redhat.com> > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a copy > > + * of this software and associated documentation files (the "Software"), to deal > > + * in the Software without restriction, including without limitation the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > > + * THE SOFTWARE. > > Is there an intent to license this binary as BSD (by restricting sources > that can be linked in), or is it going to end up as GPLv2+ for other > reasons? If the latter, would it be better to license this file GPLv2+? The binary will be GPLv2 either way. Maybe even GPLv2+, not sure about that. This file copies quite a bit from qemu-img.c and vl.c, so I'm using the same license as there. Of course, the "bug" in my patch is that I also need to keep not only the license text, but also the actual copyright line from there. Will fix. > > > + */ > > + > > +#include "qemu/osdep.h" > > + > > +#include "block/block.h" > > +#include "crypto/init.h" > > + > > +#include "qapi/error.h" > > +#include "qemu-common.h" > > +#include "qemu-version.h" > > +#include "qemu/config-file.h" > > +#include "qemu/error-report.h" > > +#include "qemu/log.h" > > +#include "qemu/main-loop.h" > > +#include "qemu/module.h" > > + > > +#include "trace/control.h" > > + > > +#include <getopt.h> > > Shouldn't system headers appear right after qemu/osdep.h? I wasn't aware of this, but CODING_STYLE.rst agrees with you. > > + > > +static void help(void) > > +{ > > + printf( > > +"Usage: %s [options]\n" > > +"QEMU storage daemon\n" > > +"\n" > > +" -h, --help display this help and exit\n" > > +" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" > > +" specify tracing options\n" > > +" -V, --version output version information and exit\n" > > +"\n" > > +QEMU_HELP_BOTTOM "\n", > > + error_get_progname()); > > +} > > + > > +static int process_options(int argc, char *argv[], Error **errp) > > +{ > > + int c; > > + char *trace_file = NULL; > > + int ret = -EINVAL; > > + > > + static const struct option long_options[] = { > > + {"help", no_argument, 0, 'h'}, > > + {"version", no_argument, 0, 'V'}, > > + {"trace", required_argument, NULL, 'T'}, > > I find it harder to maintain lists of options (which will get longer over > time) when the order of the struct... > > > + {0, 0, 0, 0} > > + }; > > + > > + while ((c = getopt_long(argc, argv, ":hT:V", long_options, NULL)) != -1) { > > ...the order of the short options... > > > + switch (c) { > > + case '?': > > + error_setg(errp, "Unknown option '%s'", argv[optind - 1]); > > + goto out; > > + case ':': > > + error_setg(errp, "Missing option argument for '%s'", > > + argv[optind - 1]); > > + goto out; > > + case 'h': > > + help(); > > + exit(EXIT_SUCCESS); > > + case 'V': > > + printf("qemu-storage-daemon version " > > + QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n"); > > + exit(EXIT_SUCCESS); > > + case 'T': > > + g_free(trace_file); > > + trace_file = trace_opt_parse(optarg); > > + break; > > ...and the order of the case statements are not identical. Case-insensitive > alphabetical may be easiest (matching your shortopt ordering of ":hT:V"). Makes sense. (I think I tried to remember to keep things in alphabetical order at least sometimes, but apparently I didn't try hard enough.) > > + } > > + } > > + if (optind != argc) { > > + error_setg(errp, "Unexpected argument: %s", argv[optind]); > > + goto out; > > + } > > + > > + if (!trace_init_backends()) { > > + error_setg(errp, "Could not initialize trace backends"); > > + goto out; > > + } > > + trace_init_file(trace_file); > > + qemu_set_log(LOG_TRACE); > > + > > + ret = 0; > > +out: > > + g_free(trace_file); > > Mark trace_file as g_auto, and you can avoid the out: label altogether. Oooh, magic! :-) Kevin
diff --git a/configure b/configure index 08ca4bcb46..bb3d55fb25 100755 --- a/configure +++ b/configure @@ -6034,7 +6034,7 @@ tools="" if test "$want_tools" = "yes" ; then tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-edid\$(EXESUF) $tools" if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then - tools="qemu-nbd\$(EXESUF) $tools" + tools="qemu-nbd\$(EXESUF) qemu-storage-daemon\$(EXESUF) $tools" fi if [ "$ivshmem" = "yes" ]; then tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools" diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c new file mode 100644 index 0000000000..a251dc255c --- /dev/null +++ b/qemu-storage-daemon.c @@ -0,0 +1,141 @@ +/* + * QEMU storage daemon + * + * Copyright (c) 2019 Kevin Wolf <kwolf@redhat.com> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" + +#include "block/block.h" +#include "crypto/init.h" + +#include "qapi/error.h" +#include "qemu-common.h" +#include "qemu-version.h" +#include "qemu/config-file.h" +#include "qemu/error-report.h" +#include "qemu/log.h" +#include "qemu/main-loop.h" +#include "qemu/module.h" + +#include "trace/control.h" + +#include <getopt.h> + +static void help(void) +{ + printf( +"Usage: %s [options]\n" +"QEMU storage daemon\n" +"\n" +" -h, --help display this help and exit\n" +" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" +" specify tracing options\n" +" -V, --version output version information and exit\n" +"\n" +QEMU_HELP_BOTTOM "\n", + error_get_progname()); +} + +static int process_options(int argc, char *argv[], Error **errp) +{ + int c; + char *trace_file = NULL; + int ret = -EINVAL; + + static const struct option long_options[] = { + {"help", no_argument, 0, 'h'}, + {"version", no_argument, 0, 'V'}, + {"trace", required_argument, NULL, 'T'}, + {0, 0, 0, 0} + }; + + while ((c = getopt_long(argc, argv, ":hT:V", long_options, NULL)) != -1) { + switch (c) { + case '?': + error_setg(errp, "Unknown option '%s'", argv[optind - 1]); + goto out; + case ':': + error_setg(errp, "Missing option argument for '%s'", + argv[optind - 1]); + goto out; + case 'h': + help(); + exit(EXIT_SUCCESS); + case 'V': + printf("qemu-storage-daemon version " + QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n"); + exit(EXIT_SUCCESS); + case 'T': + g_free(trace_file); + trace_file = trace_opt_parse(optarg); + break; + } + } + if (optind != argc) { + error_setg(errp, "Unexpected argument: %s", argv[optind]); + goto out; + } + + if (!trace_init_backends()) { + error_setg(errp, "Could not initialize trace backends"); + goto out; + } + trace_init_file(trace_file); + qemu_set_log(LOG_TRACE); + + ret = 0; +out: + g_free(trace_file); + return ret; +} + +int main(int argc, char *argv[]) +{ + Error *local_err = NULL; + int ret; + +#ifdef CONFIG_POSIX + signal(SIGPIPE, SIG_IGN); +#endif + + error_init(argv[0]); + qemu_init_exec_dir(argv[0]); + + module_call_init(MODULE_INIT_QOM); + module_call_init(MODULE_INIT_TRACE); + qemu_add_opts(&qemu_trace_opts); + qcrypto_init(&error_fatal); + bdrv_init(); + + if (qemu_init_main_loop(&local_err)) { + error_report_err(local_err); + return EXIT_FAILURE; + } + + ret = process_options(argc, argv, &local_err); + if (ret < 0) { + error_report_err(local_err); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +} diff --git a/Makefile b/Makefile index 30f0abfb42..76338d0ab4 100644 --- a/Makefile +++ b/Makefile @@ -558,6 +558,7 @@ qemu-img.o: qemu-img-cmds.h qemu-img$(EXESUF): qemu-img.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) qemu-nbd$(EXESUF): qemu-nbd.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) qemu-io$(EXESUF): qemu-io.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) +qemu-storage-daemon$(EXESUF): qemu-storage-daemon.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
This adds a new binary qemu-storage-daemon that doesn't yet do more than some typical initialisation for tools and parsing the basic command options --version, --help and --trace. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- configure | 2 +- qemu-storage-daemon.c | 141 ++++++++++++++++++++++++++++++++++++++++++ Makefile | 1 + 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 qemu-storage-daemon.c