From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5EA95C433B4 for ; Tue, 20 Apr 2021 02:28:02 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8A27360C3E for ; Tue, 20 Apr 2021 02:28:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8A27360C3E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:33832 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lYg7L-0008Mp-E0 for qemu-devel@archiver.kernel.org; Mon, 19 Apr 2021 22:27:59 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:42384) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lYg6B-0007wh-Nt for qemu-devel@nongnu.org; Mon, 19 Apr 2021 22:26:48 -0400 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:35175) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lYg68-0005Ht-2G for qemu-devel@nongnu.org; Mon, 19 Apr 2021 22:26:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1618885602; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fed1aZbNag5WaKzt3QKqvDyJ4GTz94CMVdS00tOpGR4=; b=Vjmp8j5BEQRGuZWTZyY+25wZd0qKJ+wXJf0I1Y1LAT/jom+UYImeXBAqJHfFibE5WW2qix xjSZk852XqxvmGokZ8gygAhiSD9iDGlOYrr5noWi39G6Wo+tmR9kakTo8Gr7h8cSzV9xIY Y+w+uqVSs1RQcrTjwGnkVuw6pmM8Mno= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-580-iP93iX_sMNKI3Qb88zvn7Q-1; Mon, 19 Apr 2021 22:26:40 -0400 X-MC-Unique: iP93iX_sMNKI3Qb88zvn7Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 08E9D102CB6D for ; Tue, 20 Apr 2021 02:26:40 +0000 (UTC) Received: from [10.10.118.219] (ovpn-118-219.rdu2.redhat.com [10.10.118.219]) by smtp.corp.redhat.com (Postfix) with ESMTP id D9B765C1C4; Tue, 20 Apr 2021 02:26:34 +0000 (UTC) Subject: Re: [PATCH RFC 0/7] RFC: Asynchronous QMP Draft To: Stefan Hajnoczi References: <20210413155553.2660523-1-jsnow@redhat.com> <79eb174a-8e08-aac8-6a0c-e0c7b03eb782@redhat.com> From: John Snow Message-ID: <19a736c7-aced-39d3-e1fb-c0dd5031b2ff@redhat.com> Date: Mon, 19 Apr 2021 22:26:34 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jsnow@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=63.128.21.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: armbru@redhat.com, crosa@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 4/15/21 5:52 AM, Stefan Hajnoczi wrote: > Yeah, it seems very nice for allowing multiple event listeners that > don't steal each other's events. I like it. > > qmp.event_listener() could take a sequence of QMP event names to trigger > on. If the sequence is empty then all QMP events will be reported. I made something like this: # Example 1 with qmp.listener('STOP') as listener: await qmp.execute('stop') await listener.get() # Example 2 with qmp.listener('JOB_STATUS_CHANGE') as listener: await qmp.execute('blockdev-create', ...) async for event in listener: if event['data']['status'] == 'concluded': break await qmp.execute('job-dismiss', ...) # Example 3 - all events with qmp.listener() as events: async for event in events: print(f"got '{event['event']}' event!") # Example 4 - several events on one listener job_events = ( 'JOB_STATUS_CHANGE', 'BLOCK_JOB_COMPLETED', 'BLOCK_JOB_CANCELLED', 'BLOCK_JOB_ERROR', 'BLOCK_JOB_READY', 'BLOCK_JOB_PENDING' ) with qmp.listener(job_events) as events: ... There is a *post-filtering* syntax available to EventListener.get(). It will filter events out using a very simplistic syntax. # Example 5 -- a short-hand form of Example 2. with qmp.listener('JOB_STATUS_CHANGE') as job_events: await qmp.execute('blockdev-create', ...) await job_events.get(status='concluded') await qmp.execute('job-dismiss', ...) A shortcoming with this interface is that it's easy to create a listener that hears multiple events, but it's not easy to create *several listeners*. I am not sure what syntax will be the nicest for this, but I tried by allowing the manual creation of listeners: # Example 6 listener1 = EventListener('JOB_STATUS_CHANGE') listener2 = EventListener(job_events) # Note the use of listen() instead of listener() with qmp.listen(listener1, listener2) as (ev1, ev2): # listeners are now active. ... # listeners are now inactive. # The context manager clears any stale events in the listener(s). I thought this might be nicer than trying to extend the listener syntax: with qmp.listeners( 'JOB_STATUS_CHANGE', (job_events) ) as ( listener1, listener2, ): ... especially because it might get confusing when trying to separate "one listener with multiple events" vs "several listeners with one event each, and it makes things a little ambiguous: with qmp.listeners('STOP') as (stop_events,): ... And this isn't any prettier, and also likely to confuse: with qmp.listeners('STOP', 'RESUME') as (stops, resumes): ... because it's only so very subtly different from this: with qmp.listeners(('STOP', 'RESUME')) as (runstate_events,): ... This also doesn't begin to address one of the worst headaches of writing iotests where transactions are involved: accidentally eating events meant for other jobs. I prototyped something where it's possible to create an EventListener with an optional pre-filter, but it's a little bulky: # Example 7 listener = EventListener('JOB_STATUS_CHANGE', lambda e: e['data']['id'] == 'job0') with qmp.listen(listener): await qmp.execute('blockdev-create', arguments={'job-id': 'job0'}) await listener.get(status='created') ... Some thoughts on this: - Pre-filters are powerful, but involve a lot of boilerplate. - Accepting two kinds of parameters, name(s) and filter both, makes it even trickier to write concise context blocks; especially with multiple jobs. Here's a final example of something you may very well want to do in iotest code: # Example 8 def job_filter(job_id: str) -> EventFilter: def filter(event: Message) -> bool: return event.get('data', {}).get('id') == job_id return filter listener1 = EventListener('JOB_STATUS_CHANGE', job_filter('job0')) listener2 = EventListener('JOB_STATUS_CHANGE', job_filter('job1')) with qmp.listen(listener1, listener2) as (job0, job1): await asyncio.gather( qmp.execute('blockdev-create', arguments={'job-id': 'job0'}), qmp.execute('blockdev-create', arguments={'job-id': 'job1'}), job0.get(status='concluded'), job1.get(status='concluded') ) (Note: gather isn't required here. You could write the execute and get statements individually and in whichever order you wanted, as long as the execute statement for a given job appears prior to the corresponding wait!) The difficulty I have here is extending that backwards to the "create listener on the fly" syntax, for the reasons stated above with making it ambiguous as to whether we're creating one or two listeners, etc. Trying to minimize boilerplate while leaving the interfaces generic and powerful is tough. I'm still playing around with different options and solutions, but your feedback/input is welcome. --js