Re: [Qemu-devel] [PATCH] hw/qxl: warn on sync io usage

[Alon Levy <alevy@redhat.com>]

> Hi,
> 
> > +static void sync_io_warning(PCIQXLDevice *qxl, uint32_t io_port)
> > +{
> > +    fprintf(stderr, "qxl-%d: WARNING: sync io used, see (RHBZ
> > 747011)",
> > +            qxl->id);
> > +    fprintf(stderr, "qxl-%d: WARNING: virt-viewer/remote-viewer
> > can hang\n",
> > +            qxl->id);
> > +    if (qxl->revision < 3) {
> > +        fprintf(stderr, "qxl-%d: WARNING: revision >= 3 should be
> > used\n",
> > +                qxl->id);
> > +    }
> > +}
> 
> The message should also include hints how to fix that.
> 

Yes, that was the idea of the last line, but I like your idea of a QMP message below. Users divide into those who would launch qemu directly, and via administration interfaces, so I think a stderr message should be made for the former plus a QMP message for the later. The revision can be changed directly or via machine type like you noted. So I'll make a patch to warn on the former during initialization (revision = 2 due to [direct parameter|machine type] suggest [increase to 3|change machine type to >pc-0.12]). And I'll fix the later per your suggestion. I should also change spice to warn (we already warn when the keyboard is cleartext, but that warning is never displayed, so adding another such warning and fixing remote-viewer to display it).

> For the revision this probably means to update the machine type from
> pc-0.12 (which sets rev=2 via compat properties) to something newer.
> Telling the user what to do about it is tricky though as there seems
> to
> be no simple GUI way to do that, at least not in virt-manager.  In
> the
> other hand if the user manages to find the message in
> /var/log/libvirt/qemu/${guest}.log he might be experienced enough to
> just "virsh edit ${guest}".
> 
> For the sync I/O it's easy, just say something like "Update qxl
> drivers
> in the guest."
> 
> BTW: You can print multi-line messages this way ...
> 
> 	fprintf(stderr,
>                 "long line one\n"
> 		"long line two\n",
>                 args, here);

Thanks for that.

> 
> ... which I find more readable in the source code.
> 
> Do we wanna have a "suggest to update guest drivers for device $foo"
> qmp message for management?
I think so.

> Or has ovirt/rhev better ways (guest agent?) to deal with that?

I'll check.

> 
> cheers,
>   Gerd
> 
>