qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole


From: Gerd Hoffmann
Subject: Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
Date: Tue, 03 Aug 2010 18:42:05 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.4) Gecko/20100628 Red Hat/3.1-1.el6 Thunderbird/3.1

  Hi,

/me wonders what the point of the 'backlog' struct element is then.

Because it could be used for host event but let's ignore that for now.

I doubt using the same beast for both host and guest is going to fly ...

Yes, we can do that. I don't think it is useful. Oh, and it also
changes the command line interface. Todays ...

qemu -chardev soemthing,id=foo \
-device virtserport,chardev=foo

... would turn into something like ...

qemu -chardev something,if=foo \
-charsrv simple,chardev=foo,id=bar \
-device virtserport,charsrv=bar

If we think this is useful, then we can find a way to make the command
line syntax work. If we don't think it's useful, then there's no point
in doing it.

Well, when using the new model everythere this is a moot point as -chardev would just define a CharServerState instead of a CharDriverState then.

Spice would then implement it's own CharServerState and would use it to
understand what state the session is in.

Spice would basically (ab-)use it as event delivery mechanism.

Can you explain what spice uses these events for?

spice-vmc code registers/unregisters the interface within the spice server. So the interface is only activated in case the guest uses it. Spice client sees the interface being active or not and can act accordingly.

http://cgit.freedesktop.org/spice/qemu/tree/hw/spice-vmc.c?h=spice.v13#n128

Well. I disagree. Checking the state is needed nevertheless. The
places where virtio-serial checks port->state today it would have to
check whenever port->chardev is non-NULL then. The only difference is
that failures to do so might become a bit more obvious as qemu will
segfault due to the NULL pointer dereferences then. I still think this
isn't worth the effort though.

But I think we ultimately need to switch to having the front-ends having
a NULL check. Even beyond front-end initiated connect/disconnect,
front-end's need to learn to deal with back-end initiated
disconnect/connect.

I don't think we have to go with a NULL check. Providing chr_is_*() functions to query state and adding asserts() to the chr_*() function should provide the same level of robustness IMHO. Also having CharDriverStates come and go brings its own share of problems.

If you look at something like the serial device's chardev usage, right
now, it writes to the stream regardless of whether the back-end is
connected and we have different semantics in each backend about what we
do. It would be far better to just expose the fact that the backend
isn't connected to the device such that it can either present that to
the guest or make it's own decision about what to do.

Indeed.

I think the model where we always write to a chardev is fundamentally
broken. Sending life cycle events over an always open stream is even
more broken and I think it's a good opportunity to introduce life cycle
awareness into the API (especially since it can be done as a pretty
small incremental change).

The chardevs API can use a major overhaul, no question. I doubt that creating CharServerState helps here much though. Especially I don't see how that could be used for *both* Host and Guest side.

cheers,
  Gerd




reply via email to

[Prev in Thread] Current Thread [Next in Thread]