[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 1/2] qxl: store channel id in qxl->id
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [PULL 1/2] qxl: store channel id in qxl->id |
Date: |
Mon, 5 Nov 2018 09:43:51 +0100 |
User-agent: |
NeoMutt/20180716 |
Hi,
> > vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
> > + qxl->id = qemu_console_get_index(vga->con); /* == channel_id */
> > + if (qxl->id != 0) {
> > + error_setg(errp, "primary qxl-vga device must be console 0 "
> > + "(first display device on the command line)");
> > + return;
> > + }
>
> In the comment this seems no more required so why testing it?
I'd expect it'll confuse guests.
If you are sure this is not the case feel free to send a patch dropping
this check.
> > static void qxl_realize_secondary(PCIDevice *dev, Error **errp)
> > {
> > - static int device_id = 1;
> > PCIQXLDevice *qxl = PCI_QXL(dev);
> >
> > - qxl->id = device_id++;
> > qxl_init_ramsize(qxl);
> > memory_region_init_ram(&qxl->vga.vram, OBJECT(dev), "qxl.vgavram",
> > qxl->vga.vram_size, &error_fatal);
> > qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram);
> > qxl->vga.con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
> > + qxl->id = qemu_console_get_index(qxl->vga.con); /* == channel_id */
> >
>
> As these IDs must be contiguous this means that there must be the requirement
> that if there is a qxl interface only qxl interfaces are used and no other
> console which seems to me wrong.
Hmm. Didn't know this is a requirement. But as far I know qxl->id is
also expected to be the channel_id. So with qxl and non-qxl mixed we
are in trouble no matter what:
* with the patch we break the contiguous id requirement.
* without the patch we break the qxl->id == channel_id requirement.
So, what now?
cheers,
Gerd