From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40744) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGQoc-0006tx-QS for qemu-devel@nongnu.org; Tue, 25 Sep 2012 04:48:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGQoW-00018g-T4 for qemu-devel@nongnu.org; Tue, 25 Sep 2012 04:48:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9744) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGQoW-00018Y-Ky for qemu-devel@nongnu.org; Tue, 25 Sep 2012 04:48:20 -0400 From: Markus Armbruster References: <1348247243-12446-1-git-send-email-lcapitulino@redhat.com> <1348247243-12446-4-git-send-email-lcapitulino@redhat.com> Date: Tue, 25 Sep 2012 10:48:05 +0200 In-Reply-To: <1348247243-12446-4-git-send-email-lcapitulino@redhat.com> (Luiz Capitulino's message of "Fri, 21 Sep 2012 14:07:23 -0300") Message-ID: <87lify7aka.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: jan.kiszka@siemens.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, d.hatayama@jp.fujitsu.com Luiz Capitulino writes: > Today, it's necessary to specify the protocol you want to use > when dumping the guest memory, for example: > > (qemu) dump-guest-memory file:/tmp/guest-memory > > This has a few issues: > > 1. It's cumbersome to type > 2. We loose file path autocompletion > 3. Being able to specify fd:X in HMP makes little sense for humans > > Because of these reasons, hardcode the 'protocol' argument to > 'file:' in HMP. No objection. The QMP wart remains: protocol is a string that needs to be parsed. I feel that should be avoided in QMP. We should have expressed "either filename or file descriptor name" in the schema. Let's avoid such mistakes in the future. One remark inline. > Signed-off-by: Luiz Capitulino > --- > hmp-commands.hx | 8 +++----- > hmp.c | 11 ++++++++--- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index ed67e99..0302458 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -914,12 +914,11 @@ ETEXI > #if defined(CONFIG_HAVE_CORE_DUMP) > { > .name = "dump-guest-memory", > - .args_type = "paging:-p,protocol:s,begin:i?,length:i?", > - .params = "[-p] protocol [begin] [length]", > + .args_type = "paging:-p,filename:F,begin:i?,length:i?", > + .params = "[-p] filename [begin] [length]", > .help = "dump guest memory to file" > "\n\t\t\t begin(optional): the starting physical address" > "\n\t\t\t length(optional): the memory size, in bytes", > - .user_print = monitor_user_noop, > .mhandler.cmd = hmp_dump_guest_memory, > }, > > @@ -929,8 +928,7 @@ STEXI > @findex dump-guest-memory > Dump guest memory to @var{protocol}. The file can be processed with crash or > gdb. > - protocol: destination file(started with "file:") or destination file > - descriptor (started with "fd:") > + filename: dump file name > paging: do paging to get guest's memory mapping > begin: the starting physical address. It's optional, and should be > specified with length together. > diff --git a/hmp.c b/hmp.c > index ba6fbd3..513b40b 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) > { > Error *errp = NULL; > int paging = qdict_get_try_bool(qdict, "paging", 0); > - const char *file = qdict_get_str(qdict, "protocol"); > + const char *file = qdict_get_str(qdict, "filename"); > bool has_begin = qdict_haskey(qdict, "begin"); > bool has_length = qdict_haskey(qdict, "length"); > int64_t begin = 0; > int64_t length = 0; > + QString *prot; > > if (has_begin) { > begin = qdict_get_int(qdict, "begin"); > @@ -1055,9 +1056,13 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) > length = qdict_get_int(qdict, "length"); > } > > - qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length, > - &errp); > + prot = qstring_from_str("file:"); > + qstring_append(prot, file); What about prot = g_strconcat("file:", file, NULL) ... g_free(prot) ? > + > + qmp_dump_guest_memory(paging, qstring_get_str(prot), has_begin, begin, > + has_length, length, &errp); > hmp_handle_error(mon, &errp); > + QDECREF(prot); > } > > void hmp_netdev_add(Monitor *mon, const QDict *qdict)