From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X09tE-0001IR-9e for qemu-devel@nongnu.org; Thu, 26 Jun 2014 09:39:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X09t6-0007xo-Jp for qemu-devel@nongnu.org; Thu, 26 Jun 2014 09:39:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18168) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X09t6-0007xi-BV for qemu-devel@nongnu.org; Thu, 26 Jun 2014 09:38:52 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5QDcpsM008317 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 26 Jun 2014 09:38:51 -0400 Message-ID: <53AC2268.2000503@redhat.com> Date: Thu, 26 Jun 2014 15:38:48 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1403781075-30989-1-git-send-email-lersek@redhat.com> <1403781075-30989-3-git-send-email-lersek@redhat.com> <53AC0D67.7040402@redhat.com> In-Reply-To: <53AC0D67.7040402@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] char: report frontend open/closed state in 'query-chardev' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org, mprivozn@redhat.com, kraxel@redhat.com, amit.shah@redhat.com, lcapitulino@redhat.com On 06/26/14 14:09, Eric Blake wrote: > On 06/26/2014 05:11 AM, Laszlo Ersek wrote: >> In addition to the on-line reporting added in the previous patch, allow >> libvirt to query frontend state independently of events. >> >> Libvirt's path to identify the guest agent channel it cares about differs >> between the event added in the previous patch and the QMP response field >> added here. The event identifies the frontend device, by "id". The >> 'query-chardev' QMP command identifies the backend device (again by "id"). >> The association is under libvirt's control. >> >> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1080376 >> Signed-off-by: Laszlo Ersek >> --- >> > >> ## >> -{ 'type': 'ChardevInfo', 'data': {'label': 'str', 'filename': 'str'} } >> +{ 'type': 'ChardevInfo', 'data': {'label': 'str', >> + 'filename': 'str', >> + 'frontend-open': 'bool'} } > > Hmm; I wonder if this should instead be > 'frontend-status':'VirtioSerialPortStatus', to reuse the type from patch > 1/2. That way, if we ever add a third state, then both the event and > the poll will be reusing the same enum values to report that state. I expected this remark :) The difference is rooted in the fact that the event approaches the virtio-serial port status from the frontend (ie. guest) side, while the query approaches the same from the backend (ie. host, chardev) side. If I wanted to bring those in future-proof sync, then I would have to change the underlying, generic chardev machinery -- namely, the fe_open field, and everything that operates on it. I actually considered the other direction too: rather than introducing status:VirtioSerialPortStatus, just add open:bool (which was your original suggestion in your v1 review). I decided against it because the current list of enum constants (connected, disconnected) expresses just the same, and it'll be a *tiny* bit easier to extend, should that necessity arise. Sounds acceptible? :) If not, then I'm OK to replace status:VirtioSerialPortStatus with open:bool in the first patch. Thanks, Laszlo