[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values |
Date: |
Tue, 10 Jan 2017 10:44:37 +0000 |
On 10 January 2017 at 10:34, Dr. David Alan Gilbert <address@hidden> 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