[Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Amit Shah
From: Alon Levy al...@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]

Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@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;
+}
+}
+
 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




Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread 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 al...@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 al...@redhat.com
  Acked-by: Michael S. Tsirkin m...@redhat.com
  Signed-off-by: Amit Shah amit.s...@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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Michael S. Tsirkin
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 al...@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 al...@redhat.com
   Acked-by: Michael S. Tsirkin m...@redhat.com
   Signed-off-by: Amit Shah amit.s...@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?

-- 
MST



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Amit Shah
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 al...@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 al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Michael S. Tsirkin
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 al...@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 al...@redhat.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Amit Shah amit.s...@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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Andreas Färber
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 al...@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 al...@redhat.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Amit Shah amit.s...@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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Amit Shah
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 al...@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 al...@redhat.com
  Acked-by: Michael S. Tsirkin m...@redhat.com
  Signed-off-by: Amit Shah amit.s...@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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Alon Levy
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 al...@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 al...@redhat.com
  Acked-by: Michael S. Tsirkin m...@redhat.com
  Signed-off-by: Amit Shah amit.s...@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
 



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Michael S. Tsirkin
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



Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

2012-04-24 Thread Michael S. Tsirkin
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
 From: Alon Levy al...@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 al...@redhat.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Amit Shah amit.s...@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