qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>,
	eric.auger.pro@gmail.com, qemu-devel@nongnu.org,
	ehabkost@redhat.com, pbonzini@redhat.com
Subject: Re: [PATCH] qom: Allow object_property_add_child() to fail
Date: Wed, 24 Jun 2020 09:51:33 +0100	[thread overview]
Message-ID: <20200624085133.GC774096@redhat.com> (raw)
In-Reply-To: <87eeq4dgax.fsf@dusky.pond.sub.org>

On Wed, Jun 24, 2020 at 10:22:14AM +0200, Markus Armbruster wrote:
> Eric Auger <eric.auger@redhat.com> writes:
> 
> > object_property_add() does not allow object_property_try_add()
> > to gracefully fail as &error_abort is passed as an error handle.
> >
> > However such failure can easily be triggered from the QMP shell when,
> > for instance, one attempts to create an object with an id that already
> > exists:
> >
> > For instance, call twice:
> > object-add qom-type=memory-backend-ram id=mem1 props.size=1073741824
> > and QEMU aborts.
> >
> > This behavior is undesired as a user/management application mistake
> > in reusing a property ID shouldn't result in loss of the VM and live
> > data within.
> >
> > This patch introduces two new functions, object_property_add_err() and
> > object_property_add_child_err() whose prototype features an error handle.
> > object_property_add_child_err() now gets called from user_creatable_add_type.
> > This solution was chosen instead of changing the prototype of existing
> > functions because the number of existing callers is huge.
> >
> > The error now is returned gracefully to the QMP client.
> >
> > (QEMU) object-add qom-type=memory-backend-ram id=mem2  props.size=4294967296
> > {"return": {}}
> > (QEMU) object-add qom-type=memory-backend-ram id=mem2  props.size=4294967296
> > {"error": {"class": "GenericError", "desc": "attempt to add duplicate property
> > 'mem2' to object (type 'container')"}}
> >
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> Recent regression, my fault.  Please point that out, and add
> 
>   Fixes: d2623129a7dec1d3041ad1221dda1ca49c667532

I noticed tests/qtest/qmp-cmd-test.c exercises object-add. Probably a
good idea to extend that to test duplicate ID scenario, as that would
have caught the accidental regression.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



      parent reply	other threads:[~2020-06-24  8:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 15:54 [PATCH] qom: Allow object_property_add_child() to fail Eric Auger
2020-06-23 16:05 ` Daniel P. Berrangé
2020-06-23 16:20 ` Paolo Bonzini
2020-06-23 16:22 ` Paolo Bonzini
2020-06-23 16:24   ` Auger Eric
2020-06-23 16:23 ` Paolo Bonzini
2020-06-24  8:22 ` Markus Armbruster
2020-06-24  8:40   ` Auger Eric
2020-06-24  8:51   ` Daniel P. Berrangé [this message]

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=20200624085133.GC774096@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=eric.auger.pro@gmail.com \
    --cc=eric.auger@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).