Re: [Qemu-devel] [PATCH 2/2] qdev: safely fail device_add if unable to allocate device

[Eric Blake]

On 12/18/2015 09:48 AM, Daniel P. Berrange wrote:
> On Fri, Dec 18, 2015 at 04:30:47PM +0100, Igor Mammedov wrote:
>> qdev_device_add() currently uses object_new() which
>> will abort if there memory allocation for device instance
>> fails. While it's fine it startup, it is not desirable
>> diring hotplug.
>>
>> Try to allocate memory for object first and fail safely
>> if allocation fails.
>>

> This just avoids one small malloc failure.
> 
>> +    object_initialize(dev, obj_size, driver);
> 
> This is going to call g_new many more times, so you'll
> still hit OOM almost immediately. eg the call to
> g_hash_table_new_full() in object_initialize_with_type
> will abort on OOM, not to mention anything run in a
> instance constructor function registered against the
> class. There's no way to avoid this given that we have
> chosen to use GLib in QEMU, so I don't really see any
> point in replacing the 'object_new' call with g_try_malloc

We just had a recent thread on it, and I tend to agree that this series
isn't helpful.

https://lists.gnu.org/archive/html/qemu-devel/2015-12/threads.html#01238

If the allocation is BIG (as in one megabyte or more), then doing
tentative allocation and reporting failure is useful (we still have
enough small memory left to meaningfully report the error).  But here,
objects are unlikely to require a megabyte each (and if you really do
have an obj_size that large, given that it is a sizeof(type), we may
have other problems on our hands - what does the .h look like that
defines a type that big, and how slow does it compile?).  We can assume
that small allocations will always succeed, because if they fail, even
the mere act of trying to gracefully recover and report that failure is
likely to malloc more memory, which will recursively fail and hose us
worse than exiting right away.

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

signature.asc
Description: OpenPGP digital signature