[PATCH v2 1/3] xen: fix usage of ENODATA

[PATCH v2 1/3] xen: fix usage of ENODATA

[Roger Pau Monne]

ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
hypervisor are translated to ENOENT.

Also, the error code is returned in errno if the call returns -1, so
compare the error code with the value in errno instead of the value
returned by the function.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
---
Changes since v1:
 - Define ENODATA to ENOENT for platforms that don't have ENODATA.
---
 xen-hvm.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen-hvm.c b/xen-hvm.c
index a64486c..a414105 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
                                  start_addr >> TARGET_PAGE_BITS, npages,
                                  bitmap);
     if (rc < 0) {
-        if (rc != -ENODATA) {
+#ifndef ENODATA
+#define ENODATA  ENOENT
+#endif
+        if (errno == ENODATA) {
             memory_region_set_dirty(framebuffer, 0, size);
             DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
                     ", 0x" TARGET_FMT_plx "): %s\n",
-                    start_addr, start_addr + size, strerror(-rc));
+                    start_addr, start_addr + size, strerror(errno));
         }
         return;
     }
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Roger Pau Monne]

The current behaviour of tap_open for BSD systems differ greatly from
it's Linux counterpart. Since FreeBSD supports interface renaming and
tap device cloning by opening /dev/tap, implement a FreeBSD specific
version of tap_open that behaves like it's Linux counterpart.

This is specially important for toolstacks that use Qemu (like Xen
libxl), in order to have a unified behaviour across suported
platforms.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 90f8a02..bf91bd0 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -27,12 +27,13 @@
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
 
-#ifdef __NetBSD__
+#if defined(__NetBSD__) || defined(__FreeBSD__)
 #include <sys/ioctl.h>
 #include <net/if.h>
 #include <net/if_tap.h>
 #endif
 
+#ifndef __FreeBSD__
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
              int vnet_hdr_required, int mq_required)
 {
@@ -108,6 +109,73 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
     return fd;
 }
 
+#else /* __FreeBSD__ */
+
+#define PATH_NET_TAP "/dev/tap"
+
+int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
+             int vnet_hdr_required, int mq_required)
+{
+    int fd, s, ret;
+    struct ifreq ifr;
+
+    TFR(fd = open(PATH_NET_TAP, O_RDWR));
+    if (fd < 0) {
+        error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+        return -1;
+    }
+
+    memset(&ifr, 0, sizeof(ifr));
+
+    ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
+    if (ret < 0) {
+        error_report("could not get tap interface name");
+        goto error;
+    }
+
+    if (ifname[0] != '\0') {
+        /* User requested the interface to have a specific name */
+        s = socket(AF_LOCAL, SOCK_DGRAM, 0);
+        if (s < 0) {
+            error_report("could not open socket to set interface name");
+            goto error;
+        }
+        ifr.ifr_data = ifname;
+        ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
+        close(s);
+        if (ret < 0) {
+            error_report("could not set tap interface name");
+            goto error;
+        }
+    } else {
+        pstrcpy(ifname, ifname_size, ifr.ifr_name);
+    }
+
+    if (*vnet_hdr) {
+        /* BSD doesn't have IFF_VNET_HDR */
+        *vnet_hdr = 0;
+
+        if (vnet_hdr_required && !*vnet_hdr) {
+            error_report("vnet_hdr=1 requested, but no kernel "
+                         "support for IFF_VNET_HDR available");
+            goto error;
+        }
+    }
+    if (mq_required) {
+        error_report("mq_required requested, but not kernel support"
+                     "for IFF_MULTI_QUEUE available");
+        goto error;
+    }
+
+    fcntl(fd, F_SETFL, O_NONBLOCK);
+    return fd;
+
+error:
+    close(fd);
+    return -1;
+}
+#endif /* __FreeBSD__ */
+
 int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
 {
     return 0;
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Stefan Hajnoczi]

