From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54677) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQHQv-0002w5-IO for qemu-devel@nongnu.org; Thu, 21 Jul 2016 13:06:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQHQs-0007hh-7u for qemu-devel@nongnu.org; Thu, 21 Jul 2016 13:06:49 -0400 Received: from 4.mo178.mail-out.ovh.net ([46.105.49.171]:55827) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQHQs-0007hX-1F for qemu-devel@nongnu.org; Thu, 21 Jul 2016 13:06:46 -0400 Received: from player169.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo178.mail-out.ovh.net (Postfix) with ESMTP id 2CA651000E0A for ; Thu, 21 Jul 2016 19:06:45 +0200 (CEST) Date: Thu, 21 Jul 2016 19:06:39 +0200 From: Greg Kurz Message-ID: <20160721190639.0ebf903c@bahia.lan> In-Reply-To: <20160721190239.0153576f@bahia.lan> References: <146911619705.23920.6002060635853247073.stgit@bahia.lan> <5790FDCC.2010509@gmail.com> <20160721190239.0153576f@bahia.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] virtio-pci: error out when both legacy and modern modes are disabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: "Michael S. Tsirkin" , Cornelia Huck , qemu-devel@nongnu.org On Thu, 21 Jul 2016 19:02:39 +0200 Greg Kurz wrote: > On Thu, 21 Jul 2016 19:52:28 +0300 > Marcel Apfelbaum wrote: > > > On 07/21/2016 06:52 PM, Greg Kurz wrote: > > > From: Greg Kurz > > > > > > Without presuming if we got there because of a user mistake or some > > > more subtle bug in the tooling, it really does not make sense to > > > implement a non-functional device. > > > > > > Signed-off-by: Greg Kurz > > > Signed-off-by: Greg Kurz > > > --- > > > v2: - error out at realize time as suggested by Connie > > > - updated title and changelog > > > --- > > > hw/virtio/virtio-pci.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > > index f0677b73d860..8d707aac0c21 100644 > > > --- a/hw/virtio/virtio-pci.c > > > +++ b/hw/virtio/virtio-pci.c > > > @@ -1838,6 +1838,12 @@ static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp) > > > VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev); > > > PCIDevice *pci_dev = &proxy->pci_dev; > > > > > > + if (proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY && > > > + proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN) { > > > + error_setg(errp, "device is unserviceable when both legacy and modern modes are disabled. At least one of the disable-modern or disable-legacy properties should be set to false."); > > > + return; > > > + } > > > + > > > > Hi Greg, > > > > I agree with the patch, but other patch in the mailing list > > replaces the flags with standalone fields: > > > > https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04744.html > > > > v3 will be using virtio_pci_legacy() and virtio_pci_modern(). > > > Also the error line may be too long and need to be truncated. > > > > I'll rephrase the message. BTW is there a formal limit for the length > of error lines ? 80 chars ? > Not even sure it makes sense since the final error message contains the full -device command line option which can be of arbitrary length... > > Thanks, > > Marcel > > > > > if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) && > > > !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN)) { > > > pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; > > > > > > > > >