| Message ID | 20220726192150.2435175-9-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-gpio and various virtio cleanups | expand |
在 2022/7/27 03:21, Alex Bennée 写道: > The assert() protecting against leakage is a little aggressive and > causes needless crashes if a device is shutdown without having been > configured. In this case no descriptors are lost because none have > been assigned. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > hw/virtio/virtio-pci.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 45327f0b31..5ce61f9b45 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -996,9 +996,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) > > nvqs = MIN(nvqs, VIRTIO_QUEUE_MAX); > > - /* When deassigning, pass a consistent nvqs value > - * to avoid leaking notifiers. > + /* > + * When deassigning, pass a consistent nvqs value to avoid leaking > + * notifiers. But first check we've actually been configured, exit > + * early if we haven't. > */ > + if (!assign && !proxy->nvqs_with_notifiers) { > + return 0; > + } I think the logic is the caller is in charge of tracking whether the vhost device is started so it can avoid calling this function? Thanks > assert(assign || nvqs == proxy->nvqs_with_notifiers); > > proxy->nvqs_with_notifiers = nvqs;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 45327f0b31..5ce61f9b45 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -996,9 +996,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) nvqs = MIN(nvqs, VIRTIO_QUEUE_MAX); - /* When deassigning, pass a consistent nvqs value - * to avoid leaking notifiers. + /* + * When deassigning, pass a consistent nvqs value to avoid leaking + * notifiers. But first check we've actually been configured, exit + * early if we haven't. */ + if (!assign && !proxy->nvqs_with_notifiers) { + return 0; + } assert(assign || nvqs == proxy->nvqs_with_notifiers); proxy->nvqs_with_notifiers = nvqs;
The assert() protecting against leakage is a little aggressive and causes needless crashes if a device is shutdown without having been configured. In this case no descriptors are lost because none have been assigned. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- hw/virtio/virtio-pci.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)