[PATCH] add a qemu-ifup script on NetBSD

[PATCH] add a qemu-ifup script on NetBSD

[Manuel Bouyer]

On NetBSD, qemu-xen will use a qemu-ifup script to setup the tap interfaces
(as qemu-xen-traditional used to). Copy the script from qemu-xen-traditional,
and install it on NetBSD. While there document parameters and environnement
variables.

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/hotplug/NetBSD/Makefile  | 1 +
 tools/hotplug/NetBSD/qemu-ifup | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 tools/hotplug/NetBSD/qemu-ifup

diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
index 114b223207..f909ffa367 100644
--- a/tools/hotplug/NetBSD/Makefile
+++ b/tools/hotplug/NetBSD/Makefile
@@ -7,6 +7,7 @@ XEN_SCRIPTS += locking.sh
 XEN_SCRIPTS += block
 XEN_SCRIPTS += vif-bridge
 XEN_SCRIPTS += vif-ip
+XEN_SCRIPTS += qemu-ifup
 
 XEN_SCRIPT_DATA =
 XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
diff --git a/tools/hotplug/NetBSD/qemu-ifup b/tools/hotplug/NetBSD/qemu-ifup
new file mode 100644
index 0000000000..4305419f44
--- /dev/null
+++ b/tools/hotplug/NetBSD/qemu-ifup
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+#called by qemu when a HVM domU is started.
+# first parameter is tap interface, second is the bridge name
+# environement variable $XEN_DOMAIN_ID contains the domU's ID,
+# which can be used to retrieve extra parameters from the xenstore.
+
+ifconfig $1 up
+exec /sbin/brconfig $2 add $1
-- 
2.29.2

[PATCH] xenstored: close socket connections on error

[Manuel Bouyer]

On error, don't keep socket connection in ignored state but close them.
When the remote end of a socket is closed, xenstored will flag it as an
error and switch the connection to ignored. But on some OSes (e.g.
NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
state will stay open forever in xenstored (and it will loop with CPU 100%
busy).

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083
---
 tools/xenstore/xenstored_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 1ab6f162cb..0fea598352 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1440,6 +1440,9 @@ static void ignore_connection(struct connection *conn)
 
 	talloc_free(conn->in);
 	conn->in = NULL;
+	/* if this is a socket connection, drop it now */
+	if (conn->fd >= 0)
+		talloc_free(conn);
 }
 
 static const char *sockmsg_string(enum xsd_sockmsg_type type)
-- 
2.29.2

[PATCH v3] Document qemu-ifup on NetBSD

[Manuel Bouyer]

Document that on NetBSD, the tap interface will be configured by the
qemu-ifup script.

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
---
 docs/man/xl-network-configuration.5.pod | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/man/xl-network-configuration.5.pod b/docs/man/xl-network-configuration.5.pod
index af058d4d3c..8e5fd909fa 100644
--- a/docs/man/xl-network-configuration.5.pod
+++ b/docs/man/xl-network-configuration.5.pod
@@ -172,6 +172,9 @@ add it to the relevant bridge). Defaults to
 C<XEN_SCRIPT_DIR/vif-bridge> but can be set to any script. Some example
 scripts are installed in C<XEN_SCRIPT_DIR>.
 
+Note on NetBSD HVM guests will ignore the script option for tap
+(emulated) interfaces and always use
+C<XEN_SCRIPT_DIR/qemu-ifup> to configure the interface in bridged mode.
 
 =head2 ip
 
-- 
2.29.2

[PATCH v3] NetBSD: use system-provided headers

[Manuel Bouyer]