On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
> The current behaviour of tap_open for BSD systems differ greatly from
> it's Linux counterpart. Since FreeBSD supports interface renaming and
> tap device cloning by opening /dev/tap, implement a FreeBSD specific
> version of tap_open that behaves like it's Linux counterpart.
> 
> This is specially important for toolstacks that use Qemu (like Xen
> libxl), in order to have a unified behaviour across suported
> platforms.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 69 insertions(+), 1 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Roger Pau Monné]

On 27/05/14 15:29, Stefan Hajnoczi wrote:
> On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
>> The current behaviour of tap_open for BSD systems differ greatly from
>> it's Linux counterpart. Since FreeBSD supports interface renaming and
>> tap device cloning by opening /dev/tap, implement a FreeBSD specific
>> version of tap_open that behaves like it's Linux counterpart.
>>
>> This is specially important for toolstacks that use Qemu (like Xen
>> libxl), in order to have a unified behaviour across suported
>> platforms.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: xen-devel@lists.xenproject.org
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Anthony Liguori <aliguori@us.ibm.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 69 insertions(+), 1 deletions(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

I still don't see this committed to the repository, should I ping someone?

Thanks, Roger.

Re: [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

On Tue, 22 Jul 2014, Roger Pau Monné wrote:
> On 27/05/14 15:29, Stefan Hajnoczi wrote:
> > On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
> >> The current behaviour of tap_open for BSD systems differ greatly from
> >> it's Linux counterpart. Since FreeBSD supports interface renaming and
> >> tap device cloning by opening /dev/tap, implement a FreeBSD specific
> >> version of tap_open that behaves like it's Linux counterpart.
> >>
> >> This is specially important for toolstacks that use Qemu (like Xen
> >> libxl), in order to have a unified behaviour across suported
> >> platforms.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> Cc: xen-devel@lists.xenproject.org
> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> Cc: Anthony Liguori <aliguori@us.ibm.com>
> >> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> >> ---
> >>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 files changed, 69 insertions(+), 1 deletions(-)
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> I still don't see this committed to the repository, should I ping someone?

I was assuming that this patch would go via some other tree.  But if
Stefan is OK I could pick it up and submit a pull request for both patch
1 and 2 of this series.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 1628 bytes --]

On Tue, Jul 22, 2014 at 01:26:00PM +0100, Stefano Stabellini wrote:
> On Tue, 22 Jul 2014, Roger Pau Monné wrote:
> > On 27/05/14 15:29, Stefan Hajnoczi wrote:
> > > On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
> > >> The current behaviour of tap_open for BSD systems differ greatly from
> > >> it's Linux counterpart. Since FreeBSD supports interface renaming and
> > >> tap device cloning by opening /dev/tap, implement a FreeBSD specific
> > >> version of tap_open that behaves like it's Linux counterpart.
> > >>
> > >> This is specially important for toolstacks that use Qemu (like Xen
> > >> libxl), in order to have a unified behaviour across suported
> > >> platforms.
> > >>
> > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > >> Cc: xen-devel@lists.xenproject.org
> > >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >> Cc: Anthony Liguori <aliguori@us.ibm.com>
> > >> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > >> ---
> > >>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >>  1 files changed, 69 insertions(+), 1 deletions(-)
> > > 
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > I still don't see this committed to the repository, should I ping someone?
> 
> I was assuming that this patch would go via some other tree.  But if
> Stefan is OK I could pick it up and submit a pull request for both patch
> 1 and 2 of this series.

I'm fine with that.  I only reviewed this patch since it affects the net
subsystem and left the series as a whole for someone to merge.

Stefan

[-- Attachment #1.2: Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Roger Pau Monné]

On 22/07/14 14:26, Stefano Stabellini wrote:
> On Tue, 22 Jul 2014, Roger Pau Monné wrote:
>> On 27/05/14 15:29, Stefan Hajnoczi wrote:
>>> On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
>>>> The current behaviour of tap_open for BSD systems differ greatly from
>>>> it's Linux counterpart. Since FreeBSD supports interface renaming and
>>>> tap device cloning by opening /dev/tap, implement a FreeBSD specific
>>>> version of tap_open that behaves like it's Linux counterpart.
>>>>
>>>> This is specially important for toolstacks that use Qemu (like Xen
>>>> libxl), in order to have a unified behaviour across suported
>>>> platforms.
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> Cc: xen-devel@lists.xenproject.org
>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>> Cc: Anthony Liguori <aliguori@us.ibm.com>
>>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>>> ---
>>>>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>  1 files changed, 69 insertions(+), 1 deletions(-)
>>>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>
>> I still don't see this committed to the repository, should I ping someone?
> 
> I was assuming that this patch would go via some other tree.  But if
> Stefan is OK I could pick it up and submit a pull request for both patch
> 1 and 2 of this series.

Would you do the backport of those three patches (one is already
committed as e02bc6) to the qemu-xen repo at the same time, or would you
like me to remind you about this in a month or so?

I would really like to have all this patches in Xen 4.5 if possible.

Thanks, Roger.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/3] tap-bsd: implement a FreeBSD only version of tap_open

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On Wed, 23 Jul 2014, Roger Pau Monné wrote:
> On 22/07/14 14:26, Stefano Stabellini wrote:
> > On Tue, 22 Jul 2014, Roger Pau Monné wrote:
> >> On 27/05/14 15:29, Stefan Hajnoczi wrote:
> >>> On Fri, May 23, 2014 at 05:57:48PM +0200, Roger Pau Monne wrote:
> >>>> The current behaviour of tap_open for BSD systems differ greatly from
> >>>> it's Linux counterpart. Since FreeBSD supports interface renaming and
> >>>> tap device cloning by opening /dev/tap, implement a FreeBSD specific
> >>>> version of tap_open that behaves like it's Linux counterpart.
> >>>>
> >>>> This is specially important for toolstacks that use Qemu (like Xen
> >>>> libxl), in order to have a unified behaviour across suported
> >>>> platforms.
> >>>>
> >>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >>>> Cc: xen-devel@lists.xenproject.org
> >>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>> Cc: Anthony Liguori <aliguori@us.ibm.com>
> >>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> >>>> ---
> >>>>  net/tap-bsd.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>>  1 files changed, 69 insertions(+), 1 deletions(-)
> >>>
> >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>
> >> I still don't see this committed to the repository, should I ping someone?
> > 
> > I was assuming that this patch would go via some other tree.  But if
> > Stefan is OK I could pick it up and submit a pull request for both patch
> > 1 and 2 of this series.
> 
> Would you do the backport of those three patches (one is already
> committed as e02bc6) to the qemu-xen repo at the same time, or would you
> like me to remind you about this in a month or so?
> 
> I would really like to have all this patches in Xen 4.5 if possible.

I should remember when I'll send a pull request (when 2.1 will be
out). But please remind me if I'll forget.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v2 3/3] serial: poll the serial console with G_IO_HUP

