Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values

[Peter Maydell <peter.maydell@linaro.org>]

On 10 January 2017 at 10:34, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> However, it's a bit optimistic of the memory region to claim the name
> is just for debug; Migration/ram.c transmits the RAMBlock's idstr on
> the wire (as does postcopy) - so I think the memory.h comment
> is wrong.

We'd better fix that, then, or we'll find ourselves breaking
migration compat by accident...

> I don't think it's a big problem since you're unlikely to hit these
> big names in practice; but it would be better to return an error
> rather than assert/abort since then you wouldn't abort as part
> of a hot-add.

Almost all of the calls aren't going to be hot-add, though.

> So it's worth taking the common cases as this patch does; I don't
> think it's worth the hastle of changing 100+ calls though.

You also have the code paths via memory_region_allocate_system_memory
which end up calling vmstate_register_ram_global which then calls
qemu_ram_set_idstr -- none of that has support for returning an
error.

(Aside: at some point I want to introduce a
memory_region_allocate_aux_memory() which wraps the common
pattern
    memory_region_init_ram(mr, NULL, name, size, &error_fatal);
    vmstate_register_ram_global(mr);
so we have a simple way to create RAM blocks which aren't
the main system ram, by analogy with memory_region_allocate_system_memory().)

thanks
-- PMM