[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command '
From: |
Wen Congyang |
Subject: |
Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory |
Date: |
Mon, 19 Mar 2012 16:31:19 +0800 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100413 Fedora/3.0.4-2.fc13 Thunderbird/3.0.4 |
At 03/19/2012 10:28 AM, Wen Congyang Wrote:
> At 03/15/2012 01:18 AM, Luiz Capitulino Wrote:
>> On Wed, 14 Mar 2012 10:11:35 +0800
>> Wen Congyang <address@hidden> wrote:
>>
>>> The command's usage:
>>> dump [-p] file
>>> file should be start with "file:"(the file's path) or "fd:"(the fd's name).
>>>
>>> Note:
>>> 1. If you want to use gdb to analyse the core, please specify -p option.
>>> 2. This command doesn't support the fd that is is associated with a pipe,
>>> socket, or FIFO(lseek will fail with such fd).
>>>
>>> Signed-off-by: Wen Congyang <address@hidden>
>
> <cut>
>
>>> +
>>> +static DumpState *dump_init(bool paging, Error **errp)
>>> +{
>>> + CPUState *env;
>>> + DumpState *s = dump_get_current();
>>> + int ret;
>>> +
>>> + if (runstate_is_running()) {
>>> + vm_stop(RUN_STATE_PAUSED);
>>> + s->resume = true;
>>
>> Hmm, you actually stop the VM. Seems obvious now, but when people talked
>> about
>> making this asynchronous I automatically assumed that what we didn't want was
>> having the global mutex held for too much time (ie. while this command was
>> running).
>
> Yes, In the earlier version, I add a vm state change handler. If the vm is
> resumed
> by the user, qemu dump will be auto cancelled.
>
>>
>> The only disadvantage of having this as a synchronous command is that libvirt
>> won't be able to cancel it and won't be able to run other commands in
>> parallel.
>> Doesn't seem that serious to me.
>>
>> Btw, RUN_STATE_PAUSED is not a good one. Doesn't matter that much, as this
>> is unlikely to be visible, but you should use RUN_STATE_SAVE_VM or
>> RUN_STATE_DEBUG.
>
> OK, I will use RUN_STATE_SAVE_VM.
>
>>
>>> + } else {
>
> <cut>
>
>>> + ret = cpu_get_dump_info(&s->dump_info);
>>> + if (ret < 0) {
>>> + error_set(errp, QERR_UNSUPPORTED);
>>
>> This will let the VM paused.
>
> Hmm, in which function the vm is paused?
Sorry for my misundestand. I forgot to resume vm before dump_init() returns.
Thanks
Wen Congyang
>
>>
>>> + return NULL;
>
> <cut>
>
>>> + ret = write(fd, buf, size);
>>> + if (ret != size) {
>>> + return -1;
>>> + }
>>
>> I think you should use send_all() instead of plain write().
>
> OK, I will use qemu_write_full() you mentioned in anohter mail.
>
>>
>>> +
>>> + return 0;
>>> +}
>
> <cut>
>
>>> +
>>> + s->f = fd_write_vmcore;
>>> + s->cleanup = fd_cleanup;
>>> + s->opaque = (void *)(intptr_t)fd;
>>
>> Do we really need all these indirections?
>
> At 02/15/2012 01:31 AM, Jan Kiszka Wrote:
>> Is writing to file descriptor generic enough? What if we want to dump
>> via QMP, letting the receiver side decide about where to write it?
>
> So I use these indirections.
>
>>
>>> +
>>> + return s;
>>> +}
>>> +
>>> +void qmp_dump(bool paging, const char *file, Error **errp)
>>> +{
>>> + const char *p;
>>> + int fd = -1;
>>> + DumpState *s;
>>> +
>>> +#if !defined(WIN32)
>>> + if (strstart(file, "fd:", &p)) {
>>> + fd = qemu_get_fd(p);
>>
>> qemu_get_fd() won't be merged, you should use monitor_get_fd(cur_mon, p);
>
> OK
>
>>
>>> + if (fd == -1) {
>>> + error_set(errp, QERR_FD_NOT_FOUND, p);
>>> + return;
>>> + }
>>> + }
>>> +#endif
>>> +
>>> + if (strstart(file, "file:", &p)) {
>>> + fd = open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
>>
>> This is minor, but I'd use qemu_open() here.
>
> OK
>
>>
>>> + if (fd < 0) {
>
> <cut>
>
>>> +
>>> + qmp_dump(!!paging, file, &errp);
>>
>> Why the double negation on 'paging'?
>
> OK, I will remove double negation.
>
>>
>>> + hmp_handle_error(mon, &errp);
>
> <cut>
>
>>> +
>>> +##
>>> +# @dump
>>
>> 'dump' is too generic, please call this dump-guest-memory-vmcore or something
>> more descriptive.
>
> Hmm, dump-guest-memory-vmcore is too long. What about dump-guest-memory or
> dump-memory?
>
>>
>>> +#
>>> +# Dump guest's memory to vmcore.
>>> +#
>>> +# @paging: if true, do paging to get guest's memory mapping
>>> +# @file: the filename or file descriptor of the vmcore.
>>
>> 'file' is not a good name because it can also dump to an fd, maybe
>> 'protocol'?
>
> OK
>
> Thanks for you reviewing
> Wen Congyang
>
>
- Re: [Qemu-devel] [RFC][PATCH 08/14 v9] target-i386: Add API to write cpu status to core file, (continued)
[Qemu-devel] [RFC][PATCH 09/14 v9] target-i386: add API to get dump info, Wen Congyang, 2012/03/13
[Qemu-devel] [RFC][PATCH 10/14 v9] make gdb_id() generally avialable, Wen Congyang, 2012/03/13
[Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Wen Congyang, 2012/03/13
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Luiz Capitulino, 2012/03/14
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Wen Congyang, 2012/03/14
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Luiz Capitulino, 2012/03/15
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Wen Congyang, 2012/03/16
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Wen Congyang, 2012/03/18
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory,
Wen Congyang <=
- Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, Luiz Capitulino, 2012/03/19
Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory, HATAYAMA Daisuke, 2012/03/15
[Qemu-devel] [RFC][PATCH 12/14 v9] support to cancel the current dumping, Wen Congyang, 2012/03/13
[Qemu-devel] [RFC][PATCH 13/14 v9] support to query dumping status, Wen Congyang, 2012/03/13
[Qemu-devel] [RFC][PATCH 14/14 v9] allow user to dump a fraction of the memory, Wen Congyang, 2012/03/13
Re: [Qemu-devel] [RFC][PATCH 00/14 v9] introducing a new, dedicated memory dump mechanism, Luiz Capitulino, 2012/03/14