All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, "Don Slutz" <dslutz@verizon.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH] qdev: Skip non-existing properties when setting globals
Date: Sat, 7 Jun 2014 01:22:09 +0200	[thread overview]
Message-ID: <20140607012209.70d48228@thinkpad> (raw)
In-Reply-To: <20140606222136.GD17594@otherpad.lan.raisama.net>

On Fri, 6 Jun 2014 19:21:36 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Jun 06, 2014 at 11:38:58PM +0200, Igor Mammedov wrote:
> > On Fri, 6 Jun 2014 17:14:29 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > This avoids QEMU from aborting on cases like this:
> > > 
> > >   $ ./install/bin/qemu-system-x86_64 -global cpu.foobar=5
> > >   qemu-system-x86_64: Property '.foobar' not found
> > >   Aborted (core dumped)
> > That is expected behavior.
> 
> Why?
> 
> QEMU should never dump core due to user error.
> 
> QEMU should not abort when handling a device_add command due to user
> error.
I've meant QEMU shouldn't start if CLI has error. whether it's abort or
exit(FAIL) doesn't matter much.

> 
> > 
> > > 
> > > The code sets dev->not_used if the property is not found as an effort to
> > > to allow errors to be reported even if the device is hotpluggable, but
> > > it won't catch all errors. We can't know the property is not going to be
> > > available for hotpluggable devices, unless we actually try to create the
> > > device.
> > Instead of ignoring users errors, DeviceState should have async_error
> > field which could be set by device_post_init() instead of aborting and
> > later device_add could gracefully fail hotadd operation if error is set.
> > 
> > PS:
> > initfn-s could also reuse this, instead of ignoring errors as they do now.
> 
> Your proposal sounds good, and would allow reporting error without
> creating an object_new() variation that accepts Error**.
> 
> But I believe we need to choose what to do in the meantime, while we
> don't have that mechanism implemented. Dumping core is not acceptable.
> Exiting QEMU while handling device_add doesn't seem acceptable to me,
> either.
Exiting at startup is fine and allows to filter out user errors early.

During hotplug exiting is certainly not an option, that's why I'm
suggesting add async_error so that hotplug operation could fail gracefully.
It's a cleaner approach and not much more complex.

Ignoring errors on the other side would introduce relaxed CLI interface that
we would have to support forever for compatibility reasons.

> 
> -- 
> Eduardo


-- 
Regards,
  Igor

  reply	other threads:[~2014-06-06 23:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 20:14 [Qemu-devel] [PATCH] qdev: Skip non-existing properties when setting globals Eduardo Habkost
2014-06-06 21:06 ` Don Slutz
2014-06-06 21:38 ` Igor Mammedov
2014-06-06 22:21   ` Eduardo Habkost
2014-06-06 23:22     ` Igor Mammedov [this message]
2014-06-06 23:45       ` Peter Maydell
2014-06-07  1:09         ` Eduardo Habkost
2014-06-07  1:26           ` [Qemu-devel] [PATCH] qdev: Don't abort() in case globals can't be set Eduardo Habkost
2014-06-08 10:48             ` Michael S. Tsirkin
2014-06-09 13:00             ` Igor Mammedov
2014-06-07  8:55           ` [Qemu-devel] [PATCH] qdev: Skip non-existing properties when setting globals Peter Maydell

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=20140607012209.70d48228@thinkpad \
    --to=imammedo@redhat.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=dslutz@verizon.com \
    --cc=ehabkost@redhat.com \
    --cc=mst@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 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.