Date: Wed, 13 Sep 2017 20:22:27 +0800
From: Peter Xu <peterx@redhat.com>
Message-ID: <20170913122227.GH3617@pxdev.xzpeter.org>
References: <1502788964-30517-1-git-send-email-thuth@redhat.com>
	<20170913104118.GF3617@pxdev.xzpeter.org>
	<8046ee88-1d7e-b34c-e5db-f55c67f98a7e@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <8046ee88-1d7e-b34c-e5db-f55c67f98a7e@redhat.com>
Subject: Re: [Qemu-devel] [RFC PATCH for 2.11] tests: Introduce generic
 device hot-plug/hot-unplug functions
To: Thomas Huth <thuth@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Markus Armbruster <armbru@redhat.com>, Amit Shah <amit@kernel.org>

On Wed, Sep 13, 2017 at 12:56:20PM +0200, Thomas Huth wrote:

[...]

> >> +
> >> +/**
> >> + * Generic hot-plugging test via the device_add QMP command
> >> + */
> >> +void qtest_hot_plug_device(const char *driver, const char *id,
> >> +                           const char *fmt, ...)
> >> +{
> >> +    QDict *response;
> >> +    char *cmd, *opts = NULL;
> >> +    va_list va;
> >> +
> >> +    if (fmt) {
> >> +        va_start(va, fmt);
> >> +        opts = g_strdup_vprintf(fmt, va);
> >> +        va_end(va);
> >> +    }
> >> +
> >> +    cmd = g_strdup_printf("{'execute': 'device_add',"
> >> +                          " 'arguments': { 'driver': '%s', 'id': '%s'%s%s }}",
> >> +                          driver, id, opts ? ", " : "", opts ? opts : "");
> >> +    g_free(opts);
> >> +
> >> +    response = qmp(cmd);
> >> +    g_free(cmd);
> >> +    g_assert(response);
> >> +    while (qdict_haskey(response, "event")) {
> >> +        /* We can get DEVICE_DELETED events in case something went wrong */
> >> +        g_assert_cmpstr(qdict_get_str(response, "event"), !=, "DEVICE_DELETED");
> >> +        QDECREF(response);
> >> +        response = qmp("");
> >> +        g_assert(response);
> >> +    }
> > 
> > I guess we don't need this block for hot plug? :-)
> 
> I'm pretty sure I've seen this happening somewhere ... but maybe it was
> just a fall-out of the mis-behaving usb test...
> I'll replace it with a  g_assert(!qdict_haskey(response, "event"))
> instead, since we should not get any events in this case, should we?

I think we should not, at least there is no such DEVICE_NEW message
defined.  Maybe that's the thing you mentioned? (say, an old test
didn't flush the read pipe so the message from the old one leaked into
a new test)

Thanks,

-- 
Peter Xu