[Roger Pau Monne]

On FreeBSD polling a master pty while the other end is not connected
with G_IO_OUT only results in an endless wait. This is different from
the Linux behaviour, that returns immediately. In order to demonstrate
this, I have the following example code:

http://xenbits.xen.org/people/royger/test_poll.c

When executed on Linux:

$ ./test_poll
In callback

On FreeBSD instead, the callback never gets called:

$ ./test_poll

So, in order to workaround this, poll the source with G_IO_HUP (which
makes the code behave the same way on both Linux and FreeBSD).

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: xen-devel@lists.xenproject.org
---
Changes since v1:
 - Fix other users of qemu_chr_fe_add_watch to use G_IO_HUP.
---
 hw/char/serial.c         |    2 +-
 hw/char/virtio-console.c |    3 ++-
 hw/usb/redirect.c        |    2 +-
 monitor.c                |    2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index f4d167f..2a2c9e5 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -246,7 +246,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
         serial_receive1(s, &s->tsr, 1);
     } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
         if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
-            qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
+            qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) {
             s->tsr_retry++;
             return FALSE;
         }
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6c8be0f..38e290a 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -69,7 +69,8 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
         if (!k->is_console) {
             virtio_serial_throttle_port(port, true);
             if (!vcon->watch) {
-                vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
+                vcon->watch = qemu_chr_fe_add_watch(vcon->chr,
+                                                    G_IO_OUT|G_IO_HUP,
                                                     chr_write_unblocked, vcon);
             }
         }
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 287a505..06e757d 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -284,7 +284,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
     r = qemu_chr_fe_write(dev->cs, data, count);
     if (r < count) {
         if (!dev->watch) {
-            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
+            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT|G_IO_HUP,
                                                usbredir_write_unblocked, dev);
         }
         if (r < 0) {
diff --git a/monitor.c b/monitor.c
index 593679a..ae1c539 100644
--- a/monitor.c
+++ b/monitor.c
@@ -304,7 +304,7 @@ void monitor_flush(Monitor *mon)
             mon->outbuf = tmp;
         }
         if (mon->watch == 0) {
-            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
+            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT|G_IO_HUP,
                                                monitor_unblocked, mon);
         }
     }
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 1/3] xen: fix usage of ENODATA

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]

