qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Question] dump memory when host pci device is used by


From: Jan Kiszka
Subject: Re: [Qemu-devel] [Question] dump memory when host pci device is used by guest
Date: Tue, 18 Oct 2011 10:30:34 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

On 2011-10-18 10:25, Daniel P. Berrange wrote:
> On Tue, Oct 18, 2011 at 10:17:27AM +0200, Jan Kiszka wrote:
>> On 2011-10-18 09:58, Daniel P. Berrange wrote:
>>> On Tue, Oct 18, 2011 at 03:15:29PM +0800, Wen Congyang wrote:
>>>> Hi, Jan Kiszka
>>>>
>>>> At 10/10/2011 05:34 PM, Jan Kiszka Write:
>>>>> On 2011-10-10 11:02, Daniel P. Berrange wrote:
>>>>>> On Mon, Oct 10, 2011 at 08:52:08AM +0200, Jan Kiszka wrote:
>>>>
>>>>>
>>>>> Run gdb with "set debug remote 1" and watch the communication, it is not
>>>>> that complex. But a dump command is probably simpler for those
>>>>> scenarios, I agree.
>>>>
>>>> I have implemented the command dump and reuse migration's code. But I meet 
>>>> a problem
>>>> when I test it.
>>>>
>>>> My qemu-kvm's tree is not updated about 2 months ago, because kernel.org 
>>>> is down, and
>>>> I forgot to pull from github.
>>>>
>>>> After I pull it from github, I find the following changes:
>>>> @@ -1523,9 +1523,7 @@ static void 
>>>> assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
>>>>  
>>>>  static const VMStateDescription vmstate_assigned_device = {
>>>>      .name = "pci-assign",
>>>> -    .fields = (VMStateField []) {
>>>> -        VMSTATE_END_OF_LIST()
>>>> -    }
>>>> +    .unmigratable = 1,
>>>>  };
>>>>  
>>>>  static void reset_assigned_device(DeviceState *dev)
>>>>
>>>> Why do you remove fields from vmstate_assigned_device?
>>>> It is useful for dump because it does not check unmigratable. If 
>>>> vmstate_assigned_device
>>>> does not contain .fields, qemu will crash in vmstate_save_state().
>>>
>>> Given that '.fields' is allowed to be NULL for some devices, I'd say
>>> even for normal migration, QEMU should be checking for NULL in the
>>> vmstate_save_state() code. This would prevent QEMU crashes in the case
>>> where someone removed the .unmigratable member, but forgot to add back
>>> a .fields member.
>>
>> That crash wouldn't be bad because removinb unmigratable without adding
>> proper fields is almost always a bug.
> 
> This is the kind of example of why QEMU is too crashy in general. We
> should be defensive against such bugs, by checking that preconditions
> are valid in the code, even if we don't expect them to happen often.

A serious bug in a device model is tricky to handle gracefully. Given
that this case is fairly clear (specifically for the code reviewer), I'm
not sure what defensive measures you want to apply and what benefit they
would provide.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



reply via email to

[Prev in Thread] Current Thread [Next in Thread]