| Message ID | 9e749100b07b9935d73f7f9740deedf510222a1c.1335259354.git.amit.shah@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > From: Alon Levy <alevy@redhat.com> > > guest_connected should be false before guest driver initialization, and > true after, both for multiport aware and non multiport aware drivers. > > Don't set it before the guest_features are available; instead use > set_status which is called by io to VIRTIO_PCI_STATUS with > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > [Amit: Add comment, tweak summary] Logic also changed fron Alon's patch. Why? > > Signed-off-by: Alon Levy <alevy@redhat.com> > Acked-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > 1 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index e22940e..796224b 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > memcpy(&config, config_data, sizeof(config)); > } > > +static void set_status(VirtIODevice *vdev, uint8_t status) > +{ > + VirtIOSerial *vser; > + VirtIOSerialPort *port; > + > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > + port = find_port_by_id(vser, 0); > + > + if (port && !use_multiport(port->vser) > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > + /* > + * Non-multiport guests won't be able to tell us guest > + * open/close status. Such guests can only have a port at id > + * 0, so set guest_connected for such ports as soon as guest > + * is up. > + */ > + port->guest_connected = true; > + } > +} > + Weird. Don't you want to set guest_connected = false when driver is unloaded? This is what Alon's patch did: port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; > static void virtio_serial_save(QEMUFile *f, void *opaque) > { > VirtIOSerial *s = opaque; > @@ -798,14 +818,6 @@ static int virtser_port_qdev_init(DeviceState *qdev) > return ret; > } > > - if (!use_multiport(port->vser)) { > - /* > - * Allow writes to guest in this case; we have no way of > - * knowing if a guest port is connected. > - */ > - port->guest_connected = true; > - } > - > port->elem.out_num = 0; > > QTAILQ_INSERT_TAIL(&port->vser->ports, port, next); > @@ -905,6 +917,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf) > vser->vdev.get_features = get_features; > vser->vdev.get_config = get_config; > vser->vdev.set_config = set_config; > + vser->vdev.set_status = set_status; > > vser->qdev = dev; > > -- > 1.7.7.6
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > > From: Alon Levy <alevy@redhat.com> > > > > guest_connected should be false before guest driver initialization, and > > true after, both for multiport aware and non multiport aware drivers. > > > > Don't set it before the guest_features are available; instead use > > set_status which is called by io to VIRTIO_PCI_STATUS with > > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > > > [Amit: Add comment, tweak summary] > > Logic also changed fron Alon's patch. Why? Yes, forgot to mention that here. > > Signed-off-by: Alon Levy <alevy@redhat.com> > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > --- > > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > > 1 files changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > index e22940e..796224b 100644 > > --- a/hw/virtio-serial-bus.c > > +++ b/hw/virtio-serial-bus.c > > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > > memcpy(&config, config_data, sizeof(config)); > > } > > > > +static void set_status(VirtIODevice *vdev, uint8_t status) > > +{ > > + VirtIOSerial *vser; > > + VirtIOSerialPort *port; > > + > > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > > + port = find_port_by_id(vser, 0); > > + > > + if (port && !use_multiport(port->vser) > > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > > + /* > > + * Non-multiport guests won't be able to tell us guest > > + * open/close status. Such guests can only have a port at id > > + * 0, so set guest_connected for such ports as soon as guest > > + * is up. > > + */ > > + port->guest_connected = true; > > + } > > +} > > + > > Weird. Don't you want to set guest_connected = false when driver > is unloaded? > This is what Alon's patch did: > port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; Setting guest_connected to false when driver is unloaded is something that's not done before this patch (and not noted in the commit log too). And that case isn't specific to just port 0 (in the !multiport case); all ports will have to be updated for such a change. Is that something we should do? It's obviously correct to do that. Will it affect anything? I doubt it. But needs a separate patch and discussion for that change. Amit
On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote: > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > > On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > > > From: Alon Levy <alevy@redhat.com> > > > > > > guest_connected should be false before guest driver initialization, and > > > true after, both for multiport aware and non multiport aware drivers. > > > > > > Don't set it before the guest_features are available; instead use > > > set_status which is called by io to VIRTIO_PCI_STATUS with > > > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > > > > > [Amit: Add comment, tweak summary] > > > > Logic also changed fron Alon's patch. Why? > > Yes, forgot to mention that here. > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > > --- > > > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > > > 1 files changed, 21 insertions(+), 8 deletions(-) > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > index e22940e..796224b 100644 > > > --- a/hw/virtio-serial-bus.c > > > +++ b/hw/virtio-serial-bus.c > > > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > > > memcpy(&config, config_data, sizeof(config)); > > > } > > > > > > +static void set_status(VirtIODevice *vdev, uint8_t status) > > > +{ > > > + VirtIOSerial *vser; > > > + VirtIOSerialPort *port; > > > + > > > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > > > + port = find_port_by_id(vser, 0); > > > + > > > + if (port && !use_multiport(port->vser) > > > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > > > + /* > > > + * Non-multiport guests won't be able to tell us guest > > > + * open/close status. Such guests can only have a port at id > > > + * 0, so set guest_connected for such ports as soon as guest > > > + * is up. > > > + */ > > > + port->guest_connected = true; > > > + } > > > +} > > > + > > > > Weird. Don't you want to set guest_connected = false when driver > > is unloaded? > > This is what Alon's patch did: > > port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; > > Setting guest_connected to false when driver is unloaded is something > that's not done before this patch (and not noted in the commit log > too). > > And that case isn't specific to just port 0 (in the !multiport case); > all ports will have to be updated for such a change. > > Is that something we should do? It's obviously correct to do that. > Will it affect anything? I doubt it. But needs a separate patch and > discussion for that change. > > Amit Let's not add code to preserve a bug so we can have a discussion. Let's have a discussion right here and fix it properly. BTW guest_connected is not cleared on reset either - a bug too?
On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote: > On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote: > > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > > > On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > > > > From: Alon Levy <alevy@redhat.com> > > > > > > > > guest_connected should be false before guest driver initialization, and > > > > true after, both for multiport aware and non multiport aware drivers. > > > > > > > > Don't set it before the guest_features are available; instead use > > > > set_status which is called by io to VIRTIO_PCI_STATUS with > > > > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > > > > > > > [Amit: Add comment, tweak summary] > > > > > > Logic also changed fron Alon's patch. Why? > > > > Yes, forgot to mention that here. > > > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > > > --- > > > > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > > > > 1 files changed, 21 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > > index e22940e..796224b 100644 > > > > --- a/hw/virtio-serial-bus.c > > > > +++ b/hw/virtio-serial-bus.c > > > > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > > > > memcpy(&config, config_data, sizeof(config)); > > > > } > > > > > > > > +static void set_status(VirtIODevice *vdev, uint8_t status) > > > > +{ > > > > + VirtIOSerial *vser; > > > > + VirtIOSerialPort *port; > > > > + > > > > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > > > > + port = find_port_by_id(vser, 0); > > > > + > > > > + if (port && !use_multiport(port->vser) > > > > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > > > > + /* > > > > + * Non-multiport guests won't be able to tell us guest > > > > + * open/close status. Such guests can only have a port at id > > > > + * 0, so set guest_connected for such ports as soon as guest > > > > + * is up. > > > > + */ > > > > + port->guest_connected = true; > > > > + } > > > > +} > > > > + > > > > > > Weird. Don't you want to set guest_connected = false when driver > > > is unloaded? > > > This is what Alon's patch did: > > > port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; > > > > Setting guest_connected to false when driver is unloaded is something > > that's not done before this patch (and not noted in the commit log > > too). > > > > And that case isn't specific to just port 0 (in the !multiport case); > > all ports will have to be updated for such a change. > > > > Is that something we should do? It's obviously correct to do that. > > Will it affect anything? I doubt it. But needs a separate patch and > > discussion for that change. > > > Let's not add code to preserve a bug so we can have a discussion. Let's > have a discussion right here and fix it properly. It can be added to the series, but has to be a different patch. But I don't think we should hold up this fix because we found other bugs. > BTW guest_connected is not cleared on reset either - a bug too? Yes, I think we'll just have to scan the list of things that we want zero'ed at start. <Better to use a non-zeroed memory alloc than zero'ed, we can at least be careful to set and reset values explicitly rather than invite such bugs.> Amit
On Tue, Apr 24, 2012 at 04:44:10PM +0530, Amit Shah wrote: > On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote: > > On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote: > > > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > > > > On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > > > > > From: Alon Levy <alevy@redhat.com> > > > > > > > > > > guest_connected should be false before guest driver initialization, and > > > > > true after, both for multiport aware and non multiport aware drivers. > > > > > > > > > > Don't set it before the guest_features are available; instead use > > > > > set_status which is called by io to VIRTIO_PCI_STATUS with > > > > > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > > > > > > > > > [Amit: Add comment, tweak summary] > > > > > > > > Logic also changed fron Alon's patch. Why? > > > > > > Yes, forgot to mention that here. > > > > > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > > > > --- > > > > > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > > > > > 1 files changed, 21 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > > > index e22940e..796224b 100644 > > > > > --- a/hw/virtio-serial-bus.c > > > > > +++ b/hw/virtio-serial-bus.c > > > > > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > > > > > memcpy(&config, config_data, sizeof(config)); > > > > > } > > > > > > > > > > +static void set_status(VirtIODevice *vdev, uint8_t status) > > > > > +{ > > > > > + VirtIOSerial *vser; > > > > > + VirtIOSerialPort *port; > > > > > + > > > > > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > > > > > + port = find_port_by_id(vser, 0); > > > > > + > > > > > + if (port && !use_multiport(port->vser) > > > > > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > > > > > + /* > > > > > + * Non-multiport guests won't be able to tell us guest > > > > > + * open/close status. Such guests can only have a port at id > > > > > + * 0, so set guest_connected for such ports as soon as guest > > > > > + * is up. > > > > > + */ > > > > > + port->guest_connected = true; > > > > > + } > > > > > +} > > > > > + > > > > > > > > Weird. Don't you want to set guest_connected = false when driver > > > > is unloaded? > > > > This is what Alon's patch did: > > > > port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; > > > > > > Setting guest_connected to false when driver is unloaded is something > > > that's not done before this patch (and not noted in the commit log > > > too). > > > > > > And that case isn't specific to just port 0 (in the !multiport case); > > > all ports will have to be updated for such a change. > > > > > > Is that something we should do? It's obviously correct to do that. > > > Will it affect anything? I doubt it. But needs a separate patch and > > > discussion for that change. > > > > > Let's not add code to preserve a bug so we can have a discussion. Let's > > have a discussion right here and fix it properly. > > It can be added to the series, but has to be a different patch. But I > don't think we should hold up this fix because we found other bugs. Sure. So fix it for single port now and for multiport in a separate patch. Alon's patch is a single line and it made sense, yours differs in that you have added more code to implement a bug. > > BTW guest_connected is not cleared on reset either - a bug too? > > Yes, I think we'll just have to scan the list of things that we want > zero'ed at start. Not at start, at reset. > <Better to use a non-zeroed memory alloc than zero'ed, we can at least > be careful to set and reset values explicitly rather than invite such > bugs.> > > Amit
Am 24.04.2012 12:26, schrieb Amit Shah: > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: >> On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: >>> From: Alon Levy <alevy@redhat.com> >>> >>> guest_connected should be false before guest driver initialization, and >>> true after, both for multiport aware and non multiport aware drivers. >>> >>> Don't set it before the guest_features are available; instead use >>> set_status which is called by io to VIRTIO_PCI_STATUS with >>> VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. >>> >>> [Amit: Add comment, tweak summary] >> >> Logic also changed fron Alon's patch. Why? > > Yes, forgot to mention that here. Re "here": FWIW I learnt to add these sort of comments before the new SoB, i.e. between mst's Acked-by and your Signed-off-by so that it is clear that he ack'ed the previous version given the logic change. In this case it's arguably irrelevant since there's only one SoB by you. Andreas >>> Signed-off-by: Alon Levy <alevy@redhat.com> >>> Acked-by: Michael S. Tsirkin <mst@redhat.com> >>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
On (Tue) 24 Apr 2012 [14:21:36], Michael S. Tsirkin wrote: > On Tue, Apr 24, 2012 at 04:44:10PM +0530, Amit Shah wrote: > > On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote: > > > On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote: > > > > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > > > > > On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > > > > > > From: Alon Levy <alevy@redhat.com> > > > > > > > > > > > > guest_connected should be false before guest driver initialization, and > > > > > > true after, both for multiport aware and non multiport aware drivers. > > > > > > > > > > > > Don't set it before the guest_features are available; instead use > > > > > > set_status which is called by io to VIRTIO_PCI_STATUS with > > > > > > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > > > > > > > > > > > > [Amit: Add comment, tweak summary] > > > > > > > > > > Logic also changed fron Alon's patch. Why? > > > > > > > > Yes, forgot to mention that here. > > > > > > > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > > > > > --- > > > > > > hw/virtio-serial-bus.c | 29 +++++++++++++++++++++-------- > > > > > > 1 files changed, 21 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > > > > index e22940e..796224b 100644 > > > > > > --- a/hw/virtio-serial-bus.c > > > > > > +++ b/hw/virtio-serial-bus.c > > > > > > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) > > > > > > memcpy(&config, config_data, sizeof(config)); > > > > > > } > > > > > > > > > > > > +static void set_status(VirtIODevice *vdev, uint8_t status) > > > > > > +{ > > > > > > + VirtIOSerial *vser; > > > > > > + VirtIOSerialPort *port; > > > > > > + > > > > > > + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); > > > > > > + port = find_port_by_id(vser, 0); > > > > > > + > > > > > > + if (port && !use_multiport(port->vser) > > > > > > + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { > > > > > > + /* > > > > > > + * Non-multiport guests won't be able to tell us guest > > > > > > + * open/close status. Such guests can only have a port at id > > > > > > + * 0, so set guest_connected for such ports as soon as guest > > > > > > + * is up. > > > > > > + */ > > > > > > + port->guest_connected = true; > > > > > > + } > > > > > > +} > > > > > > + > > > > > > > > > > Weird. Don't you want to set guest_connected = false when driver > > > > > is unloaded? > > > > > This is what Alon's patch did: > > > > > port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK; > > > > > > > > Setting guest_connected to false when driver is unloaded is something > > > > that's not done before this patch (and not noted in the commit log > > > > too). > > > > > > > > And that case isn't specific to just port 0 (in the !multiport case); > > > > all ports will have to be updated for such a change. > > > > > > > > Is that something we should do? It's obviously correct to do that. > > > > Will it affect anything? I doubt it. But needs a separate patch and > > > > discussion for that change. > > > > > > > Let's not add code to preserve a bug so we can have a discussion. Let's > > > have a discussion right here and fix it properly. > > > > It can be added to the series, but has to be a different patch. But I > > don't think we should hold up this fix because we found other bugs. > > Sure. So fix it for single port now and for multiport in a > separate patch. Alon's patch is a single line and it made sense, yours > differs in that you have added more code to implement a bug. Disagree; it was not obvious in that patch; a single patch did more thing than one, commit log didn't note it (it was a side-effect), and the code was not easy to read. With the patch I will propose, there won't be any differentiation for the multiport and not multiport case. And for virtio-serial, multiport is the more important use-case (i.e. all recent guests have multiport support). > > > BTW guest_connected is not cleared on reset either - a bug too? > > > > Yes, I think we'll just have to scan the list of things that we want > > zero'ed at start. > > Not at start, at reset. Similar case. Amit
On Tue, Apr 24, 2012 at 05:10:23PM +0530, Amit Shah wrote: > > Sure. So fix it for single port now and for multiport in a > > separate patch. Alon's patch is a single line and it made sense, yours > > differs in that you have added more code to implement a bug. > > Disagree; it was not obvious in that patch; a single patch did more > thing than one, commit log didn't note it (it was a side-effect), and > the code was not easy to read. > > With the patch I will propose, there won't be any differentiation for > the multiport and not multiport case. And for virtio-serial, > multiport is the more important use-case (i.e. all recent guests have > multiport support). OK, so let's see the complete fix. > > > > BTW guest_connected is not cleared on reset either - a bug too? > > > > > > Yes, I think we'll just have to scan the list of things that we want > > > zero'ed at start. > > > > Not at start, at reset. > > Similar case. > > Amit
On Tue, Apr 24, 2012 at 01:23:53PM +0200, Andreas Färber wrote: > Am 24.04.2012 12:26, schrieb Amit Shah: > > On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: > >> On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: > >>> From: Alon Levy <alevy@redhat.com> > >>> > >>> guest_connected should be false before guest driver initialization, and > >>> true after, both for multiport aware and non multiport aware drivers. > >>> > >>> Don't set it before the guest_features are available; instead use > >>> set_status which is called by io to VIRTIO_PCI_STATUS with > >>> VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. > >>> > >>> [Amit: Add comment, tweak summary] > >> > >> Logic also changed fron Alon's patch. Why? > > > > Yes, forgot to mention that here. > > Re "here": FWIW I learnt to add these sort of comments before the new > SoB, i.e. between mst's Acked-by and your Signed-off-by so that it is > clear that he ack'ed the previous version given the logic change. In > this case it's arguably irrelevant since there's only one SoB by you. Good idea. Note about the original patch - I didn't notice the side effect I didn't put in the commit log, and then I missed Amit's change of my code as well :) > > Andreas > > >>> Signed-off-by: Alon Levy <alevy@redhat.com> > >>> Acked-by: Michael S. Tsirkin <mst@redhat.com> > >>> Signed-off-by: Amit Shah <amit.shah@redhat.com> > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index e22940e..796224b 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) memcpy(&config, config_data, sizeof(config)); } +static void set_status(VirtIODevice *vdev, uint8_t status) +{ + VirtIOSerial *vser; + VirtIOSerialPort *port; + + vser = DO_UPCAST(VirtIOSerial, vdev, vdev); + port = find_port_by_id(vser, 0); + + if (port && !use_multiport(port->vser) + && (status & VIRTIO_CONFIG_S_DRIVER_OK)) { + /* + * Non-multiport guests won't be able to tell us guest + * open/close status. Such guests can only have a port at id + * 0, so set guest_connected for such ports as soon as guest + * is up. + */ + port->guest_connected = true; + } +} + static void virtio_serial_save(QEMUFile *f, void *opaque) { VirtIOSerial *s = opaque; @@ -798,14 +818,6 @@ static int virtser_port_qdev_init(DeviceState *qdev) return ret; } - if (!use_multiport(port->vser)) { - /* - * Allow writes to guest in this case; we have no way of - * knowing if a guest port is connected. - */ - port->guest_connected = true; - } - port->elem.out_num = 0; QTAILQ_INSERT_TAIL(&port->vser->ports, port, next); @@ -905,6 +917,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf) vser->vdev.get_features = get_features; vser->vdev.get_config = get_config; vser->vdev.set_config = set_config; + vser->vdev.set_status = set_status; vser->qdev = dev;