qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor


From: Dr. David Alan Gilbert
Subject: Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor
Date: Tue, 3 May 2016 14:27:57 +0100
User-agent: Mutt/1.6.0 (2016-04-01)

* Eric Blake (address@hidden) wrote:
> On 05/03/2016 06:26 AM, Markus Armbruster wrote:
> 
> >>> +        visit_type_int(vmdesc, "size", &size, &error_abort);
> >>> +        visit_start_list(vmdesc, "fields", NULL, 0, &error_abort);
> >>> +        visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);
> >>
> >> Please avoid error_abort in migration code, especially on the source side.
> >> You've got an apparently happily working VM, we must never kill it 
> >> while attempting migration.
> > 
> > These functions cannot fail, and &error_abort is a concise way to
> > express that.  It's the same as
> > 
> >             visit_type_int(vmdesc, "size", &size, &err);
> >             assert(!err);
> 
> &error_abort is ONLY supposed to be used to flag programming errors (ie.
> they should never be reachable).  I'm asserting that the errors don't
> happen, and therefore this cannot make the migration fail - in other
> words, this is NOT going to kill a VM that attempts migration.

OK, but remember that I work on the basis that there are programming errors
in both the migration code and the VMState descriptions for devices.
If those break it still shouldn't kill the source.
(Note this isn't just true of migration - we need to be careful about
it in all cases where we're doing stuff to an otherwise happy VM).

> > * Conditions where the JSON output visitor itself sets an error:
> > 
> >   - None.
> 
> The JSON output visitor itself may be adding an error for an attempt to
> output Inf or NaN for a floating point number - but since vmstate
> doesn't use visit_type_number(), this is not possible.  And if we are
> really worried about it, then in my next spin of the patch I may make it
> user-configurable whether we stick to strict JSON or whether we relax
> things and output Inf/NaN anyways.

If that's the only case, and you're already saying it doesn't use it, then
I don't see there's a point in making that bit any more configurable.

Dave

> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK



reply via email to

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