qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de,
	armbru@redhat.com
Subject: Re: [Qemu-devel] [PATCH 2/2] qdev: safely fail device_add if unable to allocate device
Date: Fri, 18 Dec 2015 10:26:02 -0700	[thread overview]
Message-ID: <567441AA.1010309@redhat.com> (raw)
In-Reply-To: <20151218164823.GH7228@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-12-18 17:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-18 15:30 [Qemu-devel] [PATCH 0/2] qdev: fail safely if can't allocate device in device_add() Igor Mammedov
2015-12-18 15:30 ` [Qemu-devel] [PATCH 1/2] qom: add object_class_get_instance_size() Igor Mammedov
2016-01-11 15:37   ` Andreas Färber
2015-12-18 15:30 ` [Qemu-devel] [PATCH 2/2] qdev: safely fail device_add if unable to allocate device Igor Mammedov
2015-12-18 16:48   ` Daniel P. Berrange
2015-12-18 17:26     ` Eric Blake [this message]
2015-12-18 21:15       ` Markus Armbruster
2016-01-11 16:04         ` Andreas Färber
2016-01-12 15:43           ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=567441AA.1010309@redhat.com \
    --to=eblake@redhat.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).