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

["Daniel P. Berrange" <berrange@redhat.com>]

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.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|