qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>,
	qemu-devel@nongnu.org, mprivozn@redhat.com, kraxel@redhat.com,
	amit.shah@redhat.com, lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] char: report frontend open/closed state in 'query-chardev'
Date: Thu, 26 Jun 2014 09:09:14 -0600	[thread overview]
Message-ID: <53AC379A.2090902@redhat.com> (raw)
In-Reply-To: <53AC2268.2000503@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]

On 06/26/2014 07:38 AM, Laszlo Ersek wrote:

>>>  ##
>>> -{ '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.

If we ever add another state in the future, then both places would be
touched at the same time to figure out how to support that new state.
So I agree with your counter-proposal of just simplifying things to use
open:bool in the first patch; it also has the benefit of less code (no
need to add an enum).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2014-06-26 15:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-26 11:11 [Qemu-devel] [PATCH for-2.1 v2 0/2] help libvirt know what's up with qga Laszlo Ersek
2014-06-26 11:11 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] virtio-serial: report frontend connection state via monitor Laszlo Ersek
2014-06-26 12:15   ` Eric Blake
2014-06-26 11:11 ` [Qemu-devel] [PATCH for-2.1 v2 2/2] char: report frontend open/closed state in 'query-chardev' Laszlo Ersek
2014-06-26 12:09   ` Eric Blake
2014-06-26 13:38     ` Laszlo Ersek
2014-06-26 15:09       ` Eric Blake [this message]
2014-06-26 11:30 ` [Qemu-devel] [PATCH for-2.1 v2 0/2] help libvirt know what's up with qga Amit Shah

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=53AC379A.2090902@redhat.com \
    --to=eblake@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mprivozn@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).