| Message ID | 146304635825.4689.14165883840595728421.stgit@bahia.lab.toulouse-stg.fr.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 12 May 2016 11:45:58 +0200 Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > Without presuming if we got there because of a user mistake or some > more subtile bug in the tooling, it doesn't hurt to log somewhere that > the device won't be functional. > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > --- Michael, This patch still applies to your tree. Please nack if this is not worth being upstreamed. Cc'ing Connie for broader audience :) Cheers. -- Greg > hw/virtio/virtio-pci.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 0f634d2d776e..a74978cb5e83 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > uint32_t size; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > + if (!legacy && !modern) { > + error_report("Warning: 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."); > + } > + > config = proxy->pci_dev.config; > if (proxy->class_code) { > pci_config_set_class(config, proxy->class_code); > >
On Thu, 21 Jul 2016 11:24:45 +0200 Greg Kurz <groug@kaod.org> wrote: > On Thu, 12 May 2016 11:45:58 +0200 > Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > > Without presuming if we got there because of a user mistake or some > > more subtile bug in the tooling, it doesn't hurt to log somewhere that s/subtile/subtle/ > > the device won't be functional. > > > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > --- > > Michael, > > This patch still applies to your tree. Please nack if this is not worth > being upstreamed. > > Cc'ing Connie for broader audience :) > > Cheers. > > -- > Greg > > > hw/virtio/virtio-pci.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index 0f634d2d776e..a74978cb5e83 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > > uint32_t size; > > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > > > + if (!legacy && !modern) { > > + error_report("Warning: 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."); > > + } Do you maybe want to fail this at the realize stage already? A device that is neither legacy nor modern should not exist at all. > > + > > config = proxy->pci_dev.config; > > if (proxy->class_code) { > > pci_config_set_class(config, proxy->class_code); > > > > >
On Thu, 21 Jul 2016 13:07:46 +0200 Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > On Thu, 21 Jul 2016 11:24:45 +0200 > Greg Kurz <groug@kaod.org> wrote: > > > On Thu, 12 May 2016 11:45:58 +0200 > > Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > > > Without presuming if we got there because of a user mistake or some > > > more subtile bug in the tooling, it doesn't hurt to log somewhere that > > s/subtile/subtle/ > Frenglish stroke again :) > > > the device won't be functional. > > > > > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > > --- > > > > Michael, > > > > This patch still applies to your tree. Please nack if this is not worth > > being upstreamed. > > > > Cc'ing Connie for broader audience :) > > > > Cheers. > > > > -- > > Greg > > > > > hw/virtio/virtio-pci.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > > index 0f634d2d776e..a74978cb5e83 100644 > > > --- a/hw/virtio/virtio-pci.c > > > +++ b/hw/virtio/virtio-pci.c > > > @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > > > uint32_t size; > > > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > > > > > + if (!legacy && !modern) { > > > + error_report("Warning: 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."); > > > + } > > Do you maybe want to fail this at the realize stage already? A device > that is neither legacy nor modern should not exist at all. > Makes sense. I'll send a v2. Thanks ! > > > + > > > config = proxy->pci_dev.config; > > > if (proxy->class_code) { > > > pci_config_set_class(config, proxy->class_code); > > > > > > > > >
On 07/21/2016 05:43 PM, Greg Kurz wrote: > On Thu, 21 Jul 2016 13:07:46 +0200 > Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > >> On Thu, 21 Jul 2016 11:24:45 +0200 >> Greg Kurz <groug@kaod.org> wrote: >> >>> On Thu, 12 May 2016 11:45:58 +0200 >>> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: >>>> Without presuming if we got there because of a user mistake or some >>>> more subtile bug in the tooling, it doesn't hurt to log somewhere that >> >> s/subtile/subtle/ >> > > Frenglish stroke again :) > >>>> the device won't be functional. >>>> >>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>>> --- >>> >>> Michael, >>> >>> This patch still applies to your tree. Please nack if this is not worth >>> being upstreamed. >>> >>> Cc'ing Connie for broader audience :) >>> >>> Cheers. >>> >>> -- >>> Greg >>> >>>> hw/virtio/virtio-pci.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >>>> index 0f634d2d776e..a74978cb5e83 100644 >>>> --- a/hw/virtio/virtio-pci.c >>>> +++ b/hw/virtio/virtio-pci.c >>>> @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >>>> uint32_t size; >>>> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); >>>> >>>> + if (!legacy && !modern) { >>>> + error_report("Warning: 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."); >>>> + } >> >> Do you maybe want to fail this at the realize stage already? A device >> that is neither legacy nor modern should not exist at all. >> > > Makes sense. I'll send a v2. Thanks ! > I suggest to rebase it on top of: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04744.html which touches the same area... or the maintainer will deal with it :) Thanks, Marcel >>>> + >>>> config = proxy->pci_dev.config; >>>> if (proxy->class_code) { >>>> pci_config_set_class(config, proxy->class_code); >>>> >>>> >>> >> > >
On Thu, 21 Jul 2016 18:32:40 +0300 Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote: > On 07/21/2016 05:43 PM, Greg Kurz wrote: > > On Thu, 21 Jul 2016 13:07:46 +0200 > > Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > > > >> On Thu, 21 Jul 2016 11:24:45 +0200 > >> Greg Kurz <groug@kaod.org> wrote: > >> > >>> On Thu, 12 May 2016 11:45:58 +0200 > >>> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > >>>> Without presuming if we got there because of a user mistake or some > >>>> more subtile bug in the tooling, it doesn't hurt to log somewhere that > >> > >> s/subtile/subtle/ > >> > > > > Frenglish stroke again :) > > > >>>> the device won't be functional. > >>>> > >>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>>> --- > >>> > >>> Michael, > >>> > >>> This patch still applies to your tree. Please nack if this is not worth > >>> being upstreamed. > >>> > >>> Cc'ing Connie for broader audience :) > >>> > >>> Cheers. > >>> > >>> -- > >>> Greg > >>> > >>>> hw/virtio/virtio-pci.c | 4 ++++ > >>>> 1 file changed, 4 insertions(+) > >>>> > >>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > >>>> index 0f634d2d776e..a74978cb5e83 100644 > >>>> --- a/hw/virtio/virtio-pci.c > >>>> +++ b/hw/virtio/virtio-pci.c > >>>> @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > >>>> uint32_t size; > >>>> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > >>>> > >>>> + if (!legacy && !modern) { > >>>> + error_report("Warning: 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."); > >>>> + } > >> > >> Do you maybe want to fail this at the realize stage already? A device > >> that is neither legacy nor modern should not exist at all. > >> > > > > Makes sense. I'll send a v2. Thanks ! > > > > I suggest to rebase it on top of: > https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04744.html > which touches the same area... or the maintainer will deal with it :) > Oh I had missed this patch... and I would never leave the chore to the Maintainer :) I'll rebase and send a v3. > Thanks, > Marcel > Thanks Marcel for raising the flag ! Cheers. -- Greg > >>>> + > >>>> config = proxy->pci_dev.config; > >>>> if (proxy->class_code) { > >>>> pci_config_set_class(config, proxy->class_code); > >>>> > >>>> > >>> > >> > > > > >
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 0f634d2d776e..a74978cb5e83 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1639,6 +1639,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) uint32_t size; VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + if (!legacy && !modern) { + error_report("Warning: 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."); + } + config = proxy->pci_dev.config; if (proxy->class_code) { pci_config_set_class(config, proxy->class_code);
Without presuming if we got there because of a user mistake or some more subtile bug in the tooling, it doesn't hurt to log somewhere that the device won't be functional. Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> --- hw/virtio/virtio-pci.c | 4 ++++ 1 file changed, 4 insertions(+)