qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH] spice: add qxl device
Date: Wed, 17 Nov 2010 16:20:45 +0100	[thread overview]
Message-ID: <4CE3F2CD.4030701@redhat.com> (raw)
In-Reply-To: <20101117135842.GA13690@redhat.com>

   Hi,

>>>> +        qxl0 = qxl;
>>>
>>> What happens when this device is then removed?
>>
>> Better don't try ...
>
> Better prevent it then?

How can I do that?

>> The primary vga can't be hot-unplugged in qemu.  Not only because
>> the qxl0 pointer would point into nowhere in this case, but also
>> because you can't unregister the graphic console.
>
> But do we really have to make this part of qxl design, with global
> vars and stuff? Even if we have a limitation in qemu, qxl should be able
> to keep all its data in device state I think ...

It isn't part of the qxl design.  It is just that 
register_displaychangelistener() doesn't provide a way to pass through a 
device state pointer, so these callbacks use qxl0 instead.

>>   Also having non-pci ressources (legacy vga I/O ports) is a problem.
>
> I'm not sure why is this a problem. It shouldn't be.

Because it has ressources not visible in the pci config space?

But you know the pci specs better than me.  If you think it would work 
you are probably right ;)

>> Because the first one actually *is* different. It is the only one
>> which is vga compatible.  It serves as primary display.  You'll see
>> the boot messages there.
>
> I thought it's up to the guest where to send boot messages.
> Modern BIOSes have options to control this behaviour.

It isn't.  For one qemu has no multihead support.  You'll see 
non-primary displays only when connecting via spice because of that.

I also doubt seabios supports this.  How does this work btw?  Only one 
vga adapter can drive the legacy vga ports, right?  Is there some way to 
enable/disable this per vga device?  If so: does qemu emulate this 
correctly?

> I think it's a mistake to make things such as device class
> depend on the order devices are created in.
> It would be better to make it a separate property, or give it a
> different name.

Having qxl-vga and qxl devices would certainly be an option.  Works 
probably better than some qdev property usability-wise.

>> I doubt you'll see it wrap in any real world scenario.  Even with one
>> hotplug + unplug cycle per second you'll need a bunch of years to
>> see it wrap.  Beside that at least in windows the device can't be
>> unplugged in the first place, windows will veto the unplug request.
>
> Yes but we are splitting the unplug part from the guest eject, so user
> will in the future be able to perform surprise removal just like with
> real hardware. And with this in place, it need not take even a
> millisecond to unplug a device.

If you do that your guest will likely be quite upset and you'll have 
much bigger problems than the counter wrapping after millions of 
hotplugs.  I also don't see the point in plugging a display like mad.

Can we care about the *real* problems please?

thanks,
   Gerd

  reply	other threads:[~2010-11-17 15:20 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-02 13:34 [Qemu-devel] [PATCH] spice: add qxl device Gerd Hoffmann
2010-11-16 14:24 ` Anthony Liguori
2010-11-17 13:14   ` Gerd Hoffmann
2010-11-16 17:43 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-17 13:28   ` Gerd Hoffmann
2010-11-17 13:58     ` Michael S. Tsirkin
2010-11-17 15:20       ` Gerd Hoffmann [this message]
2010-11-17 16:42         ` Michael S. Tsirkin
2010-11-17 17:02           ` Gerd Hoffmann
2010-11-17 18:00             ` Michael S. Tsirkin
2010-11-18  8:09               ` Gleb Natapov
2010-11-18  8:22                 ` Gerd Hoffmann
2010-11-18  9:08                   ` Michael S. Tsirkin
2010-11-18 10:46                     ` Gerd Hoffmann
2010-11-18  9:03                 ` Michael S. Tsirkin
2010-11-18  9:11                   ` Gleb Natapov
2010-11-18  9:30                     ` Michael S. Tsirkin
2010-11-18  9:57                       ` Gleb Natapov
2010-11-18 11:33                         ` Michael S. Tsirkin
2010-11-18 11:55                           ` Gleb Natapov
2010-11-18 12:03                             ` Michael S. Tsirkin
2010-11-18 12:27                               ` Gleb Natapov
2010-11-18 14:04                                 ` Michael S. Tsirkin
2010-11-18 14:57                                   ` Gleb Natapov
2010-11-18 15:25                                     ` Michael S. Tsirkin
2010-11-18 15:42                                       ` Gleb Natapov
2010-11-18 16:04                                         ` Michael S. Tsirkin
2010-11-18 16:10                                           ` Gleb Natapov
2010-11-18 16:14                                             ` Michael S. Tsirkin

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=4CE3F2CD.4030701@redhat.com \
    --to=kraxel@redhat.com \
    --cc=mst@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).