[Qemu-devel] [PULL 2/9] slirp/smb: Replace constant strings by glib string
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> gcc 7 (on fedora 26) objects to many of the snprintf's in the smb path and command creation because it can't figure out that the smb_dir (i.e. the /tmp dir for the configuration) is known to be short. Replace all these fixed length buffers by g_str* functions that dynamically allocate and use g_dir_make_tmp to make the directory. (It's fairly new glib but we have a compat function for it). Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: Eric Blake <ebl...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- net/slirp.c | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 11b2dd249a..c705a60b62 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -80,7 +80,7 @@ typedef struct SlirpState { Slirp *slirp; Notifier exit_notifier; #ifndef _WIN32 -char smb_dir[128]; +gchar *smb_dir; #endif } SlirpState; @@ -558,11 +558,10 @@ int net_slirp_redir(const char *redir_str) /* automatic user mode samba server configuration */ static void slirp_smb_cleanup(SlirpState *s) { -char cmd[128]; int ret; -if (s->smb_dir[0] != '\0') { -snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir); +if (s->smb_dir) { +gchar *cmd = g_strdup_printf("rm -rf %s", s->smb_dir); ret = system(cmd); if (ret == -1 || !WIFEXITED(ret)) { error_report("'%s' failed.", cmd); @@ -570,15 +569,17 @@ static void slirp_smb_cleanup(SlirpState *s) error_report("'%s' failed. Error code: %d", cmd, WEXITSTATUS(ret)); } -s->smb_dir[0] = '\0'; +g_free(cmd); +g_free(s->smb_dir); +s->smb_dir = NULL; } } static int slirp_smb(SlirpState* s, const char *exported_dir, struct in_addr vserver_addr) { -char smb_conf[128]; -char smb_cmdline[128]; +char *smb_conf; +char *smb_cmdline; struct passwd *passwd; FILE *f; @@ -600,19 +601,19 @@ static int slirp_smb(SlirpState* s, const char *exported_dir, return -1; } -snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.XX"); -if (!mkdtemp(s->smb_dir)) { -error_report("could not create samba server dir '%s'", s->smb_dir); -s->smb_dir[0] = 0; +s->smb_dir = g_dir_make_tmp("qemu-smb.XX", NULL); +if (!s->smb_dir) { +error_report("could not create samba server dir"); return -1; } -snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf"); +smb_conf = g_strdup_printf("%s/%s", s->smb_dir, "smb.conf"); f = fopen(smb_conf, "w"); if (!f) { slirp_smb_cleanup(s); error_report("could not create samba server configuration file '%s'", smb_conf); +g_free(smb_conf); return -1; } fprintf(f, @@ -651,15 +652,18 @@ static int slirp_smb(SlirpState* s, const char *exported_dir, ); fclose(f); -snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s", +smb_cmdline = g_strdup_printf("%s -l %s -s %s", CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf); +g_free(smb_conf); if (slirp_add_exec(s->slirp, 0, smb_cmdline, _addr, 139) < 0 || slirp_add_exec(s->slirp, 0, smb_cmdline, _addr, 445) < 0) { slirp_smb_cleanup(s); +g_free(smb_cmdline); error_report("conflicting/invalid smbserver address"); return -1; } +g_free(smb_cmdline); return 0; } -- 2.11.0
[Qemu-devel] [PULL 3/9] slirp: tftp, copy sockaddr_size
From: Marc-André Lureau <marcandre.lur...@redhat.com> ASAN detects an "unknown-crash" when running pxe-test: /ppc64/pxe/spapr-vlan: = ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0 READ of size 128 at 0x7f6dcd298d30 thread T2 #0 0x55e22218830c in tftp_session_allocate /home/elmarco/src/qq/slirp/tftp.c:73 #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289 #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446 #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82 #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67 Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13 This frame has 3 object(s): [32, 48) '' [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this variable [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows this variable The sockaddr_storage pointer is the sockaddr_in6 lhost on the stack. Copy only the source addr size. Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> Reviewed-by: Thomas Huth <th...@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/tftp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 50e714807d..a9bc4bb1b6 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas, found: memset(spt, 0, sizeof(*spt)); - spt->client_addr = *srcsas; + memcpy(>client_addr, srcsas, sockaddr_size(srcsas)); spt->fd = -1; spt->block_size = 512; spt->client_port = tp->udp.uh_sport; -- 2.11.0
[Qemu-devel] [PULL 1/9] slirp: allow host port 0 for hostfwd
From: Vincent Bernat <vinc...@bernat.im> The OS will allocate automatically a free port. This is useful if you want to be sure to not get any port conflict. You still have to figure out which port you got, for example with "lsof" (this could be exposed in the monitor if needed). Example of use: $ qemu-system-x86_64 -net user,hostfwd=127.0.0.1:0-:22 ... Then, get your port with: $ lsof -np 1474 | grep LISTEN qemu-syst 31777 bernat 12u IPv4 [...] TCP 127.0.0.1:35145 (LISTEN) Signed-off-by: Vincent Bernat <vinc...@bernat.im> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- net/slirp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/slirp.c b/net/slirp.c index f97ec23345..11b2dd249a 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -487,7 +487,7 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, goto fail_syntax; } host_port = strtol(buf, , 0); -if (*end != '\0' || host_port < 1 || host_port > 65535) { +if (*end != '\0' || host_port < 0 || host_port > 65535) { goto fail_syntax; } -- 2.11.0
[Qemu-devel] [PULL 0/9] slirp updates
The following changes since commit 81b2d5ceb0cfb4cdc2163492e3169ed714b0cda9: Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20170426' into staging (2017-04-26 20:50:49 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to eb5d4f5329df83ea15244b47f7fbca21adaae41b: slirp: VMStatify remaining except for loop (2017-04-29 18:44:16 +0200) slirp updates Dr. David Alan Gilbert (6): slirp/smb: Replace constant strings by glib string slirp: VMState conversion; tcpcb slirp: VMStatify sbuf slirp: Common lhost/fhost union slirp: VMStatify socket level slirp: VMStatify remaining except for loop Marc-André Lureau (1): slirp: tftp, copy sockaddr_size Samuel Thibault (1): slirp: fix pinging the virtual ipv4 DNS server Vincent Bernat (1): slirp: allow host port 0 for hostfwd net/slirp.c | 32 ++-- slirp/ip_icmp.c | 5 +- slirp/sbuf.h| 4 +- slirp/slirp.c | 494 +++- slirp/socket.h | 24 ++- slirp/tcp_var.h | 6 +- slirp/tftp.c| 2 +- 7 files changed, 310 insertions(+), 257 deletions(-)
[Qemu-devel] [PATCH] slirp: fix pinging the virtual ipv4 DNS server
so that people do not think it is not working at least basically. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c index 5ffc7a683d..0b667a429a 100644 --- a/slirp/ip_icmp.c +++ b/slirp/ip_icmp.c @@ -152,8 +152,9 @@ icmp_input(struct mbuf *m, int hlen) switch (icp->icmp_type) { case ICMP_ECHO: ip->ip_len += hlen; /* since ip_input subtracts this */ -if (ip->ip_dst.s_addr == slirp->vhost_addr.s_addr) { - icmp_reflect(m); +if (ip->ip_dst.s_addr == slirp->vhost_addr.s_addr || +ip->ip_dst.s_addr == slirp->vnameserver_addr.s_addr) { +icmp_reflect(m); } else if (slirp->restricted) { goto freeit; } else {
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
Hello, FONNEMANN Mark, on mer. 19 avril 2017 18:11:59 +, wrote: > I have tested 2.9-rc4 and the problem still exists there as well. FONNEMANN Mark, on lun. 24 avril 2017 23:43:02 +, wrote: > I just confirmed that the problem exists in 2.9 release using > qemu-system-i386.exe as well. FONNEMANN Mark, on mer. 26 avril 2017 18:51:00 +, wrote: > I just verified the problem exists on Linux host as well. I cannot ping the > DNS or SMB server and the DNS server does not respond to nslookup requests. One first thing to note: the DNS or SMB servers can not be pinged, since that was never implemented (I'll post a patch for that), so there is no surprise on that end. Also, your issue happens even with explictly requesting ipv4, so it's probably not a problem with the v6 updates by themselves. When running with a Linux host (since that's what I'm most familiar with), what do you have in /etc/resolv.conf? Samuel
Re: [Qemu-devel] [PATCH 1/1] slirp: don't zero ti_i since we acccess it later.
Hello, Thomas Huth, on lun. 24 avril 2017 11:15:56 +0200, wrote: > On 20.04.2017 22:43, Tao Wu wrote: > > The current code looks buggy, we zero ti_i while we access > > ti_dst/ti_src later. Indeed. > > Signed-off-by: Tao Wu> > *mtod(m, struct tcpiphdr *) = *ti; > > ti = mtod(m, struct tcpiphdr *); > > - memset(>ti, 0, sizeof(ti->ti)); But then we don't make sure that ih_x1 is 0, which is needed for the checksum to be correct, but possibly not set by the caller. So please replace the memset call with setting the proper ih_x1 field to 0 (which thus needs the introductino of a switch over af like below in the code). Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
Stefan Weil, on ven. 21 avril 2017 21:58:18 +0200, wrote: > Am 17.04.2017 um 00:10 schrieb FONNEMANN Mark: > > I hadn't seen the original report on the list, sorry. There is too much > > traffic on qemu-devel for me to manage to catch these :/ > > > > This problem was fixed by > > e42f869b ("slirp: Make RA build more flexible") and a2f80fdf ("slirp: Send > > RDNSS in RA only if host has an IPv6 DNS server") which will be included in > > qemu 2.9. > > > > Samuel > > See report on > http://stackoverflow.com/questions/43308310/dns-server-not-working-in-qemu-usermode-networking. > > Mark, did you also get that problem with QEMU running on a Linux host, > or is it specific for QEMU running on Windows? Ah, I hadn't noticed the report was about windows. The abovementioned fix should still be fine for windows: there, get_dns6_addr just always returns -1, and thus the RDNSS option is never added. So I don't know what to do on my side, more investigation is needed there. Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
Hello, I hadn't seen the original report on the list, sorry. There is too much traffic on qemu-devel for me to manage to catch these :/ This problem was fixed by e42f869b ("slirp: Make RA build more flexible") and a2f80fdf ("slirp: Send RDNSS in RA only if host has an IPv6 DNS server") which will be included in qemu 2.9. Samuel
Re: [Qemu-devel] [PATCH v2 5/5] slirp: add a fake NC-SI backend
Hello, Philippe Mathieu-Daudé, on jeu. 13 avril 2017 08:45:23 -0300, wrote: > > The NCSI header file comes from mainline Linux Please mention within the file which file it comes from exactly. > > +case NCSI_PKT_CMD_SMA: > > +rnh->common.length = htons(4); > > +break; > > +case NCSI_PKT_CMD_GVI: > > +rnh->common.length = htons(36); > > +break; > > +case NCSI_PKT_CMD_GC: { > > +rnh->common.length = htons(32); ... > > +break; > > +} > > + > > +case NCSI_PKT_CMD_GLS: { > > +rnh->common.length = htons(16); > > +break; > > +} > > + > > +slirp_output(slirp->opaque, ncsi_reply, sizeof(ncsi_reply)); Are we really supposed to send sizeof(ncsi_reply), and not accordingly to the size announced withing the packet? Appart from that, Acked-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Samuel
Re: [Qemu-devel] [PATCH for 2.10] slirp/smb: Replace constant strings by glib string
Hello, Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v2 1/1] slirp: add SOCKS5 support
Hello, Thanks for the patch! Laurent Vivier, on mar. 28 mars 2017 21:07:56 +0200, wrote: > @@ -617,6 +621,10 @@ void slirp_pollfds_poll(GArray *pollfds, int > select_error) > * Check sockets for reading > */ > else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) { > +if (so->so_state & SS_ISFCONNECTING) { > +socks5_recv(so->s, >so_proxy_state); > +continue; > +} It looks odd to be calling socks5_recv in all cases. Shouldn't this check for so_proxy_state != 0? > @@ -645,11 +653,19 @@ void slirp_pollfds_poll(GArray *pollfds, int > select_error) > /* > * Check for non-blocking, still-connecting sockets > */ > -if (so->so_state & SS_ISFCONNECTING) { > -/* Connected */ > -so->so_state &= ~SS_ISFCONNECTING; > > -ret = send(so->s, (const void *) , 0, 0); > +if (so->so_state & SS_ISFCONNECTING) { > +ret = socks5_send(so->s, slirp->proxy_user, Ditto. Socks5 and non-socks5 code should be properly separated I guess. > @@ -1069,6 +1085,48 @@ int slirp_add_exec(Slirp *slirp, int do_pty, const > void *args, > htons(guest_port)); > } > > +int slirp_add_proxy(Slirp *slirp, const char *server, int port, > +const char *user, const char *password) > +{ > +int fd; > +socks5_state_t state; > +struct sockaddr_storage addr; > + > +/* check the connection */ Please be a bit more verbose :) For instance: > + /* just check that the connection to the socks5 server works with > the given credentials, and close without doing anything with it. */ > diff --git a/slirp/socks5.c b/slirp/socks5.c > new file mode 100644 > index 000..1c5e071 > --- /dev/null > +++ b/slirp/socks5.c > @@ -0,0 +1,284 @@ > +/* some parts from nmap/ncat GPLv2 */ One can't just steal code like this. You *have* to copy over the copyright notice, since there's notably the author information. > +static int socks5_proxy_connect(int fd, const char *server, int port) > +{ > +struct sockaddr_in saddr; > +struct hostent *he; > + > +he = gethostbyname(server); > +if (he == NULL) { > +errno = EINVAL; > +return -1; > +} > +saddr.sin_family = AF_INET; > +saddr.sin_addr = *(struct in_addr *)he->h_addr; > +saddr.sin_port = htons(port); Please add a TODO: v6 version > +static int socks5_recv_authenticate(int fd) > +{ > +char socksbuf[2]; > +if (recv(fd, socksbuf, 2, 0) != 2) { > +return -1; > +} > +if (socksbuf[0] != 1 || socksbuf[1] != 0) { These look like magic numbers :) Please document what they represent. > +int len; > + > +memset(, 0, sizeof(socks5msg)); > + > +socks5msg.ver = SOCKS5_VERSION; > +socks5msg.cmd = SOCKS_CONNECT; > +socks5msg.rsv = 0; Please rather set len to 4 here, and increment later on > +switch (addr->ss_family) { > +case AF_INET: { > +struct sockaddr_in *a = (struct sockaddr_in *)addr; > + > +socks5msg.atyp = SOCKS5_ATYP_IPv4; > +memcpy(socks5msg.dst, >sin_addr, sizeof(a->sin_addr)); > +len = sizeof(a->sin_addr); here > +memcpy(socks5msg.dst + len, >sin_port, sizeof(a->sin_port)); > +len += sizeof(a->sin_port); > +} > +break; > +case AF_INET6: { > +struct sockaddr_in6 *a = (struct sockaddr_in6 *)addr; > + > +socks5msg.atyp = SOCKS5_ATYP_IPv6; > +memcpy(socks5msg.dst, >sin6_addr, sizeof(a->sin6_addr)); > +len = sizeof(a->sin6_addr); and there. > +memcpy(socks5msg.dst + len, >sin6_port, sizeof(a->sin6_port)); > +len += sizeof(a->sin6_port); > +} > +break; > +default: > +errno = EINVAL; > +return -1; > +} > +len += 4; and not have this here, where people wonder what that is :) > +static int socks5_recv_connect(int fd) > +{ > +struct socks5_request socks5msg; > + > +if (recv(fd, , 4, 0) != 4) { Thinking about it (there are more like that above and below). You can theoretically have a short read here... So please at least leave a note that we actually should manage buffering of recv()s. > +++ b/slirp/socks5.h > +struct socks4_data { > +char version; > +char type; > +unsigned short port; > +uint32_t address; > +char data[SOCKS_BUFF_SIZE]; /* to hold FQDN and username */ > +} __attribute__((packed)); > + > +/* SOCKS4 protocol responses */ > +#define SOCKS4_VERSION 4 > +#define SOCKS_CONNECT 1 > +#define SOCKS_BIND 2 > +#define SOCKS4_CONN_ACC 90 > +#define SOCKS4_CONN_REF 91 > +#define SOCKS4_CONN_IDENT 92 > +#define SOCKS4_CONN_IDENTDIFF 93
Re: [Qemu-devel] [PATCH 14/43] usb: made printf always compile in debug output
Danil Antonov, on sam. 01 avril 2017 16:48:58 +0300, wrote: > From 02b1e378eea154c0169a3d16b64ae27c64919c2c Mon Sep 17 00:00:00 2001 > From: Danil Antonov <[1]g.danil.a...@gmail.com> > Date: Wed, 29 Mar 2017 12:28:51 +0300 > Subject: [PATCH 14/43] usb: made printf always compile in debug output > > Wrapped printf calls inside debug macros (DPRINTF) in `if` statement. > This will ensure that printf function will always compile even if debug > output is turned off and, in turn, will prevent bitrot of the format > strings. > > Signed-off-by: Danil Antonov <[2]g.danil.a...@gmail.com> Acked-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> > --- > hw/usb/dev-serial.c | 17 + > hw/usb/dev-storage.c | 17 + > hw/usb/hcd-ehci.h | 10 +- > hw/usb/hcd-xhci.c | 19 --- > 4 files changed, 35 insertions(+), 28 deletions(-) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index 6d51373..739e79b 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -17,14 +17,15 @@ > #include "hw/usb/desc.h" > #include "sysemu/char.h" > > -//#define DEBUG_Serial > - > -#ifdef DEBUG_Serial > -#define DPRINTF(fmt, ...) \ > -do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while (0) > -#else > -#define DPRINTF(fmt, ...) do {} while(0) > -#endif > +#ifndef DEBUG_Serial > +#define DEBUG_Serial 0 > +#endif > + > +#define DPRINTF(fmt, ...) do { \ > + if (DEBUG_Serial) { \ > + fprintf(stderr, "usb-serial: " fmt,## __VA_ARGS__); \ > + } \ > +} while (0); > > #define RECV_BUF 384 > > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c > index 8a61ec9..b85c006 100644 > --- a/hw/usb/dev-storage.c > +++ b/hw/usb/dev-storage.c > @@ -24,14 +24,15 @@ > #include "qapi/visitor.h" > #include "qemu/cutils.h" > > -//#define DEBUG_MSD > - > -#ifdef DEBUG_MSD > -#define DPRINTF(fmt, ...) \ > -do { printf("usb-msd: " fmt , ## __VA_ARGS__); } while (0) > -#else > -#define DPRINTF(fmt, ...) do {} while(0) > -#endif > +#ifndef DEBUG_MSD > +#define DEBUG_MSD 0 > +#endif > + > +#define DPRINTF(fmt, ...) do { \ > + if (DEBUG_MSD) { \ > + fprintf(stderr, "usb-msd: " fmt , ## __VA_ARGS__); \ > + } \ > +} while (0); > > /* USB requests. */ > #define MassStorageReset 0xff > diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h > index 938d8aa..ca6094e 100644 > --- a/hw/usb/hcd-ehci.h > +++ b/hw/usb/hcd-ehci.h > @@ -30,11 +30,11 @@ > #define EHCI_DEBUG 0 > #endif > > -#if EHCI_DEBUG > -#define DPRINTF printf > -#else > -#define DPRINTF(...) > -#endif > +#define DPRINTF(fmt, ...) do { \ > + if (EHCI_DEBUG) { \ > + fprintf(stderr, fmt, ## __VA_ARGS__); \ > + } \ > +} while (0); > > #define MMIO_SIZE 0x1000 > #define CAPA_SIZE 0x10 > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index f0af852..9f5e5b2 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -32,11 +32,16 @@ > //#define DEBUG_XHCI > //#define DEBUG_DATA > > -#ifdef DEBUG_XHCI > -#define DPRINTF(...) fprintf(stderr, __VA_ARGS__) > -#else > -#define DPRINTF(...) do {} while (0) > -#endif > +#ifndef DEBUG_XHCI > +#define DEBUG_XHCI 0 > +#endif > + > +#define DPRINTF(fmt, ...) do { \ > + if (DEBUG_XHCI) { \ > + fprintf(stderr, fmt , ## __VA_ARGS__); \ > + } \ > +} while (0); > + > #define FIXME(_msg) do { fprintf(stderr, "FIXME %s:%d %s\n", \ > __func__, __LINE__, _msg); abort(); } while > (0) > > @@ -1806,7 +1811,7 @@ static int xhci_setup_packet(XHCITransfer *xfer) > ep = xhci_epid_to_usbep(xfer->epctx); > if (!ep) { > DPRINTF("xhci: slot %d has no device\n", > - xfer->slotid); > + xfer->epctx->slotid); > return -1; > } > } > @@ -1980,7 +1985,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer > *xfer, XHCIEPContext *epctx > { > uint64_t mfindex; > > - DPRINTF("xhci_submit(slotid=%d,epid=%d)\n", xfer->slotid, xfer->epid); > + DPRINTF("xhci_submit(slotid=%d,epid=%d)\n", xfer->epctx->slotid, xfer-> > epctx->epid); > > xfer->in_xfer = epctx->type>>2; > > -- > 2.8.0.rc3 > > > References: > > [1] mailto:g.danil.a...@gmail.com > [2] mailto:g.danil.a...@gmail.com -- Samuel c> [ ] morning [ ] afternoon [ ] evening [ ] night , everyone (choose as applicable)
[Qemu-devel] [PULL 2/3] slirp: Make RA build more flexible
Do not hardcode the RA size at all, use a pl_size variable which accounts the accumulated size, and fill rip->ip_pl at the end. This will allow to make some blocks optional. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> --- slirp/ip6_icmp.c | 24 +--- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index 298a48dd25..d0f5cc1456 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -143,17 +143,10 @@ void ndp_send_ra(Slirp *slirp) /* Build IPv6 packet */ struct mbuf *t = m_get(slirp); struct ip6 *rip = mtod(t, struct ip6 *); +size_t pl_size = 0; rip->ip_src = (struct in6_addr)LINKLOCAL_ADDR; rip->ip_dst = (struct in6_addr)ALLNODES_MULTICAST; rip->ip_nh = IPPROTO_ICMPV6; -rip->ip_pl = htons(ICMP6_NDP_RA_MINLEN -+ NDPOPT_LINKLAYER_LEN -+ NDPOPT_PREFIXINFO_LEN -#ifndef _WIN32 -+ NDPOPT_RDNSS_LEN -#endif -); -t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl); /* Build ICMPv6 packet */ t->m_data += sizeof(struct ip6); @@ -171,6 +164,7 @@ void ndp_send_ra(Slirp *slirp) ricmp->icmp6_nra.reach_time = htonl(NDP_AdvReachableTime); ricmp->icmp6_nra.retrans_time = htonl(NDP_AdvRetransTime); t->m_data += ICMP6_NDP_RA_MINLEN; +pl_size += ICMP6_NDP_RA_MINLEN; /* Source link-layer address (NDP option) */ struct ndpopt *opt = mtod(t, struct ndpopt *); @@ -178,6 +172,7 @@ void ndp_send_ra(Slirp *slirp) opt->ndpopt_len = NDPOPT_LINKLAYER_LEN / 8; in6_compute_ethaddr(rip->ip_src, opt->ndpopt_linklayer); t->m_data += NDPOPT_LINKLAYER_LEN; +pl_size += NDPOPT_LINKLAYER_LEN; /* Prefix information (NDP option) */ struct ndpopt *opt2 = mtod(t, struct ndpopt *); @@ -192,6 +187,7 @@ void ndp_send_ra(Slirp *slirp) opt2->ndpopt_prefixinfo.reserved2 = 0; opt2->ndpopt_prefixinfo.prefix = slirp->vprefix_addr6; t->m_data += NDPOPT_PREFIXINFO_LEN; +pl_size += NDPOPT_PREFIXINFO_LEN; #ifndef _WIN32 /* Prefix information (NDP option) */ @@ -203,16 +199,14 @@ void ndp_send_ra(Slirp *slirp) opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; t->m_data += NDPOPT_RDNSS_LEN; +pl_size += NDPOPT_RDNSS_LEN; #endif +rip->ip_pl = htons(pl_size); +t->m_data -= sizeof(struct ip6) + pl_size; +t->m_len = sizeof(struct ip6) + pl_size; + /* ICMPv6 Checksum */ -#ifndef _WIN32 -t->m_data -= NDPOPT_RDNSS_LEN; -#endif -t->m_data -= NDPOPT_PREFIXINFO_LEN; -t->m_data -= NDPOPT_LINKLAYER_LEN; -t->m_data -= ICMP6_NDP_RA_MINLEN; -t->m_data -= sizeof(struct ip6); ricmp->icmp6_cksum = ip6_cksum(t); ip6_output(NULL, t, 0); -- 2.11.0
[Qemu-devel] [PULL 1/3] slirp: fix compilation errors with DEBUG set
From: Laurent Vivier <laur...@vivier.eu> slirp/slirp.c: In function 'get_dns_addr_resolv_conf': slirp/slirp.c:202:29: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] char *res = inet_ntop(af, tmp_addr, s, sizeof(s)); ^ slirp/slirp.c:204:25: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] res = "(string conversion error)"; Signed-off-by: Laurent Vivier <laur...@vivier.eu> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/slirp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slirp/slirp.c b/slirp/slirp.c index 60539de7a3..5a94b06f5e 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -198,7 +198,7 @@ static int get_dns_addr_resolv_conf(int af, void *pdns_addr, void *cached_addr, #ifdef DEBUG else { char s[INET6_ADDRSTRLEN]; -char *res = inet_ntop(af, tmp_addr, s, sizeof(s)); +const char *res = inet_ntop(af, tmp_addr, s, sizeof(s)); if (!res) { res = "(string conversion error)"; } -- 2.11.0
[Qemu-devel] [PULL 3/3] slirp: Send RDNSS in RA only if host has an IPv6 DNS server
Previously we would always send an RDNSS option in the RA, making the guest try to resolve DNS through IPv6, even if the host does not actually have and IPv6 DNS server available. This makes the RDNSS option enabled only when an IPv6 DNS server is available. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> --- slirp/ip6_icmp.c | 25 ++--- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index d0f5cc1456..777eb574be 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -144,6 +144,9 @@ void ndp_send_ra(Slirp *slirp) struct mbuf *t = m_get(slirp); struct ip6 *rip = mtod(t, struct ip6 *); size_t pl_size = 0; +struct in6_addr addr; +uint32_t scope_id; + rip->ip_src = (struct in6_addr)LINKLOCAL_ADDR; rip->ip_dst = (struct in6_addr)ALLNODES_MULTICAST; rip->ip_nh = IPPROTO_ICMPV6; @@ -189,18 +192,18 @@ void ndp_send_ra(Slirp *slirp) t->m_data += NDPOPT_PREFIXINFO_LEN; pl_size += NDPOPT_PREFIXINFO_LEN; -#ifndef _WIN32 /* Prefix information (NDP option) */ -/* disabled for windows for now, until get_dns6_addr is implemented */ -struct ndpopt *opt3 = mtod(t, struct ndpopt *); -opt3->ndpopt_type = NDPOPT_RDNSS; -opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; -opt3->ndpopt_rdnss.reserved = 0; -opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); -opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; -t->m_data += NDPOPT_RDNSS_LEN; -pl_size += NDPOPT_RDNSS_LEN; -#endif +if (get_dns6_addr(, _id) >= 0) { +/* Host system does have an IPv6 DNS server, announce our proxy. */ +struct ndpopt *opt3 = mtod(t, struct ndpopt *); +opt3->ndpopt_type = NDPOPT_RDNSS; +opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; +opt3->ndpopt_rdnss.reserved = 0; +opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); +opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; +t->m_data += NDPOPT_RDNSS_LEN; +pl_size += NDPOPT_RDNSS_LEN; +} rip->ip_pl = htons(pl_size); t->m_data -= sizeof(struct ip6) + pl_size; -- 2.11.0
[Qemu-devel] [PULL 0/3] slirp updates
The following changes since commit df9046363220e57d45818312759b954c033c58ab: Update version for v2.9.0-rc2 release (2017-03-28 19:11:16 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to a2f80fdfc683019901cdf4c0863a5920c0ca7245: slirp: Send RDNSS in RA only if host has an IPv6 DNS server (2017-03-29 00:51:25 +0200) slirp updates Laurent Vivier (1): slirp: fix compilation errors with DEBUG set Samuel Thibault (2): slirp: Make RA build more flexible slirp: Send RDNSS in RA only if host has an IPv6 DNS server slirp/ip6_icmp.c | 47 ++- slirp/slirp.c| 2 +- 2 files changed, 23 insertions(+), 26 deletions(-)
Re: [Qemu-devel] [PATCH 2/2] slirp: Send RDNSS in RA only if host has an IPv6 DNS server
Hello, Philippe Mathieu-Daudé, on lun. 27 mars 2017 11:56:00 -0300, wrote: > Why don't declare at function begining and remove this { } ? Oh, right, now I can. While working on the code I still had ifdef WIN32, so it'd lead to an unused variable warning. But now that the ifdef is gone, we can just put the variable at the beginning of the function indeed. Thanks, Samuel
[Qemu-devel] [PATCH 1/2] slirp: Make RA build more flexible
Do not hardcode the RA size at all, use a pl_size variable which accounts the accumulated size, and fill rip->ip_pl at the end. This will allow to make some blocks optional. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/ip6_icmp.c | 24 +--- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index 298a48dd25..d0f5cc1456 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -143,17 +143,10 @@ void ndp_send_ra(Slirp *slirp) /* Build IPv6 packet */ struct mbuf *t = m_get(slirp); struct ip6 *rip = mtod(t, struct ip6 *); +size_t pl_size = 0; rip->ip_src = (struct in6_addr)LINKLOCAL_ADDR; rip->ip_dst = (struct in6_addr)ALLNODES_MULTICAST; rip->ip_nh = IPPROTO_ICMPV6; -rip->ip_pl = htons(ICMP6_NDP_RA_MINLEN -+ NDPOPT_LINKLAYER_LEN -+ NDPOPT_PREFIXINFO_LEN -#ifndef _WIN32 -+ NDPOPT_RDNSS_LEN -#endif -); -t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl); /* Build ICMPv6 packet */ t->m_data += sizeof(struct ip6); @@ -171,6 +164,7 @@ void ndp_send_ra(Slirp *slirp) ricmp->icmp6_nra.reach_time = htonl(NDP_AdvReachableTime); ricmp->icmp6_nra.retrans_time = htonl(NDP_AdvRetransTime); t->m_data += ICMP6_NDP_RA_MINLEN; +pl_size += ICMP6_NDP_RA_MINLEN; /* Source link-layer address (NDP option) */ struct ndpopt *opt = mtod(t, struct ndpopt *); @@ -178,6 +172,7 @@ void ndp_send_ra(Slirp *slirp) opt->ndpopt_len = NDPOPT_LINKLAYER_LEN / 8; in6_compute_ethaddr(rip->ip_src, opt->ndpopt_linklayer); t->m_data += NDPOPT_LINKLAYER_LEN; +pl_size += NDPOPT_LINKLAYER_LEN; /* Prefix information (NDP option) */ struct ndpopt *opt2 = mtod(t, struct ndpopt *); @@ -192,6 +187,7 @@ void ndp_send_ra(Slirp *slirp) opt2->ndpopt_prefixinfo.reserved2 = 0; opt2->ndpopt_prefixinfo.prefix = slirp->vprefix_addr6; t->m_data += NDPOPT_PREFIXINFO_LEN; +pl_size += NDPOPT_PREFIXINFO_LEN; #ifndef _WIN32 /* Prefix information (NDP option) */ @@ -203,16 +199,14 @@ void ndp_send_ra(Slirp *slirp) opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; t->m_data += NDPOPT_RDNSS_LEN; +pl_size += NDPOPT_RDNSS_LEN; #endif +rip->ip_pl = htons(pl_size); +t->m_data -= sizeof(struct ip6) + pl_size; +t->m_len = sizeof(struct ip6) + pl_size; + /* ICMPv6 Checksum */ -#ifndef _WIN32 -t->m_data -= NDPOPT_RDNSS_LEN; -#endif -t->m_data -= NDPOPT_PREFIXINFO_LEN; -t->m_data -= NDPOPT_LINKLAYER_LEN; -t->m_data -= ICMP6_NDP_RA_MINLEN; -t->m_data -= sizeof(struct ip6); ricmp->icmp6_cksum = ip6_cksum(t); ip6_output(NULL, t, 0); -- 2.11.0
[Qemu-devel] [PATCH 0/2] Fix spurious RDNSS announce
Hello, These two patches fix sending the IPv6 RDNSS announce only when it can actually work, fixing the bug reported by Cole Robinson <crobi...@redhat.com>. Could somebody review them before I can send a pull update? Samuel Thibault (2): slirp: Make RA build more flexible slirp: Send RDNSS in RA only if host has an IPv6 DNS server slirp/ip6_icmp.c | 48 +++- 1 file changed, 23 insertions(+), 25 deletions(-) -- 2.11.0
[Qemu-devel] [PATCH 2/2] slirp: Send RDNSS in RA only if host has an IPv6 DNS server
Previously we would always send an RDNSS option in the RA, making the guest try to resolve DNS through IPv6, even if the host does not actually have and IPv6 DNS server available. This makes the RDNSS option enabled only when an IPv6 DNS server is available. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/ip6_icmp.c | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index d0f5cc1456..00183e5945 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -189,18 +189,22 @@ void ndp_send_ra(Slirp *slirp) t->m_data += NDPOPT_PREFIXINFO_LEN; pl_size += NDPOPT_PREFIXINFO_LEN; -#ifndef _WIN32 /* Prefix information (NDP option) */ -/* disabled for windows for now, until get_dns6_addr is implemented */ -struct ndpopt *opt3 = mtod(t, struct ndpopt *); -opt3->ndpopt_type = NDPOPT_RDNSS; -opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; -opt3->ndpopt_rdnss.reserved = 0; -opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); -opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; -t->m_data += NDPOPT_RDNSS_LEN; -pl_size += NDPOPT_RDNSS_LEN; -#endif +{ +struct in6_addr addr; +uint32_t scope_id; +if (get_dns6_addr(, _id) >= 0) { +/* Host system does have an IPv6 DNS server, announce our proxy. */ +struct ndpopt *opt3 = mtod(t, struct ndpopt *); +opt3->ndpopt_type = NDPOPT_RDNSS; +opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; +opt3->ndpopt_rdnss.reserved = 0; +opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); +opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; +t->m_data += NDPOPT_RDNSS_LEN; +pl_size += NDPOPT_RDNSS_LEN; +} +} rip->ip_pl = htons(pl_size); t->m_data -= sizeof(struct ip6) + pl_size; -- 2.11.0
Re: [Qemu-devel] slirp + ipxe + ipv6 dns issue
Hello, Cole Robinson, on ven. 24 mars 2017 21:17:43 -0400, wrote: > I bisected to this commit: > > slirp: Add RDNSS advertisement Mmm, I see. Could you try the attached patch to confirm that it fixes the issue for you too? Samuel diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index 298a48dd25..00183e5945 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -143,17 +143,10 @@ void ndp_send_ra(Slirp *slirp) /* Build IPv6 packet */ struct mbuf *t = m_get(slirp); struct ip6 *rip = mtod(t, struct ip6 *); +size_t pl_size = 0; rip->ip_src = (struct in6_addr)LINKLOCAL_ADDR; rip->ip_dst = (struct in6_addr)ALLNODES_MULTICAST; rip->ip_nh = IPPROTO_ICMPV6; -rip->ip_pl = htons(ICMP6_NDP_RA_MINLEN -+ NDPOPT_LINKLAYER_LEN -+ NDPOPT_PREFIXINFO_LEN -#ifndef _WIN32 -+ NDPOPT_RDNSS_LEN -#endif -); -t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl); /* Build ICMPv6 packet */ t->m_data += sizeof(struct ip6); @@ -171,6 +164,7 @@ void ndp_send_ra(Slirp *slirp) ricmp->icmp6_nra.reach_time = htonl(NDP_AdvReachableTime); ricmp->icmp6_nra.retrans_time = htonl(NDP_AdvRetransTime); t->m_data += ICMP6_NDP_RA_MINLEN; +pl_size += ICMP6_NDP_RA_MINLEN; /* Source link-layer address (NDP option) */ struct ndpopt *opt = mtod(t, struct ndpopt *); @@ -178,6 +172,7 @@ void ndp_send_ra(Slirp *slirp) opt->ndpopt_len = NDPOPT_LINKLAYER_LEN / 8; in6_compute_ethaddr(rip->ip_src, opt->ndpopt_linklayer); t->m_data += NDPOPT_LINKLAYER_LEN; +pl_size += NDPOPT_LINKLAYER_LEN; /* Prefix information (NDP option) */ struct ndpopt *opt2 = mtod(t, struct ndpopt *); @@ -192,27 +187,30 @@ void ndp_send_ra(Slirp *slirp) opt2->ndpopt_prefixinfo.reserved2 = 0; opt2->ndpopt_prefixinfo.prefix = slirp->vprefix_addr6; t->m_data += NDPOPT_PREFIXINFO_LEN; +pl_size += NDPOPT_PREFIXINFO_LEN; -#ifndef _WIN32 /* Prefix information (NDP option) */ -/* disabled for windows for now, until get_dns6_addr is implemented */ -struct ndpopt *opt3 = mtod(t, struct ndpopt *); -opt3->ndpopt_type = NDPOPT_RDNSS; -opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; -opt3->ndpopt_rdnss.reserved = 0; -opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); -opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; -t->m_data += NDPOPT_RDNSS_LEN; -#endif +{ +struct in6_addr addr; +uint32_t scope_id; +if (get_dns6_addr(, _id) >= 0) { +/* Host system does have an IPv6 DNS server, announce our proxy. */ +struct ndpopt *opt3 = mtod(t, struct ndpopt *); +opt3->ndpopt_type = NDPOPT_RDNSS; +opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8; +opt3->ndpopt_rdnss.reserved = 0; +opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval); +opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6; +t->m_data += NDPOPT_RDNSS_LEN; +pl_size += NDPOPT_RDNSS_LEN; +} +} + +rip->ip_pl = htons(pl_size); +t->m_data -= sizeof(struct ip6) + pl_size; +t->m_len = sizeof(struct ip6) + pl_size; /* ICMPv6 Checksum */ -#ifndef _WIN32 -t->m_data -= NDPOPT_RDNSS_LEN; -#endif -t->m_data -= NDPOPT_PREFIXINFO_LEN; -t->m_data -= NDPOPT_LINKLAYER_LEN; -t->m_data -= ICMP6_NDP_RA_MINLEN; -t->m_data -= sizeof(struct ip6); ricmp->icmp6_cksum = ip6_cksum(t); ip6_output(NULL, t, 0);
Re: [Qemu-devel] [PATCH] slirp: tftp, copy sockaddr_size
Marc-André Lureau, on jeu. 23 mars 2017 15:31:56 +0400, wrote: > ASAN detects an "unknown-crash" when running pxe-test: > > /ppc64/pxe/spapr-vlan: > = > ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at > pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0 > READ of size 128 at 0x7f6dcd298d30 thread T2 > #0 0x55e22218830c in tftp_session_allocate > /home/elmarco/src/qq/slirp/tftp.c:73 > #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289 > #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446 > #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82 > #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67 > > Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame > #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13 > > This frame has 3 object(s): > [32, 48) '' > [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this > variable > [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows > this variable > > The sockaddr_storage pointer is the sockaddr_in6 lhost on the > stack. Copy only the source addr size. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> Reviewed-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> > --- > slirp/tftp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 50e714807d..a9bc4bb1b6 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct > sockaddr_storage *srcsas, > > found: >memset(spt, 0, sizeof(*spt)); > - spt->client_addr = *srcsas; > + memcpy(>client_addr, srcsas, sockaddr_size(srcsas)); >spt->fd = -1; >spt->block_size = 512; >spt->client_port = tp->udp.uh_sport; > -- > 2.12.0.191.gc5d8de91d > -- Samuel gawk; talk; nice; date; wine; grep; touch; unzip; strip; \ touch; gasp; finger; gasp; lyx; gasp; latex; mount; fsck; \ more; yes; gasp; umount; make clean; make mrproper; sleep
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:40:19 +, wrote: > * Samuel Thibault (samuel.thiba...@gnu.org) wrote: > > Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:09:26 +, wrote: > > > but only for Linux. > > > > That's what I was referring to. > > Yes I think that's OK because: >a) The alternative breaks all backwards migration >b) This doesn't casuse any problem for forward migration > > There is another way, we could add a subsection that's sent whenever > we use non-IPv4; that would (probably) fail noisily on a backwards > migration to an old setup. But IMHO it's not worth it in this case > given how rare a backwards migration of an IPv6 slirp connection on > something other than Linux is. Alright. Samuel
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:09:26 +, wrote: > but only for Linux. That's what I was referring to. Samuel
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:00:17 +, wrote: > * Samuel Thibault (samuel.thiba...@gnu.org) wrote: > > Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +, wrote: > > > I'm thinking one way to do it without changing the version would > > > be to use the existing value for IPv4, and on reading allow any other > > > value for IPv6 (or just the ones we know about); that would make > > > it inwards migration compatible. > > > > Right. I don't know if that's enough for QEMU requirements. > > If you change the version number you break backwards migration anyway; > but doing what I suggested would keep backwards working in most cases. Sure, but in some cases we're breaking upward compatibility *silently*. Samuel
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +, wrote: > I'm thinking one way to do it without changing the version would > be to use the existing value for IPv4, and on reading allow any other > value for IPv6 (or just the ones we know about); that would make > it inwards migration compatible. Right. I don't know if that's enough for QEMU requirements. Samuel
Re: [Qemu-devel] [PATCH] blk: Add discard=sparse mode
Hello, Max Reitz, on lun. 27 févr. 2017 17:12:47 +0100, wrote: > > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > > -if (s->has_discard && s->has_fallocate) { > > +if (s->has_discard && (s->has_fallocate || open_flags & > > BDRV_O_SPARSE)) { > > s->has_fallocate has a meaning. I wouldn't try to call do_fallocate() if > s->has_fallocate is false. Ah, sorry, I didn't realize that that test wasn't only to check that we'll be able to call fallocate(0) further down. > Therefore, I consider this to effectively be a no-op. Yes. > > @@ -1098,7 +1102,8 @@ static ssize_t > > handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) > > #endif > > > > #ifdef CONFIG_FALLOCATE > > -if (s->has_fallocate && aiocb->aio_offset >= > > bdrv_getlength(aiocb->bs)) { > > +if (s->has_fallocate && !(open_flags & BDRV_O_SPARSE) > > +&& aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { > > First, this part is only invoked if everything before it has failed. I misread the code indeed. > Unless I'm mistaken, unmap/trim requests from the guest should result in > a discard request in the block layer. This should always trigger > handle_aiocb_discard() here and that should do what you want it to. Mmm, indeed. I guess I got lost in the hairy block code. > Could you maybe give me the configuration that results in the issue > you're describing in the commit message? Actually I can't reproduce the issue any more. I'm now wondering how I ended up there. Anyway, I'm really sorry for the noise, and thanks for the good work :) Samuel
[Qemu-devel] [PATCH] blk: Add discard=sparse mode
By default, on discard requests, the posix block backend punches holes but re-fallocates them to keep the allocated size intact. In some situations it is however convenient, when using sparse disk images, to see disk image sizes shrink on discard requests. This commit adds a discard=sparse mode which does this, by disabling the fallocate call. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/block.c b/block.c index b663204f3f..e9cd83210a 100644 --- a/block.c +++ b/block.c @@ -665,12 +665,14 @@ static void bdrv_join_options(BlockDriverState *bs, QDict *options, */ int bdrv_parse_discard_flags(const char *mode, int *flags) { -*flags &= ~BDRV_O_UNMAP; +*flags &= ~(BDRV_O_UNMAP | BDRV_O_SPARSE); if (!strcmp(mode, "off") || !strcmp(mode, "ignore")) { /* do nothing */ } else if (!strcmp(mode, "on") || !strcmp(mode, "unmap")) { *flags |= BDRV_O_UNMAP; +} else if (!strcmp(mode, "sparse")) { +*flags |= BDRV_O_UNMAP | BDRV_O_SPARSE; } else { return -1; } diff --git a/block/file-posix.c b/block/file-posix.c index 4de1abd023..f9efadc5be 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1057,6 +1057,10 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) BDRVRawState *s = aiocb->bs->opaque; #endif +#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || defined(CONFIG_FALLOCATE) +int open_flags = aiocb->bs->open_flags; +#endif + if (aiocb->aio_type & QEMU_AIO_BLKDEV) { return handle_aiocb_write_zeroes_block(aiocb); } @@ -1079,7 +1083,7 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) #endif #ifdef CONFIG_FALLOCATE_PUNCH_HOLE -if (s->has_discard && s->has_fallocate) { +if (s->has_discard && (s->has_fallocate || open_flags & BDRV_O_SPARSE)) { int ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, aiocb->aio_offset, aiocb->aio_nbytes); @@ -1098,7 +1102,8 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) #endif #ifdef CONFIG_FALLOCATE -if (s->has_fallocate && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { +if (s->has_fallocate && !(open_flags & BDRV_O_SPARSE) +&& aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, aiocb->aio_nbytes); if (ret == 0 || ret != -ENOTSUP) { return ret; diff --git a/include/block/block.h b/include/block/block.h index bde5ebda18..103313bee0 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -97,6 +97,8 @@ typedef struct HDGeometry { select an appropriate protocol driver, ignoring the format layer */ #define BDRV_O_NO_IO 0x1 /* don't initialize for I/O */ +#define BDRV_O_SPARSE 0x2 /* make guest UNMAP/TRIM operations make image + possibly sparse */ #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH) diff --git a/qemu-nbd.texi b/qemu-nbd.texi index 9a84e81eed..8df4f58ddc 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -63,8 +63,8 @@ and @samp{native} (Linux only). @item --discard=@var{discard} Control whether @dfn{discard} (also known as @dfn{trim} or @dfn{unmap}) requests are ignored or passed to the filesystem. @var{discard} is one of -@samp{ignore} (or @samp{off}), @samp{unmap} (or @samp{on}). The default is -@samp{ignore}. +@samp{ignore} (or @samp{off}), @samp{unmap} (or @samp{on}), or @samp{sparse}. +The default is @samp{ignore}. @item --detect-zeroes=@var{detect-zeroes} Control the automatic conversion of plain zero writes by the OS to driver-specific optimized zero write commands. @var{detect-zeroes} is one of diff --git a/qemu-options.hx b/qemu-options.hx index bf458f83c3..f5c7455fd1 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -539,7 +539,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, " [,serial=s][,addr=A][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" -" [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" +" [,discard=ignore|unmap|sparse][,detect-zeroes=on|off|unmap]\n" " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n" " [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n" " [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n" @@ -582,7 +582,7 @@ These options have the same definition as they have in @option{-hdachs}. @item aio=@var{aio} @var{aio} is "threads"
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Samuel Thibault, on dim. 26 févr. 2017 21:34:27 +0100, wrote: > since we'll want to change the size of the field Ah, no, sorry, it was forced to be 16bit, so at least the size is fine. But I guess we don't want to change the values to have cross-OS compatibility without changing the version. Samuel
Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level
Hello, Dr. David Alan Gilbert, on jeu. 23 févr. 2017 11:40:45 +, wrote: > * Daniel P. Berrange (berra...@redhat.com) wrote: > > IOW if we transmit this data on the wire, we've effectively said that > > our migration data format is *not* portable across different host OS > > platforms. At that point, sending different sized types on BSD vs > > Linux is no big deal since we're already incompatible semantically. > > This is a relatively recent error; it comes from eae303ff which added > ss_family to allow IPv6. > > I suspect the right fix here is to populate a temporary for ss_family > on the wire; if we make it conveniently match Linux's AF_INET6 values > it might work. So this issue is actually not new, so I asked to pull your patch series, thanks! Now we should indeed fix the issue. Making the values match Linux' AF_INET6, why not (better have some known values than arbitrary values), but I don't think we'd want to rely on this: we'll have to introduce a versioned difference, since we'll want to change the size of the field as well as its value on other OSes (and I don't think we want to manage different versioning on different OSes). Samuel
[Qemu-devel] [PULL 1/5] slirp: VMState conversion; tcpcb
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> Convert the migration of the struct tcpcb to use a VMStateDescription, the rest of it will come later. Mostly mechanical, except for conversion of some 'char' to uint8_t to ensure portability. Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Reviewed-by: Juan Quintela <quint...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/slirp.c | 149 slirp/tcp_var.h | 6 +-- 2 files changed, 57 insertions(+), 98 deletions(-) diff --git a/slirp/slirp.c b/slirp/slirp.c index 60539de7a3..276d8cb486 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1129,53 +1129,62 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int guest_port, tcp_output(sototcpcb(so)); } -static void slirp_tcp_save(QEMUFile *f, struct tcpcb *tp) +static int slirp_tcp_post_load(void *opaque, int version) { -int i; +tcp_template((struct tcpcb *)opaque); -qemu_put_sbe16(f, tp->t_state); -for (i = 0; i < TCPT_NTIMERS; i++) -qemu_put_sbe16(f, tp->t_timer[i]); -qemu_put_sbe16(f, tp->t_rxtshift); -qemu_put_sbe16(f, tp->t_rxtcur); -qemu_put_sbe16(f, tp->t_dupacks); -qemu_put_be16(f, tp->t_maxseg); -qemu_put_sbyte(f, tp->t_force); -qemu_put_be16(f, tp->t_flags); -qemu_put_be32(f, tp->snd_una); -qemu_put_be32(f, tp->snd_nxt); -qemu_put_be32(f, tp->snd_up); -qemu_put_be32(f, tp->snd_wl1); -qemu_put_be32(f, tp->snd_wl2); -qemu_put_be32(f, tp->iss); -qemu_put_be32(f, tp->snd_wnd); -qemu_put_be32(f, tp->rcv_wnd); -qemu_put_be32(f, tp->rcv_nxt); -qemu_put_be32(f, tp->rcv_up); -qemu_put_be32(f, tp->irs); -qemu_put_be32(f, tp->rcv_adv); -qemu_put_be32(f, tp->snd_max); -qemu_put_be32(f, tp->snd_cwnd); -qemu_put_be32(f, tp->snd_ssthresh); -qemu_put_sbe16(f, tp->t_idle); -qemu_put_sbe16(f, tp->t_rtt); -qemu_put_be32(f, tp->t_rtseq); -qemu_put_sbe16(f, tp->t_srtt); -qemu_put_sbe16(f, tp->t_rttvar); -qemu_put_be16(f, tp->t_rttmin); -qemu_put_be32(f, tp->max_sndwnd); -qemu_put_byte(f, tp->t_oobflags); -qemu_put_byte(f, tp->t_iobc); -qemu_put_sbe16(f, tp->t_softerror); -qemu_put_byte(f, tp->snd_scale); -qemu_put_byte(f, tp->rcv_scale); -qemu_put_byte(f, tp->request_r_scale); -qemu_put_byte(f, tp->requested_s_scale); -qemu_put_be32(f, tp->ts_recent); -qemu_put_be32(f, tp->ts_recent_age); -qemu_put_be32(f, tp->last_ack_sent); +return 0; } +static const VMStateDescription vmstate_slirp_tcp = { +.name = "slirp-tcp", +.version_id = 0, +.post_load = slirp_tcp_post_load, +.fields = (VMStateField[]) { +VMSTATE_INT16(t_state, struct tcpcb), +VMSTATE_INT16_ARRAY(t_timer, struct tcpcb, TCPT_NTIMERS), +VMSTATE_INT16(t_rxtshift, struct tcpcb), +VMSTATE_INT16(t_rxtcur, struct tcpcb), +VMSTATE_INT16(t_dupacks, struct tcpcb), +VMSTATE_UINT16(t_maxseg, struct tcpcb), +VMSTATE_UINT8(t_force, struct tcpcb), +VMSTATE_UINT16(t_flags, struct tcpcb), +VMSTATE_UINT32(snd_una, struct tcpcb), +VMSTATE_UINT32(snd_nxt, struct tcpcb), +VMSTATE_UINT32(snd_up, struct tcpcb), +VMSTATE_UINT32(snd_wl1, struct tcpcb), +VMSTATE_UINT32(snd_wl2, struct tcpcb), +VMSTATE_UINT32(iss, struct tcpcb), +VMSTATE_UINT32(snd_wnd, struct tcpcb), +VMSTATE_UINT32(rcv_wnd, struct tcpcb), +VMSTATE_UINT32(rcv_nxt, struct tcpcb), +VMSTATE_UINT32(rcv_up, struct tcpcb), +VMSTATE_UINT32(irs, struct tcpcb), +VMSTATE_UINT32(rcv_adv, struct tcpcb), +VMSTATE_UINT32(snd_max, struct tcpcb), +VMSTATE_UINT32(snd_cwnd, struct tcpcb), +VMSTATE_UINT32(snd_ssthresh, struct tcpcb), +VMSTATE_INT16(t_idle, struct tcpcb), +VMSTATE_INT16(t_rtt, struct tcpcb), +VMSTATE_UINT32(t_rtseq, struct tcpcb), +VMSTATE_INT16(t_srtt, struct tcpcb), +VMSTATE_INT16(t_rttvar, struct tcpcb), +VMSTATE_UINT16(t_rttmin, struct tcpcb), +VMSTATE_UINT32(max_sndwnd, struct tcpcb), +VMSTATE_UINT8(t_oobflags, struct tcpcb), +VMSTATE_UINT8(t_iobc, struct tcpcb), +VMSTATE_INT16(t_softerror, struct tcpcb), +VMSTATE_UINT8(snd_scale, struct tcpcb), +VMSTATE_UINT8(rcv_scale, struct tcpcb), +VMSTATE_UINT8(request_r_scale, struct tcpcb), +VMSTATE_UINT8(requested_s_scale, struct tcpcb), +VMSTATE_UINT32(ts_recent, struct tcpcb), +VMSTATE_UINT32(ts_recent_age, struct tcpcb), +VMSTATE_UINT32(last_ack_sent, struct tcpcb), +VMSTATE_END_OF_L
[Qemu-devel] [PULL 5/5] slirp: VMStatify remaining except for loop
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> This converts the remaining components, except for the top level loop, to VMState. Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: Juan Quintela <quint...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/slirp.c | 48 +++- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/slirp/slirp.c b/slirp/slirp.c index 6583de8769..f8ee7f9d95 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1343,15 +1343,25 @@ static const VMStateDescription vmstate_slirp_socket = { } }; -static void slirp_bootp_save(QEMUFile *f, Slirp *slirp) -{ -int i; +static const VMStateDescription vmstate_slirp_bootp_client = { +.name = "slirp_bootpclient", +.fields = (VMStateField[]) { +VMSTATE_UINT16(allocated, BOOTPClient), +VMSTATE_BUFFER(macaddr, BOOTPClient), +VMSTATE_END_OF_LIST() +} +}; -for (i = 0; i < NB_BOOTP_CLIENTS; i++) { -qemu_put_be16(f, slirp->bootp_clients[i].allocated); -qemu_put_buffer(f, slirp->bootp_clients[i].macaddr, 6); +static const VMStateDescription vmstate_slirp = { +.name = "slirp", +.version_id = 4, +.fields = (VMStateField[]) { +VMSTATE_UINT16_V(ip_id, Slirp, 2), +VMSTATE_STRUCT_ARRAY(bootp_clients, Slirp, NB_BOOTP_CLIENTS, 3, + vmstate_slirp_bootp_client, BOOTPClient), +VMSTATE_END_OF_LIST() } -} +}; static void slirp_state_save(QEMUFile *f, void *opaque) { @@ -1371,22 +1381,10 @@ static void slirp_state_save(QEMUFile *f, void *opaque) } qemu_put_byte(f, 0); -qemu_put_be16(f, slirp->ip_id); - -slirp_bootp_save(f, slirp); +vmstate_save_state(f, _slirp, slirp, NULL); } -static void slirp_bootp_load(QEMUFile *f, Slirp *slirp) -{ -int i; - -for (i = 0; i < NB_BOOTP_CLIENTS; i++) { -slirp->bootp_clients[i].allocated = qemu_get_be16(f); -qemu_get_buffer(f, slirp->bootp_clients[i].macaddr, 6); -} -} - static int slirp_state_load(QEMUFile *f, void *opaque, int version_id) { Slirp *slirp = opaque; @@ -1421,13 +1419,5 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id) so->extra = (void *)ex_ptr->ex_exec; } -if (version_id >= 2) { -slirp->ip_id = qemu_get_be16(f); -} - -if (version_id >= 3) { -slirp_bootp_load(f, slirp); -} - -return 0; +return vmstate_load_state(f, _slirp, slirp, version_id); } -- 2.11.0
[Qemu-devel] [PULL 4/5] slirp: VMStatify socket level
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> Working up the stack, this replaces the slirp_socket_load/save with VMState definitions. A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there. Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: Juan Quintela <quint...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/slirp.c | 170 ++--- slirp/socket.h | 6 +- 2 files changed, 93 insertions(+), 83 deletions(-) diff --git a/slirp/slirp.c b/slirp/slirp.c index 178c2b6d14..6583de8769 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1250,40 +1250,99 @@ static const VMStateDescription vmstate_slirp_sbuf = { } }; +static bool slirp_older_than_v4(void *opaque, int version_id) +{ +return version_id < 4; +} -static void slirp_socket_save(QEMUFile *f, struct socket *so) +static bool slirp_family_inet(void *opaque, int version_id) { -qemu_put_be32(f, so->so_urgc); -qemu_put_be16(f, so->so_ffamily); -switch (so->so_ffamily) { -case AF_INET: -qemu_put_be32(f, so->so_faddr.s_addr); -qemu_put_be16(f, so->so_fport); -break; -default: -error_report("so_ffamily unknown, unable to save so_faddr and" - " so_fport"); -} -qemu_put_be16(f, so->so_lfamily); -switch (so->so_lfamily) { -case AF_INET: -qemu_put_be32(f, so->so_laddr.s_addr); -qemu_put_be16(f, so->so_lport); -break; -default: -error_report("so_ffamily unknown, unable to save so_laddr and" - " so_lport"); +union slirp_sockaddr *ssa = (union slirp_sockaddr *)opaque; +return ssa->ss.ss_family == AF_INET; +} + +static int slirp_socket_pre_load(void *opaque) +{ +struct socket *so = opaque; +if (tcp_attach(so) < 0) { +return -ENOMEM; } -qemu_put_byte(f, so->so_iptos); -qemu_put_byte(f, so->so_emu); -qemu_put_byte(f, so->so_type); -qemu_put_be32(f, so->so_state); -/* TODO: Build vmstate at this level */ -vmstate_save_state(f, _slirp_sbuf, >so_rcv, 0); -vmstate_save_state(f, _slirp_sbuf, >so_snd, 0); -vmstate_save_state(f, _slirp_tcp, so->so_tcpcb, 0); +/* Older versions don't load these fields */ +so->so_ffamily = AF_INET; +so->so_lfamily = AF_INET; +return 0; } +#ifndef _WIN32 +#define VMSTATE_SS_FAMILY(f, s) VMSTATE_UINT16(f, s) +#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_UINT32_TEST(f, s, t) +#else +/* Win has a signed family number */ +#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s) +/* Win uses u_long rather than uint32_t - but it's still 32bits long */ +#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_SINGLE_TEST(f, s, t, 0, \ + vmstate_info_uint32, u_long) +#endif + +static const VMStateDescription vmstate_slirp_socket_addr = { +.name = "slirp-socket-addr", +.version_id = 4, +.fields = (VMStateField[]) { +VMSTATE_SS_FAMILY(ss.ss_family, union slirp_sockaddr), +VMSTATE_SIN4_ADDR(sin.sin_addr.s_addr, union slirp_sockaddr, +slirp_family_inet), +VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr, +slirp_family_inet), + +#if 0 +/* Untested: Needs checking by someone with IPv6 test */ +VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr, +slirp_family_inet6), +VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr, +slirp_family_inet6), +VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr, +slirp_family_inet6), +VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr, +slirp_family_inet6), +#endif + +VMSTATE_END_OF_LIST() +} +}; + +static const VMStateDescription vmstate_slirp_socket = { +.name = "slirp-socket", +.version_id = 4, +.pre_load = slirp_socket_pre_load, +.fields = (VMStateField[]) { +VMSTATE_UINT32(so_urgc, struct socket), +/* Pre-v4 versions */ +VMSTATE_SIN4_ADDR(so_faddr.s_addr, struct socket, +slirp_older_than_v4), +VMSTATE_SIN4_ADDR(so_laddr.s_addr, struct socket, +slirp_older_than_v4), +VMSTATE_UINT16_TEST(so_fport, struct socket, slirp_older_than_v4), +VMSTATE_UINT16_TEST(so_lport, struct socket, slirp_older_than_v4), +/* v4 and newer */ +VMSTATE_STRUCT(fhost, struct socket, 4, vmstate_slirp_socket_addr, + union slirp_sockaddr), +VMSTATE_STRUCT(lhost, struct socket, 4, vmstate_slirp_socket_addr, +
[Qemu-devel] [PULL 2/5] slirp: VMStatify sbuf
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> Convert the sbuf structure to a VMStateDescription. Note this uses the VMSTATE_WITH_TMP mechanism to calculate and reload the offsets based on the pointers. Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> Reviewed-by: Juan Quintela <quint...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/sbuf.h | 4 +- slirp/slirp.c | 116 ++ 2 files changed, 78 insertions(+), 42 deletions(-) diff --git a/slirp/sbuf.h b/slirp/sbuf.h index efcec39a6b..a722ecb629 100644 --- a/slirp/sbuf.h +++ b/slirp/sbuf.h @@ -12,8 +12,8 @@ #define sbspace(sb) ((sb)->sb_datalen - (sb)->sb_cc) struct sbuf { - u_int sb_cc; /* actual chars in buffer */ - u_int sb_datalen; /* Length of data */ + uint32_t sb_cc; /* actual chars in buffer */ + uint32_t sb_datalen;/* Length of data */ char*sb_wptr; /* write pointer. points to where the next * bytes should be written in the sbuf */ char*sb_rptr; /* read pointer. points to where the next diff --git a/slirp/slirp.c b/slirp/slirp.c index 276d8cb486..178c2b6d14 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1185,19 +1185,72 @@ static const VMStateDescription vmstate_slirp_tcp = { } }; -static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf) +/* The sbuf has a pair of pointers that are migrated as offsets; + * we calculate the offsets and restore the pointers using + * pre_save/post_load on a tmp structure. + */ +struct sbuf_tmp { +struct sbuf *parent; +uint32_t roff, woff; +}; + +static void sbuf_tmp_pre_save(void *opaque) +{ +struct sbuf_tmp *tmp = opaque; +tmp->woff = tmp->parent->sb_wptr - tmp->parent->sb_data; +tmp->roff = tmp->parent->sb_rptr - tmp->parent->sb_data; +} + +static int sbuf_tmp_post_load(void *opaque, int version) { -uint32_t off; - -qemu_put_be32(f, sbuf->sb_cc); -qemu_put_be32(f, sbuf->sb_datalen); -off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data); -qemu_put_sbe32(f, off); -off = (uint32_t)(sbuf->sb_rptr - sbuf->sb_data); -qemu_put_sbe32(f, off); -qemu_put_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen); +struct sbuf_tmp *tmp = opaque; +uint32_t requested_len = tmp->parent->sb_datalen; + +/* Allocate the buffer space used by the field after the tmp */ +sbreserve(tmp->parent, tmp->parent->sb_datalen); + +if (tmp->parent->sb_datalen != requested_len) { +return -ENOMEM; +} +if (tmp->woff >= requested_len || +tmp->roff >= requested_len) { +error_report("invalid sbuf offsets r/w=%u/%u len=%u", + tmp->roff, tmp->woff, requested_len); +return -EINVAL; +} + +tmp->parent->sb_wptr = tmp->parent->sb_data + tmp->woff; +tmp->parent->sb_rptr = tmp->parent->sb_data + tmp->roff; + +return 0; } + +static const VMStateDescription vmstate_slirp_sbuf_tmp = { +.name = "slirp-sbuf-tmp", +.post_load = sbuf_tmp_post_load, +.pre_save = sbuf_tmp_pre_save, +.version_id = 0, +.fields = (VMStateField[]) { +VMSTATE_UINT32(woff, struct sbuf_tmp), +VMSTATE_UINT32(roff, struct sbuf_tmp), +VMSTATE_END_OF_LIST() +} +}; + +static const VMStateDescription vmstate_slirp_sbuf = { +.name = "slirp-sbuf", +.version_id = 0, +.fields = (VMStateField[]) { +VMSTATE_UINT32(sb_cc, struct sbuf), +VMSTATE_UINT32(sb_datalen, struct sbuf), +VMSTATE_WITH_TMP(struct sbuf, struct sbuf_tmp, vmstate_slirp_sbuf_tmp), +VMSTATE_VBUFFER_UINT32(sb_data, struct sbuf, 0, NULL, sb_datalen), +VMSTATE_END_OF_LIST() +} +}; + + static void slirp_socket_save(QEMUFile *f, struct socket *so) { qemu_put_be32(f, so->so_urgc); @@ -1225,8 +1278,9 @@ static void slirp_socket_save(QEMUFile *f, struct socket *so) qemu_put_byte(f, so->so_emu); qemu_put_byte(f, so->so_type); qemu_put_be32(f, so->so_state); -slirp_sbuf_save(f, >so_rcv); -slirp_sbuf_save(f, >so_snd); +/* TODO: Build vmstate at this level */ +vmstate_save_state(f, _slirp_sbuf, >so_rcv, 0); +vmstate_save_state(f, _slirp_sbuf, >so_snd, 0); vmstate_save_state(f, _slirp_tcp, so->so_tcpcb, 0); } @@ -1263,31 +1317,9 @@ static void slirp_state_save(QEMUFile *f, void *opaque) slirp_bootp_save(f, slirp); } -static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf) -{ -uint32_t off, sb_cc, sb_datalen; - -sb_cc = qemu_get_be32(f); -sb_datalen = qemu_get_be32(f); - -sbreserve(sbuf, sb_da
[Qemu-devel] [PULL 3/5] slirp: Common lhost/fhost union
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> The socket structure has a pair of unions for lhost and fhost addresses; the unions are identical so split them out into a separate union declaration. Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Reviewed-by: Juan Quintela <quint...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/socket.h | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/slirp/socket.h b/slirp/socket.h index 8feed2aea4..c1be77eaf3 100644 --- a/slirp/socket.h +++ b/slirp/socket.h @@ -15,6 +15,12 @@ * Our socket structure */ +union slirp_sockaddr { +struct sockaddr_storage ss; +struct sockaddr_in sin; +struct sockaddr_in6 sin6; +}; + struct socket { struct socket *so_next,*so_prev; /* For a linked list of sockets */ @@ -31,22 +37,14 @@ struct socket { struct tcpiphdr *so_ti; /* Pointer to the original ti within * so_mconn, for non-blocking connections */ int so_urgc; - union { /* foreign host */ - struct sockaddr_storage ss; - struct sockaddr_in sin; - struct sockaddr_in6 sin6; - } fhost; + union slirp_sockaddr fhost; /* Foreign host */ #define so_faddr fhost.sin.sin_addr #define so_fport fhost.sin.sin_port #define so_faddr6 fhost.sin6.sin6_addr #define so_fport6 fhost.sin6.sin6_port #define so_ffamily fhost.ss.ss_family - union { /* local host */ - struct sockaddr_storage ss; - struct sockaddr_in sin; - struct sockaddr_in6 sin6; - } lhost; + union slirp_sockaddr lhost; /* Local host */ #define so_laddr lhost.sin.sin_addr #define so_lport lhost.sin.sin_port #define so_laddr6 lhost.sin6.sin6_addr -- 2.11.0
[Qemu-devel] [PULL 0/5] slirp updates
The following changes since commit 685783c5b69c83c942d1fc21679311eeb8f79ab9: Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2017-02-26 16:38:40 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to c363a5b7f9ca9e802665587900b7ea1aefcf26ea: slirp: VMStatify remaining except for loop (2017-02-26 21:16:38 +0100) slirp updates Dr. David Alan Gilbert (5): slirp: VMState conversion; tcpcb slirp: VMStatify sbuf slirp: Common lhost/fhost union slirp: VMStatify socket level slirp: VMStatify remaining except for loop slirp/sbuf.h| 4 +- slirp/slirp.c | 449 slirp/socket.h | 24 ++- slirp/tcp_var.h | 6 +- 4 files changed, 238 insertions(+), 245 deletions(-)
[Qemu-devel] [PULL 2/3] slirp: Convert mbufs to use g_malloc() and g_free()
From: Peter Maydell <peter.mayd...@linaro.org> The mbuf code currently doesn't check the result of doing a malloc() or realloc() of its data (spotted by Coverity, CID 1238946). Since the m_inc() API assumes that extending an mbuf must succeed, just convert to g_malloc() and g_free(). Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/mbuf.c | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/slirp/mbuf.c b/slirp/mbuf.c index 7eddc217e4..5ff24559fd 100644 --- a/slirp/mbuf.c +++ b/slirp/mbuf.c @@ -10,7 +10,7 @@ * FreeBSD. They are fixed size, determined by the MTU, * so that one whole packet can fit. Mbuf's cannot be * chained together. If there's more data than the mbuf - * could hold, an external malloced buffer is pointed to + * could hold, an external g_malloced buffer is pointed to * by m_ext (and the data pointers) and M_EXT is set in * the flags */ @@ -41,26 +41,26 @@ void m_cleanup(Slirp *slirp) while ((struct quehead *) m != >m_usedlist) { next = m->m_next; if (m->m_flags & M_EXT) { -free(m->m_ext); +g_free(m->m_ext); } -free(m); +g_free(m); m = next; } m = (struct mbuf *) slirp->m_freelist.qh_link; while ((struct quehead *) m != >m_freelist) { next = m->m_next; -free(m); +g_free(m); m = next; } } /* * Get an mbuf from the free list, if there are none - * malloc one + * allocate one * * Because fragmentation can occur if we alloc new mbufs and * free old mbufs, we mark all mbufs above mbuf_thresh as M_DOFREE, - * which tells m_free to actually free() it + * which tells m_free to actually g_free() it */ struct mbuf * m_get(Slirp *slirp) @@ -71,8 +71,7 @@ m_get(Slirp *slirp) DEBUG_CALL("m_get"); if (slirp->m_freelist.qh_link == >m_freelist) { - m = (struct mbuf *)malloc(SLIRP_MSIZE); - if (m == NULL) goto end_error; +m = g_malloc(SLIRP_MSIZE); slirp->mbuf_alloced++; if (slirp->mbuf_alloced > MBUF_THRESH) flags = M_DOFREE; @@ -94,7 +93,6 @@ m_get(Slirp *slirp) m->m_prevpkt = NULL; m->resolution_requested = false; m->expiration_date = (uint64_t)-1; -end_error: DEBUG_ARG("m = %p", m); return m; } @@ -112,15 +110,15 @@ m_free(struct mbuf *m) remque(m); /* If it's M_EXT, free() it */ - if (m->m_flags & M_EXT) - free(m->m_ext); - +if (m->m_flags & M_EXT) { +g_free(m->m_ext); +} /* * Either free() it or put it on the free list */ if (m->m_flags & M_DOFREE) { m->slirp->mbuf_alloced--; - free(m); +g_free(m); } else if ((m->m_flags & M_FREELIST) == 0) { insque(m,>slirp->m_freelist); m->m_flags = M_FREELIST; /* Clobber other flags */ @@ -130,7 +128,7 @@ m_free(struct mbuf *m) /* * Copy data from one mbuf to the end of - * the other.. if result is too big for one mbuf, malloc() + * the other.. if result is too big for one mbuf, allocate * an M_EXT data segment */ void @@ -160,12 +158,12 @@ m_inc(struct mbuf *m, int size) if (m->m_flags & M_EXT) { datasize = m->m_data - m->m_ext; - m->m_ext = (char *)realloc(m->m_ext,size); + m->m_ext = g_realloc(m->m_ext, size); m->m_data = m->m_ext + datasize; } else { char *dat; datasize = m->m_data - m->m_dat; - dat = (char *)malloc(size); + dat = g_malloc(size); memcpy(dat, m->m_dat, m->m_size); m->m_ext = dat; -- 2.11.0
[Qemu-devel] [PULL 0/3] slirp updates
The following changes since commit 6528a4c1f20c1ba5a22ab84bec6788a574ac04c8: Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2017-02-26 11:47:00 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to bd5d2353aa69e68e45d8a89787bab17c155e9e24: slirp: tcp_listen(): Don't try to close() an fd we never opened (2017-02-26 15:39:29 +0100) slirp updates Peter Maydell (3): slirp: Check qemu_socket() return value in udp_listen() slirp: Convert mbufs to use g_malloc() and g_free() slirp: tcp_listen(): Don't try to close() an fd we never opened slirp/mbuf.c | 30 ++ slirp/socket.c | 4 +++- slirp/udp.c| 4 3 files changed, 21 insertions(+), 17 deletions(-)
[Qemu-devel] [PULL 1/3] slirp: Check qemu_socket() return value in udp_listen()
From: Peter Maydell <peter.mayd...@linaro.org> Check the return value from qemu_socket() rather than trying to pass it to bind() as an fd argument even if it's negative. This wouldn't have caused any negative consequences, because it won't be a valid fd number and the bind call will fail; but Coverity complains (CID 1005723). Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/udp.c | 4 1 file changed, 4 insertions(+) diff --git a/slirp/udp.c b/slirp/udp.c index 93d7224792..227d779022 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -335,6 +335,10 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, return NULL; } so->s = qemu_socket(AF_INET,SOCK_DGRAM,0); +if (so->s < 0) { +sofree(so); +return NULL; +} so->so_expire = curtime + SO_EXPIRE; insque(so, >udb); -- 2.11.0
[Qemu-devel] [PULL 3/3] slirp: tcp_listen(): Don't try to close() an fd we never opened
From: Peter Maydell <peter.mayd...@linaro.org> Coverity points out (CID 1005725) that an error-exit path in tcp_listen() will try to close(s) even if the reason it got there was that the qemu_socket() failed and s was never opened. Not only that, this isn't even the right function to use, because we need closesocket() to do the right thing on Windows. Change to using the right function and only calling it if needed. Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/socket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/slirp/socket.c b/slirp/socket.c index 6c18971368..86927722e1 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -713,7 +713,9 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, (listen(s,1) < 0)) { int tmperrno = errno; /* Don't clobber the real reason we failed */ - close(s); +if (s >= 0) { +closesocket(s); +} sofree(so); /* Restore the real errno */ #ifdef _WIN32 -- 2.11.0
Re: [Qemu-devel] libslirp and QEMU slirp
Hello, Stefan Hajnoczi, on Mon 06 Feb 2017 11:55:05 +, wrote: > I'm proposing this topic for discussion because there might be common > ground for Samuel from QEMU and the VDE folks to collaborate. > > Samuel: Should QEMU join forces with libslirp? Well, there are not that many forces from the QEMU side :) Personally, I'm fine with seeing slirp move to a separate playground that qemu would depend on and others could contribute to. It just needs to be integrated to distros so that qemu can get it. That being said, the integration of slirp and qemu is not so loose, notably on the top of my head: - we are using timers for icmp announcements - qemu needs to be able to save/restore state, with compatibility with previous versions We'd need to see how that can be expressed as a library. Samuel
[Qemu-devel] [PATCH] Drop duplicate display option documentation
The curses and none possibilities are already documented on a separate line, so documenting it on the sdl line was both unneeded and confusing. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/qemu-options.hx b/qemu-options.hx index c534a2f7f9..9c81d3752d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -927,7 +927,7 @@ ETEXI DEF("display", HAS_ARG, QEMU_OPTION_display, "-display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n" -"[,window_close=on|off][,gl=on|off]|curses|none|\n" +"[,window_close=on|off][,gl=on|off]\n" "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n" "-display vnc=[,]\n" "-display curses\n"
Re: [Qemu-devel] [PULL v2 05/11] sdl2: set window ID
Hello, Gerd Hoffmann, on Thu 12 Jan 2017 16:56:49 +0100, wrote: > On Do, 2017-01-12 at 16:10 +0100, Stefan Weil wrote: > > This commit breaks builds for Windows. See below for details. > > > Windows does not use X11. gcc fails: > > > > CC ui/sdl2.o > > /qemu/ui/sdl2.c: In function ‘sdl_display_init’: > > /qemu/ui/sdl2.c:821:54: error: ‘union ’ has no member named ‘x11’ > > qemu_console_set_window_id(con, info.info.x11.window); > > Oops. Oops, sorry, it seems to have been overlooked indeed. Can you easily test the attached patch? Samuel diff --git a/ui/sdl2.c b/ui/sdl2.c index 9a79b17b92..0f41d1fa90 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -762,6 +762,9 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) char *filename; int i; SDL_SysWMinfo info; +#if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11) +int window_id; +#endif if (no_frame) { gui_noframe = 1; @@ -817,9 +820,17 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) sdl2_console[i].dcl.con = con; register_displaychangelistener(_console[i].dcl); +#if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11) if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, )) { -qemu_console_set_window_id(con, info.info.x11.window); +#ifdef SDL_VIDEO_DRIVER_WINDOWS +window_id = (int)(uintptr_t) info.info.win.hwnd; +#endif +#ifdef SDL_VIDEO_DRIVER_X11 +window_id = info.info.x11.window; +#endif +qemu_console_set_window_id(con, window_id); } +#endif } /* Load a 32x32x4 image. White pixels are transparent. */
[Qemu-devel] [PATCH 2/3] console: move window ID code from baum to sdl
This moves the SDL bits for window ID from the baum driver to SDL, as well as fixing the build for non-X11. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- Difference from v3: Use qemu_console_lookup_by_index and qemu_console_is_graphic to access the console --- backends/baum.c | 31 --- ui/sdl.c| 25 + 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/backends/baum.c b/backends/baum.c index b92369d840..b045ef49c5 100644 --- a/backends/baum.c +++ b/backends/baum.c @@ -27,12 +27,10 @@ #include "sysemu/char.h" #include "qemu/timer.h" #include "hw/usb.h" +#include "ui/console.h" #include #include #include -#ifdef CONFIG_SDL -#include -#endif #if 0 #define DPRINTF(fmt, ...) \ @@ -227,12 +225,8 @@ static const uint8_t nabcc_translation[2][256] = { /* The guest OS has started discussing with us, finish initializing BrlAPI */ static int baum_deferred_init(BaumDriverState *baum) { -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -SDL_SysWMinfo info; -#endif -#endif -int tty; +int tty = BRLAPI_TTY_DEFAULT; +QemuConsole *con; if (baum->deferred_init) { return 1; @@ -243,21 +237,12 @@ static int baum_deferred_init(BaumDriverState *baum) return 0; } -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -memset(, 0, sizeof(info)); -SDL_VERSION(); -if (SDL_GetWMInfo()) { -tty = info.info.x11.wmwindow; -} else { -#endif -#endif -tty = BRLAPI_TTY_DEFAULT; -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) +con = qemu_console_lookup_by_index(0); +if (con && qemu_console_is_graphic(con)) { +tty = qemu_console_get_window_id(con); +if (tty == -1) +tty = BRLAPI_TTY_DEFAULT; } -#endif -#endif if (brlapi__enterTtyMode(baum->brlapi, tty, NULL) == -1) { brlapi_perror("baum: brlapi__enterTtyMode"); diff --git a/ui/sdl.c b/ui/sdl.c index d8cf5bcf74..19e8a848a7 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -947,6 +947,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) int flags; uint8_t data = 0; const SDL_VideoInfo *vi; +SDL_SysWMinfo info; char *filename; #if defined(__APPLE__) @@ -1023,5 +1024,29 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) sdl_cursor_hidden = SDL_CreateCursor(, , 8, 1, 0, 0); sdl_cursor_normal = SDL_GetCursor(); +memset(, 0, sizeof(info)); +SDL_VERSION(); +if (SDL_GetWMInfo()) { +int i; +for (i = 0; ; i++) { +/* All consoles share the same window */ +QemuConsole *con = qemu_console_lookup_by_index(i); +if (con) { +#if defined(SDL_VIDEO_DRIVER_X11) +qemu_console_set_window_id(con, info.info.x11.wmwindow); +#elif defined(SDL_VIDEO_DRIVER_NANOX) || \ + defined(SDL_VIDEO_DRIVER_WINDIB) || defined(SDL_VIDEO_DRIVER_DDRAW) || \ + defined(SDL_VIDEO_DRIVER_GAPI) || \ + defined(SDL_VIDEO_DRIVER_RISCOS) +qemu_console_set_window_id(con, (int) (uintptr_t) info.window); +#else +qemu_console_set_window_id(con, info.data); +#endif +} else { +break; +} +} +} + atexit(sdl_cleanup); } -- 2.11.0
[Qemu-devel] [PATCH 3/3] sdl2: set window ID
This uses the console API to record the window ID of the SDL2 windows. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- ui/sdl2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/sdl2.c b/ui/sdl2.c index 30d2a3c35d..9a79b17b92 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -761,6 +761,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) uint8_t data = 0; char *filename; int i; +SDL_SysWMinfo info; if (no_frame) { gui_noframe = 1; @@ -786,6 +787,8 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) exit(1); } SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1"); +memset(, 0, sizeof(info)); +SDL_VERSION(); for (i = 0;; i++) { QemuConsole *con = qemu_console_lookup_by_index(i); @@ -813,6 +816,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) #endif sdl2_console[i].dcl.con = con; register_displaychangelistener(_console[i].dcl); + +if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, )) { +qemu_console_set_window_id(con, info.info.x11.window); +} } /* Load a 32x32x4 image. White pixels are transparent. */ -- 2.11.0
[Qemu-devel] [PATCH 1/3] console: add API to get underlying gui window ID
This adds two console functions, qemu_console_set_window_id and qemu_graphic_console_get_window_id, to let graphical backend record the window id in the QemuConsole structure, and let the baum driver read it. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- Difference from v4: select console with a QemuConsole* parameter instead of an integer index. --- include/ui/console.h | 4 ui/console.c | 11 +++ 2 files changed, 15 insertions(+) diff --git a/include/ui/console.h b/include/ui/console.h index e2589e2134..ee8c407cac 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -394,6 +394,10 @@ uint32_t qemu_console_get_head(QemuConsole *con); QemuUIInfo *qemu_console_get_ui_info(QemuConsole *con); int qemu_console_get_width(QemuConsole *con, int fallback); int qemu_console_get_height(QemuConsole *con, int fallback); +/* Return the low-level window id for the console */ +int qemu_console_get_window_id(QemuConsole *con); +/* Set the low-level window id for the console */ +void qemu_console_set_window_id(QemuConsole *con, int window_id); void console_select(unsigned int index); void qemu_console_resize(QemuConsole *con, int width, int height); diff --git a/ui/console.c b/ui/console.c index ed888e55ea..b9575f2ee5 100644 --- a/ui/console.c +++ b/ui/console.c @@ -124,6 +124,7 @@ struct QemuConsole { int dcls; DisplayChangeListener *gl; bool gl_block; +int window_id; /* Graphic console state. */ Object *device; @@ -273,6 +274,16 @@ void graphic_hw_gl_block(QemuConsole *con, bool block) } } +int qemu_console_get_window_id(QemuConsole *con) +{ +return con->window_id; +} + +void qemu_console_set_window_id(QemuConsole *con, int window_id) +{ +con->window_id = window_id; +} + void graphic_hw_invalidate(QemuConsole *con) { if (!con) { -- 2.11.0
[Qemu-devel] [PATCHv4 0/3] Move getting XWindow ID from baum driver to graphical backend
Hello, This is a respin of moving getting XWindow ID from baum to actual graphical backends. This only contains the new API, the move of existing code to the new API, and the addition of support for SDL2. Gtk will need more discussion and work. Compared to v3, this makes the API use a QemuConsole* parameter instead of an integer index, Samuel Samuel Thibault (3): console: add API to get underlying gui window ID console: move window ID code from baum to sdl sdl2: set window ID backends/baum.c | 31 --- include/ui/console.h | 4 ui/console.c | 11 +++ ui/sdl.c | 25 + ui/sdl2.c| 7 +++ 5 files changed, 55 insertions(+), 23 deletions(-) -- 2.11.0
Re: [Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks
no-re...@patchew.org, on Tue 20 Dec 2016 15:15:43 -0800, wrote: > Your series seems to have some coding style problems. See output below for > more information: > ERROR: suspect code indent for conditional statements (10, 14) > #90: FILE: slirp/tftp.c:393: > + if (blksize > 0) { > + spt->block_size = MIN(blksize, TFTP_BLOCKSIZE_MAX); This is a false positive. Samuel
Re: [Qemu-devel] [PATCH] slirp: support dynamic block size for TFTP transfers
Hervé Poussineau, on Mon 21 Nov 2016 20:45:49 +0100, wrote: > The blocksize option is defined in RFC 1783. > We now support block sizes between 1 and 1432 bytes, instead of 512 only. > > Signed-off-by: Hervé PoussineauI fixed a couple of trivial things and commited to my tree, thanks! Samuel
[Qemu-devel] [PULL 1/2] slirp, disas: Replace min/max with MIN/MAX macros
From: Yuval Shaia <yuval.sh...@oracle.com> Signed-off-by: Yuval Shaia <yuval.sh...@oracle.com> Reviewed-by: Markus Armbruster <arm...@redhat.com> Reviewed-by: Laurent Vivier <lviv...@redhat.com> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- disas/m68k.c | 8 ++-- slirp/dhcpv6.c | 2 +- slirp/ip6_icmp.c | 2 +- slirp/slirp.c | 2 +- slirp/slirp.h | 5 - slirp/tcp_input.c | 16 slirp/tcp_output.c | 6 +++--- slirp/tcp_timer.c | 2 +- slirp/tcpip.h | 2 +- 9 files changed, 18 insertions(+), 27 deletions(-) diff --git a/disas/m68k.c b/disas/m68k.c index 8e7c3f76c4..073abb9efd 100644 --- a/disas/m68k.c +++ b/disas/m68k.c @@ -4698,10 +4698,6 @@ get_field (const unsigned char *data, enum floatformat_byteorders order, return result; } -#ifndef min -#define min(a, b) ((a) < (b) ? (a) : (b)) -#endif - /* Convert from FMT to a double. FROM is the address of the extended float. Store the double in *TO. */ @@ -4733,7 +4729,7 @@ floatformat_to_double (const struct floatformat *fmt, nan = 0; while (mant_bits_left > 0) { - mant_bits = min (mant_bits_left, 32); + mant_bits = MIN(mant_bits_left, 32); if (get_field (ufrom, fmt->byteorder, fmt->totalsize, mant_off, mant_bits) != 0) @@ -4793,7 +4789,7 @@ floatformat_to_double (const struct floatformat *fmt, while (mant_bits_left > 0) { - mant_bits = min (mant_bits_left, 32); + mant_bits = MIN(mant_bits_left, 32); mant = get_field (ufrom, fmt->byteorder, fmt->totalsize, mant_off, mant_bits); diff --git a/slirp/dhcpv6.c b/slirp/dhcpv6.c index 02c51c7756..d266611e85 100644 --- a/slirp/dhcpv6.c +++ b/slirp/dhcpv6.c @@ -168,7 +168,7 @@ static void dhcpv6_info_request(Slirp *slirp, struct sockaddr_in6 *srcsas, sa[0], sa[1], sa[2], sa[3], sa[4], sa[5], sa[6], sa[7], sa[8], sa[9], sa[10], sa[11], sa[12], sa[13], sa[14], sa[15], slirp->bootp_filename); -slen = min(slen, smaxlen); +slen = MIN(slen, smaxlen); *resp++ = slen >> 8;/* option-len high byte */ *resp++ = slen; /* option-len low byte */ resp += slen; diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c index 6d18e28985..298a48dd25 100644 --- a/slirp/ip6_icmp.c +++ b/slirp/ip6_icmp.c @@ -95,7 +95,7 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code) #endif rip->ip_nh = IPPROTO_ICMPV6; -const int error_data_len = min(m->m_len, +const int error_data_len = MIN(m->m_len, IF_MTU - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN)); rip->ip_pl = htons(ICMP6_ERROR_MINLEN + error_data_len); t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl); diff --git a/slirp/slirp.c b/slirp/slirp.c index 6e2b4e5a90..60539de7a3 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -774,7 +774,7 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error) static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) { struct slirp_arphdr *ah = (struct slirp_arphdr *)(pkt + ETH_HLEN); -uint8_t arp_reply[max(ETH_HLEN + sizeof(struct slirp_arphdr), 64)]; +uint8_t arp_reply[MAX(ETH_HLEN + sizeof(struct slirp_arphdr), 64)]; struct ethhdr *reh = (struct ethhdr *)arp_reply; struct slirp_arphdr *rah = (struct slirp_arphdr *)(arp_reply + ETH_HLEN); int ar_op; diff --git a/slirp/slirp.h b/slirp/slirp.h index a1f3139134..3877f667f0 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -292,9 +292,4 @@ int tcp_emu(struct socket *, struct mbuf *); int tcp_ctl(struct socket *); struct tcpcb *tcp_drop(struct tcpcb *tp, int err); -#ifndef _WIN32 -#define min(x,y) ((x) < (y) ? (x) : (y)) -#define max(x,y) ((x) > (y) ? (x) : (y)) -#endif - #endif diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c index c5063a918d..edb98f06f3 100644 --- a/slirp/tcp_input.c +++ b/slirp/tcp_input.c @@ -596,7 +596,7 @@ findso: win = sbspace(>so_rcv); if (win < 0) win = 0; - tp->rcv_wnd = max(win, (int)(tp->rcv_adv - tp->rcv_nxt)); + tp->rcv_wnd = MAX(win, (int)(tp->rcv_adv - tp->rcv_nxt)); } switch (tp->t_state) { @@ -1065,8 +1065,8 @@ trimthenstep6: else if (++tp->t_dupacks == TCPREXMTTHRESH) { tcp_seq onxt = tp->snd_nxt; u_int win = - min(tp->snd_wnd, tp->snd_cwnd) / 2 / - tp->t_maxseg; +MIN(tp->snd_wnd, tp->snd_cwnd) / +2 / tp-&
[Qemu-devel] [PULL 2/2] slirp: support dynamic block size for TFTP transfers
From: Hervé Poussineau <hpous...@reactos.org> The blocksize option is defined in RFC 1783 and RFC 2348. We now support block sizes between 1 and 1428 bytes, instead of 512 only. Signed-off-by: Hervé Poussineau <hpous...@reactos.org> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/tftp.c | 26 ++ slirp/tftp.h | 8 +--- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index c1859066cc..50e714807d 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -72,6 +72,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas, memset(spt, 0, sizeof(*spt)); spt->client_addr = *srcsas; spt->fd = -1; + spt->block_size = 512; spt->client_port = tp->udp.uh_sport; spt->slirp = slirp; @@ -115,7 +116,7 @@ static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, } if (len) { -lseek(spt->fd, block_nr * 512, SEEK_SET); +lseek(spt->fd, block_nr * spt->block_size, SEEK_SET); bytes_read = read(spt->fd, buf, len); } @@ -189,7 +190,8 @@ static int tftp_send_oack(struct tftp_session *spt, values[i]) + 1; } -m->m_len = sizeof(struct tftp_t) - 514 + n - sizeof(struct udphdr); +m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX + 2) + n + - sizeof(struct udphdr); tftp_udp_output(spt, m, recv_tp); return 0; @@ -214,7 +216,7 @@ static void tftp_send_error(struct tftp_session *spt, tp->x.tp_error.tp_error_code = htons(errorcode); pstrcpy((char *)tp->x.tp_error.tp_msg, sizeof(tp->x.tp_error.tp_msg), msg); - m->m_len = sizeof(struct tftp_t) - 514 + 3 + strlen(msg) + m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX + 2) + 3 + strlen(msg) - sizeof(struct udphdr); tftp_udp_output(spt, m, recv_tp); @@ -240,7 +242,8 @@ static void tftp_send_next_block(struct tftp_session *spt, tp->tp_op = htons(TFTP_DATA); tp->x.tp_data.tp_block_nr = htons((spt->block_nr + 1) & 0x); - nobytes = tftp_read_data(spt, spt->block_nr, tp->x.tp_data.tp_buf, 512); + nobytes = tftp_read_data(spt, spt->block_nr, tp->x.tp_data.tp_buf, + spt->block_size); if (nobytes < 0) { m_free(m); @@ -252,10 +255,11 @@ static void tftp_send_next_block(struct tftp_session *spt, return; } - m->m_len = sizeof(struct tftp_t) - (512 - nobytes) - sizeof(struct udphdr); + m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX - nobytes) + - sizeof(struct udphdr); tftp_udp_output(spt, m, recv_tp); - if (nobytes == 512) { + if (nobytes == spt->block_size) { tftp_session_update(spt); } else { @@ -385,13 +389,11 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas, } else if (strcasecmp(key, "blksize") == 0) { int blksize = atoi(value); - /* If blksize option is bigger than what we will - * emit, accept the option with our packet size. - * Otherwise, simply do as we didn't see the option. - */ - if (blksize >= 512) { + /* Accept blksize up to our maximum size */ + if (blksize > 0) { + spt->block_size = MIN(blksize, TFTP_BLOCKSIZE_MAX); option_name[nb_options] = "blksize"; - option_value[nb_options] = 512; + option_value[nb_options] = spt->block_size; nb_options++; } } diff --git a/slirp/tftp.h b/slirp/tftp.h index 2cd276dec6..a4c4a64e64 100644 --- a/slirp/tftp.h +++ b/slirp/tftp.h @@ -15,6 +15,7 @@ #define TFTP_OACK 6 #define TFTP_FILENAME_MAX 512 +#define TFTP_BLOCKSIZE_MAX 1428 struct tftp_t { struct udphdr udp; @@ -22,13 +23,13 @@ struct tftp_t { union { struct { uint16_t tp_block_nr; - uint8_t tp_buf[512]; + uint8_t tp_buf[TFTP_BLOCKSIZE_MAX]; } tp_data; struct { uint16_t tp_error_code; - uint8_t tp_msg[512]; + uint8_t tp_msg[TFTP_BLOCKSIZE_MAX]; } tp_error; -char tp_buf[512 + 2]; +char tp_buf[TFTP_BLOCKSIZE_MAX + 2]; } x; } __attribute__((packed)); @@ -36,6 +37,7 @@ struct tftp_session { Slirp *slirp; char *filename; int fd; +uint16_t block_size; struct sockaddr_storage client_addr; uint16_t client_port; -- 2.11.0
[Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks
The following changes since commit 82ecffa8c050bf5bbc13329e9b65eac1caa5b55c: Open 2.9 development tree (2016-12-20 16:20:16 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 9443598d7e2f2c0a6493d97b3f11dd04837b08e8: slirp: support dynamic block size for TFTP transfers (2016-12-21 00:02:15 +0100) slirp updates Hervé Poussineau (1): slirp: support dynamic block size for TFTP transfers Yuval Shaia (1): slirp, disas: Replace min/max with MIN/MAX macros disas/m68k.c | 8 ++-- slirp/dhcpv6.c | 2 +- slirp/ip6_icmp.c | 2 +- slirp/slirp.c | 2 +- slirp/slirp.h | 5 - slirp/tcp_input.c | 16 slirp/tcp_output.c | 6 +++--- slirp/tcp_timer.c | 2 +- slirp/tcpip.h | 2 +- slirp/tftp.c | 26 ++ slirp/tftp.h | 8 +--- 11 files changed, 37 insertions(+), 42 deletions(-)
Re: [Qemu-devel] [PATCH v2] slirp, disas: Replace min/max with MIN/MAX macros
I have applied it to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v2 4/5] slirp: VMStatify socket level
Hello, Dr. David Alan Gilbert, on Mon 28 Nov 2016 09:08:16 +, wrote: > Hmm, I don't really know IPv6 but I'm thinking this code will become > something like > the following (says he not knowing whether a scope-id or a flowinfo > is something that needs migrating) (untested): > > > static const VMStateDescription vmstate_slirp_socket_addr = { > .name = "slirp-socket-addr", > .version_id = 4, > .fields = (VMStateField[]) { > VMSTATE_UINT16(ss.ss_family, union slirp_sockaddr), > VMSTATE_UINT32_TEST(sin.sin_addr.s_addr, union slirp_sockaddr, > slirp_family_inet), > VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr, > slirp_family_inet), > > VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr, > slirp_family_inet6), > VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr, > slirp_family_inet6), > VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr, > slirp_family_inet6), > VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr, > slirp_family_inet6), > > VMSTATE_END_OF_LIST() > } > }; Ok, that looks good :) > So to me that looks pretty clean, we need to add another slirp_family_inet6 > test function, but then we just add the extra fields for the IPv6 stuff. Yes, now I see. > Can you suggest an IPv6 command line for testing that ? Well, it doesn't exist yet, that's the problem. And applying your patch would have made it to exist even harder, so that's why I was afraid. I would say that your patch should contain these IPv6 lines, but as comments since they are untested, for the person who will at some point want to implement the IPv6 case here. Samuel
Re: [Qemu-devel] [PATCH] cutils: Define min and max marcos
Yuval Shaia, on Mon 28 Nov 2016 00:18:26 +0200, wrote: > On Sun, Nov 27, 2016 at 03:10:04PM +0100, Samuel Thibault wrote: > > Yuval Shaia, on Fri 25 Nov 2016 12:31:26 +0200, wrote: > > > -#ifndef _WIN32 > > > -#define min(x,y) ((x) < (y) ? (x) : (y)) > > > -#define max(x,y) ((x) > (y) ? (x) : (y)) > > > -#endif > > > > This has protection against _WIN32, I guess that was on purpose. > > I'm not following. > Are you suggesting that this was there to prevent code from compiling when > _WIN32 was define? I mean that min and max are already defined on windows: https://msdn.microsoft.com/en-us/library/windows/desktop/dd757290%28v=vs.85%29.aspx > > Perhaps qemu should avoid risking a clash with OS-provided min/max > > macros, by renaming these to qemu_min/max? > > On MIN and MAX? Yes. > I have noticed some other approach which was taken in osdep.h with ifdef, > for example: > 193 #ifndef ROUND_UP > 194 #define ROUND_UP(n,d) (((n) + (d) - 1) & -(d)) > 195 #endif That could probably be enough for our use indeed. Samuel
Re: [Qemu-devel] [PATCH v2 4/5] slirp: VMStatify socket level
Samuel Thibault, on Sun 27 Nov 2016 16:13:46 +0100, wrote: > Dr. David Alan Gilbert (git), on Wed 23 Nov 2016 18:52:57 +, wrote: > > +static const VMStateDescription vmstate_slirp_socket_addr = { > > +.name = "slirp-socket-addr", > > +.version_id = 4, > > +.fields = (VMStateField[]) { > > +VMSTATE_UINT16(ss.ss_family, union slirp_sockaddr), > > +VMSTATE_UINT32_TEST(sin.sin_addr.s_addr, union slirp_sockaddr, > > +slirp_family_inet), > > +VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr, > > +slirp_family_inet), > > +VMSTATE_END_OF_LIST() > > +} > > +}; > > How will we be able to add the IPv6 case here? Reading again your previous post, it seemed it'd be in slirp_family_inet, but I don't immediately see how. Applying your patch for 2.9 would thus make porting the code to IPv6 more difficult than how it is now, so I'm quite reluctant :) Could you perhaps simply add the IPv6 case in your patch series already? It shouldn't be much work for you who actually know how the VMSTATE machinery is supposed to work (I guess the amount of people who care about slirp *and* know about VMSTATE is extremely small), and a proof of concept for the portability to non-ipv4 addresse spaces. Samuel
Re: [Qemu-devel] [PATCH v2 4/5] slirp: VMStatify socket level
Hello, Dr. David Alan Gilbert (git), on Wed 23 Nov 2016 18:52:57 +, wrote: > +static const VMStateDescription vmstate_slirp_socket_addr = { > +.name = "slirp-socket-addr", > +.version_id = 4, > +.fields = (VMStateField[]) { > +VMSTATE_UINT16(ss.ss_family, union slirp_sockaddr), > +VMSTATE_UINT32_TEST(sin.sin_addr.s_addr, union slirp_sockaddr, > +slirp_family_inet), > +VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr, > +slirp_family_inet), > +VMSTATE_END_OF_LIST() > +} > +}; How will we be able to add the IPv6 case here? Samuel
Re: [Qemu-devel] [PATCH] cutils: Define min and max marcos
Hello, Yuval Shaia, on Fri 25 Nov 2016 12:31:26 +0200, wrote: > -#ifndef _WIN32 > -#define min(x,y) ((x) < (y) ? (x) : (y)) > -#define max(x,y) ((x) > (y) ? (x) : (y)) > -#endif This has protection against _WIN32, I guess that was on purpose. Perhaps qemu should avoid risking a clash with OS-provided min/max macros, by renaming these to qemu_min/max? Samuel
Re: [Qemu-devel] [v2] tftp: fake support for netascii protocol
Stefan Hajnoczi, on Mon 21 Nov 2016 14:46:16 +, wrote: > On Sun, Nov 20, 2016 at 09:41:36AM +0100, Vincent Bernat wrote: > > From: Vincent Bernat> > > > Some network equipments are requesting a file using the netascii > > protocol and this is not configurable. Currently, qemu's tftpd only > > supports the octet protocol. This commit makes it accept the netascii > > protocol as well but do not perform the requested transformation (LF -> > > CR,LF) as it would be far more complex. The current implementation is > > good enough. A user has always the choice to preencode the served file > > correctly. > > > > Signed-off-by: Vincent Bernat > > --- > > slirp/tftp.c | 11 --- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/slirp/tftp.c b/slirp/tftp.c > > index c1859066ccb2..6907d5b92074 100644 > > --- a/slirp/tftp.c > > +++ b/slirp/tftp.c > > @@ -26,6 +26,7 @@ > > #include "slirp.h" > > #include "qemu-common.h" > > #include "qemu/cutils.h" > > +#include "qemu/log.h" > > > > static inline int tftp_session_in_use(struct tftp_session *spt) > > { > > @@ -326,13 +327,17 @@ static void tftp_handle_rrq(Slirp *slirp, struct > > sockaddr_storage *srcsas, > > return; > >} > > > > - if (strcasecmp(>x.tp_buf[k], "octet") != 0) { > > + if (strcasecmp(>x.tp_buf[k], "octet") == 0) { > > + k += 6; > > + } else if (strcasecmp(>x.tp_buf[k], "netascii") == 0) { > > + qemu_log_mask(LOG_UNIMP, "tftp: netascii protocol not implemented, " > > +"no CR-LF conversion\n"); > > + k += 9; > > + } else { > > This is an RFC violation. I don't think it's suitable for upstream QEMU. > > The commit description says it would be "far more complex" to implement > netascii. Is the LF -> CR LF and CR -> CR NUL transformation so hard? I guess the question is that while the patch above could be accepted for the upcoming 2.8 (I don't see it breaking existing systems), a patch that would implement the transformation would be a lot more involved, and really not suitable for 2.8. Samuel
[Qemu-devel] [PULL] tftp: fake support for netascii protocol
From: Vincent Bernat <vinc...@bernat.im> Some network equipments are requesting a file using the netascii protocol and this is not configurable. Currently, qemu's tftpd only supports the octet protocol. This commit makes it accept the netascii protocol as well but do not perform the requested transformation (LF -> CR,LF) as it would be far more complex. The current implementation is good enough. A user has always the choice to preencode the served file correctly. Signed-off-by: Vincent Bernat <vinc...@bernat.im> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/tftp.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index c185906..6907d5b 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -26,6 +26,7 @@ #include "slirp.h" #include "qemu-common.h" #include "qemu/cutils.h" +#include "qemu/log.h" static inline int tftp_session_in_use(struct tftp_session *spt) { @@ -326,13 +327,17 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas, return; } - if (strcasecmp(>x.tp_buf[k], "octet") != 0) { + if (strcasecmp(>x.tp_buf[k], "octet") == 0) { + k += 6; + } else if (strcasecmp(>x.tp_buf[k], "netascii") == 0) { + qemu_log_mask(LOG_UNIMP, "tftp: netascii protocol not implemented, " +"no CR-LF conversion\n"); + k += 9; + } else { tftp_send_error(spt, 4, "Unsupported transfer mode", tp); return; } - k += 6; /* skipping octet */ - /* do sanity checks on the filename */ if (!strncmp(req_fname, "../", 3) || req_fname[strlen(req_fname) - 1] == '/' || -- 2.10.2
[Qemu-devel] [PULL] tftp: fake support for netascii protocol
The following changes since commit b0bcc86d2a87456f5a276f941dc775b265b309cf: Update version for v2.8.0-rc0 release (2016-11-15 20:55:12 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to b4a952793033e322f9ab0a3661b3faeee17ba4e6: tftp: fake support for netascii protocol (2016-11-20 18:02:38 +0100) slirp updates Vincent Bernat (1): tftp: fake support for netascii protocol slirp/tftp.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-)
Re: [Qemu-devel] [PULL] tftp: fake support for netascii protocol
Vincent Bernat, on Sat 19 Nov 2016 09:03:08 +0100, wrote: > ❦ 19 novembre 2016 08:32 +0100, Thomas Huth: > > >> Some network equipments are requesting a file using the netascii > >> protocol and this is not configurable. Currently, qemu's tftpd only > >> supports the octet protocol. This commit makes it accept the netascii > >> protocol as well but do not perform the requested transformation (LF -> > >> CR,LF) as it would be far more complex. > > > > That sounds somewhat wrong to me. QEMU now seems to support a transfer > > mode that is not really implemented. > > On the other hand, the current implementation may not RFC compliant as > the three modes (netascii, octet, mail) are not supported (but the RFC > predates the usage of MAY/SHOULD/MUST, so it's unclear for me). > > I have tried to add proper support, but this is more invasive as we > cannot just seek in the file to get each block. However, this is > something that I can do if compliance is important for QEMU. > > > I think you should at least issue a qemu_log_mask(LOG_UNIMP, "...") > > call in that case. > > I can do that if needed. That'd be better indeed. Otherwise people might wonder why things are not working. Warning that they have to do the LF -> CR,LF conversion by hand is important here. Samuel
Re: [Qemu-devel] [PATCH for-2.8] curses: Fix compiler warnings (Mingw-w64 redefinition of macro KEY_EVENT)
Stefan Weil, on Sat 19 Nov 2016 19:53:18 +0100, wrote: > For builds with Mingw-w64 as it is included in Cygwin, there are two > header files which define KEY_EVENT with different values. > > This results in lots of compiler warnings like this one: > > CC vl.o > In file included from /qemu/include/ui/console.h:340:0, > from /qemu/vl.c:76: > /usr/i686-w64-mingw32/sys-root/mingw/include/curses.h:1522:0: warning: > "KEY_EVENT" redefined > #define KEY_EVENT 0633 /* We were interrupted by an event */ > > In file included from /usr/share/mingw-w64/include/windows.h:74:0, > from /usr/share/mingw-w64/include/winsock2.h:23, > from /qemu/include/sysemu/os-win32.h:29, > from /qemu/include/qemu/osdep.h:100, > from /qemu/vl.c:24: > /usr/share/mingw-w64/include/wincon.h:101:0: note: this is the location of > the previous definition > #define KEY_EVENT 0x1 > > QEMU only uses the KEY_EVENT macro from wincon.h. > Therefore we can undefine the macro coming from curses.h. > > The explicit include statement for curses.h in ui/curses.c is not needed > and was removed. > > Those two modifications fix the redefinition warnings. > > Signed-off-by: Stefan Weil <s...@weilnetz.de> Acked-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> > --- > include/ui/console.h | 3 +++ > ui/curses.c | 1 - > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/ui/console.h b/include/ui/console.h > index e2589e2..e5ae506 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -337,7 +337,10 @@ static inline pixman_format_code_t > surface_format(DisplaySurface *s) > } > > #ifdef CONFIG_CURSES > +/* KEY_EVENT is defined in wincon.h and in curses.h. Avoid redefinition. */ > +#undef KEY_EVENT > #include > +#undef KEY_EVENT > typedef chtype console_ch_t; > extern chtype vga_to_curses[]; > #else > diff --git a/ui/curses.c b/ui/curses.c > index 2e132a7..03cefdf 100644 > --- a/ui/curses.c > +++ b/ui/curses.c > @@ -22,7 +22,6 @@ > * THE SOFTWARE. > */ > #include "qemu/osdep.h" > -#include > > #ifndef _WIN32 > #include > -- > 2.10.2 > > -- Samuel Now I know someone out there is going to claim, "Well then, UNIX is intuitive, because you only need to learn 5000 commands, and then everything else follows from that! Har har har!" (Andy Bates in comp.os.linux.misc, on "intuitive interfaces", slightly defending Macs.)
Re: [Qemu-devel] [PATCH] tftp: fake support for netascii protocol
Hello, Vincent Bernat, on Thu 17 Nov 2016 13:22:32 +0100, wrote: > ❦ 17 novembre 2016 13:20 +0100, Vincent Bernat: > > > Some network equipments are requesting a file using the netascii > > protocol and this is not configurable. Currently, qemu's tftpd only > > supports the octet protocol. This commit makes it accept the netascii > > protocol as well but do not perform the requested transformation (LF -> > > CR,LF) as it would be far more complex. The current implementation is > > good enough. A user has always the choice to preencode the served file > > correctly. > > Signed-off-by: Vincent Bernat Thanks, I've pushed to my tree and requested a pull. Samuel
[Qemu-devel] [PULL] tftp: fake support for netascii protocol
From: Vincent Bernat <vinc...@bernat.im> Some network equipments are requesting a file using the netascii protocol and this is not configurable. Currently, qemu's tftpd only supports the octet protocol. This commit makes it accept the netascii protocol as well but do not perform the requested transformation (LF -> CR,LF) as it would be far more complex. The current implementation is good enough. A user has always the choice to preencode the served file correctly. Signed-off-by: Vincent Bernat <vinc...@bernat.im> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- slirp/tftp.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index c185906..ab1c05d 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -326,13 +326,15 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas, return; } - if (strcasecmp(>x.tp_buf[k], "octet") != 0) { + if (strcasecmp(>x.tp_buf[k], "octet") == 0) { + k += 6; + } else if (strcasecmp(>x.tp_buf[k], "netascii") == 0) { + k += 9; + } else { tftp_send_error(spt, 4, "Unsupported transfer mode", tp); return; } - k += 6; /* skipping octet */ - /* do sanity checks on the filename */ if (!strncmp(req_fname, "../", 3) || req_fname[strlen(req_fname) - 1] == '/' || -- 2.10.2
[Qemu-devel] [PULL] tftp: fake support for netascii protocol
The following changes since commit b0bcc86d2a87456f5a276f941dc775b265b309cf: Update version for v2.8.0-rc0 release (2016-11-15 20:55:12 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 43fccf87c92a6a88a7294597b719f17fd1b41d3d: tftp: fake support for netascii protocol (2016-11-18 18:49:02 +0100) slirp updates Vincent Bernat (1): tftp: fake support for netascii protocol slirp/tftp.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-)
[Qemu-devel] [PULL] slirp: Fix access to freed memory
The following changes since commit 83c83f9a5266ff113060f887f106a47920fa6974: Merge remote-tracking branch 'bonzini/tags/for-upstream' into staging (2016-11-11 12:51:50 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to ea64d5f08817b5e79e17135dce516c7583107f91: slirp: Fix access to freed memory (2016-11-14 17:36:33 +0100) slirp updates Samuel Thibault (1): slirp: Fix access to freed memory slirp/socket.c | 17 + 1 file changed, 17 insertions(+)
[Qemu-devel] [PULL] slirp: Fix access to freed memory
if_start() goes through the slirp->if_fastq and slirp->if_batchq list of pending messages, and accesses ifm->ifq_so->so_nqueued of its elements if ifm->ifq_so != NULL. When freeing a socket, we thus need to make sure that any pending message for this socket does not refer to the socket any more. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Tested-by: Brian Candler <b.cand...@pobox.com> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> --- slirp/socket.c | 17 + 1 file changed, 17 insertions(+) diff --git a/slirp/socket.c b/slirp/socket.c index 280050a..6c18971 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -66,6 +66,23 @@ void sofree(struct socket *so) { Slirp *slirp = so->slirp; + struct mbuf *ifm; + + for (ifm = (struct mbuf *) slirp->if_fastq.qh_link; + (struct quehead *) ifm != >if_fastq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } + + for (ifm = (struct mbuf *) slirp->if_batchq.qh_link; + (struct quehead *) ifm != >if_batchq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } if (so->so_emu==EMU_RSH && so->extra) { sofree(so->extra); -- 2.10.2
Re: [Qemu-devel] [PATCH] slirp: Fix access to freed memory
Hello, Note: no-re...@patchew.org, on Sun 13 Nov 2016 15:13:47 -0800, wrote: > Your series seems to have some coding style problems. See output below for > more information: > > === OUTPUT BEGIN === > fatal: unrecognized argument: --no-patch > Checking PATCH 1/1: ... > ERROR: suspect code indent for conditional statements (4, 6) > #29: FILE: slirp/socket.c:74: > +if (ifm->ifq_so == so) { > + ifm->ifq_so = NULL; > > ERROR: suspect code indent for conditional statements (4, 6) > #37: FILE: slirp/socket.c:82: > +if (ifm->ifq_so == so) { > + ifm->ifq_so = NULL; This is due to that portion of the slirp code using 2-space indentation instead of 4-space indentation. Samuel
[Qemu-devel] [PATCH] slirp: Fix access to freed memory
if_start() goes through the slirp->if_fastq and slirp->if_batchq list of pending messages, and accesses ifm->ifq_so->so_nqueued of its elements if ifm->ifq_so != NULL. When freeing a socket, we thus need to make sure that any pending message for this socket does not refer to the socket any more. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Tested-by: Brian Candler <b.cand...@pobox.com> --- slirp/socket.c | 17 + 1 file changed, 17 insertions(+) diff --git a/slirp/socket.c b/slirp/socket.c index 280050a..6c18971 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -66,6 +66,23 @@ void sofree(struct socket *so) { Slirp *slirp = so->slirp; + struct mbuf *ifm; + + for (ifm = (struct mbuf *) slirp->if_fastq.qh_link; + (struct quehead *) ifm != >if_fastq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } + + for (ifm = (struct mbuf *) slirp->if_batchq.qh_link; + (struct quehead *) ifm != >if_batchq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } if (so->so_emu==EMU_RSH && so->extra) { sofree(so->extra); -- 2.10.2
Re: [Qemu-devel] Crashing in tcp_close
Hello, Brian Candler, on Sat 12 Nov 2016 09:33:55 +, wrote: > On 11/11/2016 22:09, Samuel Thibault wrote: > >Ooh, I see. Now it's obvious, now that it's not coming from the tcb > >loop:) Could you try the attached patch? > > It looks like it now goes into an infinite loop when a connection is closed. Oops, sorry, my patch was completely bogus, here is a proper one. Samuel diff --git a/slirp/socket.c b/slirp/socket.c index 280050a..6c18971 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -66,6 +66,23 @@ void sofree(struct socket *so) { Slirp *slirp = so->slirp; + struct mbuf *ifm; + + for (ifm = (struct mbuf *) slirp->if_fastq.qh_link; + (struct quehead *) ifm != >if_fastq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } + + for (ifm = (struct mbuf *) slirp->if_batchq.qh_link; + (struct quehead *) ifm != >if_batchq; + ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } if (so->so_emu==EMU_RSH && so->extra) { sofree(so->extra);
Re: [Qemu-devel] Crashing in tcp_close
Brian Candler, on Fri 11 Nov 2016 20:53:12 +, wrote: > On 11/11/2016 16:17, Samuel Thibault wrote: > >Could you increase the value given to valgrind's --num-callers= so we > >can make sure the context of this call? > > OK: re-run with --num-callers=250. It took a few iterations, but I captured > it again. (I have grepped out all the "invalid file descriptor" lines). Thanks! > ==1217== Thread 1: > ==1217== Invalid read of size 4 > ==1217==at 0x550B5B: if_start (if.c:230) > ==1217==by 0x5550E2: slirp_pollfds_poll (slirp.c:770) > ==1217==by 0x5891EB: main_loop_wait (main-loop.c:508) > ==1217==by 0x2F4430: main_loop (vl.c:1908) > ==1217==by 0x2F4430: main (vl.c:4604) Ooh, I see. Now it's obvious, now that it's not coming from the tcb loop :) Could you try the attached patch? Samuel diff --git a/slirp/socket.c b/slirp/socket.c index 280050a..1a50d30 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -66,6 +66,13 @@ void sofree(struct socket *so) { Slirp *slirp = so->slirp; + struct mbuf *ifm; + + for (ifm = slirp->next_m; ifm; ifm = ifm->ifq_next) { +if (ifm->ifq_so == so) { + ifm->ifq_so = NULL; +} + } if (so->so_emu==EMU_RSH && so->extra) { sofree(so->extra);
Re: [Qemu-devel] Crashing in tcp_close
Hello, Brian Candler, on Fri 11 Nov 2016 16:02:44 +, wrote: > Aha!! Looking carefully at valgrind output, I see some definite cases of > use-after-free in tcp_output. Does the info below help? Ok, that's interesting. I however still don't see how that could happen :) > ==18350== Invalid read of size 4 > ==18350==at 0x550B5B: if_start (if.c:230) > ==18350==by 0x552E6C: ip_output (ip_output.c:85) > ==18350==by 0x55AA31: tcp_output (tcp_output.c:469) > ==18350==by 0x558FD7: tcp_input (tcp_input.c:1386) > ==18350==by 0x55543F: slirp_input (slirp.c:867) > ==18350==by 0x54AFBF: net_slirp_receive (slirp.c:118) > ==18350==by 0x540B18: nc_sendv_compat (net.c:701) > ==18350==by 0x540B18: qemu_deliver_packet_iov (net.c:728) > ==18350==by 0x5438DA: qemu_net_queue_deliver_iov (queue.c:179) > ==18350==by 0x5438DA: qemu_net_queue_send_iov (queue.c:224) > ==18350==by 0x36B428: virtio_net_flush_tx (virtio-net.c:1282) > ==18350==by 0x36B624: virtio_net_tx_bh (virtio-net.c:1387) > ==18350==by 0x5804EC: aio_bh_call (async.c:67) > ==18350==by 0x5804EC: aio_bh_poll (async.c:95) > ==18350==by 0x58A8FF: aio_dispatch (aio-posix.c:308) Could you increase the value given to valgrind's --num-callers= so we can make sure the context of this call? Here tcp_input get the buffer being freed below from the slirp->tcb list, and sofree happens to drop it from that list before calling free... I'm wondering whether we have a kind of concurrency or recursivity here. > ==18350== Address 0x9eabec4 is 340 bytes inside a block of size 432 free'd > ==18350==at 0x4C2EDEB: free (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==18350==by 0x55B25E: tcp_close (tcp_subr.c:334) > ==18350==by 0x55C7AE: tcp_timers (tcp_timer.c:289) > ==18350==by 0x55C7AE: tcp_slowtimo (tcp_timer.c:89) > ==18350==by 0x555187: slirp_pollfds_poll (slirp.c:576) > ==18350==by 0x5891EB: main_loop_wait (main-loop.c:508) > ==18350==by 0x2F4430: main_loop (vl.c:1908) > ==18350==by 0x2F4430: main (vl.c:4604) Samuel
[Qemu-devel] [PATCH] Fix cursesw detection
On systems which do not provide ncursesw.pc and whose /usr/include/curses.h does not include wide support, we should not only try with no -I, i.e. /usr/include, but also with -I/usr/include/ncursesw. To properly detect for wide support with and without -Werror, we need to check for the presence of e.g. the WACS_DEGREE macro. We also want to stop at the first curses_inc_list configuration which works, and make sure to set IFS to : at each new loop. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Tested-by: Cornelia Huck <cornelia.h...@de.ibm.com> --- configure | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/configure b/configure index fd6f898..7d2a34e 100755 --- a/configure +++ b/configure @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no @@ -2941,11 +2941,13 @@ int main(void) { resize_term(0, 0); addwstr(L"wide chars\n"); addnwstr(, 1); + add_wch(WACS_DEGREE); return s != 0; } EOF IFS=: for curses_inc in $curses_inc_list; do +IFS=: for curses_lib in $curses_lib_list; do unset IFS if compile_prog "$curses_inc" "$curses_lib" ; then @@ -2955,6 +2957,9 @@ EOF break fi done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then -- 2.10.2
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Sergey Smolov, on Wed 09 Nov 2016 13:15:18 +0300, wrote: > Is it planned to publish this patch into master? Yes. Samuel
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Hello, Cornelia Huck, on Wed 09 Nov 2016 10:40:28 +0100, wrote: > Still curses=no... log attached. Oops, sorry, I misplaced my code, and it somehow worked in my case. Could you give a try at the attached patch instead? Thanks, Samuel commit cc8965eb848f53599895a650a6e062319189be1f Author: Samuel Thibault <samuel.thiba...@ens-lyon.org> Date: Tue Nov 8 20:57:27 2016 +0100 Fix cursesw detection On systems which do not provide ncursesw.pc and whose /usr/include/curses.h does not include wide support, we should not only try with no -I, i.e. /usr/include, but also with -I/usr/include/ncursesw. To properly detect for wide support with and without -Werror, we need to check for the presence of e.g. the WACS_DEGREE macro. We also want to stop at the first curses_inc_list configuration which works, and make sure to set IFS to : at each new loop. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/configure b/configure index fd6f898..7d2a34e 100755 --- a/configure +++ b/configure @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no @@ -2941,11 +2941,13 @@ int main(void) { resize_term(0, 0); addwstr(L"wide chars\n"); addnwstr(, 1); + add_wch(WACS_DEGREE); return s != 0; } EOF IFS=: for curses_inc in $curses_inc_list; do +IFS=: for curses_lib in $curses_lib_list; do unset IFS if compile_prog "$curses_inc" "$curses_lib" ; then @@ -2955,6 +2957,9 @@ EOF break fi done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Hello, Cornelia Huck, on Wed 09 Nov 2016 10:12:36 +0100, wrote: > On Wed, 9 Nov 2016 10:04:02 +0100 > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > Please post config.log so we can have a clue about what is going > > wrong. All these error messages are meant to be reported verbatim, not > > reinterpreted :) > > Well, no error here - just curses=no. The errors are in config.log, that's what one is supposed to look at when there are configure issues. > config.log attached. The difference seems to be that the statement you > added in the sample program causes a real error instead of a warning. Yes, that was on purpose, to avoid the -Werror issue. It's expected to happen because /usr/include/curses.h doesn't seem to have the wide support. But -I/usr/include/ncursesw/curses.h is supposed to have. But: > cc -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels > -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration > -Wold-style-definition -Wtype-limits -fstack-protector-all > -I/usr/include/libpng16 -I/usr/include/ncursesw -o config-temp/qemu-conf.exe > config-temp/qemu-conf.c -m64 -g :-lncursesw:-lcursesw > cc: error: :-lncursesw:-lcursesw: No such file or directory That's what the real issue is in your case. I now see why, could you try the attached patch instead? Samuel commit ea32127ca780b0945827776bf27f99383529621c Author: Samuel Thibault <samuel.thiba...@ens-lyon.org> Date: Tue Nov 8 20:57:27 2016 +0100 Fix cursesw detection On systems which do not provide ncursesw.pc and whose /usr/include/curses.h does not include wide support, we should not only try with no -I, i.e. /usr/include, but also with -I/usr/include/ncursesw. To properly detect for wide support with and without -Werror, we need to check for the presence of e.g. the WACS_DEGREE macro. We also want to stop at the first curses_inc_list configuration which works, and make sure to set IFS to : at each new loop. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/configure b/configure index fd6f898..bac7bcc 100755 --- a/configure +++ b/configure @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no @@ -2941,6 +2941,7 @@ int main(void) { resize_term(0, 0); addwstr(L"wide chars\n"); addnwstr(, 1); + add_wch(WACS_DEGREE); return s != 0; } EOF @@ -2954,7 +2955,11 @@ EOF libs_softmmu="$curses_lib $libs_softmmu" break fi + IFS=: done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Hello, Cornelia Huck, on Wed 09 Nov 2016 09:58:59 +0100, wrote: > On Tue, 8 Nov 2016 21:10:19 +0100 > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > Cornelia Huck, on Tue 08 Nov 2016 12:34:49 +0100, wrote: > > “ > > configure test passed without -Werror but failed with -Werror. > > This is probably a bug in the configure script. The failing command > > will be at the bottom of config.log. > > You can run configure with --disable-werror to bypass this check. > > ” > > > > If so, you should really have said it, I was really wondering how > > configure could just stopping in your case. That does explain things > > indeed. > > I said so in my very first mail for the issue... appears I was unclear. Do you mean "configure barfs about -Werror."? Yes it was unclear to me :) > > Could you try the attached patch? It should be able to really fail > > without Werror too. > > With your patch, configure runs through and detects curses=no. Not sure > that's correct, though: SLES12SP1 _does_ have curses, but not a .pc > file for ncursesw. I don't know enough about curses to say whether it > should be that way... Please post config.log so we can have a clue about what is going wrong. All these error messages are meant to be reported verbatim, not reinterpreted :) Samuel
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Cornelia Huck, on Tue 08 Nov 2016 12:34:49 +0100, wrote: > > diff --git a/configure b/configure > > index fd6f898..e200aa8 100755 > > --- a/configure > > +++ b/configure > > @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then > > curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" > > curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" > >else > > -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" > > +curses_inc_list="$($pkg_config --cflags ncursesw > > 2>/dev/null):-I/usr/include/ncursesw:" > > This arrives at > > curses_inc_list=":-I/usr/include/ncursesw:" > > which causes the parser below to start with an empty curses_inc (with : > as separator). Yes, this is expected. > configure fails as before (with -Werror; passes without). Ah! So are you getting the following message? “ configure test passed without -Werror but failed with -Werror. This is probably a bug in the configure script. The failing command will be at the bottom of config.log. You can run configure with --disable-werror to bypass this check. ” If so, you should really have said it, I was really wondering how configure could just stopping in your case. That does explain things indeed. Could you try the attached patch? It should be able to really fail without Werror too. > > @@ -2955,6 +2955,9 @@ EOF > > break > >fi > > done > > +if test "$curses_found" = yes ; then > > + break > > +fi > > Breaking out as soon as we've found a working config seems like a good > idea, but I don't think it's related to this issue. Actually it is: in your case it's the second config which will work, the third one will fail. Not breaking would mean we keep the failing one. Samuel commit 4c5e78e8843fa919f2601d8e44029ed0e711c6d9 Author: Samuel Thibault <samuel.thiba...@ens-lyon.org> Date: Tue Nov 8 20:57:27 2016 +0100 Fix cursesw detection On systems which do not provide ncursesw.pc and whose /usr/include/curses.h does not include wide support, we should not only try with no -I, i.e. /usr/include, but also with -I/usr/include/ncursesw. To properly detect for wide support with and without -Werror, we need to check for the presence of e.g. the WACS_DEGREE macro. We also want to stop at the first curses_inc_list configuration which works. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> diff --git a/configure b/configure index fd6f898..f35edf8 100755 --- a/configure +++ b/configure @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no @@ -2941,6 +2941,7 @@ int main(void) { resize_term(0, 0); addwstr(L"wide chars\n"); addnwstr(, 1); + add_wch(WACS_DEGREE); return s != 0; } EOF @@ -2955,6 +2956,9 @@ EOF break fi done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then
Re: [Qemu-devel] [PATCH] Fix legacy ncurses detection.
Hello, On 7 November 2016 at 13:38, Michal Suchanekwrote: > -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" > +curses_inc_list="$($pkg_config --cflags ncursesw > 2>/dev/null)-I/usr/include/ncursesw:" Cornelia Huck, on Mon 07 Nov 2016 17:26:56 +0100, wrote: > +if [ "$curses_inc_list" == ":" ]; then > + # some systems don't provide a proper .pc file for ncursesw > + curses_inc_list="-I/usr/include/ncursesw:" > +fi Could you rather try the attached patch? Samuel diff --git a/configure b/configure index fd6f898..e200aa8 100755 --- a/configure +++ b/configure @@ -2926,7 +2926,7 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no @@ -2955,6 +2955,9 @@ EOF break fi done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then
Re: [Qemu-devel] Crashing in tcp_close
Hello, Stefan Hajnoczi, on Fri 04 Nov 2016 11:14:19 +, wrote: > CCing slirp maintainers to get attention on this bug Thanks! > > Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. > > 0x76a1bb5b in _int_free (av=0x76d5fb20 , > > p=, have_lock=0) at malloc.c:4006 > > 4006malloc.c: No such file or directory. > > (gdb) bt > > #0 0x76a1bb5b in _int_free (av=0x76d5fb20 , > > p=, have_lock=0) > > at malloc.c:4006 > > #1 0x76a1fabc in __GI___libc_free (mem=) at > > malloc.c:2969 > > #2 0x559a6c0f in tcp_close (tp=tp@entry=0x56621ed0) at > > slirp/tcp_subr.c:334 > > #3 0x559a6c8f in tcp_drop (tp=tp@entry=0x56621ed0, > > err=) at slirp/tcp_subr.c:298 > > #4 0x559a816b in tcp_timers (timer=, > > tp=0x56621ed0) at slirp/tcp_timer.c:179 > > #3 0x559a6c8f in tcp_drop (tp=tp@entry=0x56621ed0, > > err=) at slirp/tcp_subr.c:298 > > #4 0x559a816b in tcp_timers (timer=, > > tp=0x56621ed0) at slirp/tcp_timer.c:179 > > #5 tcp_slowtimo (slirp=slirp@entry=0x5658ecf0) at slirp/tcp_timer.c:89 > > * If so, what additional gdb output would you like me to provide? > > I wonder if this connection has already been closed/freed before and the > timer fires shortly afterward. That's just a guess based on the > backtrace. That's very unlikely: soclose removes the socket from the list, so tcp_slowtimo wouldn't be able to find it. That'd rather be a buffer overflow. But it's hard to believe it could come from the socket structure since it doesn't contain any buffer. Brian, could you run it with export MALLOC_CHECK_=2 and also this could be useful: export MALLOC_PERTURB_=1234 Also, to rule out the double-free scenario, and try to catch a buffer overflow coming from the socket structure itself, I have attached a patch which adds some debugging. > > * If developers want to reproduce this, let me know and I can probably send > > the VM qcow2 file and/or packer source privately off-list [I need to check > > permission for that] That could be useful. Samuel diff --git a/slirp/socket.c b/slirp/socket.c index 280050a..e603164 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -51,10 +51,12 @@ socreate(Slirp *slirp) so = (struct socket *)malloc(sizeof(struct socket)); if(so) { memset(so, 0, sizeof(struct socket)); +so->canary1 = 0xdeadbeef; so->so_state = SS_NOFDREF; so->s = -1; so->slirp = slirp; so->pollfds_idx = -1; +so->canary2 = 0xbe3fd3ad; } return(so); } @@ -67,6 +69,14 @@ sofree(struct socket *so) { Slirp *slirp = so->slirp; + if (so->s == -1234) +fprintf(stderr,"oops, re-freeing a freed socket!\n"); + if (so->canary1 != 0xdeadbeef) +fprintf(stderr,"oops, canary1 bogus!\n"); + if (so->canary2 != 0xbe3fd3ad) +fprintf(stderr,"oops, canary2 bogus!\n"); + so->s = -1234; + if (so->so_emu==EMU_RSH && so->extra) { sofree(so->extra); so->extra=NULL; diff --git a/slirp/socket.h b/slirp/socket.h index 8feed2a..14fac1c 100644 --- a/slirp/socket.h +++ b/slirp/socket.h @@ -17,6 +17,7 @@ struct socket { struct socket *so_next,*so_prev; /* For a linked list of sockets */ + int canary1; int s; /* The actual socket */ @@ -70,6 +71,7 @@ struct socket { struct sbuf so_rcv; /* Receive buffer */ struct sbuf so_snd; /* Send buffer */ void * extra;/* Extra pointer */ + int canary2; };
Re: [Qemu-devel] slirp: fix ipv6 guest network access with windows host
Hello, Bo Hu, on Wed 02 Nov 2016 16:02:29 -0700, wrote: > This patch is from android emulator (which is based on qemu2.2) and I hope > this patch is > also useful to upstream qemu as well. Indeed, even if normally we fill all required fields, it's probably better to memset the whole structure. > From 021eac8c593a34a6a5e106d187a8e1fd22a1522f Mon Sep 17 00:00:00 2001 > From: bohu <[1]b...@google.com> > Date: Wed, 2 Nov 2016 15:56:26 -0700 > Subject: [PATCH] slirp: fix ipv6 guest network access with windows host > > In tcp_input function, local sockaddr_storage variables lhost > and fhost are used without being cleared to zero; and consequently > tcp connect call fails on windows because there is some random data > in those variables (windows complains with WSAEADDRNOTAVAIL); > > This CL calls memset to clear those two variables so that the address > passed to connect does not have random data in it. > > Signed-off-by: Bo Hu <[2]b...@google.com> > + memset(, 0, sizeof(lhost); > + memset(, 0, sizeof(fhost); There is just a typo: missing closing parenthesis... But apart from that, Reviewed-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> Samuel
Re: [Qemu-devel] [PATCH 4/4] gtk: set window ID
Samuel Thibault, on Mon 31 Oct 2016 17:00:07 +0100, wrote: > This uses the console API to record the window ID of the GTK windows. Ah, sorry, I let this one through, please ignore it, it'll need rework as discussed in the other thread. Samuel
[Qemu-devel] [PATCH 2/4] console: move window ID code from baum to sdl
This moves the SDL bits for window ID from the baum driver to SDL, as well as fixing the build for non-X11. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- backends/baum.c | 25 +++-- ui/sdl.c| 25 + 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/backends/baum.c b/backends/baum.c index b92369d..5d7d27c 100644 --- a/backends/baum.c +++ b/backends/baum.c @@ -27,12 +27,10 @@ #include "sysemu/char.h" #include "qemu/timer.h" #include "hw/usb.h" +#include "ui/console.h" #include #include #include -#ifdef CONFIG_SDL -#include -#endif #if 0 #define DPRINTF(fmt, ...) \ @@ -227,11 +225,6 @@ static const uint8_t nabcc_translation[2][256] = { /* The guest OS has started discussing with us, finish initializing BrlAPI */ static int baum_deferred_init(BaumDriverState *baum) { -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -SDL_SysWMinfo info; -#endif -#endif int tty; if (baum->deferred_init) { @@ -243,21 +236,9 @@ static int baum_deferred_init(BaumDriverState *baum) return 0; } -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -memset(, 0, sizeof(info)); -SDL_VERSION(); -if (SDL_GetWMInfo()) { -tty = info.info.x11.wmwindow; -} else { -#endif -#endif +tty = qemu_graphic_console_get_window_id(); +if (tty == -1) tty = BRLAPI_TTY_DEFAULT; -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -} -#endif -#endif if (brlapi__enterTtyMode(baum->brlapi, tty, NULL) == -1) { brlapi_perror("baum: brlapi__enterTtyMode"); diff --git a/ui/sdl.c b/ui/sdl.c index d8cf5bc..8d9d171 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -947,6 +947,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) int flags; uint8_t data = 0; const SDL_VideoInfo *vi; +SDL_SysWMinfo info; char *filename; #if defined(__APPLE__) @@ -1023,5 +1024,29 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) sdl_cursor_hidden = SDL_CreateCursor(, , 8, 1, 0, 0); sdl_cursor_normal = SDL_GetCursor(); +memset(, 0, sizeof(info)); +SDL_VERSION(); +if (SDL_GetWMInfo()) { +int i; +for (i = 0; ; i++) { +/* All consoles share the same window */ +QemuConsole *con = qemu_console_lookup_by_index(i); +if (con) { +#if defined(SDL_VIDEO_DRIVER_X11) +qemu_console_set_window_id(i, info.info.x11.wmwindow); +#elif defined(SDL_VIDEO_DRIVER_NANOX) || \ + defined(SDL_VIDEO_DRIVER_WINDIB) || defined(SDL_VIDEO_DRIVER_DDRAW) || \ + defined(SDL_VIDEO_DRIVER_GAPI) || \ + defined(SDL_VIDEO_DRIVER_RISCOS) +qemu_console_set_window_id(i, (int) (uintptr_t) info.window); +#else +qemu_console_set_window_id(i, info.data); +#endif +} else { +break; +} +} +} + atexit(sdl_cleanup); } -- 2.10.1
[Qemu-devel] [PATCHv3 0/4] Move getting XWindow ID from baum driver to graphical backend
Hello, This is a respin of moving getting XWindow ID from baum to actual graphical backends. This only contains the new API, the move of existing code to the new API, and the addition of support for SDL2. Gtk will need more discussion and work. Compared to v2, this fixes build on 64bit windows, where the window handle needs to be explictly truncated to 32bit (such handles are guaranteed to be 32bit) Samuel Samuel Thibault (4): console: add API to get underlying gui window ID console: move window ID code from baum to sdl sdl2: set window ID gtk: set window ID backends/baum.c | 25 +++-- include/ui/console.h | 3 +++ ui/console.c | 15 +++ ui/gtk.c | 25 +++-- ui/sdl.c | 25 + ui/sdl2.c| 7 +++ 6 files changed, 76 insertions(+), 24 deletions(-) -- 2.10.1
[Qemu-devel] [PATCH 3/4] sdl2: set window ID
This uses the console API to record the window ID of the SDL2 windows. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- ui/sdl2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/sdl2.c b/ui/sdl2.c index 30d2a3c..b464f16 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -761,6 +761,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) uint8_t data = 0; char *filename; int i; +SDL_SysWMinfo info; if (no_frame) { gui_noframe = 1; @@ -786,6 +787,8 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) exit(1); } SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1"); +memset(, 0, sizeof(info)); +SDL_VERSION(); for (i = 0;; i++) { QemuConsole *con = qemu_console_lookup_by_index(i); @@ -813,6 +816,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) #endif sdl2_console[i].dcl.con = con; register_displaychangelistener(_console[i].dcl); + +if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, )) { +qemu_console_set_window_id(i, info.info.x11.window); +} } /* Load a 32x32x4 image. White pixels are transparent. */ -- 2.10.1
[Qemu-devel] [PATCH 1/4] console: add API to get underlying gui window ID
This adds two console functions, qemu_console_set_window_id and qemu_graphic_console_get_window_id, to let graphical backend record the window id in the QemuConsole structure, and let the baum driver read it. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- include/ui/console.h | 3 +++ ui/console.c | 15 +++ 2 files changed, 18 insertions(+) diff --git a/include/ui/console.h b/include/ui/console.h index e2589e2..cf07e41 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -394,6 +394,9 @@ uint32_t qemu_console_get_head(QemuConsole *con); QemuUIInfo *qemu_console_get_ui_info(QemuConsole *con); int qemu_console_get_width(QemuConsole *con, int fallback); int qemu_console_get_height(QemuConsole *con, int fallback); +/* Return the low-level window id for the first graphical console */ +int qemu_graphic_console_get_window_id(void); +void qemu_console_set_window_id(int index, int window_id); void console_select(unsigned int index); void qemu_console_resize(QemuConsole *con, int width, int height); diff --git a/ui/console.c b/ui/console.c index ed888e5..aa3c4c7 100644 --- a/ui/console.c +++ b/ui/console.c @@ -124,6 +124,7 @@ struct QemuConsole { int dcls; DisplayChangeListener *gl; bool gl_block; +int window_id; /* Graphic console state. */ Object *device; @@ -273,6 +274,20 @@ void graphic_hw_gl_block(QemuConsole *con, bool block) } } +int qemu_graphic_console_get_window_id(void) +{ +if (consoles[0]->console_type == GRAPHIC_CONSOLE) { +return consoles[0]->window_id; +} +return -1; +} + +void qemu_console_set_window_id(int index, int window_id) +{ +assert(index >= 0 && index < nb_consoles); +consoles[index]->window_id = window_id; +} + void graphic_hw_invalidate(QemuConsole *con) { if (!con) { -- 2.10.1
[Qemu-devel] [PATCH 4/4] gtk: set window ID
This uses the console API to record the window ID of the GTK windows. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- ui/gtk.c | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index ca737c4..d75f255 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -2170,6 +2170,13 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover) GtkDisplayState *s = g_malloc0(sizeof(*s)); char *filename; GdkDisplay *window_display; +GdkWindow *gdk_window; +#ifdef GDK_WINDOWING_X11 +Window window_id; +#elif defined(GDK_WINDOWING_WIN32) +HWND window_id; +#endif +int i; if (!gtkinit) { fprintf(stderr, "gtk initialization failed\n"); @@ -2232,8 +2239,6 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover) { VirtualConsole *cur = gd_vc_find_current(s); if (cur) { -int i; - for (i = 0; i < s->nb_vcs; i++) { VirtualConsole *vc = >vc[i]; if (vc && vc->type == GD_VC_VTE && vc != cur) { @@ -2253,6 +2258,22 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover) } gd_set_keycode_type(s); + +gdk_window = gtk_widget_get_window(s->window); +#ifdef GDK_WINDOWING_X11 +window_id = GDK_WINDOW_XID(gdk_window); +#elif defined(GDK_WINDOWING_WIN32) +window_id = gdk_win32_window_get_impl_hwnd(gdk_window); +#endif +for (i = 0; ; i++) { +/* All consoles share the same window */ +QemuConsole *con = qemu_console_lookup_by_index(i); +if (con) { +qemu_console_set_window_id(i, (int) window_id); +} else { +break; +} +} } void early_gtk_display_init(int opengl) -- 2.10.1
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Cornelia Huck, on Mon 31 Oct 2016 14:08:48 +0100, wrote: > On Mon, 31 Oct 2016 15:03:33 +0100 > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > > Cornelia Huck, on Mon 31 Oct 2016 13:48:01 +0100, wrote: > > > On Mon, 31 Oct 2016 13:39:30 +0100 > > > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > > > > > > Cornelia Huck, on Mon 31 Oct 2016 13:08:06 +0100, wrote: > > > > > You mean in configure, right? Including cursesw.h in the test program > > > > > gets configure going again. > > > > > > > > Could you try the attached patch which fixes both configure and > > > > ui/curses.c? > > > > > > Sadly, this does not fix it for me. I get the same errors in the > > > configure test build as before. > > > > Could you post the config.log so we get a better view at what is going > > wrong? > > Attached. > cc -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels > -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration > -Wold-style-definition -Wtype-limits -fstack-protector-all > -I/usr/include/libpng16 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c > -m64 -g > config-temp/qemu-conf.c: In function ‘main’: > config-temp/qemu-conf.c:13:3: warning: implicit declaration of function > ‘addwstr’ [-Wimplicit-function-declaration] ... > cc -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels > -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration > -Wold-style-definition -Wtype-limits -fstack-protector-all > -I/usr/include/libpng16 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c > -m64 -g -lncursesw > config-temp/qemu-conf.c: In function ‘main’: > config-temp/qemu-conf.c:13:3: warning: implicit declaration of function > ‘addwstr’ [-Wimplicit-function-declaration] ... > cc -Werror -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels > -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration > -Wold-style-definition -Wtype-limits -fstack-protector-all > -I/usr/include/libpng16 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c > -m64 -g -lncursesw > config-temp/qemu-conf.c: In function ‘main’: > config-temp/qemu-conf.c:13:3: error: implicit declaration of function > ‘addwstr’ [-Werror=implicit-function-declaration] >addwstr(L"wide chars\n"); >^ > config-temp/qemu-conf.c:13:3: error: nested extern declaration of ‘addwstr’ > [-Werror=nested-externs] > config-temp/qemu-conf.c:14:3: error: implicit declaration of function > ‘addnwstr’ [-Werror=implicit-function-declaration] >addnwstr(, 1); >^ > config-temp/qemu-conf.c:14:3: error: nested extern declaration of ‘addnwstr’ > [-Werror=nested-externs] > cc1: all warnings being treated as errors These are expected But doesn't configure also try with -DCONFIG_CURSESW_H? Does it really stop here? Samuel
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Cornelia Huck, on Mon 31 Oct 2016 13:48:01 +0100, wrote: > On Mon, 31 Oct 2016 13:39:30 +0100 > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > > Cornelia Huck, on Mon 31 Oct 2016 13:08:06 +0100, wrote: > > > You mean in configure, right? Including cursesw.h in the test program > > > gets configure going again. > > > > Could you try the attached patch which fixes both configure and > > ui/curses.c? > > Sadly, this does not fix it for me. I get the same errors in the > configure test build as before. Could you post the config.log so we get a better view at what is going wrong? Thanks, Samuel
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Cornelia Huck, on Mon 31 Oct 2016 13:08:06 +0100, wrote: > You mean in configure, right? Including cursesw.h in the test program > gets configure going again. Could you try the attached patch which fixes both configure and ui/curses.c? Thanks, Samuel diff --git a/configure b/configure index f83cdf8..bae01f0 100755 --- a/configure +++ b/configure @@ -2920,13 +2920,17 @@ if test "$curses" != "no" ; then curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):" curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses" else -curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):" +curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-DCONFIG_CURSESW_H:" curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw" fi curses_found=no cat > $TMPC << EOF #include +#ifdef CONFIG_CURSESW_H +#include +#else #include +#endif #include int main(void) { const char *s = curses_version(); @@ -2949,6 +2953,9 @@ EOF break fi done +if test "$curses_found" = yes ; then + break +fi done unset IFS if test "$curses_found" = "yes" ; then diff --git a/ui/curses.c b/ui/curses.c index 2e132a7..cb61073 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -22,7 +22,11 @@ * THE SOFTWARE. */ #include "qemu/osdep.h" +#ifdef CONFIG_CURSESW_H +#include +#else #include +#endif #ifndef _WIN32 #include
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Cornelia Huck, on Mon 31 Oct 2016 13:08:06 +0100, wrote: > On Mon, 31 Oct 2016 13:01:59 +0100 > Samuel Thibault <samuel.thiba...@gnu.org> wrote: > > > Cornelia Huck, on Mon 31 Oct 2016 12:45:30 +0100, wrote: > > > config-temp/qemu-conf.c:9:3: warning: implicit declaration of function > > > ‘addwstr’ [-Wimplicit-function-declaration] > > > > Bleh. > > > > Could you try to replace #include with #include > > in ui/curses.c, to see whether that fixes the missing declaration on > > that system? We could be trying both headers to look for the wide > > functions and include the one which works. > > You mean in configure, right? Ah, sorry, I read too fast, I thought what you had was a build failure, not a configuration failure. > Including cursesw.h in the test program gets configure going again. Ok, that makes sense, thanks. Samuel
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Samuel Thibault, on Mon 31 Oct 2016 13:01:59 +0100, wrote: > Cornelia Huck, on Mon 31 Oct 2016 12:45:30 +0100, wrote: > > > From: Samuel Thibault <samuel.thiba...@ens-lyon.org> > > > > > > Use ncursesw package instead of curses on non-mingw, and check a few > > > functions. > > > Also take cflags from pkg-config, since cursesw headers may be in a > > > separate, non-default directory. > > > > > > Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> > > > Message-id: 20161015195308.20473-3-samuel.thiba...@ens-lyon.org > > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > > > --- > > > configure | 29 - > > > 1 file changed, 20 insertions(+), 9 deletions(-) > > > > This seems to break configure on one of the systems I use (which may or > > may not have a broken setup). SLES12SP1 (s390x) without curses in the > > output of pkg-config --list-all, but headers seem to be present (? -- > > I'm not the admin). Other systems (Fedora and Ubuntu) are fine. > > > config-temp/qemu-conf.c:9:3: warning: implicit declaration of function > > ‘addwstr’ [-Wimplicit-function-declaration] > > We could be trying both headers to look for the wide > functions and include the one which works. I'm however surprised that this didn't get catched by configure, we do look for addwstr there. Could you also post the config.log? Thanks, Samuel
Re: [Qemu-devel] [PULL 6/6] curses: Use cursesw instead of curses
Cornelia Huck, on Mon 31 Oct 2016 12:45:30 +0100, wrote: > > From: Samuel Thibault <samuel.thiba...@ens-lyon.org> > > > > Use ncursesw package instead of curses on non-mingw, and check a few > > functions. > > Also take cflags from pkg-config, since cursesw headers may be in a > > separate, non-default directory. > > > > Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> > > Message-id: 20161015195308.20473-3-samuel.thiba...@ens-lyon.org > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > > --- > > configure | 29 - > > 1 file changed, 20 insertions(+), 9 deletions(-) > > This seems to break configure on one of the systems I use (which may or > may not have a broken setup). SLES12SP1 (s390x) without curses in the > output of pkg-config --list-all, but headers seem to be present (? -- > I'm not the admin). Other systems (Fedora and Ubuntu) are fine. > config-temp/qemu-conf.c:9:3: warning: implicit declaration of function > ‘addwstr’ [-Wimplicit-function-declaration] Bleh. Could you try to replace #include with #include in ui/curses.c, to see whether that fixes the missing declaration on that system? We could be trying both headers to look for the wide functions and include the one which works. Samuel
Re: [Qemu-devel] [PATCH 3/3] Move getting XWindow ID from baum driver to graphical backend
Gerd Hoffmann, on Wed 26 Oct 2016 12:17:44 +0200, wrote: > > +/* All consoles share the same window */ > > No. That is the default setup, but try "View / Detach tab". Window ID > changing at runtime ... So we would need to make baum register for notification of Window ID change. It could be a mere typedef void QemuConsoleWindowIDListener(void); qemu_console_window_id_add_listener(QemuConsoleWindowIDListener listener); qemu_console_window_id_remove_listener(QemuConsoleWindowIDListener listener); that adds/removes the listener to a list to be called when qemu_console_set_window_id is called. Or we could generalize a bit: typedef void QemuConsoleConfigListener(void); qemu_console_config_add_listener(QemuConsoleConfigListener listener); qemu_console_config_remove_listener(QemuConsoleConfigListener listener); Or even more generalized: struct QemuConsoleListener { void (*window_id)(void); }; typedef struct QemuConsoleListener QemuConsoleListener; qemu_console_add_listener(QemuConsoleListener *listener); qemu_console_remove_listener(QemuConsoleListener *listener); What would be preferrable? Samuel
[Qemu-devel] [PATCHv2 0/3] Move getting XWindow ID from baum driver to graphical backend
Hello, This is a respin of moving getting XWindow ID from baum to actual graphical backends. This only contains the new API, the move of existing code to the new API, and the addition of support for SDL2. Gtk will need more discussion and work. Samuel Samuel Thibault (3): console: add API to get underlying gui window ID console: move window ID code from baum to sdl sdl2: set window ID backends/baum.c | 25 +++-- include/ui/console.h | 3 +++ ui/console.c | 15 +++ ui/sdl.c | 25 + ui/sdl2.c| 7 +++ 5 files changed, 53 insertions(+), 22 deletions(-) -- 2.10.1
[Qemu-devel] [PATCH 2/3] console: move window ID code from baum to sdl
This moves the SDL bits for window ID from the baum driver to SDL, as well as fixing the build for non-X11. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- backends/baum.c | 25 +++-- ui/sdl.c| 25 + 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/backends/baum.c b/backends/baum.c index b92369d..5d7d27c 100644 --- a/backends/baum.c +++ b/backends/baum.c @@ -27,12 +27,10 @@ #include "sysemu/char.h" #include "qemu/timer.h" #include "hw/usb.h" +#include "ui/console.h" #include #include #include -#ifdef CONFIG_SDL -#include -#endif #if 0 #define DPRINTF(fmt, ...) \ @@ -227,11 +225,6 @@ static const uint8_t nabcc_translation[2][256] = { /* The guest OS has started discussing with us, finish initializing BrlAPI */ static int baum_deferred_init(BaumDriverState *baum) { -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -SDL_SysWMinfo info; -#endif -#endif int tty; if (baum->deferred_init) { @@ -243,21 +236,9 @@ static int baum_deferred_init(BaumDriverState *baum) return 0; } -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -memset(, 0, sizeof(info)); -SDL_VERSION(); -if (SDL_GetWMInfo()) { -tty = info.info.x11.wmwindow; -} else { -#endif -#endif +tty = qemu_graphic_console_get_window_id(); +if (tty == -1) tty = BRLAPI_TTY_DEFAULT; -#if defined(CONFIG_SDL) -#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0) -} -#endif -#endif if (brlapi__enterTtyMode(baum->brlapi, tty, NULL) == -1) { brlapi_perror("baum: brlapi__enterTtyMode"); diff --git a/ui/sdl.c b/ui/sdl.c index d8cf5bc..7fa3772 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -947,6 +947,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) int flags; uint8_t data = 0; const SDL_VideoInfo *vi; +SDL_SysWMinfo info; char *filename; #if defined(__APPLE__) @@ -1023,5 +1024,29 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) sdl_cursor_hidden = SDL_CreateCursor(, , 8, 1, 0, 0); sdl_cursor_normal = SDL_GetCursor(); +memset(, 0, sizeof(info)); +SDL_VERSION(); +if (SDL_GetWMInfo()) { +int i; +for (i = 0; ; i++) { +/* All consoles share the same window */ +QemuConsole *con = qemu_console_lookup_by_index(i); +if (con) { +#if defined(SDL_VIDEO_DRIVER_X11) +qemu_console_set_window_id(i, info.info.x11.wmwindow); +#elif defined(SDL_VIDEO_DRIVER_NANOX) || \ + defined(SDL_VIDEO_DRIVER_WINDIB) || defined(SDL_VIDEO_DRIVER_DDRAW) || \ + defined(SDL_VIDEO_DRIVER_GAPI) || \ + defined(SDL_VIDEO_DRIVER_RISCOS) +qemu_console_set_window_id(i, (int) info.window); +#else +qemu_console_set_window_id(i, info.data); +#endif +} else { +break; +} +} +} + atexit(sdl_cleanup); } -- 2.10.1
[Qemu-devel] [PATCH 1/3] console: add API to get underlying gui window ID
This adds two console functions, qemu_console_set_window_id and qemu_graphic_console_get_window_id, to let graphical backend record the window id in the QemuConsole structure, and let the baum driver read it. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- include/ui/console.h | 3 +++ ui/console.c | 15 +++ 2 files changed, 18 insertions(+) diff --git a/include/ui/console.h b/include/ui/console.h index e2589e2..cf07e41 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -394,6 +394,9 @@ uint32_t qemu_console_get_head(QemuConsole *con); QemuUIInfo *qemu_console_get_ui_info(QemuConsole *con); int qemu_console_get_width(QemuConsole *con, int fallback); int qemu_console_get_height(QemuConsole *con, int fallback); +/* Return the low-level window id for the first graphical console */ +int qemu_graphic_console_get_window_id(void); +void qemu_console_set_window_id(int index, int window_id); void console_select(unsigned int index); void qemu_console_resize(QemuConsole *con, int width, int height); diff --git a/ui/console.c b/ui/console.c index ed888e5..aa3c4c7 100644 --- a/ui/console.c +++ b/ui/console.c @@ -124,6 +124,7 @@ struct QemuConsole { int dcls; DisplayChangeListener *gl; bool gl_block; +int window_id; /* Graphic console state. */ Object *device; @@ -273,6 +274,20 @@ void graphic_hw_gl_block(QemuConsole *con, bool block) } } +int qemu_graphic_console_get_window_id(void) +{ +if (consoles[0]->console_type == GRAPHIC_CONSOLE) { +return consoles[0]->window_id; +} +return -1; +} + +void qemu_console_set_window_id(int index, int window_id) +{ +assert(index >= 0 && index < nb_consoles); +consoles[index]->window_id = window_id; +} + void graphic_hw_invalidate(QemuConsole *con) { if (!con) { -- 2.10.1
[Qemu-devel] [PATCH 3/3] sdl2: set window ID
This uses the console API to record the window ID of the SDL2 windows. Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- ui/sdl2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/sdl2.c b/ui/sdl2.c index 30d2a3c..b464f16 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -761,6 +761,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) uint8_t data = 0; char *filename; int i; +SDL_SysWMinfo info; if (no_frame) { gui_noframe = 1; @@ -786,6 +787,8 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) exit(1); } SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1"); +memset(, 0, sizeof(info)); +SDL_VERSION(); for (i = 0;; i++) { QemuConsole *con = qemu_console_lookup_by_index(i); @@ -813,6 +816,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) #endif sdl2_console[i].dcl.con = con; register_displaychangelistener(_console[i].dcl); + +if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, )) { +qemu_console_set_window_id(i, info.info.x11.window); +} } /* Load a 32x32x4 image. White pixels are transparent. */ -- 2.10.1