On Fri, 23 May 2014, Roger Pau Monne wrote:
> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
> hypervisor are translated to ENOENT.
> 
> Also, the error code is returned in errno if the call returns -1, so
> compare the error code with the value in errno instead of the value
> returned by the function.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> ---
> Changes since v1:
>  - Define ENODATA to ENOENT for platforms that don't have ENODATA.
> ---
>  xen-hvm.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/xen-hvm.c b/xen-hvm.c
> index a64486c..a414105 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>                                   start_addr >> TARGET_PAGE_BITS, npages,
>                                   bitmap);
>      if (rc < 0) {
> -        if (rc != -ENODATA) {
> +#ifndef ENODATA
> +#define ENODATA  ENOENT
> +#endif

I wonder if it makes sense to have this in a more generic header, lile
include/qemu/osdep.h?

> +        if (errno == ENODATA) {
>              memory_region_set_dirty(framebuffer, 0, size);
>              DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
>                      ", 0x" TARGET_FMT_plx "): %s\n",
> -                    start_addr, start_addr + size, strerror(-rc));
> +                    start_addr, start_addr + size, strerror(errno));
>          }
>          return;
>      }
> -- 
> 1.7.7.5 (Apple Git-26)
> 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 1/3] xen: fix usage of ENODATA

[Roger Pau Monné]

On 27/05/14 18:18, Stefano Stabellini wrote:
> On Fri, 23 May 2014, Roger Pau Monne wrote:
>> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
>> hypervisor are translated to ENOENT.
>>
>> Also, the error code is returned in errno if the call returns -1, so
>> compare the error code with the value in errno instead of the value
>> returned by the function.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: xen-devel@lists.xenproject.org
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> ---
>> Changes since v1:
>>  - Define ENODATA to ENOENT for platforms that don't have ENODATA.
>> ---
>>  xen-hvm.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen-hvm.c b/xen-hvm.c
>> index a64486c..a414105 100644
>> --- a/xen-hvm.c
>> +++ b/xen-hvm.c
>> @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>>                                   start_addr >> TARGET_PAGE_BITS, npages,
>>                                   bitmap);
>>      if (rc < 0) {
>> -        if (rc != -ENODATA) {
>> +#ifndef ENODATA
>> +#define ENODATA  ENOENT
>> +#endif
> 
> I wonder if it makes sense to have this in a more generic header, lile
> include/qemu/osdep.h?

Well, I think this is something really specific to Xen, because FreeBSD
is mapping the error returned by Xen (ENODATA), to a error code
available on FreeBSD (ENOENT), see:

http://xenbits.xen.org/gitweb/?p=people/royger/freebsd.git;a=patch;h=f09eeed01bc884b37e978f6ec6e3e7a86778ef4b

But I don't think this is useful outside of this context, since there's
no syscall that would return ENODATA on FreeBSD that instead returns
ENOENT. All the paths in Qemu that check for ENODATA are Linux specific
AFAICT.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v2 1/3] xen: fix usage of ENODATA

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]

