From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMDNr-0007Nn-Ks for qemu-devel@nongnu.org; Thu, 11 Oct 2012 03:40:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMDNl-0003Gv-OG for qemu-devel@nongnu.org; Thu, 11 Oct 2012 03:40:43 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:41135) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMDNl-0003G9-FT for qemu-devel@nongnu.org; Thu, 11 Oct 2012 03:40:37 -0400 Date: Thu, 11 Oct 2012 03:40:36 -0400 (EDT) From: Alon Levy Message-ID: <740361087.12933055.1349941236346.JavaMail.root@redhat.com> In-Reply-To: <50766D01.2030509@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/qxl: warn on sync io usage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org > 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 > >