[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol a
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file: |
Date: |
Tue, 25 Sep 2012 09:02:24 -0300 |
On Tue, 25 Sep 2012 10:48:05 +0200
Markus Armbruster <address@hidden> wrote:
> Luiz Capitulino <address@hidden> 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.
I think that the right solution would be to have an URI.
> One remark inline.
>
> > Signed-off-by: Luiz Capitulino <address@hidden>
> > ---
> > 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) ?
Good suggestion.
>
> > +
> > + 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)
>
Re: [Qemu-devel] [PATCH 0/3]: qmp/hmp: dump-guest-memory fixes, Markus Armbruster, 2012/09/25