On Tue, 27 May 2014, Roger Pau Monné wrote:
> On 27/05/14 18:18, Stefano Stabellini wrote:
> > On Fri, 23 May 2014, Roger Pau Monne wrote:
> >> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
> >> hypervisor are translated to ENOENT.
> >>
> >> Also, the error code is returned in errno if the call returns -1, so
> >> compare the error code with the value in errno instead of the value
> >> returned by the function.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> Cc: xen-devel@lists.xenproject.org
> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> Cc: Anthony Perard <anthony.perard@citrix.com>
> >> ---
> >> Changes since v1:
> >>  - Define ENODATA to ENOENT for platforms that don't have ENODATA.
> >> ---
> >>  xen-hvm.c |    7 +++++--
> >>  1 files changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/xen-hvm.c b/xen-hvm.c
> >> index a64486c..a414105 100644
> >> --- a/xen-hvm.c
> >> +++ b/xen-hvm.c
> >> @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
> >>                                   start_addr >> TARGET_PAGE_BITS, npages,
> >>                                   bitmap);
> >>      if (rc < 0) {
> >> -        if (rc != -ENODATA) {
> >> +#ifndef ENODATA
> >> +#define ENODATA  ENOENT
> >> +#endif
> > 
> > I wonder if it makes sense to have this in a more generic header, lile
> > include/qemu/osdep.h?
> 
> Well, I think this is something really specific to Xen, because FreeBSD
> is mapping the error returned by Xen (ENODATA), to a error code
> available on FreeBSD (ENOENT), see:
> 
> http://xenbits.xen.org/gitweb/?p=people/royger/freebsd.git;a=patch;h=f09eeed01bc884b37e978f6ec6e3e7a86778ef4b
> 
> But I don't think this is useful outside of this context, since there's
> no syscall that would return ENODATA on FreeBSD that instead returns
> ENOENT. All the paths in Qemu that check for ENODATA are Linux specific
> AFAICT.

Fair enough. In that case the patch is fine as it is.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 3/3] serial: poll the serial console with G_IO_HUP

[Roger Pau Monné]

Ping?

On 23/05/14 17:57, Roger Pau Monne wrote:
> On FreeBSD polling a master pty while the other end is not connected
> with G_IO_OUT only results in an endless wait. This is different from
> the Linux behaviour, that returns immediately. In order to demonstrate
> this, I have the following example code:
> 
> http://xenbits.xen.org/people/royger/test_poll.c
> 
> When executed on Linux:
> 
> $ ./test_poll
> In callback
> 
> On FreeBSD instead, the callback never gets called:
> 
> $ ./test_poll
> 
> So, in order to workaround this, poll the source with G_IO_HUP (which
> makes the code behave the same way on both Linux and FreeBSD).
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: xen-devel@lists.xenproject.org
> ---
> Changes since v1:
>  - Fix other users of qemu_chr_fe_add_watch to use G_IO_HUP.
> ---
>  hw/char/serial.c         |    2 +-
>  hw/char/virtio-console.c |    3 ++-
>  hw/usb/redirect.c        |    2 +-
>  monitor.c                |    2 +-
>  4 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index f4d167f..2a2c9e5 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -246,7 +246,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
>          serial_receive1(s, &s->tsr, 1);
>      } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
>          if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
> -            qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
> +            qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) {
>              s->tsr_retry++;
>              return FALSE;
>          }
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 6c8be0f..38e290a 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -69,7 +69,8 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
>          if (!k->is_console) {
>              virtio_serial_throttle_port(port, true);
>              if (!vcon->watch) {
> -                vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
> +                vcon->watch = qemu_chr_fe_add_watch(vcon->chr,
> +                                                    G_IO_OUT|G_IO_HUP,
>                                                      chr_write_unblocked, vcon);
>              }
>          }
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 287a505..06e757d 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -284,7 +284,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
>      r = qemu_chr_fe_write(dev->cs, data, count);
>      if (r < count) {
>          if (!dev->watch) {
> -            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
> +            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT|G_IO_HUP,
>                                                 usbredir_write_unblocked, dev);
>          }
>          if (r < 0) {
> diff --git a/monitor.c b/monitor.c
> index 593679a..ae1c539 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -304,7 +304,7 @@ void monitor_flush(Monitor *mon)
>              mon->outbuf = tmp;
>          }
>          if (mon->watch == 0) {
> -            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
> +            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT|G_IO_HUP,
>                                                 monitor_unblocked, mon);
>          }
>      }
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v2 3/3] serial: poll the serial console with G_IO_HUP