On NetBSD use the system-provided headers for ioctl and related definitions,
they are up to date and have more chances to match the kernel's idea of
the ioctls and structures.
Remove now-unused NetBSD/evtchn.h and NetBSD/privcmd.h.
Don't fail install if xen/sys/*.h are not present.

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/debugger/gdbsx/xg/xg_main.c      |   4 +
 tools/include/Makefile                 |   2 +
 tools/include/xen-sys/NetBSD/evtchn.h  |  86 --------------------
 tools/include/xen-sys/NetBSD/privcmd.h | 106 -------------------------
 tools/libs/call/private.h              |   4 +
 tools/libs/ctrl/xc_private.h           |   4 +
 tools/libs/foreignmemory/private.h     |   6 ++
 7 files changed, 20 insertions(+), 192 deletions(-)
 delete mode 100644 tools/include/xen-sys/NetBSD/evtchn.h
 delete mode 100644 tools/include/xen-sys/NetBSD/privcmd.h

diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
index 4576c762af..903d60baed 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -49,7 +49,11 @@
 #include "xg_public.h"
 #include <xen/version.h>
 #include <xen/domctl.h>
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 #include <xen/foreign/x86_32.h>
 #include <xen/foreign/x86_64.h>
 
diff --git a/tools/include/Makefile b/tools/include/Makefile
index 4d4ec5f974..65fb67a771 100644
--- a/tools/include/Makefile
+++ b/tools/include/Makefile
@@ -68,7 +68,9 @@ install: all
 	$(INSTALL_DATA) xen/foreign/*.h $(DESTDIR)$(includedir)/xen/foreign
 	$(INSTALL_DATA) xen/hvm/*.h $(DESTDIR)$(includedir)/xen/hvm
 	$(INSTALL_DATA) xen/io/*.h $(DESTDIR)$(includedir)/xen/io
+ifneq ($(wildcard xen/sys/*.h),)
 	$(INSTALL_DATA) xen/sys/*.h $(DESTDIR)$(includedir)/xen/sys
+endif
 	$(INSTALL_DATA) xen/xsm/*.h $(DESTDIR)$(includedir)/xen/xsm
 
 .PHONY: uninstall
diff --git a/tools/include/xen-sys/NetBSD/evtchn.h b/tools/include/xen-sys/NetBSD/evtchn.h
deleted file mode 100644
index 2d8a1f9164..0000000000
--- a/tools/include/xen-sys/NetBSD/evtchn.h
+++ /dev/null
@@ -1,86 +0,0 @@
-/* $NetBSD: evtchn.h,v 1.1.1.1 2007/06/14 19:39:45 bouyer Exp $ */
-/******************************************************************************
- * evtchn.h
- * 
- * Interface to /dev/xen/evtchn.
- * 
- * Copyright (c) 2003-2005, K A Fraser
- * 
- * This file may be distributed separately from the Linux kernel, or
- * incorporated into other software packages, subject to the following license:
- * 
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this source file (the "Software"), to deal in the Software without
- * restriction, including without limitation the rights to use, copy, modify,
- * merge, publish, distribute, sublicense, and/or sell copies of the Software,
- * and to permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- * 
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- * 
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#ifndef __NetBSD_EVTCHN_H__
-#define __NetBSD_EVTCHN_H__
-
-/*
- * Bind a fresh port to VIRQ @virq.
- */
-#define IOCTL_EVTCHN_BIND_VIRQ				\
-	_IOWR('E', 4, struct ioctl_evtchn_bind_virq)
-struct ioctl_evtchn_bind_virq {
-	unsigned int virq;
-	unsigned int port;
-};
-
-/*
- * Bind a fresh port to remote <@remote_domain, @remote_port>.
- */
-#define IOCTL_EVTCHN_BIND_INTERDOMAIN			\
-	_IOWR('E', 5, struct ioctl_evtchn_bind_interdomain)
-struct ioctl_evtchn_bind_interdomain {
-	unsigned int remote_domain, remote_port;
-	unsigned int port;
-};
-
-/*
- * Allocate a fresh port for binding to @remote_domain.
- */
-#define IOCTL_EVTCHN_BIND_UNBOUND_PORT			\
-	_IOWR('E', 6, struct ioctl_evtchn_bind_unbound_port)
-struct ioctl_evtchn_bind_unbound_port {
-	unsigned int remote_domain;
-	unsigned int port;
-};
-
-/*
- * Unbind previously allocated @port.
- */
-#define IOCTL_EVTCHN_UNBIND				\
-	_IOW('E', 7, struct ioctl_evtchn_unbind)
-struct ioctl_evtchn_unbind {
-	unsigned int port;
-};
-
-/*
- * Send event to previously allocated @port.
- */
-#define IOCTL_EVTCHN_NOTIFY				\
-	_IOW('E', 8, struct ioctl_evtchn_notify)
-struct ioctl_evtchn_notify {
-	unsigned int port;
-};
-
-/* Clear and reinitialise the event buffer. Clear error condition. */
-#define IOCTL_EVTCHN_RESET				\
-	_IO('E', 9)
-
-#endif /* __NetBSD_EVTCHN_H__ */
diff --git a/tools/include/xen-sys/NetBSD/privcmd.h b/tools/include/xen-sys/NetBSD/privcmd.h
deleted file mode 100644
index 555bad973e..0000000000
--- a/tools/include/xen-sys/NetBSD/privcmd.h
+++ /dev/null
@@ -1,106 +0,0 @@
-/*	NetBSD: xenio.h,v 1.3 2005/05/24 12:07:12 yamt Exp $	*/
-
-/******************************************************************************
- * privcmd.h
- * 
- * Copyright (c) 2003-2004, K A Fraser
- * 
- * This file may be distributed separately from the Linux kernel, or
- * incorporated into other software packages, subject to the following license:
- * 
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this source file (the "Software"), to deal in the Software without
- * restriction, including without limitation the rights to use, copy, modify,
- * merge, publish, distribute, sublicense, and/or sell copies of the Software,
- * and to permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- * 
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- * 
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#ifndef __NetBSD_PRIVCMD_H__
-#define __NetBSD_PRIVCMD_H__
-
-/* Interface to /dev/xen/privcmd */
-
-typedef struct privcmd_hypercall
-{
-    unsigned long op;
-    unsigned long arg[5];
-    long retval;
-} privcmd_hypercall_t;
-
-typedef struct privcmd_mmap_entry {
-    unsigned long va;
-    unsigned long mfn;
-    unsigned long npages;
-} privcmd_mmap_entry_t; 
-
-typedef struct privcmd_mmap {
-    int num;
-    domid_t dom; /* target domain */
-    privcmd_mmap_entry_t *entry;
-} privcmd_mmap_t; 
-
-typedef struct privcmd_mmapbatch {
-    int num;     /* number of pages to populate */
-    domid_t dom; /* target domain */
-    unsigned long addr;  /* virtual address */
-    unsigned long *arr; /* array of mfns - top nibble set on err */
-} privcmd_mmapbatch_t; 
-
-typedef struct privcmd_blkmsg
-{
-    unsigned long op;
-    void         *buf;
-    int           buf_size;
-} privcmd_blkmsg_t;
-
-/*
- * @cmd: IOCTL_PRIVCMD_HYPERCALL
- * @arg: &privcmd_hypercall_t
- * Return: Value returned from execution of the specified hypercall.
- */
-#define IOCTL_PRIVCMD_HYPERCALL         \
-    _IOWR('P', 0, privcmd_hypercall_t)
-
-#if defined(_KERNEL)
-/* compat */
-#define IOCTL_PRIVCMD_INITDOMAIN_EVTCHN_OLD \
-    _IO('P', 1)
-#endif /* defined(_KERNEL) */
-    
-#define IOCTL_PRIVCMD_MMAP             \
-    _IOW('P', 2, privcmd_mmap_t)
-#define IOCTL_PRIVCMD_MMAPBATCH        \
-    _IOW('P', 3, privcmd_mmapbatch_t)
-#define IOCTL_PRIVCMD_GET_MACH2PHYS_START_MFN \
-    _IOR('P', 4, unsigned long)
-
-/*
- * @cmd: IOCTL_PRIVCMD_INITDOMAIN_EVTCHN
- * @arg: n/a
- * Return: Port associated with domain-controller end of control event channel
- *         for the initial domain.
- */
-#define IOCTL_PRIVCMD_INITDOMAIN_EVTCHN \
-    _IOR('P', 5, int)
-
-/* Interface to /dev/xenevt */
-/* EVTCHN_RESET: Clear and reinit the event buffer. Clear error condition. */
-#define EVTCHN_RESET  _IO('E', 1)
-/* EVTCHN_BIND: Bind to the specified event-channel port. */
-#define EVTCHN_BIND   _IOW('E', 2, unsigned long)
-/* EVTCHN_UNBIND: Unbind from the specified event-channel port. */
-#define EVTCHN_UNBIND _IOW('E', 3, unsigned long)
-
-#endif /* __NetBSD_PRIVCMD_H__ */
diff --git a/tools/libs/call/private.h b/tools/libs/call/private.h
index 7944ac5baf..96922e03d5 100644
--- a/tools/libs/call/private.h
+++ b/tools/libs/call/private.h
@@ -7,7 +7,11 @@
 #include <xencall.h>
 
 #include <xen/xen.h>
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #ifndef PAGE_SHIFT /* Mini-os, Yukk */
 #define PAGE_SHIFT           12
diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h
index f0b5f83ac8..68e388f488 100644
--- a/tools/libs/ctrl/xc_private.h
+++ b/tools/libs/ctrl/xc_private.h
@@ -39,7 +39,11 @@
 #include <xenforeignmemory.h>
 #include <xendevicemodel.h>
 
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #include <xen-tools/libs.h>
 
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index 1ee3626dd2..581d8b1eef 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -8,7 +8,13 @@
 #include <xentoolcore_internal.h>
 
 #include <xen/xen.h>
+
+#ifdef __NetBSD__
+#include <xen/xen.h>
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #ifndef PAGE_SHIFT /* Mini-os, Yukk */
 #define PAGE_SHIFT           12
-- 
2.29.2

Re: [PATCH] add a qemu-ifup script on NetBSD

[Ian Jackson]

Manuel Bouyer writes ("[PATCH] add a qemu-ifup script on NetBSD"):
> On NetBSD, qemu-xen will use a qemu-ifup script to setup the tap interfaces
> (as qemu-xen-traditional used to). Copy the script from qemu-xen-traditional,
> and install it on NetBSD. While there document parameters and environnement
> variables.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

> +++ b/tools/hotplug/NetBSD/qemu-ifup
> @@ -0,0 +1,9 @@
> +#!/bin/sh
> +
> +#called by qemu when a HVM domU is started.
> +# first parameter is tap interface, second is the bridge name
> +# environement variable $XEN_DOMAIN_ID contains the domU's ID,
> +# which can be used to retrieve extra parameters from the xenstore.
> +
> +ifconfig $1 up
> +exec /sbin/brconfig $2 add $1

Acked-by: Ian Jackson <iwj@xenproject.org>

Ian.

Re: [PATCH] xenstored: close socket connections on error

[Ian Jackson]

Manuel Bouyer writes ("[PATCH] xenstored: close socket connections on error"):
> On error, don't keep socket connection in ignored state but close them.
> When the remote end of a socket is closed, xenstored will flag it as an
> error and switch the connection to ignored. But on some OSes (e.g.
> NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
> state will stay open forever in xenstored (and it will loop with CPU 100%
> busy).

Juergen, I think you probably know this code the best.  Would you be
able to review this ?

I'm not sure I understand what the specific behaviour on NetBSD is
that is upsetting xenstored.  Or rather, what it is that xenstored is
using to tell when the socket should be closed.

I grepped for POLLERR and nothing came up.

Or to put it another way, is this commit

> Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083

broken on Linux too ?

Ian.

Re: [PATCH v3] NetBSD: use system-provided headers

[Ian Jackson]

Manuel Bouyer writes ("[PATCH v3] NetBSD: use system-provided headers"):
> +#ifdef __NetBSD__
> +#include <xen/xenio.h>
> +#else
>  #include <xen/sys/privcmd.h>
> +#endif
>  #include <xen/foreign/x86_32.h>
>  #include <xen/foreign/x86_64.h>
Maneul, thanks.  I think this is a bugfix and ought in principle to go
in but I think we probably want to do this with configure rather than
ad-hoc ifdefs.

Roger, what do you think ?  Were you going to add a configure test for
the #ifdef that we put in earlier ?

Thanks,
Ian.

Re: [PATCH] xenstored: close socket connections on error

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH] xenstored: close socket connections on error"):
> Manuel Bouyer writes ("[PATCH] xenstored: close socket connections on error"):
> > On error, don't keep socket connection in ignored state but close them.
> > When the remote end of a socket is closed, xenstored will flag it as an
> > error and switch the connection to ignored. But on some OSes (e.g.
> > NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
> > state will stay open forever in xenstored (and it will loop with CPU 100%
> > busy).
> 
> Juergen, I think you probably know this code the best.  Would you be
> able to review this ?
> 
> I'm not sure I understand what the specific behaviour on NetBSD is
> that is upsetting xenstored.  Or rather, what it is that xenstored is
> using to tell when the socket should be closed.
> 
> I grepped for POLLERR and nothing came up.
> 
> Or to put it another way, is this commit
> 
> > Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083
> 
> broken on Linux too ?

Andy pointed me to the recent thread "xenstored file descriptor leak"
which answers all these questions.  I think it would have been nice if
some tools maintainer(s) had been CC'd on that :-).

Juergen, I guess I will get a formal R-b from you ?

Release-Acked-by: Ian Jackson <iwj@xenproject.org>


Manuel, in response to this:

> When I started, I looked at the wiki for instructions about
> patches, but didn't find any ...

Earlier I offered you help with git, in private email.  I agree that
git is confusing and sometimes impenetrable.  But it seems that what
you are doing now is worse!  Please take me up on my offer of help.

Our wiki doesn't give instructions on how to use git to maintain a
patch series.  Those instructions would not be Xen-specific.  Perhaps
we could have a pointer or two, but everyone has their own pet methods
and tooling so the result would perhaps be more confusing than
helpful.

Regards,
Ian.

Re: [PATCH] xenstored: close socket connections on error

[Manuel Bouyer]

On Wed, Feb 03, 2021 at 05:38:59PM +0000, Ian Jackson wrote:
> > [...]
> > broken on Linux too ?
> 
> Andy pointed me to the recent thread "xenstored file descriptor leak"
> which answers all these questions.  I think it would have been nice if
> some tools maintainer(s) had been CC'd on that :-).

I did use add_maintainers.pl against it (or at last it was my intent)

> 
> Juergen, I guess I will get a formal R-b from you ?
> 
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
> 
> 
> Manuel, in response to this:
> 
> > When I started, I looked at the wiki for instructions about
> > patches, but didn't find any ...
> 
> Earlier I offered you help with git, in private email.  I agree that
> git is confusing and sometimes impenetrable.  But it seems that what
> you are doing now is worse!  Please take me up on my offer of help.

I didn't forget. It's just that I don't even know what to ask to start
with.  It seems that StGit will help a lot though.

> 
> Our wiki doesn't give instructions on how to use git to maintain a
> patch series.  Those instructions would not be Xen-specific.  Perhaps
> we could have a pointer or two, but everyone has their own pet methods
> and tooling so the result would perhaps be more confusing than
> helpful.

a howto is alwaus helpfull. Even if it's not the one and only way
to do, at last it gives a start point.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH v3] NetBSD: use system-provided headers

[Roger Pau Monné]

On Wed, Feb 03, 2021 at 05:26:44PM +0000, Ian Jackson wrote:
> Manuel Bouyer writes ("[PATCH v3] NetBSD: use system-provided headers"):
> > +#ifdef __NetBSD__
> > +#include <xen/xenio.h>
> > +#else
> >  #include <xen/sys/privcmd.h>
> > +#endif
> >  #include <xen/foreign/x86_32.h>
> >  #include <xen/foreign/x86_64.h>
> Maneul, thanks.  I think this is a bugfix and ought in principle to go
> in but I think we probably want to do this with configure rather than
> ad-hoc ifdefs.
> 
> Roger, what do you think ?  Were you going to add a configure test for
> the #ifdef that we put in earlier ?

Yes, sorry, I owe you that patch. Will try to do tomorrow so that we
can have a model for other headers. AFAICT I will have to build
something around AC_CHECK_HEADER so that we can get a define that
contains a path that can be used with an #include directive.

Thanks, Roger.

Re: [PATCH] xenstored: close socket connections on error

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 596 bytes --]

On 03.02.21 17:54, Manuel Bouyer wrote:
> On error, don't keep socket connection in ignored state but close them.
> When the remote end of a socket is closed, xenstored will flag it as an
> error and switch the connection to ignored. But on some OSes (e.g.
> NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
> state will stay open forever in xenstored (and it will loop with CPU 100%
> busy).
> 
> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] xenstored: close socket connections on error

[Manuel Bouyer]

On Thu, Feb 04, 2021 at 12:11:02PM +0100, Jürgen Groß wrote:
> On 03.02.21 17:54, Manuel Bouyer wrote:
> > On error, don't keep socket connection in ignored state but close them.
> > When the remote end of a socket is closed, xenstored will flag it as an
> > error and switch the connection to ignored. But on some OSes (e.g.
> > NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
> > state will stay open forever in xenstored (and it will loop with CPU 100%
> > busy).
> > 
> > Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> > Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

thanks.
I still don't know if I'm supposed to send a new version of the patch with
these tags, even if the patch itself doesn't change, or if the commiter
will handle them ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH] xenstored: close socket connections on error

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 956 bytes --]

On 04.02.21 12:16, Manuel Bouyer wrote:
> On Thu, Feb 04, 2021 at 12:11:02PM +0100, Jürgen Groß wrote:
>> On 03.02.21 17:54, Manuel Bouyer wrote:
>>> On error, don't keep socket connection in ignored state but close them.
>>> When the remote end of a socket is closed, xenstored will flag it as an
>>> error and switch the connection to ignored. But on some OSes (e.g.
>>> NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
>>> state will stay open forever in xenstored (and it will loop with CPU 100%
>>> busy).
>>>
>>> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
>>> Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083
>>
>> Reviewed-by: Juergen Gross <jgross@suse.com>
> 
> thanks.
> I still don't know if I'm supposed to send a new version of the patch with
> these tags, even if the patch itself doesn't change, or if the commiter
> will handle them ?
> 

Will be done by the committer.


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] xenstored: close socket connections on error

[Ian Jackson]

Manuel Bouyer writes ("Re: [PATCH] xenstored: close socket connections on error"):
> On Thu, Feb 04, 2021 at 12:11:02PM +0100, Jürgen Groß wrote:
> > On 03.02.21 17:54, Manuel Bouyer wrote:
> > > On error, don't keep socket connection in ignored state but close them.
> > > When the remote end of a socket is closed, xenstored will flag it as an
> > > error and switch the connection to ignored. But on some OSes (e.g.
> > > NetBSD), poll(2) will return only POLLIN in this case, so sockets in ignored
> > > state will stay open forever in xenstored (and it will loop with CPU 100%
> > > busy).
> > > 
> > > Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> > > Fixes: d2fa370d3ef9cbe22d7256c608671cdcdf6e0083
> > 
> > Reviewed-by: Juergen Gross <jgross@suse.com>
> 
> thanks.
> I still don't know if I'm supposed to send a new version of the patch with
> these tags, even if the patch itself doesn't change, or if the commiter
> will handle them ?

The committer will handle them.

Thanks,
Ian.