[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/qxl: warn on sync io usage
From: |
Alon Levy |
Subject: |
Re: [Qemu-devel] [PATCH] hw/qxl: warn on sync io usage |
Date: |
Thu, 11 Oct 2012 03:40:36 -0400 (EDT) |
> 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
>
>