[Roger Pau Monné]

Do I need to resend this? it's been more than a month without review.

Roger.

On 13/06/14 17:35, Roger Pau Monné wrote:
> Ping?
> 
> On 23/05/14 17:57, Roger Pau Monne wrote:
>> On FreeBSD polling a master pty while the other end is not connected
>> with G_IO_OUT only results in an endless wait. This is different from
>> the Linux behaviour, that returns immediately. In order to demonstrate
>> this, I have the following example code:
>>
>> http://xenbits.xen.org/people/royger/test_poll.c
>>
>> When executed on Linux:
>>
>> $ ./test_poll
>> In callback
>>
>> On FreeBSD instead, the callback never gets called:
>>
>> $ ./test_poll
>>
>> So, in order to workaround this, poll the source with G_IO_HUP (which
>> makes the code behave the same way on both Linux and FreeBSD).
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>> Cc: "Andreas Färber" <afaerber@suse.de>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: xen-devel@lists.xenproject.org
>> ---
>> Changes since v1:
>>  - Fix other users of qemu_chr_fe_add_watch to use G_IO_HUP.
>> ---
>>  hw/char/serial.c         |    2 +-
>>  hw/char/virtio-console.c |    3 ++-
>>  hw/usb/redirect.c        |    2 +-
>>  monitor.c                |    2 +-
>>  4 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/char/serial.c b/hw/char/serial.c
>> index f4d167f..2a2c9e5 100644
>> --- a/hw/char/serial.c
>> +++ b/hw/char/serial.c
>> @@ -246,7 +246,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
>>          serial_receive1(s, &s->tsr, 1);
>>      } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
>>          if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
>> -            qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
>> +            qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) {
>>              s->tsr_retry++;
>>              return FALSE;
>>          }
>> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
>> index 6c8be0f..38e290a 100644
>> --- a/hw/char/virtio-console.c
>> +++ b/hw/char/virtio-console.c
>> @@ -69,7 +69,8 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
>>          if (!k->is_console) {
>>              virtio_serial_throttle_port(port, true);
>>              if (!vcon->watch) {
>> -                vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
>> +                vcon->watch = qemu_chr_fe_add_watch(vcon->chr,
>> +                                                    G_IO_OUT|G_IO_HUP,
>>                                                      chr_write_unblocked, vcon);
>>              }
>>          }
>> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
>> index 287a505..06e757d 100644
>> --- a/hw/usb/redirect.c
>> +++ b/hw/usb/redirect.c
>> @@ -284,7 +284,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
>>      r = qemu_chr_fe_write(dev->cs, data, count);
>>      if (r < count) {
>>          if (!dev->watch) {
>> -            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
>> +            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT|G_IO_HUP,
>>                                                 usbredir_write_unblocked, dev);
>>          }
>>          if (r < 0) {
>> diff --git a/monitor.c b/monitor.c
>> index 593679a..ae1c539 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -304,7 +304,7 @@ void monitor_flush(Monitor *mon)
>>              mon->outbuf = tmp;
>>          }
>>          if (mon->watch == 0) {
>> -            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
>> +            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT|G_IO_HUP,
>>                                                 monitor_unblocked, mon);
>>          }
>>      }
>>
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v2 3/3] serial: poll the serial console with G_IO_HUP

