From: Thomas Huth <thuth@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
Eduardo Habkost <ehabkost@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] hw/core/qdev: Do not allow hot-plugging without hotplug controller
Date: Mon, 11 Sep 2017 16:31:39 +0200 [thread overview]
Message-ID: <f62b660d-596a-f5ed-b4c7-d5adf58d2913@redhat.com> (raw)
In-Reply-To: <20170911145344.4f2e192c@nial.brq.redhat.com>
On 11.09.2017 14:53, Igor Mammedov wrote:
> On Thu, 7 Sep 2017 11:22:42 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> qdev_unplug() bails out with an assertion if the user tries to device_del
>> a hot-plugged device that does not have a hotplug controller. Unfortunately,
>> our devices are all marked with hotpluggable = true by default (see the
>> device_class_init() function in qdev.c), so it currently can happen that
>> the user runs into this situation and QEMU gets terminated unexpectedly:
>>
>> $ qemu-system-aarch64 -M virt -nographic -nodefaults -monitor stdio -S
>> QEMU 2.10.50 monitor - type 'help' for more information
>> (qemu) device_add aux-to-i2c-bridge,id=x
>> (qemu) device_del x
>> **
>> ERROR:qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
>> Aborted (core dumped)
>> Hotplugging devices without a hotplug controller does not make much sense,
>> so we should disallow this during the device_add process already!
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/core/qdev.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 606ab53..d9ccce6 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -908,6 +908,11 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
>> if (local_err != NULL) {
>> goto fail;
>> }
>> + } else if (dev->hotplugged) {
>> + /* Hot-plugged device without hotplug controller? No way! */
>> + error_setg(&local_err, QERR_DEVICE_NO_HOTPLUG,
>> + object_get_typename(obj));
>> + goto fail;
>> }
>>
>> if (dc->realize) {
>
> maybe it should be other way around, i.e, fix device so that following would work
>
> device_set_realized()
> if (dev->hotplugged && !dc->hotpluggable) {
> error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
> return;
> }
>
> instead of leaving device broken, like in yours
> 84ebd3e watchdog/wdt_diag288: Mark diag288 watchdog as non-hotpluggable
No, that apparently does not work right for new devices since people
keep forgetting to set hotpluggable = false there. Both, Paolo and Peter
suggested that we should not allow hot-plugging if there's no hot plug
controller - it indeed does not make sense, so we should not allow it.
Thomas
next prev parent reply other threads:[~2017-09-11 14:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-07 9:22 [Qemu-devel] [PATCH] hw/core/qdev: Do not allow hot-plugging without hotplug controller Thomas Huth
2017-09-11 12:53 ` Igor Mammedov
2017-09-11 14:31 ` Thomas Huth [this message]
2017-09-11 15:06 ` Igor Mammedov
2017-09-12 18:05 ` Eduardo Habkost
2017-09-13 7:12 ` Igor Mammedov
2017-09-13 10:58 ` Peter Maydell
2017-09-21 17:56 ` Eduardo Habkost
2017-09-21 18:37 ` Thomas Huth
2017-09-26 10:05 ` Thomas Huth
2017-09-26 13:41 ` Eduardo Habkost
2017-09-26 14:33 ` Thomas Huth
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=f62b660d-596a-f5ed-b4c7-d5adf58d2913@redhat.com \
--to=thuth@redhat.com \
--cc=armbru@redhat.com \
--cc=dgilbert@redhat.com \
--cc=ehabkost@redhat.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.