[Paolo Bonzini]

Il 30/06/2014 13:00, Roger Pau Monné ha scritto:
> Do I need to resend this? it's been more than a month without review.

I'll send a pull request for it.

Sorry for the delay.

Paolo

> On 13/06/14 17:35, Roger Pau Monné wrote:
>> Ping?
>>
>> On 23/05/14 17:57, Roger Pau Monne wrote:
>>> On FreeBSD polling a master pty while the other end is not connected
>>> with G_IO_OUT only results in an endless wait. This is different from
>>> the Linux behaviour, that returns immediately. In order to demonstrate
>>> this, I have the following example code:
>>>
>>> http://xenbits.xen.org/people/royger/test_poll.c
>>>
>>> When executed on Linux:
>>>
>>> $ ./test_poll
>>> In callback
>>>
>>> On FreeBSD instead, the callback never gets called:
>>>
>>> $ ./test_poll
>>>
>>> So, in order to workaround this, poll the source with G_IO_HUP (which
>>> makes the code behave the same way on both Linux and FreeBSD).
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>>> Cc: "Andreas Färber" <afaerber@suse.de>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: xen-devel@lists.xenproject.org
>>> ---
>>> Changes since v1:
>>>  - Fix other users of qemu_chr_fe_add_watch to use G_IO_HUP.
>>> ---
>>>  hw/char/serial.c         |    2 +-
>>>  hw/char/virtio-console.c |    3 ++-
>>>  hw/usb/redirect.c        |    2 +-
>>>  monitor.c                |    2 +-
>>>  4 files changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/char/serial.c b/hw/char/serial.c
>>> index f4d167f..2a2c9e5 100644
>>> --- a/hw/char/serial.c
>>> +++ b/hw/char/serial.c
>>> @@ -246,7 +246,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
>>>          serial_receive1(s, &s->tsr, 1);
>>>      } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
>>>          if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
>>> -            qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
>>> +            qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) {
>>>              s->tsr_retry++;
>>>              return FALSE;
>>>          }
>>> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
>>> index 6c8be0f..38e290a 100644
>>> --- a/hw/char/virtio-console.c
>>> +++ b/hw/char/virtio-console.c
>>> @@ -69,7 +69,8 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
>>>          if (!k->is_console) {
>>>              virtio_serial_throttle_port(port, true);
>>>              if (!vcon->watch) {
>>> -                vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
>>> +                vcon->watch = qemu_chr_fe_add_watch(vcon->chr,
>>> +                                                    G_IO_OUT|G_IO_HUP,
>>>                                                      chr_write_unblocked, vcon);
>>>              }
>>>          }
>>> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
>>> index 287a505..06e757d 100644
>>> --- a/hw/usb/redirect.c
>>> +++ b/hw/usb/redirect.c
>>> @@ -284,7 +284,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
>>>      r = qemu_chr_fe_write(dev->cs, data, count);
>>>      if (r < count) {
>>>          if (!dev->watch) {
>>> -            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
>>> +            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT|G_IO_HUP,
>>>                                                 usbredir_write_unblocked, dev);
>>>          }
>>>          if (r < 0) {
>>> diff --git a/monitor.c b/monitor.c
>>> index 593679a..ae1c539 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -304,7 +304,7 @@ void monitor_flush(Monitor *mon)
>>>              mon->outbuf = tmp;
>>>          }
>>>          if (mon->watch == 0) {
>>> -            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
>>> +            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT|G_IO_HUP,
>>>                                                 monitor_unblocked, mon);
>>>          }
>>>      }
>>>
>>
>>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel