Re: Request for Support: QEMU IPv6 Port Forwarding Issue
Hello, Thomas Huth, le mar. 19 mars 2024 15:28:12 +0100, a ecrit: > On 19/03/2024 06.53, Srinivasu Kandukuri (MS/ECP2-ETAS-VOS) wrote: > > Dear QEMU Support Team, > > > > We are currently encountering difficulties in utilizing QEMU for > > starting a virtual machine image on Windows with IPv6 networking, > > specifically related to port forwarding. We are using QEMU emulator > > version 7.1.93 (v7.2.0-rc3-11946-gb68e69cdcc-dirty). > > > > Our objective is to establish IPv6 network connectivity within the > > virtual machine and forward ports to allow external access. However, we > > are encountering errors when attempting to configure the port forwarding > > mechanism. > > > > Here is the command we are using: > > > > *qemu-system-x86_64: -netdev > > user,id=net1,ipv6=on,ipv6-net=fe80::5054:ff:fecd:585a/64,hostfwd=tcp::2210-[fe80::5054:ff:fecd:585a]:22* > > > > However, upon executing this command, we encounter the following error: > > > > *Invalid host forwarding rule 'tcp::2210-[fe80::5054:ff:fecd:585a]:22' > > (Bad guest address)* > > > > We understand that the format for specifying host forwarding rules > > follows the pattern: > > > > *protocol:[listen_address]:listen_port-[dest_address]:dest_port* > > > > We believe that we are following this pattern correctly, but still, we > > are encountering errors. > > Looking at the code: > > https://gitlab.com/qemu-project/qemu/-/blob/v8.2.0/net/slirp.c#L824 > > it seems like QEMU is only using inet_aton() here, which means IPv4 only, > sorry, but this likely needs some additional changes first to support IPv6 > addresses here. Nicholas Ngai's work is still pending integration: https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg05643.html https://gitlab.com/qemu-project/qemu/-/issues/347 Samuel
Re: [PATCH 2/4] Avoid multiple definitions of copy_file_range
Manolo de Medici, le mer. 17 janv. 2024 16:08:34 +0100, a ecrit: > Understood, but I cannot judge if it is a bug in qemu or it fixes > another host os, > since qemu doesn't target only glibc. Yes, but freebsd too uses ssize_t: https://man.freebsd.org/cgi/man.cgi?copy_file_range(2) glib mentions that it only exists on linux and freebsd. https://www.gnu.org/software/gnulib/manual/html_node/copy_005ffile_005frange.html > In order to avoid breaking other hosts, I consider it more cautious to > ignore the difference. Ignoring a bug is not a good thing on the long run :) When there is something suspicious, it's useful to fix it. > In the long term the Hurd is going to implement copy_file_range Yes and by just fixing the prototype, we'll keep qemu able to automatically use it when it becomes available. Really, please, no tinkering, rather fix bugs. Samuel
Re: [PATCH 2/4] Avoid multiple definitions of copy_file_range
Hello,
Manolo de Medici, le mer. 17 janv. 2024 15:47:09 +0100, a ecrit:
> ../../../block/file-posix.c:2003:14: error: conflicting types for
> 'copy_file_range'; have 'off_t(int, off_t *, int, off_t *, size_t,
> unsigned int)' {aka 'long long int(int, long long int *, int, long
> long int *, unsigned int, unsigned int)'}
> 2003 | static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
> | ^~~
> In file included from /root/qemu/include/qemu/osdep.h:122,
> from ../../../block/file-posix.c:25:
> /usr/include/unistd.h:1142:9: note: previous declaration of
> 'copy_file_range' with type 'ssize_t(int, __off64_t *, int,
> __off64_t *, size_t, unsigned int)' {aka 'int(int, long long int *,
> int, long long int *, unsigned int, unsigned int)'}
> 1142 | ssize_t copy_file_range (int __infd, __off64_t *__pinoff,
> | ^~~
>
> the current patch fixes this compilation error.
Yes, but by ignoring the difference :)
The prototype of copy_file_range in glibc really does say that it
returns an ssize_t, not an off_t, so that should be fixed so.
Samuel
Re: [PATCH 2/4] Avoid multiple definitions of copy_file_range
Manolo de Medici, le mer. 17 janv. 2024 15:09:39 +0100, a ecrit:
> Hello Philippe,
> thank you for the feedback, I've checked that. The problem is that the
> Hurd fails that test due to the following:
>
> #if defined __stub_copy_file_range || defined __stub___copy_file_range
> fail fail fail this function is not going to work
> #endifefines the stub __copy_file_ran
>
> rightfully so I'd say, because copy_file_range is just a stub on the Hurd.
Yes.
> As such, we really need to exclude the code that defines the stub in
> qemu on the Hurd.
But how do things work without the qemu stub?
Or put another way: what problem exactly the presence of the qemu stub
makes?
Samuel
> On Wed, Jan 17, 2024 at 2:56 PM Philippe Mathieu-Daudé
> wrote:
> >
> > Hi Manolo,
> >
> > On 17/1/24 13:31, Manolo de Medici wrote:
> > > It's already defined as a stub on the GNU Hurd.
> >
> > Meson checks for this function and defines
> > HAVE_COPY_FILE_RANGE if available, see in meson.build:
> >
> >config_host_data.set('HAVE_COPY_FILE_RANGE',
> > cc.has_function('copy_file_range'))
> >
> > Maybe some header is missing in "osdep.h" for GNU Hurd?
> >
> > > Signed-off-by: Manolo de Medici
> > >
> > > diff --git a/block/file-posix.c b/block/file-posix.c
> > > index 35684f7e21..05426abb7d 100644
> > > --- a/block/file-posix.c
> > > +++ b/block/file-posix.c
> > > @@ -1999,7 +1999,7 @@ static int handle_aiocb_write_zeroes_unmap(void
> > > *opaque)
> > > return handle_aiocb_write_zeroes(aiocb);
> > > }
> > >
> > > -#ifndef HAVE_COPY_FILE_RANGE
> > > +#if !defined(HAVE_COPY_FILE_RANGE) && !defined(__GNU__)
> > > static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
> > >off_t *out_off, size_t len, unsigned int
> > > flags)
> > > {
> > > ---
> > > block/file-posix.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/block/file-posix.c b/block/file-posix.c
> > > index 35684f7e21..05426abb7d 100644
> > > --- a/block/file-posix.c
> > > +++ b/block/file-posix.c
> > > @@ -1999,7 +1999,7 @@ static int handle_aiocb_write_zeroes_unmap(void
> > > *opaque)
> > > return handle_aiocb_write_zeroes(aiocb);
> > > }
> > >
> > > -#ifndef HAVE_COPY_FILE_RANGE
> > > +#if !defined(HAVE_COPY_FILE_RANGE) && !defined(__GNU__)
> > > static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
> > >off_t *out_off, size_t len, unsigned int
> > > flags)
> > > {
> > > --
> > > 2.43.0
> > >
> >
Re: [PATCH v1] Allowing setting and overriding parameters in smb.conf
Henrik Carlqvist, le jeu. 03 août 2023 01:26:02 +0200, a ecrit: > On Thu, 3 Aug 2023 01:13:24 +0200 > Samuel Thibault wrote: > > > Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit: > > > On Wed, 2 Aug 2023 21:53:56 +0200 > > > Samuel Thibault wrote: > > > > > > > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit: > > > > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char > > > > > *exported_dir, > > > > > "printing = bsd\n" > > > > > "disable spoolss = yes\n" > > > > > "usershare max shares = 0\n" > > > > > -"[qemu]\n" > > > > > -"path=%s\n" > > > > > "read only=no\n" > > > > > "guest ok=yes\n" > > > > > + "%s" > > > > > +"[qemu]\n" > > > > > +"path=%s\n" > > > > > > > > ?This is moving read only and guest ok to the global section? > > > > > > Thanks for your quick reply! > > > > > > Yes, I thought that would be OK because the [qemu] share will inherit > > > those settings from the global section. By placing those in the global > > > section it is possible to override them with later contradicting > > > statements like "read only=yes\n" in the %s before the [qemu] section. > > > > Ok, it would be useful to mention that in the changelog. > > Is there a changelog in the repo? Or do you mean that I should mention that in > the git commit message? I mean the latter, yes. Samuel
Re: [PATCH v1] Allowing setting and overriding parameters in smb.conf
Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit: > On Wed, 2 Aug 2023 21:53:56 +0200 > Samuel Thibault wrote: > > > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit: > > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char > > > *exported_dir, > > > "printing = bsd\n" > > > "disable spoolss = yes\n" > > > "usershare max shares = 0\n" > > > -"[qemu]\n" > > > -"path=%s\n" > > > "read only=no\n" > > > "guest ok=yes\n" > > > + "%s" > > > +"[qemu]\n" > > > +"path=%s\n" > > > > ?This is moving read only and guest ok to the global section? > > Thanks for your quick reply! > > Yes, I thought that would be OK because the [qemu] share will inherit those > settings from the global section. By placing those in the global section it is > possible to override them with later contradicting statements like > "read only=yes\n" in the %s before the [qemu] section. Ok, it would be useful to mention that in the changelog. Thanks, Samuel
Re: [PATCH v1] Allowing setting and overriding parameters in smb.conf
Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit: > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char > *exported_dir, > "printing = bsd\n" > "disable spoolss = yes\n" > "usershare max shares = 0\n" > -"[qemu]\n" > -"path=%s\n" > "read only=no\n" > "guest ok=yes\n" > + "%s" > +"[qemu]\n" > +"path=%s\n" ?This is moving read only and guest ok to the global section? Samuel
Re: Tips for local testing guestfwd
Hello, Felix Wu, le mar. 18 juil. 2023 18:12:16 -0700, a ecrit: > 02 == SYN so it looks good. But both tcpdump and wireshark (looking into > packet > dump provided by QEMU invocation) Which packet dump? > I added multiple prints inside slirp and confirmed the ipv6 version of [1] was > reached. > in tcp_output function [2], I got following print: > qemu-system-aarch64: info: Slirp: AF_INET6 out dst ip = > fdb5:481:10ce:0:8c41:aaff:fea9:f674, port = 52190 > qemu-system-aarch64: info: Slirp: AF_INET6 out src ip = fec0::105, port = > 54322 > It looks like there should be something being sent back to the guest, That's what it is. > unless my understanding of tcp_output is wrong. It looks so. > To understand the datapath of guestfwd better, I have the following questions: > 1. What's the meaning of tcp_input and tcp_output? My guess is the following > graph, but I would like to confirm. No, tcp_input is for packets that come from the guest, and tcp_output is for packets that are send to the guest. So it's like that: > tcp_inputwrite_cb host send() > QEMU > slirp ---> QEMU > host > < <- <- > tcp_output slirp_socket_recvhost recv() > 2. I don't see port 6655 in the above process. How does slirp know 6655 is the > port that needs to be visited on the host side? Slirp itself *doesn't* know that port. The guestfwd piece just calls the SlirpWriteCb when it has data coming from the guest. See the documentation: /* Set up port forwarding between a port in the guest network and a * callback that will receive the data coming from the port */ SLIRP_EXPORT int slirp_add_guestfwd(Slirp *slirp, SlirpWriteCb write_cb, void *opaque, struct in_addr *guest_addr, int guest_port); and /* This is called by the application for a guestfwd, to provide the data to be * sent on the forwarded port */ SLIRP_EXPORT void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int guest_port, const uint8_t *buf, int size); Samuel
Re: Tips for local testing guestfwd
Hello, Felix Wu wrote: > 2. I want to understand what ip I should use. Currently I have following > formats for the QEMU invocation in ipv6: > ``` > guestfwd=tcp:[::1]:1234-tcp:[my:host:ip:from:ifconfig]:22 > ``` > I know the general form is `guestfwd=tcp:server:port-dev`, where > server:port is for guest, Yes, the address to be used within the guest network. So it needs to be within the guest network. > Is the aforementioned invocation correct? No, because ::1 isn't in the guest network. > Or in this case [::1] is the local host address and I should put qemu > address for it instead? You can use whatever IP you want, as long as it's in the guest network. e.g. [fec0::1234] if you're with the default fec0::/64 network. > 3. Is there a default ipv6 address for QEMU instance? I think I need it in > the invocation. By default it's address 2 within the prefix, i.e. fec0::2 with the default fec0::/64 network. Samuel
[PULL 1/1] usb-braille: Better explain that one also has to create a chardev backend
Users have reported not to understand the documentation. This completes it to give an explicit example how one is supposed to set up a virtual braille USB device. Signed-off-by: Samuel Thibault --- docs/system/devices/usb.rst | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst index f39a88f080..37cb9b33ae 100644 --- a/docs/system/devices/usb.rst +++ b/docs/system/devices/usb.rst @@ -178,8 +178,20 @@ option or the ``device_add`` monitor command. Available devices are: host character device id. ``usb-braille,chardev=id`` - Braille device. This will use BrlAPI to display the braille output on - a real or fake device referenced by id. + Braille device. This emulates a Baum Braille device USB port. id has to + specify a character device defined with ``-chardev ???,id=id``. One will + normally use BrlAPI to display the braille output on a BRLTTY-supported + device with + + .. parsed-literal:: + + |qemu_system| [...] -chardev braille,id=brl -device usb-braille,chardev=brl + + or alternatively, use the following equivalent shortcut: + + .. parsed-literal:: + + |qemu_system| [...] -usbdevice braille ``usb-net[,netdev=id]`` Network adapter that supports CDC ethernet and RNDIS protocols. id -- 2.35.1
[PULL 0/1] baum: better document usb-braille configuration
The following changes since commit 3e01455edd5fce06c14e2926b6ef408d9a94c9fb: usb-braille: Better explain that one also has to create a chardev backend (2022-09-06 00:09:50 +0200) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 3e01455edd5fce06c14e2926b6ef408d9a94c9fb: usb-braille: Better explain that one also has to create a chardev backend (2022-09-06 00:09:50 +0200) baum: better document usb-braille configuration Samuel Thibault (1): usb-braille: Better explain that one also has to create a chardev backend
Re: slirp: Can I get IPv6-only DHCP working?
Peter Delevoryas, le jeu. 25 août 2022 16:15:26 -0700, a ecrit:
> On Fri, Aug 26, 2022 at 12:56:10AM +0200, Samuel Thibault wrote:
> > Peter Delevoryas, le jeu. 25 août 2022 15:38:53 -0700, a ecrit:
> > > It seems like there's support for an IPv6 dns proxy, and there's
> > > literally a
> > > file called "dhcpv6.c" in slirp, but it has a comment saying it only
> > > supports
> > > whatever is necessary for TFTP network boot I guess.
> >
> > For which DNS support is welcome :)
> >
> > > Maybe there's no support then?
> >
> > It seems there is:
> >
> > if (ri.want_dns) {
> > *resp++ = OPTION_DNS_SERVERS >> 8; /* option-code high byte */
> > *resp++ = OPTION_DNS_SERVERS; /* option-code low byte */
> > *resp++ = 0; /* option-len high byte */
> > *resp++ = 16; /* option-len low byte */
> > memcpy(resp, >vnameserver_addr6, 16);
> > resp += 16;
> > }
>
> Well, that's great, but actually I just care about whether slirp supports
> DHCPv6
> address requests. Sorry if I didn't explain that properly.
Ah. It doesn't seem to indeed.
> since the DHCPv6 hook "dhcpv6_input" is already there, maybe I can
> just get something going through there?
Probably.
Samuel
Re: slirp: Can I get IPv6-only DHCP working?
Hello,
Peter Delevoryas, le jeu. 25 août 2022 15:38:53 -0700, a ecrit:
> It seems like there's support for an IPv6 dns proxy, and there's literally a
> file called "dhcpv6.c" in slirp, but it has a comment saying it only supports
> whatever is necessary for TFTP network boot I guess.
For which DNS support is welcome :)
> Maybe there's no support then?
It seems there is:
if (ri.want_dns) {
*resp++ = OPTION_DNS_SERVERS >> 8; /* option-code high byte */
*resp++ = OPTION_DNS_SERVERS; /* option-code low byte */
*resp++ = 0; /* option-len high byte */
*resp++ = 16; /* option-len low byte */
memcpy(resp, >vnameserver_addr6, 16);
resp += 16;
}
Samuel
Re: [PATCH v2 for-7.2 0/6] Drop libslirp submodule
Thomas Huth, le mer. 24 août 2022 17:11:16 +0200, a ecrit: > At the point in time we're going to release QEMU 7.2, all supported > host OS distributions will have a libslirp package available, so > there is no need anymore for us to ship the slirp submodule. Thus > let's clean up the related tests and finally remove the submodule now. Acked-by: Samuel Thibault Thanks! > v2: > - Added patches to clean up and adapt the tests > - Rebased the removal patch to the latest version of the master branch > > Thomas Huth (6): > tests/docker: Update the debian-all-test-cross container to Debian 11 > tests/vm: Add libslirp to the VM tests > tests/lcitool/libvirt-ci: Update the lcitool module to the latest > version > tests: Refresh dockerfiles and FreeBSD vars with lcitool > tests/avocado: Do not run tests that require libslirp if it is not > available > Remove the slirp submodule (i.e. compile only with an external > libslirp) > > configure | 24 > meson.build | 121 -- > .gitlab-ci.d/buildtest.yml| 20 ++- > .gitlab-ci.d/cirrus/freebsd-12.vars | 2 +- > .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- > .gitlab-ci.d/container-cross.yml | 1 - > .gitmodules | 3 - > MAINTAINERS | 1 - > meson_options.txt | 5 +- > scripts/archive-source.sh | 2 +- > scripts/meson-buildoptions.sh | 4 +- > slirp | 1 - > tests/avocado/avocado_qemu/__init__.py| 7 + > tests/avocado/info_usernet.py | 1 + > tests/avocado/replay_linux.py | 1 + > tests/docker/Makefile.include | 1 - > .../dockerfiles/debian-all-test-cross.docker | 9 +- > tests/docker/dockerfiles/opensuse-leap.docker | 2 +- > tests/docker/dockerfiles/ubuntu2004.docker| 2 +- > tests/lcitool/libvirt-ci | 2 +- > tests/vm/freebsd | 3 + > tests/vm/haiku.x86_64 | 3 +- > tests/vm/netbsd | 3 + > 23 files changed, 64 insertions(+), 156 deletions(-) > delete mode 16 slirp > > -- > 2.31.1 > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 4/4] slirp: Add oob-eth-addr to -netdev options
Peter Delevoryas, le mer. 15 juin 2022 18:05:26 -0700, a ecrit:
> With this change, you can now request the out-of-band MAC address from
> slirp in fby35-bmc:
>
> wget
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd \
> -nographic \
> -netdev
> user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
>
> ...
> username: root
> password: 0penBmc
> ...
>
> root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
> NC-SI Command Response:
> cmd: NCSI_OEM_CMD(0x50)
> Response: COMMAND_COMPLETED(0x) Reason: NO_ERROR(0x)
> Payload length = 24
>
> 20: 0x00 0x00 0x81 0x19
> 24: 0x01 0x00 0x1b 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0xde 0xad 0xbe 0xef
> 36: 0xca 0xfe 0x00 0x00
>
> root@bmc-oob:~# ifconfig
> eth0 Link encap:Ethernet HWaddr DE:AD:BE:EF:CA:FE
> inet addr:10.0.2.15 Bcast:10.0.2.255 Mask:255.255.255.0
> inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
> inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
> RX packets:253 errors:0 dropped:0 overruns:0 frame:0
> TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:24638 (24.0 KiB) TX bytes:18876 (18.4 KiB)
> Interrupt:32
>
> loLink encap:Local Loopback
> inet addr:127.0.0.1 Mask:255.0.0.0
> inet6 addr: ::1/128 Scope:Host
> UP LOOPBACK RUNNING MTU:65536 Metric:1
> RX packets:2 errors:0 dropped:0 overruns:0 frame:0
> TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:120 (120.0 B) TX bytes:120 (120.0 B)
>
> Signed-off-by: Peter Delevoryas
> ---
> net/slirp.c | 13 +++--
> qapi/net.json | 5 -
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 231068c1e2..858d3da859 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
>const char *tftp_server_name, uint32_t mfr_id,
> - Error **errp)
> + uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
> {
> /* default settings according to historic slirp */
> struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> cfg.vdnssearch = dnssearch;
> cfg.vdomainname = vdomainname;
> cfg.mfr_id = mfr_id;
> +memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);
And similarly here.
> s->slirp = slirp_new(, _cb, s);
> QTAILQ_INSERT_TAIL(_stacks, s, entry);
>
> @@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> const NetdevUserOptions *user;
> const char **dnssearch;
> bool ipv4 = true, ipv6 = true;
> +MACAddr oob_eth_addr = {};
>
> assert(netdev->type == NET_CLIENT_DRIVER_USER);
> user = >u.user;
> @@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
> net_init_slirp_configs(user->guestfwd, 0);
>
> +if (user->has_oob_eth_addr &&
> +net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
> +error_setg(errp, "invalid syntax for OOB ethernet address");
> +return -1;
> +}
> +
> ret = net_slirp_init(peer, "user", name, user->q_restrict,
> ipv4, vnet, user->host,
> ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
> @@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> user->bootfile, user->dhcpstart,
> user->dns, user->ipv6_dns, user->smb,
> user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, user->mfr_id, errp);
> + user->tftp_server_name, user->mfr_id,
> oob_eth_addr.a,
> + errp);
>
> while (slirp_configs) {
> config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index efc5cb3fb6..7b2c3c205c 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -169,6 +169,8 @@
> #
> # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> #
> +# @oob-eth-addr:
Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options
Peter Delevoryas, le mer. 15 juin 2022 18:05:25 -0700, a ecrit:
> This lets you set the manufacturer's ID for a slirp netdev, which can be
> queried from the guest through the Get Version ID NC-SI command. For
> example, by setting the manufacturer's ID to 0x8119:
>
> wget
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd -nographic \
> -netdev user,id=nic,mfr-id=0x8119,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
> ...
> username: root
> password: 0penBmc
> ...
> root@bmc-oob:~# ncsi-util 0x15
> NC-SI Command Response:
> cmd: GET_VERSION_ID(0x15)
> Response: COMMAND_COMPLETED(0x) Reason: NO_ERROR(0x)
> Payload length = 40
>
> 20: 0xf1 0xf0 0xf0 0x00
> 24: 0x00 0x00 0x00 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0x00 0x00 0x00 0x00
> 36: 0x00 0x00 0x00 0x00
> 40: 0x00 0x00 0x00 0x00
> 44: 0x00 0x00 0x00 0x00
> 48: 0x00 0x00 0x00 0x00
> 52: 0x00 0x00 0x81 0x19
>
> Signed-off-by: Peter Delevoryas
> ---
> net/slirp.c | 5 +++--
> qapi/net.json | 5 -
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 75e5ccafd9..231068c1e2 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>const char *vnameserver, const char *vnameserver6,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
> - const char *tftp_server_name,
> + const char *tftp_server_name, uint32_t mfr_id,
>Error **errp)
> {
> /* default settings according to historic slirp */
> @@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> cfg.vnameserver6 = ip6_dns;
> cfg.vdnssearch = dnssearch;
> cfg.vdomainname = vdomainname;
> +cfg.mfr_id = mfr_id;
You will need a #if to only include it with slirp 4.8.0 indeed.
Otherwise the mfr_id field won't exist.
In the #else part, it would probably useful to give the user at least a
warning that tells her to upgrade slirp to 4.8.0 to get the mfr_id
functionality working.
> s->slirp = slirp_new(, _cb, s);
> QTAILQ_INSERT_TAIL(_stacks, s, entry);
>
> @@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> user->bootfile, user->dhcpstart,
> user->dns, user->ipv6_dns, user->smb,
> user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, errp);
> + user->tftp_server_name, user->mfr_id, errp);
>
> while (slirp_configs) {
> config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d6..efc5cb3fb6 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -167,6 +167,8 @@
> #
> # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
> #
> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> +#
> # Since: 1.2
> ##
> { 'struct': 'NetdevUserOptions',
> @@ -192,7 +194,8 @@
> '*smbserver': 'str',
> '*hostfwd': ['String'],
> '*guestfwd': ['String'],
> -'*tftp-server-name': 'str' } }
> +'*tftp-server-name': 'str',
> +'*mfr-id': 'uint32' } }
>
> ##
> # @NetdevTapOptions:
> --
> 2.30.2
>
Re: [PATCH 2/4] slirp: Update SlirpConfig version to 5
Hello, Peter Delevoryas, le mer. 15 juin 2022 18:05:24 -0700, a ecrit: > I think we probably need a new Slirp release > (4.8.0) and a switch statement here instead, right? > > So that we can preserve the behavior for 4.7.0? Yes, that's the idea. Samuel
Re: [RFC PATCH 3/4] net: slirp: add support for CFI-friendly timer API
Paolo Bonzini, le mar. 12 avril 2022 14:13:36 +0200, a ecrit:
> libslirp 4.7 introduces a CFI-friendly version of the .timer_new callback.
> The new callback replaces the function pointer with an enum; invoking the
> callback is done with a new function slirp_handle_timer.
>
> Support the new API so that CFI can be made compatible with using a system
> libslirp.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Samuel Thibault
> ---
> net/slirp.c | 41 -
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index b3a92d6e38..57af42299d 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,10 +184,43 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
> return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> }
>
> +typedef struct SlirpTimer SlirpTimer;
> struct SlirpTimer {
> QEMUTimer timer;
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +Slirp *slirp;
> +SlirpTimerId id;
> +void *cb_opaque;
> +#endif
> +};
> +
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +static void net_slirp_init_completed(Slirp *slirp, void *opaque)
> +{
> +SlirpState *s = opaque;
> +s->slirp = slirp;
> }
>
> +static void net_slirp_timer_cb(void *opaque)
> +{
> +SlirpTimer *t = opaque;
> +slirp_handle_timer(t->slirp, t->id, t->cb_opaque);
> +}
> +
> +static void *net_slirp_timer_new_opaque(SlirpTimerId id,
> +void *cb_opaque, void *opaque)
> +{
> +SlirpState *s = opaque;
> +SlirpTimer *t = g_new(SlirpTimer, 1);
> +t->slirp = s->slirp;
> +t->id = id;
> +t->cb_opaque = cb_opaque;
> +timer_init_full(>timer, NULL, QEMU_CLOCK_VIRTUAL,
> +SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +net_slirp_timer_cb, t);
> +return t;
> +}
> +#else
> static void *net_slirp_timer_new(SlirpTimerCb cb,
> void *cb_opaque, void *opaque)
> {
> @@ -197,6 +230,7 @@ static void *net_slirp_timer_new(SlirpTimerCb cb,
> cb, cb_opaque);
> return t;
> }
> +#endif
>
> static void net_slirp_timer_free(void *timer, void *opaque)
> {
> @@ -231,7 +265,12 @@ static const SlirpCb slirp_cb = {
> .send_packet = net_slirp_send_packet,
> .guest_error = net_slirp_guest_error,
> .clock_get_ns = net_slirp_clock_get_ns,
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +.init_completed = net_slirp_init_completed,
> +.timer_new_opaque = net_slirp_timer_new_opaque,
> +#else
> .timer_new = net_slirp_timer_new,
> +#endif
> .timer_free = net_slirp_timer_free,
> .timer_mod = net_slirp_timer_mod,
> .register_poll_fd = net_slirp_register_poll_fd,
> @@ -578,7 +617,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>
> s = DO_UPCAST(SlirpState, nc, nc);
>
> -cfg.version = 3;
> +cfg.version = SLIRP_CHECK_VERSION(4,7,0) ? 4 : 3;
> cfg.restricted = restricted;
> cfg.in_enabled = ipv4;
> cfg.vnetwork = net;
> --
> 2.35.1
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [RFC PATCH 2/4] net: slirp: switch to slirp_new
Paolo Bonzini, le mar. 12 avril 2022 14:13:35 +0200, a ecrit:
> Replace slirp_init with slirp_new, so that a more recent cfg.version
> can be specified.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Samuel Thibault
> ---
> net/slirp.c | 27 +--
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index f1e25d741f..b3a92d6e38 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -389,6 +389,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> #if defined(CONFIG_SMBD_COMMAND)
> struct in_addr smbsrv = { .s_addr = 0 };
> #endif
> +SlirpConfig cfg = { 0 };
> NetClientState *nc;
> SlirpState *s;
> char buf[20];
> @@ -577,12 +578,26 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>
> s = DO_UPCAST(SlirpState, nc, nc);
>
> -s->slirp = slirp_init(restricted, ipv4, net, mask, host,
> - ipv6, ip6_prefix, vprefix6_len, ip6_host,
> - vhostname, tftp_server_name,
> - tftp_export, bootfile, dhcp,
> - dns, ip6_dns, dnssearch, vdomainname,
> - _cb, s);
> +cfg.version = 3;
> +cfg.restricted = restricted;
> +cfg.in_enabled = ipv4;
> +cfg.vnetwork = net;
> +cfg.vnetmask = mask;
> +cfg.vhost = host;
> +cfg.in6_enabled = ipv6;
> +cfg.vprefix_addr6 = ip6_prefix;
> +cfg.vprefix_len = vprefix6_len;
> +cfg.vhost6 = ip6_host;
> +cfg.vhostname = vhostname;
> +cfg.tftp_server_name = tftp_server_name;
> +cfg.tftp_path = tftp_export;
> +cfg.bootfile = bootfile;
> +cfg.vdhcp_start = dhcp;
> +cfg.vnameserver = dns;
> +cfg.vnameserver6 = ip6_dns;
> +cfg.vdnssearch = dnssearch;
> +cfg.vdomainname = vdomainname;
> +s->slirp = slirp_new(, _cb, s);
> QTAILQ_INSERT_TAIL(_stacks, s, entry);
>
> /*
> --
> 2.35.1
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [RFC PATCH 4/4] net: slirp: allow CFI with libslirp >= 4.7
Paolo Bonzini, le mar. 12 avril 2022 14:13:37 +0200, a ecrit:
> slirp 4.7 introduces a new CFI-friendly timer callback that does
> not pass function pointers within libslirp as callbacks for timers.
> Check the version number and, if it is new enough, allow using CFI
> even with a system libslirp.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Samuel Thibault
> ---
> meson.build | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 861de93c4f..92a83580a3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2485,21 +2485,21 @@ if have_system
> slirp = declare_dependency(link_with: libslirp,
> dependencies: slirp_deps,
> include_directories: slirp_inc)
> + else
> +# slirp <4.7 is incompatible with CFI support in QEMU. This is because
> +# it passes function pointers within libslirp as callbacks for timers.
> +# When using a system-wide shared libslirp, the type information for the
> +# callback is missing and the timer call produces a false positive with
> CFI.
> +#
> +# Now that slirp_opt has been defined, check if the selected slirp is
> compatible
> +# with control-flow integrity.
> +if get_option('cfi') and slirp.found() and
> slirp.version().version_compare('<4.7')
> + error('Control-Flow Integrity is not compatible with system-wide
> slirp.' \
> + + ' Please configure with --enable-slirp=git or upgrade to
> libslirp 4.7')
> +endif
>endif
> endif
>
> -# For CFI, we need to compile slirp as a static library together with qemu.
> -# This is because we register slirp functions as callbacks for QEMU Timers.
> -# When using a system-wide shared libslirp, the type information for the
> -# callback is missing and the timer call produces a false positive with CFI.
> -#
> -# Now that slirp_opt has been defined, check if the selected slirp is
> compatible
> -# with control-flow integrity.
> -if get_option('cfi') and slirp_opt == 'system'
> - error('Control-Flow Integrity is not compatible with system-wide slirp.' \
> - + ' Please configure with --enable-slirp=git')
> -endif
> -
> fdt = not_found
> if have_system
>fdt_opt = get_option('fdt')
> --
> 2.35.1
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [RFC PATCH 1/4] net: slirp: introduce a wrapper struct for QemuTimer
Paolo Bonzini, le mar. 12 avril 2022 14:13:34 +0200, a ecrit:
> This struct will be extended in the next few patches to support the
> new slirp_handle_timer() call. For that we need to store an additional
> "int" for each SLIRP timer, in addition to the cb_opaque.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Samuel Thibault
> ---
> net/slirp.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index bc5e9e4f77..f1e25d741f 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,23 +184,32 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
> return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> }
>
> +struct SlirpTimer {
> +QEMUTimer timer;
> +}
> +
> static void *net_slirp_timer_new(SlirpTimerCb cb,
> void *cb_opaque, void *opaque)
> {
> -return timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
> - SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> - cb, cb_opaque);
> +SlirpTimer *t = g_new(SlirpTimer, 1);
> +timer_init_full(>timer, NULL, QEMU_CLOCK_VIRTUAL,
> +SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +cb, cb_opaque);
> +return t;
> }
>
> static void net_slirp_timer_free(void *timer, void *opaque)
> {
> -timer_free(timer);
> +SlirpTimer *t = timer;
> +timer_del(>timer);
> +g_free(t);
> }
>
> static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
> void *opaque)
> {
> -timer_mod(timer, expire_timer);
> +SlirpTimer *t = timer;
> +timer_mod(>timer, expire_timer);
> }
>
> static void net_slirp_register_poll_fd(int fd, void *opaque)
> --
> 2.35.1
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH] configure: Disable capstone and slirp in the --without-default-features mode
Thomas Huth, le lun. 21 févr. 2022 10:06:47 +0100, a ecrit: > For the users, it looks a little bit weird that capstone and slirp are > not disabled automatically if they run the configure script with the > "--without-default-features" option, so let's do that now. > Note: fdt is *not* changed accordingly since this affects the targets > that we can build, so disabling fdt automatically here might have > unexpected side-effects for the users. > > Signed-off-by: Thomas Huth Acked-by: Samuel Thibault > --- > I thought I sent out that patch a couple of weeks ago already, but > I cannot find it in the archives, so I likely missed to send it > correctly. Anyway, sorry if you've got this twice! > > configure | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 3a29eff5cc..36d10d95bb 100755 > --- a/configure > +++ b/configure > @@ -361,9 +361,14 @@ slirp_smbd="$default_feature" > # are included in the automatically generated help message) > > # 1. Track which submodules are needed > -capstone="auto" > +if test "$default_feature" = no ; then > + capstone="disabled" > + slirp="disabled" > +else > + capstone="auto" > + slirp="auto" > +fi > fdt="auto" > -slirp="auto" > > # 2. Support --with/--without option > default_devices="true" > -- > 2.27.0
Re: [PATCH] ide: Cap LBA28 capacity announcement to 2^28-1
Ping?
Samuel Thibault, le mar. 24 août 2021 12:43:44 +0200, a ecrit:
> The LBA28 capacity (at offsets 60/61 of identification) is supposed to
> express the maximum size supported by LBA28 commands. If the device is
> larger than this, we have to cap it to 2^28-1.
>
> At least NetBSD happens to be using this value to determine whether to use
> LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
> LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.
>
> Signed-off-by: Samuel Thibault
> ---
> hw/ide/core.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fd69ca3167..e28f8aad61 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
> static void ide_identify_size(IDEState *s)
> {
> uint16_t *p = (uint16_t *)s->identify_data;
> -put_le16(p + 60, s->nb_sectors);
> -put_le16(p + 61, s->nb_sectors >> 16);
> +int64_t nb_sectors_lba28 = s->nb_sectors;
> +if (nb_sectors_lba28 >= 1 << 28) {
> +nb_sectors_lba28 = (1 << 28) - 1;
> +}
> +put_le16(p + 60, nb_sectors_lba28);
> +put_le16(p + 61, nb_sectors_lba28 >> 16);
> put_le16(p + 100, s->nb_sectors);
> put_le16(p + 101, s->nb_sectors >> 16);
> put_le16(p + 102, s->nb_sectors >> 32);
> --
> 2.32.0
>
Re: [PATCH] net/slirp: Use newer slirp_*_hostxfwd API
Nicholas Ngai, le sam. 25 sept. 2021 16:22:02 -0700, a ecrit:
> Sorry for the duplicate email. The cc’s for the maintainers on the email
> didn’t go through the first time.
>
> Nicholas Ngai
>
> On 9/25/21 2:48 PM, Nicholas Ngai wrote:
> > libslirp provides a newer slirp_*_hostxfwd API meant for
> > address-agnostic forwarding instead of the is_udp parameter which is
> > limited to just TCP/UDP.
> >
> > Signed-off-by: Nicholas Ngai
Reviewed-by: Samuel Thibault
> > ---
> > net/slirp.c | 64 +++--
> > 1 file changed, 42 insertions(+), 22 deletions(-)
> >
> > diff --git a/net/slirp.c b/net/slirp.c
> > index ad3a838e0b..49ae01a2f0 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -643,12 +643,17 @@ static SlirpState *slirp_lookup(Monitor *mon, const
> > char *id)
> > void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
> > {
> > -struct in_addr host_addr = { .s_addr = INADDR_ANY };
> > -int host_port;
> > +struct sockaddr_in host_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = INADDR_ANY,
> > +},
> > +};
> > +int port;
> > +int flags = 0;
> > char buf[256];
> > const char *src_str, *p;
> > SlirpState *s;
> > -int is_udp = 0;
> > int err;
> > const char *arg1 = qdict_get_str(qdict, "arg1");
> > const char *arg2 = qdict_get_try_str(qdict, "arg2");
> > @@ -670,9 +675,9 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict
> > *qdict)
> > }
> > if (!strcmp(buf, "tcp") || buf[0] == '\0') {
> > -is_udp = 0;
> > +/* Do nothing; already TCP. */
> > } else if (!strcmp(buf, "udp")) {
> > -is_udp = 1;
> > +flags |= SLIRP_HOSTFWD_UDP;
> > } else {
> > goto fail_syntax;
> > }
> > @@ -680,15 +685,17 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict
> > *qdict)
> > if (get_str_sep(buf, sizeof(buf), , ':') < 0) {
> > goto fail_syntax;
> > }
> > -if (buf[0] != '\0' && !inet_aton(buf, _addr)) {
> > +if (buf[0] != '\0' && !inet_aton(buf, _addr.sin_addr)) {
> > goto fail_syntax;
> > }
> > -if (qemu_strtoi(p, NULL, 10, _port)) {
> > +if (qemu_strtoi(p, NULL, 10, )) {
> > goto fail_syntax;
> > }
> > +host_addr.sin_port = htons(port);
> > -err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
> > +err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) _addr,
> > +sizeof(host_addr), flags);
> > monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
> > err ? "not found" : "removed");
> > @@ -700,12 +707,22 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict
> > *qdict)
> > static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error
> > **errp)
> > {
> > -struct in_addr host_addr = { .s_addr = INADDR_ANY };
> > -struct in_addr guest_addr = { .s_addr = 0 };
> > -int host_port, guest_port;
> > +struct sockaddr_in host_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = INADDR_ANY,
> > +},
> > +};
> > +struct sockaddr_in guest_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = 0,
> > +},
> > +};
> > +int flags = 0;
> > +int port;
> > const char *p;
> > char buf[256];
> > -int is_udp;
> > char *end;
> > const char *fail_reason = "Unknown reason";
> > @@ -715,9 +732,9 @@ static int slirp_hostfwd(SlirpState *s, const char
> > *redir_str, Error **errp)
> > goto fail_syntax;
> > }
> > if (!strcmp(buf, "tcp") || buf[0] == '\0') {
> > -is_udp = 0;
> > +/* Do nothing; already TCP. */
> > } else if (!strcmp(buf, "udp")) {
> > -is_udp = 1;
> > +flags |= SLIRP_HOSTFWD_UDP;
> > } else {
> > fail_reason = "Bad protocol name";
> > goto fail_syntax;
> > @@ -727,7 +744,7 @@ static int slirp_hostfwd(SlirpState *s, const char
> > *redir_str, Error **
Re: [PATCH] ide: Cap LBA28 capacity announcement to 2^28-1
Ping?
Samuel Thibault, le mar. 24 août 2021 12:43:44 +0200, a ecrit:
> The LBA28 capacity (at offsets 60/61 of identification) is supposed to
> express the maximum size supported by LBA28 commands. If the device is
> larger than this, we have to cap it to 2^28-1.
>
> At least NetBSD happens to be using this value to determine whether to use
> LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
> LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.
>
> Signed-off-by: Samuel Thibault
> ---
> hw/ide/core.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fd69ca3167..e28f8aad61 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
> static void ide_identify_size(IDEState *s)
> {
> uint16_t *p = (uint16_t *)s->identify_data;
> -put_le16(p + 60, s->nb_sectors);
> -put_le16(p + 61, s->nb_sectors >> 16);
> +int64_t nb_sectors_lba28 = s->nb_sectors;
> +if (nb_sectors_lba28 >= 1 << 28) {
> +nb_sectors_lba28 = (1 << 28) - 1;
> +}
> +put_le16(p + 60, nb_sectors_lba28);
> +put_le16(p + 61, nb_sectors_lba28 >> 16);
> put_le16(p + 100, s->nb_sectors);
> put_le16(p + 101, s->nb_sectors >> 16);
> put_le16(p + 102, s->nb_sectors >> 32);
> --
> 2.32.0
>
[PATCH] ide: Cap LBA28 capacity announcement to 2^28-1
The LBA28 capacity (at offsets 60/61 of identification) is supposed to
express the maximum size supported by LBA28 commands. If the device is
larger than this, we have to cap it to 2^28-1.
At least NetBSD happens to be using this value to determine whether to use
LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.
Signed-off-by: Samuel Thibault
---
hw/ide/core.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index fd69ca3167..e28f8aad61 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
static void ide_identify_size(IDEState *s)
{
uint16_t *p = (uint16_t *)s->identify_data;
-put_le16(p + 60, s->nb_sectors);
-put_le16(p + 61, s->nb_sectors >> 16);
+int64_t nb_sectors_lba28 = s->nb_sectors;
+if (nb_sectors_lba28 >= 1 << 28) {
+nb_sectors_lba28 = (1 << 28) - 1;
+}
+put_le16(p + 60, nb_sectors_lba28);
+put_le16(p + 61, nb_sectors_lba28 >> 16);
put_le16(p + 100, s->nb_sectors);
put_le16(p + 101, s->nb_sectors >> 16);
put_le16(p + 102, s->nb_sectors >> 32);
--
2.32.0
Re: [OSS-Fuzz] Assertion Failure: !in6_zero(_addr) (#111)
Hello,
Alexander Bulekov, le lun. 03 mai 2021 16:09:33 -0400, a ecrit:
> Forwarding this along to the list, so it doesn't get burried during the
> gitlab issue migration.
Thanks!
Pushed a proposed fix on
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/86
Samuel
> - Forwarded message from "Alexander Bulekov (@a1xndr)"
> -
>
> Alexander Bulekov created an issue:
> https://gitlab.com/qemu-project/qemu/-/issues/111
>
> Hello,
> Reproducer
> ```
> cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> 512M -M q35 -nodefaults -device e1000e,netdev=net0 -netdev user,id=net0 \
> -qtest stdio
> outl 0xcf8 0x8813
> outl 0xcfc 0x56
> outl 0xcf8 0x8801
> outl 0xcfc 0x0600
> write 0x56000403 0x1 0x02
> write 0x5600042b 0x1 0x80
> write 0x20a 0x1 0x86
> write 0x20b 0x1 0xdd
> write 0x20c 0x1 0x60
> write 0x212 0x1 0x11
> write 0x213 0x1 0x01
> write 0x224 0x1 0xfe
> write 0x225 0x1 0xc0
> write 0x233 0x1 0x02
> write 0x237 0x1 0x45
> write 0x23d 0x1 0x01
> write 0xb 0x1 0x24
> write 0x10 0x1 0xfe
> write 0x11 0x1 0x01
> write 0x19 0x1 0x01
> write 0x1a 0x1 0x10
> write 0x1b 0x1 0x25
> write 0x5600043a 0x1 0x04
> EOF
> ```
>
> Stack-trace:
> ```
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../net/eth.c:374:27 in
> ../net/eth.c:375:27: runtime error: member access within misaligned address
> 0x63114846 for type 'struct ip6_header', which requires 4 byte alignment
> 0x63114846: note: pointer points here
> 00 00 11 11 60 00 00 00 00 00 11 11 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 fe c0 00 00
> ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../net/eth.c:375:27 in
> qemu-fuzz-i386: ../slirp/src/ndp_table.c:59: _Bool ndp_table_search(Slirp *,
> struct in6_addr, uint8_t *): Assertion `!in6_zero(_addr)' failed.
>
> #8 in __assert_fail assert/assert.c:101:3
> #9 in ndp_table_search /slirp/src/ndp_table.c:59:5
> #10 in if_encap6 /slirp/src/slirp.c:926:10
> #11 in if_encap /slirp/src/slirp.c:967:15
> #12 in if_start /slirp/src/if.c:183:45
> #13 in ip6_output /slirp/src/ip6_output.c:35:9
> #14 in tftp_udp_output /slirp/src/tftp.c:161:9
> #15 in tftp_send_error /slirp/src/tftp.c:223:5
> #16 in tftp_handle_rrq /slirp/src/tftp.c
> #17 in tftp_input /slirp/src/tftp.c:453:9
> #18 in udp6_input /slirp/src/udp6.c:81:9
> #19 in slirp_input /slirp/src/slirp.c:847:13
> #20 in net_slirp_receive /net/slirp.c:136:5
> #21 in nc_sendv_compat /net/net.c
> #22 in qemu_deliver_packet_iov /net/net.c:765:15
> #23 in qemu_net_queue_deliver_iov /net/queue.c:179:11
> #24 in qemu_net_queue_send_iov /net/queue.c:246:11
> #25 in net_tx_pkt_sendv /hw/net/net_tx_pkt.c:558:9
> #26 in net_tx_pkt_send /hw/net/net_tx_pkt.c:633:9
> #27 in e1000e_tx_pkt_send /hw/net/e1000e_core.c:659:16
> #28 in e1000e_process_tx_desc /hw/net/e1000e_core.c:736:17
> #29 in e1000e_start_xmit /hw/net/e1000e_core.c:927:9
> #30 in e1000e_set_tdt /hw/net/e1000e_core.c:2444:9
> #31 in e1000e_core_write /hw/net/e1000e_core.c:3256:9
> #32 in memory_region_write_accessor /softmmu/memory.c:491:5
> #33 in access_with_adjusted_size /softmmu/memory.c:552:18
> #34 in memory_region_dispatch_write /softmmu/memory.c
> #35 in flatview_write_continue /softmmu/physmem.c:2746:23
> #36 in flatview_write /softmmu/physmem.c:2786:14
> #37 in address_space_write /softmmu/physmem.c:2878:18
> ```
>
> Test-case:
> ```
> /*
> * Autogenerated Fuzzer Test Case
> *
> * Copyright (c) 2021
> *
> * This work is licensed under the terms of the GNU GPL, version 2 or later.
> * See the COPYING file in the top-level directory.
> */
>
> #include "qemu/osdep.h"
>
> #include "libqos/libqtest.h"
>
> static void test_fuzz(void)
> {
> QTestState *s = qtest_init("-display none , -m 512M -M q35 -nodefaults
> -device "
>"e1000e,netdev=net0 -netdev user,id=net0");
> qtest_outl(s, 0xcf8, 0x8813);
> qtest_outl(s, 0xcfc, 0x56);
> qtest_outl(s, 0xcf8, 0x8801);
> qtest_outl(s, 0xcfc, 0x0600);
> qtest_bufwrite(s, 0x56000403, "\x02", 0x1);
> qtest_bufwrite(s, 0x5600042b, "\x80", 0x1);
> qtest_bufwrite(s, 0x20a, "\x86", 0x1);
> qtest_bufwrite(s, 0x20b, "\xdd", 0x1);
> qtest_bufwrite(s, 0x20c, "\x60", 0x1);
> qtest_bufwrite(s, 0x212, "\x11", 0x1);
> qtest_bufwrite(s, 0x213, "\x01", 0x1);
> qtest_bufwrite(s, 0x224, "\xfe", 0x1);
> qtest_bufwrite(s, 0x225, "\xc0", 0x1);
> qtest_bufwrite(s, 0x233, "\x02", 0x1);
> qtest_bufwrite(s, 0x237, "\x45", 0x1);
> qtest_bufwrite(s, 0x23d, "\x01", 0x1);
> qtest_bufwrite(s, 0xb, "\x24", 0x1);
> qtest_bufwrite(s, 0x10, "\xfe", 0x1);
> qtest_bufwrite(s, 0x11, "\x01", 0x1);
> qtest_bufwrite(s, 0x19, "\x01", 0x1);
> qtest_bufwrite(s, 0x1a, "\x10", 0x1);
> qtest_bufwrite(s, 0x1b, "\x25", 0x1);
> qtest_bufwrite(s, 0x5600043a, "\x04", 0x1);
> qtest_quit(s);
> }
> int main(int argc, char **argv)
> {
> const char *arch =
Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation
Marc-André Lureau, le jeu. 06 mai 2021 01:27:25 +0400, a ecrit: > @@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const > uint8_t *buf, int len) > } > timer_del(baum->cellCount_timer); > > - memset(zero, 0, sizeof(zero)); > + memset(zero, 0, baum->x * baum->y); > > > eh, I would have left the sizeof(zero).. I was wondering too, but below this it's clear that only baum->x * baum->y is getting used. Samuel
Re: [PATCH 04/23] chardev/baum: Avoid dynamic stack allocation
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:28 +0200, a ecrit:
> Use autofree heap allocation instead of variable-length
> array on the stack.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Samuel Thibault
> ---
> chardev/baum.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/baum.c b/chardev/baum.c
> index 0822e9ed5f3..bc09cda3471 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -299,7 +299,8 @@ static void baum_chr_accept_input(struct Chardev *chr)
> static void baum_write_packet(BaumChardev *baum, const uint8_t *buf, int len)
> {
> Chardev *chr = CHARDEV(baum);
> -uint8_t io_buf[1 + 2 * len], *cur = io_buf;
> +g_autofree uint8_t *io_buf = g_malloc(1 + 2 * len);
> +uint8_t *cur = io_buf;
> int room;
> *cur++ = ESC;
> while (len--)
> --
> 2.26.3
>
Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:27 +0200, a ecrit:
> We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not
> a big value, it is actually 84). Instead of having the compiler
> use variable-length array, declare an array able to hold the
> maximum 'x * y'.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Samuel Thibault
> ---
> chardev/baum.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/chardev/baum.c b/chardev/baum.c
> index adc3d7b3b56..0822e9ed5f3 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -383,9 +383,9 @@ static int baum_eat_packet(BaumChardev *baum, const
> uint8_t *buf, int len)
> switch (req) {
> case BAUM_REQ_DisplayData:
> {
> -uint8_t cells[baum->x * baum->y], c;
> -uint8_t text[baum->x * baum->y];
> -uint8_t zero[baum->x * baum->y];
> +uint8_t cells[X_MAX * Y_MAX], c;
> +uint8_t text[X_MAX * Y_MAX];
> +uint8_t zero[X_MAX * Y_MAX];
> int cursor = BRLAPI_CURSOR_OFF;
> int i;
>
> @@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const
> uint8_t *buf, int len)
> }
> timer_del(baum->cellCount_timer);
>
> -memset(zero, 0, sizeof(zero));
> +memset(zero, 0, baum->x * baum->y);
>
> brlapi_writeArguments_t wa = {
> .displayNumber = BRLAPI_DISPLAY_DEFAULT,
> --
> 2.26.3
>
Re: [PATCH 02/23] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:26 +0200, a ecrit:
> Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Samuel Thibault
> ---
> chardev/baum.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/chardev/baum.c b/chardev/baum.c
> index 5deca778bc4..adc3d7b3b56 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -87,6 +87,9 @@
>
> #define BUF_SIZE 256
>
> +#define X_MAX 84
> +#define Y_MAX 1
> +
> struct BaumChardev {
> Chardev parent;
>
> @@ -244,11 +247,11 @@ static int baum_deferred_init(BaumChardev *baum)
> brlapi_perror("baum: brlapi__getDisplaySize");
> return 0;
> }
> -if (baum->y > 1) {
> -baum->y = 1;
> +if (baum->y > Y_MAX) {
> +baum->y = Y_MAX;
> }
> -if (baum->x > 84) {
> -baum->x = 84;
> +if (baum->x > X_MAX) {
> +baum->x = X_MAX;
> }
>
> con = qemu_console_lookup_by_index(0);
> --
> 2.26.3
>
Re: [PATCH] net/slirp: Fix incorrect permissions on samba >= 2.0.5
Laurent Vivier, le ven. 30 avril 2021 18:48:29 +0200, a ecrit: > CC: +Samuel I don't know the smb code at all. > Le 23/02/2021 à 03:41, Niklas Hambüchen a écrit : > > As the added commend and `man smb.conf` explain, starting > > with that samba version, `force user` must be configured > > in `[global]` in order to access the configured `smb_dir`. > > > > This broke `-net user,smb=/path/to/folder`: > > > > The `chdir` into e.g. `/run/user/0/qemu-smb.DCZ8Y0` failed. > > In verbose logs, this manifested as: > > > > [..., effective(65534, 65534), real(65534, 0)] > > /source3/smbd/service.c:159(chdir_current_service) > > chdir (/run/user/0) failed, reason: Permission denied > > > > [..., effective(65534, 65534), real(65534, 0)] > > /source3/smbd/service.c:167(chdir_current_service) > > chdir (/run/user/0) failed, reason: Permission denied > > > > [..., effective(65534, 65534), real(65534, 0)] > > /source3/smbd/uid.c:448(change_to_user_internal) > > change_to_user_internal: chdir_current_service() failed! > > > > This commit fixes it by setting the `[global]` force user to > > the user that owns the directories `smbd` needs to access. > > > > Signed-off-by: Niklas Hambüchen > > --- > > net/slirp.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/net/slirp.c b/net/slirp.c > > index be914c0be0..82387bdb19 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -850,6 +850,11 @@ static int slirp_smb(SlirpState* s, const char > > *exported_dir, > > } > > fprintf(f, > > "[global]\n" > > +"# In Samba 2.0.5 and above the 'force user' parameter\n" > > +"# also causes the primary group of the forced user to be > > used\n" > > +"# as the primary group for all file activity.\n" > > +"# This includes the various directories set below.\n" > > +"force user=%s\n" > > "private dir=%s\n" > > "interfaces=127.0.0.1\n" > > "bind interfaces only=yes\n" > > @@ -871,6 +876,7 @@ static int slirp_smb(SlirpState* s, const char > > *exported_dir, > > "read only=no\n" > > "guest ok=yes\n" > > "force user=%s\n", > > +passwd->pw_name, > > s->smb_dir, > > s->smb_dir, > > s->smb_dir, > > >
Re: [PATCH 3/4] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Gerd Hoffmann, le jeu. 11 mars 2021 12:37:38 +0100, a ecrit: > Which would also drop support for serial braille devices. Not sure > how much of a problem that would be these days. It is an important concern: we also need to be able to test braille devices connected through serial. Samuel
Re: [PATCH] baum: Fix crash when Braille output is not available
Peter Maydell, le mer. 10 mars 2021 17:18:11 +, a ecrit:
> On Wed, 10 Mar 2021 at 16:08, Samuel Thibault
> wrote:
> >
> > When Braille output is not available, the backend properly reports being
> > unable to be created, but 5f8e93c3e262 ("util/qemu-timer: Make timer_free()
> > imply timer_del()") made the timer_free() call now refuse any NULL
> > parameter. char_braille_finalize thus now has to be more careful with
> > calling it on baum->cellCount_timer.
>
> It wasn't the intention of that commit to make freeing a NULL
> timer invalid;
Ok!
Samuel
[PATCH] baum: Fix crash when Braille output is not available
When Braille output is not available, the backend properly reports being
unable to be created, but 5f8e93c3e262 ("util/qemu-timer: Make timer_free()
imply timer_del()") made the timer_free() call now refuse any NULL
parameter. char_braille_finalize thus now has to be more careful with
calling it on baum->cellCount_timer.
Signed-off-by: Samuel Thibault
---
chardev/baum.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/chardev/baum.c b/chardev/baum.c
index 5deca778bc..aca5bf12fb 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -631,7 +631,9 @@ static void char_braille_finalize(Object *obj)
{
BaumChardev *baum = BAUM_CHARDEV(obj);
-timer_free(baum->cellCount_timer);
+if (baum->cellCount_timer) {
+timer_free(baum->cellCount_timer);
+}
if (baum->brlapi) {
brlapi__closeConnection(baum->brlapi);
g_free(baum->brlapi);
--
2.30.1
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Daniel P. Berrangé, le mer. 10 mars 2021 15:31:48 +, a ecrit: > On Wed, Mar 10, 2021 at 04:26:46PM +0100, Paolo Bonzini wrote: > > On 10/03/21 16:02, Samuel Thibault wrote: > > > > > When trying to remove the -usbdevice option, there were complaints > > > > > that > > > > > "-usbdevice braille" is still a very useful shortcut for some people. > > > > Pointer? I missed it. > > > > > > For instance > > > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html > > > > In one sentence: "Braille is worth a special case because a subset of our > > user base (blind people) will use it 100% of the time, plus it is not > > supported by libvirt and hence virt-manager". > > If simplicity of enabling braille support is critical, we could get > something even simpler than "-usbdevice braille", and just provide > a bare "-braille" with no args required as a "do the right thing" > option ? That was discussed a bit earlier in the thread: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00681.html https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00686.html https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00687.html Just like keyboard/mouse, one would still want to specify whether the braille device is to be connected through usb or serial, so at least "-braille usb" and "-braille serial". Note https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00689.html Paolo wrote: > Adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... The thing is: creating one without the other does not make sense. Samuel
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Markus Armbruster, le mer. 10 mars 2021 14:17:48 +0100, a ecrit: > Thomas Huth writes: > > When trying to remove the -usbdevice option, there were complaints that > > "-usbdevice braille" is still a very useful shortcut for some people. > > Pointer? I missed it. For instance https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html > "braille" is the only driver with a factory. "-usbdevice braille" is > sugar for > > -device usb-braille,chardev=braille -chardev braille,id=braille > -machine usb=on > > It's buggy: > > $ qemu-system-x86_64 -S -usbdevice braille > qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, > please use '-device usb-...' instead > [three seconds tick by...] > Segmentation fault (core dumped) I don't have time to keep testing the usbdevice braille option of qemu from git indeed. But I do use the option at least weekly/monthly with releases. And some braille users do depend on it for their daily work. > It neglects to actually parse PARAMS: > > $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot" > qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' > is deprecated, please use '-device usb-...' instead I wasn't aware of the issue, thus not fixed of course. > The only one that has something approaching a leg to stand on is > braille. Still, I fail to see why having to specify a backend is fine > for any number of other devices, but not for braille. Because people who use this option have no idea how they would be supposed to make it work otherwise. > -device usb-braille,chardev=braille -chardev braille,id=braille > -machine usb=on This is *way* beyond what people would work out, even if documented. For them a braille device is just one device, there is no need for notion of frontend/backend, this is all in just one "device" for them. Put another way, it does not even make sense to build anything different from what you wrote: using another chardev backend with usb-braille frontend doesn't make sense, and exposing the braille chardev backend on another usb frontend doesn't make sense either. > If users need more time, we can extend the grace period. They don't need time, they need things that are simple to use. That three-option black magic really isn't. Samuel
Re: [PATCH v2 4/4] slirp: feature detection for smbd
Joelle van Dyne, le mar. 09 mars 2021 10:11:31 -0800, a ecrit: > On Tue, Mar 9, 2021 at 6:09 AM Philippe Mathieu-Daudé > wrote: > > On 3/9/21 1:27 AM, Joelle van Dyne wrote: > > > Replace Windows specific macro with a more generic feature detection > > > macro. Allows slirp smb feature to be disabled manually as well. > > > > > > Signed-off-by: Joelle van Dyne > > > --- > > > configure | 26 +++--- > > > meson.build | 2 +- > > > net/slirp.c | 16 > > > 3 files changed, 32 insertions(+), 12 deletions(-) > > > > Hmm v1 had: Acked-by: Samuel Thibault > > did you change something to not include Samuel A-b tag? > > Sorry, I must have missed it! NP :) Samuel
Re: [PATCH 4/4] slirp: feature detection for smbd
Joelle van Dyne, le dim. 07 mars 2021 22:48:21 -0800, a ecrit:
> Replace Windows specific macro with a more generic feature detection
> macro. Allows slirp smb feature to be disabled manually as well.
>
> Signed-off-by: Joelle van Dyne
Acked-by: Samuel Thibault
> ---
> configure | 26 +++---
> meson.build | 3 +++
> net/slirp.c | 16
> 3 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/configure b/configure
> index 34fccaa2ba..8335a3e6a0 100755
> --- a/configure
> +++ b/configure
> @@ -465,6 +465,7 @@ fuse_lseek="auto"
> multiprocess="auto"
>
> malloc_trim="auto"
> +slirp_smbd="auto"
>
> # parse CC options second
> for opt do
> @@ -834,8 +835,6 @@ do
> fi
> done
>
> -: ${smbd=${SMBD-/usr/sbin/smbd}}
> -
> # Default objcc to clang if available, otherwise use CC
> if has clang; then
>objcc=clang
> @@ -1560,6 +1559,10 @@ for opt do
>;;
>--disable-multiprocess) multiprocess="disabled"
>;;
> + --enable-slirp-smbd) slirp_smbd=yes
> + ;;
> + --disable-slirp-smbd) slirp_smbd=no
> + ;;
>*)
>echo "ERROR: unknown option $opt"
>echo "Try '$0 --help' for more information"
> @@ -1913,6 +1916,7 @@ disabled with --disable-FEATURE, default is enabled if
> available
>fuseFUSE block device export
>fuse-lseek SEEK_HOLE/SEEK_DATA support for FUSE exports
>multiprocessOut of process device emulation support
> + slirp-smbd use smbd (at path --smbd=*) in slirp networking
>
> NOTE: The object files are built at the place where configure is launched
> EOF
> @@ -5252,6 +5256,19 @@ case "$slirp" in
> ;;
> esac
>
> +# Check for slirp smbd dupport
> +: ${smbd=${SMBD-/usr/sbin/smbd}}
> +if test "$slirp_smbd" != "no" ; then
> + if test "$mingw32" = "yes" ; then
> +if test "$slirp_smbd" = "yes" ; then
> + error_exit "Host smbd not supported on this platform."
> +fi
> +slirp_smbd=no
> + else
> +slirp_smbd=yes
> + fi
> +fi
> +
> ##
> # check for usable __NR_keyctl syscall
>
> @@ -5527,7 +5544,10 @@ fi
> if test "$guest_agent" = "yes" ; then
>echo "CONFIG_GUEST_AGENT=y" >> $config_host_mak
> fi
> -echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> +if test "$slirp_smbd" = "yes" ; then
> + echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
> + echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> +fi
> if test "$vde" = "yes" ; then
>echo "CONFIG_VDE=y" >> $config_host_mak
>echo "VDE_LIBS=$vde_libs" >> $config_host_mak
> diff --git a/meson.build b/meson.build
> index ba0db9fa1f..773ce512c7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2629,6 +2629,9 @@ summary_info += {'pixman':pixman.found()}
> summary_info += {'VTE support': config_host.has_key('CONFIG_VTE')}
> # TODO: add back version
> summary_info += {'slirp support': slirp_opt == 'disabled' ? false :
> slirp_opt}
> +if slirp_opt != 'disabled' and 'CONFIG_SLIRP_SMBD' in config_host
> + summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
> +endif
> summary_info += {'libtasn1': config_host.has_key('CONFIG_TASN1')}
> summary_info += {'PAM': config_host.has_key('CONFIG_AUTH_PAM')}
> summary_info += {'iconv support': iconv.found()}
> diff --git a/net/slirp.c b/net/slirp.c
> index be914c0be0..b3ded2aac1 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -27,7 +27,7 @@
> #include "net/slirp.h"
>
>
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
> #include
> #include
> #endif
> @@ -90,7 +90,7 @@ typedef struct SlirpState {
> Slirp *slirp;
> Notifier poll_notifier;
> Notifier exit_notifier;
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
> gchar *smb_dir;
> #endif
> GSList *fwd;
> @@ -103,7 +103,7 @@ static QTAILQ_HEAD(, SlirpState) slirp_stacks =
> static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp);
> static int slirp_guestfwd(SlirpState *s, const char *config_str, Error
> **errp);
>
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
> static int slirp_smb(SlirpState *s, const char *exported_dir,
> struct in_addr vserver_addr, Error **errp);
&
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Hello,
Doug Evans, le ven. 05 mars 2021 17:00:13 -0800, a ecrit:
> Is it possible for QEMU to lazily determine the guest's IPv6
> address? I.e., postpone the ""->guest address mapping until it's
> needed and then, say, take the first entry in the NDP table?
That would probably be possible, yes, by moving the
if (!guest_addr.s_addr) {
guest_addr = slirp->vdhcp_startaddr;
}
from slirp_add_hostfwd() and alike to tcp_connect() and sorecvfrom()
(along the other sotranslate call).
> That feels a bit fragile: what if someone else gets the first entry in
> the NDP table? But is that any more fragile than assuming the first
> handed out DHCP address is to the guest?
I don't think it's really more fragile.
> [<<-- Honest question, can we assume the first handed out DHCP address
> will necessarily be the guest?]
It "cannot" be anything else. What could happen is a PXE loader that
uses DHCP/NDP, and then the OS that does it again.
> But that would mean the defaults for the guest would have to be
> different than for the host. E.g.,
> host: ",ipv4" means both,
Why would it mean both? I don't follow you here.
> whereas guest: ",ipv4" (ideally) means ipv4 (since both is meaningless)
Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Doug Evans, le ven. 05 mars 2021 16:05:05 -0800, a ecrit: > Given that the code is not supposed to be able to know brackets were present > (they're stripped off early on), what does the above mean w.r.t. the guest? > For the host we can have "" mean listen on both IPv4 and IPv6 > (by default, absent extra options like ipv4=off). > But what does a guest address of "" mean? > IPv4? IPv6? Both? It cannot really be "both" since it'd need to know. The 0.0.0.0 address used to mean the dhcp-provided address, and we don't have a way to know the ipv6 address used with the stateless selection, so I would say that empty address would be just the dhcp-provided address. Most probably the guest will pick it up anyway, and guests usually listen the same on ipv4 and ipv6, so I'd say empty most probably means the user wants to just connect to ipv4 (whatever protocol was used to connect to the host). Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Daniel P. Berrangé, le mer. 03 mars 2021 18:11:41 +, a ecrit: > On Wed, Mar 03, 2021 at 10:06:50AM -0800, Doug Evans wrote: > > On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault > > wrote: > > > > > > + Examples: > > > > + hostfwd_add net0 tcp:127.0.0.1:10022-:22 > > > > + hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22 > > > > > > Yep, that looks good to me. > > > > Daniel, you wanted me to use inet_parse(). > > Is the above syntax ok with you? > > You must have had some expectation that at least some of > > the various flags that inet_parse() recognizes would be needed here. > > It feels like the ,ipv4=on|off,ipv6=on|off flags are relevant here, > especially in the empty address case. eg > >tcp::10022 - attempt to listen on both ipv4 + ipv6 >tcp::10022,ipv4=off - listen on default address, but only for ipv6 >tcp::10022,ipv6=off - listen on default address, but only for ipv4 > > Basically this ends up bringing the hostfwd stuff into alignment with > the way other backends deal with this I'm fine with this yes, better have a coherent user interface. Samuel
[Bug 1917442] Re: [AHCI] crash when running a GNU/Hurd guest
Note: this is using the rump ahci driver.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917442
Title:
[AHCI] crash when running a GNU/Hurd guest
Status in QEMU:
New
Bug description:
QEMU git hash = 51db2d7cf2
Running guest OS using:
$ gdb --args /extra/qemu/bin/qemu-system-i386 -M q35,accel=kvm -m 4096
-net user,hostfwd=tcp::-:22 -net nic -drive
id=udisk,file=/dev/sdd,format=raw,if=none -device ide-
drive,drive=udisk,bootindex=1 -curses
...
root@zamhurd:~# .ahcisata0 channel 5: setting WDCTL_RST failed for
drive 0
Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x74f7bf00 (LWP 590666)]
ahci_commit_buf (dma=0x57335870, tx_bytes=2048) at ../hw/ide/ahci.c:1462
1462tx_bytes += le32_to_cpu(ad->cur_cmd->status);
(gdb) bt full
#0 ahci_commit_buf (dma=0x57335870, tx_bytes=2048)
at ../hw/ide/ahci.c:1462
ad = 0x57335870
#1 0x55893171 in dma_buf_commit (s=0x57335930, tx_bytes=2048)
at ../hw/ide/core.c:805
#2 0x558934f8 in ide_dma_cb (opaque=0x57335930, ret=0)
at ../hw/ide/core.c:887
s = 0x57335930
n = 4
sector_num = 4491160
offset = 140732794753312
stay_active = false
prep_size = 0
__PRETTY_FUNCTION__ = "ide_dma_cb"
#3 0x55830720 in dma_complete (dbs=0x7ffee83d5120, ret=0)
at ../softmmu/dma-helpers.c:121
__PRETTY_FUNCTION__ = "dma_complete"
#4 0x558307cd in dma_blk_cb (opaque=0x7ffee83d5120, ret=0)
at ../softmmu/dma-helpers.c:139
dbs = 0x7ffee83d5120
cur_addr = 140732794753408
cur_len = 93825013280880
mem = 0x7ffeeccfef00
__PRETTY_FUNCTION__ = "dma_blk_cb"
#5 0x55d92bce in blk_aio_complete (acb=0x7ffee847bbe0)
at ../block/block-backend.c:1412
#6 0x55d92df0 in blk_aio_read_entry (opaque=0x7ffee847bbe0)
at ../block/block-backend.c:1466
acb = 0x7ffee847bbe0
rwco = 0x7ffee847bc08
qiov = 0x7ffee83d5180
__PRETTY_FUNCTION__ = "blk_aio_read_entry"
#7 0x55e85580 in coroutine_trampoline (i0=-398117056, i1=32766)
at ../util/coroutine-ucontext.c:173
arg = {p = 0x7ffee8453740, i = {-398117056, 32766}}
self = 0x7ffee8453740
co = 0x7ffee8453740
fake_stack_save = 0x0
#8 0x76544020 in __start_context () at /lib64/libc.so.6
#9 0x7ffeefdfd680 in ()
#10 0x in ()
(gdb)
(gdb) l
1457 */
1458 static void ahci_commit_buf(const IDEDMA *dma, uint32_t tx_bytes)
1459 {
1460 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
1461
1462 tx_bytes += le32_to_cpu(ad->cur_cmd->status);
1463 ad->cur_cmd->status = cpu_to_le32(tx_bytes);
1464 }
1465
1466 static int ahci_dma_rw_buf(const IDEDMA *dma, bool is_write)
(gdb) p ad
$1 = (AHCIDevice *) 0x57335870
(gdb) p ad->cur_cmd
$2 = (AHCICmdHdr *) 0x0
(gdb)
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1917442/+subscriptions
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Samuel Thibault, le lun. 01 mars 2021 17:27:47 +0100, a ecrit: > Doug Evans, le lun. 01 mars 2021 08:23:03 -0800, a ecrit: > > On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault <[1]samuel.thiba...@gnu.org> > > wrote: > > > > [...] > > > Note that one issue I am leaving for later (i.e., I don't want to drag > > this > > > patch series out to include it), is whether and how to support > > ipv4-host-> > > > ipv6-guest forwarding and vice versa. Can libslirp support this? > > > > That would be feasible yes: since the data flow is completely rebuilt > > between the host and the guest, there is no remnant of the IP version. > > It was simpler to have e.g. udp_listen and udp6_listen separate to keep > > uint32_t / in6_addr parameters, but there is no strict reason for this: > > the haddr is only passed to the bind() call, and the laddr is only > > recorded in the so. Put another way, a refactoring patch could be to > > just hand udp_listen two sockaddrs, and it will just work fine. We'd > > then introduce a slirp_add_hostfwd that takes two sockaddr instead of > > host/port. > > > > > > > > I guess I'm not familiar enough with this code. > > Help me understand how passing two addresses to udp_listen is simpler. > > That feels confusing from an API viewpoint. > > ? udp_listen already takes two addresses + two ports. I just mean > replacing that with two sockaddr, which contains both, but also contains > the address family. I submitted this to > > https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74 And the public API to https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/75 Ideally we'd then just drop the ipv6-only public API. Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Samuel Thibault, le lun. 01 mars 2021 17:26:23 +0100, a ecrit: > We could make libslirp enable the IPV6ONLY flag to avoid that, and > make qemu pass an AF_UNSPEC address for the ipv4+ipv6 case, in which > case libslirp wouldn't set IPV6ONLY. Ah, no, AF_UNSPEC would not allow to specify the port. But we can use a flag in the slirp_add_hostxfwd call. Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Doug Evans, le lun. 01 mars 2021 08:23:03 -0800, a ecrit: > On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault <[1]samuel.thiba...@gnu.org> > wrote: > > [...] > > Note that one issue I am leaving for later (i.e., I don't want to drag > this > > patch series out to include it), is whether and how to support > ipv4-host-> > > ipv6-guest forwarding and vice versa. Can libslirp support this? > > That would be feasible yes: since the data flow is completely rebuilt > between the host and the guest, there is no remnant of the IP version. > It was simpler to have e.g. udp_listen and udp6_listen separate to keep > uint32_t / in6_addr parameters, but there is no strict reason for this: > the haddr is only passed to the bind() call, and the laddr is only > recorded in the so. Put another way, a refactoring patch could be to > just hand udp_listen two sockaddrs, and it will just work fine. We'd > then introduce a slirp_add_hostfwd that takes two sockaddr instead of > host/port. > > > > I guess I'm not familiar enough with this code. > Help me understand how passing two addresses to udp_listen is simpler. > That feels confusing from an API viewpoint. ? udp_listen already takes two addresses + two ports. I just mean replacing that with two sockaddr, which contains both, but also contains the address family. I submitted this to https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74 Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Doug Evans, le lun. 01 mars 2021 08:07:19 -0800, a ecrit: > Are there any users that this functional change would break? > [Previously the empty address meant qemu would only listen on ipv4 addr-any.] One case that could be broken would be a user having already another service listening on ipv6-only along qemu listening on ipv4-only. But I find this very little probable. > What if a user wants only ipv4 addr-any (or only ipv6 addr-any) ? "0.0.0.0" would get ipv4 addr-any. Without anything done in particular, "::" would get both ipv6 and ipv4. We could make libslirp enable the IPV6ONLY flag to avoid that, and make qemu pass an AF_UNSPEC address for the ipv4+ipv6 case, in which case libslirp wouldn't set IPV6ONLY. make that ipv6-only through a flag passed to > What does hostfwd "::12345-6.7.8.9:10" mean? > Does the presence of the empty host address mean forward both ipv4 and ipv6 to > guest ipv4 6.7.8.9? I'd say so, yes. Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Markus Armbruster, le lun. 01 mars 2021 09:15:41 +0100, a ecrit: > Samuel Thibault writes: > > Specifying [127.0.0.1] would be odd, but for instance > > > > ssh localhost -D '[127.0.0.1]':23456 > > > > happens to listen on 127.0.0.1. So I would say that common practice > > really is that [] only matters for syntax, and not semantic. > > I believe common syntactic practice is to use [brackets] only around > numeric IPv6 addresses. E.g. socat(1): > >IP address > An IPv4 address in numbers-and-dots notation, an IPv6 address in > hex notation enclosed in brackets, or a hostname that resolves > to an IPv4 or an IPv6 address. > Examples: 127.0.0.1, [::1], www.dest-unreach.org, dns1 Yes and that's also what ssh documents, but in ssh the brackets happen to also work for an IPv4 address. Samuel
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
Hello, Doug Evans, le lun. 08 févr. 2021 10:59:01 -0800, a ecrit: > Samuel, how do qemu patches involving libslirp changes usually work? Well, we haven't had many yet actually :) > Should I have held off submitting the qemu patch until the libslirp > prerequisite has been added to qemu's tree, No, as this example shows, there are iterations that can happen on the qemu side before we have something we can commit, so better do both in parallel. I don't know what qemu people think about the slirp submodule: do qemu prefers to only track stable branchs, or would it be fine to track the master branch? I guess you'd prefer to have a slirp stable release you can depend on when releasing qemu, so perhaps better wait for a slirp release before bumping in qemu, just to be safe? Which doesn't mean avoiding submitting patchqueues that depend on it before that, better go in parallel. > or maybe I should include the libslirp patch so that people can at least apply > it (with a caveat saying the patch is already in libslirp.git) until it's > added > to the qemu tree? Not sure what is best here. We at least need something so that people are not confused by the patchqueue calling some function that doesn't exist in the submodule yet. Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Samuel Thibault, le dim. 28 févr. 2021 22:39:57 +0100, a ecrit: > It was simpler to have e.g. udp_listen and udp6_listen separate to keep > uint32_t / in6_addr parameters, but there is no strict reason for this: > the haddr is only passed to the bind() call, and the laddr is only > recorded in the so. Put another way, a refactoring patch could be to > just hand udp_listen two sockaddrs, and it will just work fine. I have submitted that part to https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74 Could you review it? > We'd then introduce a slirp_add_xhostfwd that takes two sockaddr > instead of host/port. That should then be easy to introduce indeed, and immediately more powerful than the slirp_add/remove_ipv6_hostfwd. Possibly you could just replace in qemu the existing slirp_add/remove_hostfwd call, and thus make the whole code simpler: ideally it's the address parsing function that would produce a sockaddr. Samuel
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
Hello, Daniel P. Berrangé, le lun. 22 févr. 2021 09:39:41 +, a ecrit: > In general callers shouldn't care about which format was parsed. The use > of [] is just a mechanism to reliably separate the port from the address. > Once you have the address part getaddrinfo() will reliably parse the > address into a sockaddr struct on its own. Agreed. > The is_v6 flag is only needed > for the legacy compat needs in slirp, even that is only if we want to > have strict equivalence with historical behaviour, as opposed to changing > empty string to mean to listen on both IPv4+6 concurrently.. I would say that empty address meaning ipv4+6 looks better to me. Doug Evans, le lun. 22 févr. 2021 09:55:09 -0800, a ecrit: > Hi guys. I think before I submit yet another patchset in this series I need > someone with authority to define the user API for ipv6 host forwarding. > Since the hostfwd syntax is parsed in net/slirp.c, Samuel I think that means > you (based on what I'm reading in MAINTAINERS). Well, I'm not maintainer of the user API actually. That'd rather be Markus Armbruster, now Cc-ed, who devises the command-line options, QAPI, etc. > Based on what Maxim originally wrote I was going with addresses wrapped in [] > mean ipv6, but Daniel does not want that. Specifying [127.0.0.1] would be odd, but for instance ssh localhost -D '[127.0.0.1]':23456 happens to listen on 127.0.0.1. So I would say that common practice really is that [] only matters for syntax, and not semantic. > There are issues to consider of course. > Note that one issue I am leaving for later (i.e., I don't want to drag this > patch series out to include it), is whether and how to support ipv4-host-> > ipv6-guest forwarding and vice versa. Can libslirp support this? That would be feasible yes: since the data flow is completely rebuilt between the host and the guest, there is no remnant of the IP version. It was simpler to have e.g. udp_listen and udp6_listen separate to keep uint32_t / in6_addr parameters, but there is no strict reason for this: the haddr is only passed to the bind() call, and the laddr is only recorded in the so. Put another way, a refactoring patch could be to just hand udp_listen two sockaddrs, and it will just work fine. We'd then introduce a slirp_add_hostfwd that takes two sockaddr instead of host/port. > Setting cross-forwarding aside, we can't break existing uses of course, so > that > means that one issue is that if [] is not used to identify ipv6 addresses then > something like ",ipv6" will be needed as a separate argument; otherwise the > empty address "" will be ambiguous. (as I mentioned above, I'd say empty address "" should mean ipv4+ipv6) > + Examples: > + hostfwd_add net0 tcp:127.0.0.1:10022-:22 > + hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22 Yep, that looks good to me. Samuel
Re: [PATCH v2 2/2] net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands
Doug Evans, le mer. 03 févr. 2021 13:37:29 -0800, a ecrit:
> @@ -1392,6 +1392,34 @@ SRST
>Remove host-to-guest TCP or UDP redirection.
> ERST
>
> +#ifdef CONFIG_SLIRP
> +{
> +.name = "ipv6_hostfwd_add",
> +.args_type = "arg1:s,arg2:s?",
> +.params = "[netdev_id]
> [tcp|udp]:[hostaddr6]:hostport-[guestaddr6]:guestport",
Perhaps explicit that the IPv6 address should be enclosed with [] ?
> +/* Ignore the part between the ']' and addr_sep. */
> +if (get_str_sep(buf, sizeof(buf), , addr_sep) < 0) {
Mmm, I would say that we do not want to just ignore it, and rather make
sure that it is empty, so that we can possibly make extensions later
without breaking existing misuse.
Samuel
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
Doug Evans, le mer. 03 févr. 2021 13:37:28 -0800, a ecrit: > ... in preparation for adding ipv6 host forwarding support. Reviewed-by: Samuel Thibault except > diff --git a/slirp b/slirp > index 8f43a99191..358c0827d4 16 > --- a/slirp > +++ b/slirp > @@ -1 +1 @@ > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > +Subproject commit 358c0827d49778f016312bfb4167fe639900681f Which, I would say, deserves its own commit?
Re: [RFC] Change default ipv6 network from fec0/10 (site local) to fe80/10 (link local)
Hello, Philippe Mathieu-Daudé, le mer. 27 janv. 2021 22:46:13 +0100, a ecrit: > On 1/27/21 8:13 PM, Doug Evans wrote: > > I happened to notice QEMU's default for the ipv6 network is fec0::/10 > > which is deprecated (RFC3879). > > I think(!) an obvious replacement is fe80::/10, link local. fe80::/10 is really a different thing, I don't think we want to use it. We can use some prefix in fc00::/7, such as fd00::/8. It "just" needs checking with various guest OS, to check that it doesn't break the default behavior. Samuel
Re: [PATCH] net/slirp.c: Fix spelling error in error message
Doug Evans, le jeu. 21 janv. 2021 16:42:51 -0800, a ecrit:
> DNS should be DHCP
>
> Signed-off-by: Doug Evans
Reviewed-by: Samuel Thibault
> ---
> net/slirp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 8350c6d45f..be914c0be0 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -473,7 +473,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> return -1;
> }
> if (dhcp.s_addr == host.s_addr || dhcp.s_addr == dns.s_addr) {
> -error_setg(errp, "DNS must be different from host and DNS");
> +error_setg(errp, "DHCP must be different from host and DNS");
> return -1;
> }
>
> --
> 2.30.0.280.ga3ce27912f-goog
>
Re: [PATCH v2] meson: fix ncurses detection on macOS
Chris Hofstaedtler, le mer. 30 déc. 2020 23:17:27 +0100, a ecrit:
> Without this, meson fails with "curses package not usable" when using ncurses
> 6.2. Apparently the wide functions (addwstr, etc) are hidden behind the extra
> define, and meson does not define it at that detection stage.
>
> Regression from b01a4fd3bd7d6f2 ("configure: Define NCURSES_WIDECHAR if we're
> using curses"). The meson conversion has seen many iterations of the curses
> check, so pinpointing the exact commit breaking this is not so easy.
>
> Signed-off-by: Chris Hofstaedtler
> Cc: Peter Maydell
> Cc: Philippe Mathieu-Daudé
> Cc: Samuel Thibault
> Cc: Yonggang Luo
Reviewed-by: Samuel Thibault
> ---
> meson.build | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 372576f82c..fd74728674 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -500,16 +500,16 @@ if have_system and not get_option('curses').disabled()
> endif
>endforeach
>msg = get_option('curses').enabled() ? 'curses library not found' : ''
> + curses_compile_args = ['-DNCURSES_WIDECHAR']
>if curses.found()
> -if cc.links(curses_test, dependencies: [curses])
> - curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR',
> dependencies: [curses])
> +if cc.links(curses_test, args: curses_compile_args, dependencies:
> [curses])
> + curses = declare_dependency(compile_args: curses_compile_args,
> dependencies: [curses])
> else
>msg = 'curses package not usable'
>curses = not_found
> endif
>endif
>if not curses.found()
> -curses_compile_args = ['-DNCURSES_WIDECHAR']
> has_curses_h = cc.has_header('curses.h', args: curses_compile_args)
> if targetos != 'windows' and not has_curses_h
>message('Trying with /usr/include/ncursesw')
> --
> 2.29.2
>
Re: [PATCH] meson: fix ncurses detection on macOS
Philippe Mathieu-Daudé, le lun. 28 déc. 2020 18:20:13 +0100, a ecrit:
> On 12/28/20 4:16 PM, Chris Hofstaedtler wrote:
> > Without this, meson fails with "curses package not usable"
> > when using ncurses 6.2. Apparently the wide functions
> > (addwstr, etc) are hidden behind the extra define, and
> > meson does not define it at that detection stage.
>
> Seems reasonable, but still Cc'ing more developers.
That looks sensible indeed.
> > Signed-off-by: Chris Hofstaedtler
> > ---
> > meson.build | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 9c152a85bd..7b9d92c14a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -510,7 +510,7 @@ if have_system and not get_option('curses').disabled()
> >endforeach
> >msg = get_option('curses').enabled() ? 'curses library not found' : ''
> >if curses.found()
> > -if cc.links(curses_test, dependencies: [curses])
> > +if cc.links(curses_test, args: '-DNCURSES_WIDECHAR', dependencies:
> > [curses])
> >curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR',
> > dependencies: [curses])
> > else
> >msg = 'curses package not usable'
Re: Does QEMU's coverity-scan run need to track coverity issues in dtb or slirp ?
Hello, Peter Maydell, le lun. 02 nov. 2020 19:54:14 +, a ecrit: > Do dtc and slirp as upstream projects already track Coverity issues We started tracking them yes. Samuel
Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response
Mark Cave-Ayland, le lun. 26 oct. 2020 13:40:05 +, a ecrit: > On 26/10/2020 13:00, Jason Andryuk wrote: > > On Mon, Oct 26, 2020 at 7:21 AM Samuel Thibault > > wrote: > > > Aurelien, you introduced the "| 1" in > > > > > > commit abb8a13918ecc1e8160aa78582de9d5224ea70df > > > Author: Aurelien Jarno > > > Date: Wed Aug 13 04:23:17 2008 + > > > > > > usb-serial: add support for modem lines > > > > > > [...] > > > @@ -357,9 +393,9 @@ static int usb_serial_handle_control(USBDevice *dev, > > > int request, int value, > > > /* TODO: TX ON/OFF */ > > > break; > > > case DeviceInVendor | FTDI_GET_MDM_ST: > > > -/* TODO: return modem status */ > > > -data[0] = 0; > > > -ret = 1; > > > +data[0] = usb_get_modem_lines(s) | 1; > > > +data[1] = 0; > > > +ret = 2; > > > break; > > > > I'm not particularly familiar with the FTDI USB serial devices. I > > found setting FTDI_THRE | FTDI_TEMT by comparing with real hardware. > > > > A little searching found this: > > https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L541 > > > > That shows "B0 Reserved - must be 1", so maybe that is why "| 1" was > > added? > > Right - that's for the modem status returned as part of the first 2 status > bytes for incoming data which is slightly different from modem status > returned directly from FTDI_SIO_GET_MODEM_STATUS: > https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L423. > > It is the latter which this patch changes and appears to match what I see on > my Chipi-X hardware here. Aurelien, do you remember the reason for the addition of 1 here? It does look like the confusion between the incoming data bytes and the modem status bytes. Samuel
Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response
Mark Cave-Ayland, le lun. 26 oct. 2020 10:58:43 +, a ecrit: > On 26/10/2020 09:54, Samuel Thibault wrote: > > Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:00 +, a ecrit: > > > The FTDI_GET_MDM_ST response should only return a single byte indicating > > > the > > > modem status with bit 0 cleared (as documented in the Linux ftdi_sio.h > > > header > > > file). > > > > > > Signed-off-by: Mark Cave-Ayland > > > --- > > > hw/usb/dev-serial.c | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > > > index 4c374d0790..fa734bcf54 100644 > > > --- a/hw/usb/dev-serial.c > > > +++ b/hw/usb/dev-serial.c > > > @@ -360,9 +360,8 @@ static void usb_serial_handle_control(USBDevice *dev, > > > USBPacket *p, > > > /* TODO: TX ON/OFF */ > > > break; > > > case VendorDeviceRequest | FTDI_GET_MDM_ST: > > > -data[0] = usb_get_modem_lines(s) | 1; > > > -data[1] = FTDI_THRE | FTDI_TEMT; > > > -p->actual_length = 2; > > > +data[0] = usb_get_modem_lines(s); > > > +p->actual_length = 1; > > [...] > A quick test shows my Chipi-X returns 0x1 0x60 with nothing attached in > response to FTDI_SIO_GET_MODEM_STATUS_REQUEST: assuming the reply length > should be 2 bytes, the comment about B0-B3 being zero and the response from > my Chip-X above suggests that the "| 1" should still be dropped from the > response. Aurelien, you introduced the "| 1" in commit abb8a13918ecc1e8160aa78582de9d5224ea70df Author: Aurelien Jarno Date: Wed Aug 13 04:23:17 2008 + usb-serial: add support for modem lines [...] @@ -357,9 +393,9 @@ static int usb_serial_handle_control(USBDevice *dev, int request, int value, /* TODO: TX ON/OFF */ break; case DeviceInVendor | FTDI_GET_MDM_ST: -/* TODO: return modem status */ -data[0] = 0; -ret = 1; +data[0] = usb_get_modem_lines(s) | 1; +data[1] = 0; +ret = 2; break; do you know exactly what it is for? Samuel
Re: [PATCH 0/9] dev-serial: minor fixes and improvements
Hello, Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:52 +, a ecrit: > This series comes from a client project that I have been working on over the > past few months which involves communicating with serial hardware and > associated simulators using the QEMU USB serial device. Thanks for these! I only had concerned with the MDM_ST change, see the corresponding mail. The other patches can be applied independently from that change. Samuel
Re: [PATCH 9/9] dev-serial: store flow control and xon/xoff characters
Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:01 +, a ecrit:
> Note that whilst the device does not do anything with these values, they are
> logged with trace events and stored to allow future implementation.
>
> The default flow control is set to none at reset as documented in the Linux
> ftdi_sio.h header file.
>
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 38 +++---
> hw/usb/trace-events | 2 ++
> 2 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index fa734bcf54..3dbc56d77a 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -52,6 +52,7 @@
>
> /* SET_FLOW_CTRL */
>
> +#define FTDI_NO_HS 0
> #define FTDI_RTS_CTS_HS1
> #define FTDI_DTR_DSR_HS2
> #define FTDI_XON_XOFF_HS 4
> @@ -98,6 +99,9 @@ struct USBSerialState {
> uint8_t error_chr;
> uint8_t event_trigger;
> bool always_plugged;
> +uint8_t flow_control;
> +uint8_t xon;
> +uint8_t xoff;
> QEMUSerialSetParams params;
> int latency;/* ms */
> CharBackend cs;
> @@ -181,14 +185,36 @@ static const USBDesc desc_braille = {
> .str = desc_strings,
> };
>
> +static void usb_serial_set_flow_control(USBSerialState *s,
> +uint8_t flow_control)
> +{
> +USBDevice *dev = USB_DEVICE(s);
> +USBBus *bus = usb_bus_from_device(dev);
> +
> +/* TODO: ioctl */
> +s->flow_control = flow_control;
> +trace_usb_serial_set_flow_control(bus->busnr, dev->addr, flow_control);
> +}
> +
> +static void usb_serial_set_xonxoff(USBSerialState *s, int xonxoff)
> +{
> +USBDevice *dev = USB_DEVICE(s);
> +USBBus *bus = usb_bus_from_device(dev);
> +
> +s->xon = xonxoff & 0xff;
> +s->xoff = (xonxoff >> 8) & 0xff;
> +
> +trace_usb_serial_set_xonxoff(bus->busnr, dev->addr, s->xon, s->xoff);
> +}
> +
> static void usb_serial_reset(USBSerialState *s)
> {
> -/* TODO: Set flow control to none */
> s->event_chr = 0x0d;
> s->event_trigger = 0;
> s->recv_ptr = 0;
> s->recv_used = 0;
> /* TODO: purge in char driver */
> +usb_serial_set_flow_control(s, FTDI_NO_HS);
> }
>
> static void usb_serial_handle_reset(USBDevice *dev)
> @@ -285,9 +311,15 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_TIOCM, );
> break;
> }
> -case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL:
> -/* TODO: ioctl */
> +case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL: {
> +uint8_t flow_control = index >> 8;
> +
> +usb_serial_set_flow_control(s, flow_control);
> +if (flow_control & FTDI_XON_XOFF_HS) {
> +usb_serial_set_xonxoff(s, value);
> +}
> break;
> +}
> case VendorDeviceOutRequest | FTDI_SET_BAUD: {
> static const int subdivisors8[8] = { 0, 4, 2, 1, 3, 5, 6, 7 };
> int subdivisor8 = subdivisors8[((value & 0xc000) >> 14)
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 0d0a3e5f2a..725b7077ad 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -331,3 +331,5 @@ usb_serial_unsupported_data_bits(int bus, int addr, int
> value) "dev %d:%d unsupp
> usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token"
> usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d"
> usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev
> %d:%d parity %c, data bits %d, stop bits %d"
> +usb_serial_set_flow_control(int bus, int addr, int index) "dev %d:%d flow
> control %d"
> +usb_serial_set_xonxoff(int bus, int addr, uint8_t xon, uint8_t xoff) "dev
> %d:%d xon 0x%x xoff 0x%x"
> --
> 2.20.1
>
--
Samuel
> dvips -o $@ $<
Faut faire gffe de pas te couper avec ton truc, t'as mis des ciseaux ($<)
partout :))
-+- Dom in Guide du linuxien pervers - "J'aime pas les Makefile !" -+-
Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response
Hello,
(Cc-ing Aurelien who introduced the support for modem control, and Jason
who added the missing THRE and TEMT flags).
Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:00 +, a ecrit:
> The FTDI_GET_MDM_ST response should only return a single byte indicating the
> modem status with bit 0 cleared (as documented in the Linux ftdi_sio.h header
> file).
>
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/usb/dev-serial.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 4c374d0790..fa734bcf54 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -360,9 +360,8 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> /* TODO: TX ON/OFF */
> break;
> case VendorDeviceRequest | FTDI_GET_MDM_ST:
> -data[0] = usb_get_modem_lines(s) | 1;
> -data[1] = FTDI_THRE | FTDI_TEMT;
> -p->actual_length = 2;
> +data[0] = usb_get_modem_lines(s);
> +p->actual_length = 1;
Err, but Linux' drivers/usb/serial/ftdi_sio.c:ftdi_process_packet()
contradicts this:
if (len < 2) {
dev_dbg(>dev, "malformed packet\n");
return 0;
}
status = buf[0] & FTDI_STATUS_B0_MASK;
if (status != priv->prev_status) {
char diff_status = status ^ priv->prev_status;
if (diff_status & FTDI_RS0_CTS)
port->icount.cts++;
[...]
/* save if the transmitter is empty or not */
if (buf[1] & FTDI_RS_TEMT)
priv->transmit_empty = 1;
else
priv->transmit_empty = 0;
Did you actually get an issue with seeing this packet contain two bytes?
Samuel
Re: [PATCH 7/9] dev-serial: add support for setting data_bits in QEMUSerialSetParams
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:59 +, a ecrit:
> Also implement the behaviour reported in Linux's ftdi_sio.c whereby if an
> invalid
> data_bits value is provided then the hardware defaults to using 8.
>
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 17 +
> hw/usb/trace-events | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 919e25e1d9..4c374d0790 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -308,6 +308,23 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> break;
> }
> case VendorDeviceOutRequest | FTDI_SET_DATA:
> +switch (value & 0xff) {
> +case 7:
> +s->params.data_bits = 7;
> +break;
> +case 8:
> +s->params.data_bits = 8;
> +break;
> +default:
> +/*
> + * According to a comment in Linux's ftdi_sio.c original FTDI
> + * chips fall back to 8 data bits for unsupported data_bits
> + */
> +trace_usb_serial_unsupported_data_bits(bus->busnr, dev->addr,
> + value & 0xff);
> +s->params.data_bits = 8;
> +}
> +
> switch (value & FTDI_PARITY) {
> case 0:
> s->params.parity = 'N';
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 9e984b2e0c..0d0a3e5f2a 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -327,6 +327,7 @@ usb_serial_handle_control(int bus, int addr, int request,
> int value) "dev %d:%d
> usb_serial_unsupported_parity(int bus, int addr, int value) "dev %d:%d
> unsupported parity %d"
> usb_serial_unsupported_stopbits(int bus, int addr, int value) "dev %d:%d
> unsupported stop bits %d"
> usb_serial_unsupported_control(int bus, int addr, int request, int value)
> "dev %d:%d got unsupported/bogus control 0x%x, value 0x%x"
> +usb_serial_unsupported_data_bits(int bus, int addr, int value) "dev %d:%d
> unsupported data bits %d, falling back to 8"
> usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token"
> usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d"
> usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev
> %d:%d parity %c, data bits %d, stop bits %d"
> --
> 2.20.1
>
--
Samuel
> Il [e2fsck] a bien démarré, mais il m'a rendu la main aussitot en me
> disant "houlala, c'est pas beau à voir votre truc, je préfèrerai que
> vous teniez vous même la tronçonneuse" (traduction libre)
NC in Guide du linuxien pervers : "Bien configurer sa tronçonneuse."
Re: [PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:58 +, a ecrit:
> Some operating systems will generate a new device ID when a USB device is
> unplugged
> and then replugged into the USB. If this is done whilst switching between
> multiple
> applications over a virtual serial port, the change of device ID requires
> going
> back into the OS/application to locate the new device accordingly.
>
> Add a new always-plugged property that if specified will ensure that the
> device
> always remains attached to the USB regardless of the state of the backend
> chardev.
>
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 92c35615eb..919e25e1d9 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -97,6 +97,7 @@ struct USBSerialState {
> uint8_t event_chr;
> uint8_t error_chr;
> uint8_t event_trigger;
> +bool always_plugged;
> QEMUSerialSetParams params;
> int latency;/* ms */
> CharBackend cs;
> @@ -516,12 +517,12 @@ static void usb_serial_event(void *opaque, QEMUChrEvent
> event)
> s->event_trigger |= FTDI_BI;
> break;
> case CHR_EVENT_OPENED:
> -if (!s->dev.attached) {
> +if (!s->always_plugged && !s->dev.attached) {
> usb_device_attach(>dev, _abort);
> }
> break;
> case CHR_EVENT_CLOSED:
> -if (s->dev.attached) {
> +if (!s->always_plugged && s->dev.attached) {
> usb_device_detach(>dev);
> }
> break;
> @@ -556,7 +557,8 @@ static void usb_serial_realize(USBDevice *dev, Error
> **errp)
> usb_serial_event, NULL, s, NULL, true);
> usb_serial_handle_reset(dev);
>
> -if (qemu_chr_fe_backend_open(>cs) && !dev->attached) {
> +if (s->always_plugged || (qemu_chr_fe_backend_open(>cs) &&
> + !dev->attached)) {
> usb_device_attach(dev, _abort);
> }
> s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
> @@ -584,6 +586,7 @@ static const VMStateDescription vmstate_usb_serial = {
>
> static Property serial_properties[] = {
> DEFINE_PROP_CHR("chardev", USBSerialState, cs),
> +DEFINE_PROP_BOOL("always-plugged", USBSerialState, always_plugged,
> false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> --
> 2.20.1
>
--
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)
Re: [PATCH 4/9] dev-serial: add trace-events for baud rate and data parameters
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:56 +, a ecrit: > Signed-off-by: Mark Cave-Ayland Reviewed-by: Samuel Thibault > --- > hw/usb/dev-serial.c | 3 +++ > hw/usb/trace-events | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index abc316c7bf..badf8785db 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -307,6 +307,7 @@ static void usb_serial_handle_control(USBDevice *dev, > USBPacket *p, > } > > s->params.speed = (4800 / 2) / (8 * divisor + subdivisor8); > +trace_usb_serial_set_baud(bus->busnr, dev->addr, s->params.speed); > qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params); > break; > } > @@ -340,6 +341,8 @@ static void usb_serial_handle_control(USBDevice *dev, > USBPacket *p, > goto fail; > } > > +trace_usb_serial_set_data(bus->busnr, dev->addr, s->params.parity, > + s->params.data_bits, s->params.stop_bits); > qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params); > /* TODO: TX ON/OFF */ > break; > diff --git a/hw/usb/trace-events b/hw/usb/trace-events > index e5871cbbbc..9e984b2e0c 100644 > --- a/hw/usb/trace-events > +++ b/hw/usb/trace-events > @@ -328,3 +328,5 @@ usb_serial_unsupported_parity(int bus, int addr, int > value) "dev %d:%d unsupport > usb_serial_unsupported_stopbits(int bus, int addr, int value) "dev %d:%d > unsupported stop bits %d" > usb_serial_unsupported_control(int bus, int addr, int request, int value) > "dev %d:%d got unsupported/bogus control 0x%x, value 0x%x" > usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token" > +usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d" > +usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev > %d:%d parity %c, data bits %d, stop bits %d" > -- > 2.20.1 > -- Samuel J'ai un gros problème: j'ai cet exercice à rendre demain lundi, mais ma TI 89 ne sait pas le faire... Est-ce que quelqu'un pourrait m'aider?? -+- OD In Guide du Neuneu Usenet : Comment ça ! Il faut réfléchir ?-+-
Re: [PATCH 5/9] dev-serial: replace DeviceOutVendor/DeviceInVendor with equivalent macros from usb.h
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:57 +, a ecrit:
> The DeviceOutVendor and DeviceInVendor macros can be replaced with their
> equivalent VendorDeviceOutRequest and VendorDeviceRequest macros from usb.h.
>
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 25 ++---
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index badf8785db..92c35615eb 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -37,11 +37,6 @@
> #define FTDI_SET_LATENCY 9
> #define FTDI_GET_LATENCY 10
>
> -#define DeviceOutVendor \
> - ((USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> -#define DeviceInVendor \
> - ((USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> -
> /* RESET */
>
> #define FTDI_RESET_SIO 0
> @@ -253,7 +248,7 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> break;
>
> /* Class specific requests. */
> -case DeviceOutVendor | FTDI_RESET:
> +case VendorDeviceOutRequest | FTDI_RESET:
> switch (value) {
> case FTDI_RESET_SIO:
> usb_serial_reset(s);
> @@ -268,7 +263,7 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> break;
> }
> break;
> -case DeviceOutVendor | FTDI_SET_MDM_CTRL:
> +case VendorDeviceOutRequest | FTDI_SET_MDM_CTRL:
> {
> static int flags;
> qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_GET_TIOCM, );
> @@ -289,10 +284,10 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_TIOCM, );
> break;
> }
> -case DeviceOutVendor | FTDI_SET_FLOW_CTRL:
> +case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL:
> /* TODO: ioctl */
> break;
> -case DeviceOutVendor | FTDI_SET_BAUD: {
> +case VendorDeviceOutRequest | FTDI_SET_BAUD: {
> static const int subdivisors8[8] = { 0, 4, 2, 1, 3, 5, 6, 7 };
> int subdivisor8 = subdivisors8[((value & 0xc000) >> 14)
> | ((index & 1) << 2)];
> @@ -311,7 +306,7 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
> break;
> }
> -case DeviceOutVendor | FTDI_SET_DATA:
> +case VendorDeviceOutRequest | FTDI_SET_DATA:
> switch (value & FTDI_PARITY) {
> case 0:
> s->params.parity = 'N';
> @@ -346,23 +341,23 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
> /* TODO: TX ON/OFF */
> break;
> -case DeviceInVendor | FTDI_GET_MDM_ST:
> +case VendorDeviceRequest | FTDI_GET_MDM_ST:
> data[0] = usb_get_modem_lines(s) | 1;
> data[1] = FTDI_THRE | FTDI_TEMT;
> p->actual_length = 2;
> break;
> -case DeviceOutVendor | FTDI_SET_EVENT_CHR:
> +case VendorDeviceOutRequest | FTDI_SET_EVENT_CHR:
> /* TODO: handle it */
> s->event_chr = value;
> break;
> -case DeviceOutVendor | FTDI_SET_ERROR_CHR:
> +case VendorDeviceOutRequest | FTDI_SET_ERROR_CHR:
> /* TODO: handle it */
> s->error_chr = value;
> break;
> -case DeviceOutVendor | FTDI_SET_LATENCY:
> +case VendorDeviceOutRequest | FTDI_SET_LATENCY:
> s->latency = value;
> break;
> -case DeviceInVendor | FTDI_GET_LATENCY:
> +case VendorDeviceRequest | FTDI_GET_LATENCY:
> data[0] = s->latency;
> p->actual_length = 1;
> break;
> --
> 2.20.1
>
--
Samuel
How do I type "for i in *.dvi do xdvi i done" in a GUI?
(Discussion in comp.os.linux.misc on the intuitiveness of interfaces.)
Re: [PATCH 3/9] dev-serial: convert from DPRINTF to trace-events
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:55 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 28 ++--
> hw/usb/trace-events | 8
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 77ce89d38b..abc316c7bf 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -20,15 +20,8 @@
> #include "chardev/char-serial.h"
> #include "chardev/char-fe.h"
> #include "qom/object.h"
> +#include "trace.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
>
> #define RECV_BUF (512 - (2 * 8))
>
> @@ -205,8 +198,9 @@ static void usb_serial_reset(USBSerialState *s)
> static void usb_serial_handle_reset(USBDevice *dev)
> {
> USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
>
> -DPRINTF("Reset\n");
> +trace_usb_serial_reset(bus->busnr, dev->addr);
>
> usb_serial_reset(s);
> /* TODO: Reset char device, send BREAK? */
> @@ -244,9 +238,11 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
>int length, uint8_t *data)
> {
> USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
> int ret;
>
> -DPRINTF("got control %x, value %x\n", request, value);
> +trace_usb_serial_handle_control(bus->busnr, dev->addr, request, value);
> +
> ret = usb_desc_handle_control(dev, p, request, value, index, length,
> data);
> if (ret >= 0) {
> return;
> @@ -326,7 +322,8 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> s->params.parity = 'E';
> break;
> default:
> -DPRINTF("unsupported parity %d\n", value & FTDI_PARITY);
> +trace_usb_serial_unsupported_parity(bus->busnr, dev->addr,
> +value & FTDI_PARITY);
> goto fail;
> }
>
> @@ -338,7 +335,8 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> s->params.stop_bits = 2;
> break;
> default:
> -DPRINTF("unsupported stop bits %d\n", value & FTDI_STOP);
> +trace_usb_serial_unsupported_stopbits(bus->busnr, dev->addr,
> + value & FTDI_STOP);
> goto fail;
> }
>
> @@ -367,7 +365,8 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> break;
> default:
> fail:
> -DPRINTF("got unsupported/bogus control %x, value %x\n", request,
> value);
> +trace_usb_serial_unsupported_control(bus->busnr, dev->addr, request,
> + value);
> p->status = USB_RET_STALL;
> break;
> }
> @@ -431,6 +430,7 @@ static void usb_serial_token_in(USBSerialState *s,
> USBPacket *p)
> static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
> {
> USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
> uint8_t devep = p->ep->nr;
> struct iovec *iov;
> int i;
> @@ -459,7 +459,7 @@ static void usb_serial_handle_data(USBDevice *dev,
> USBPacket *p)
> break;
>
> default:
> -DPRINTF("Bad token\n");
> +trace_usb_serial_bad_token(bus->busnr, dev->addr);
> fail:
> p->status = USB_RET_STALL;
> break;
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 72e4298780..e5871cbbbc 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -320,3 +320,11 @@ usb_host_parse_interface(int bus, int addr, int num, int
> alt, int active) "dev %
> usb_host_parse_endpoint(int bus, int addr, int ep, const char *dir, const
> char *type, int active) "dev %d:%d, ep %d, %s, %s, active %d"
> usb_host_parse_error(int bus, int addr, const char *errmsg) "dev %d:%d, msg
> %s"
> usb_host_remote_wakeup_removed(int bus, int addr) "dev %d:%d"
> +
> +# dev-serial.c
> +usb_serial_reset(int bus, int addr) "dev %d:%d reset"
> +usb_serial_handle_control(int bus, int addr, int request, int valu
Re: [PATCH 2/9] dev-serial: use USB_SERIAL QOM macro for USBSerialState assignments
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:54 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 7a5fa3770e..77ce89d38b 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -204,7 +204,7 @@ static void usb_serial_reset(USBSerialState *s)
>
> static void usb_serial_handle_reset(USBDevice *dev)
> {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
>
> DPRINTF("Reset\n");
>
> @@ -243,7 +243,7 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
>int request, int value, int index,
>int length, uint8_t *data)
> {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
> int ret;
>
> DPRINTF("got control %x, value %x\n", request, value);
> @@ -430,7 +430,7 @@ static void usb_serial_token_in(USBSerialState *s,
> USBPacket *p)
>
> static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
> {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
> uint8_t devep = p->ep->nr;
> struct iovec *iov;
> int i;
> --
> 2.20.1
>
--
Samuel
t: bah c'est tendre le pattern pour se faire matcher, hein
Re: [PATCH 1/9] dev-serial: style changes to improve readability and checkpatch fixes
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:53 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: Samuel thibault
> ---
> hw/usb/dev-serial.c | 230
> 1 file changed, 126 insertions(+), 104 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index b1622b7c7f..7a5fa3770e 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -33,72 +33,75 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while
> (0)
> #define RECV_BUF (512 - (2 * 8))
>
> /* Commands */
> -#define FTDI_RESET 0
> -#define FTDI_SET_MDM_CTRL1
> -#define FTDI_SET_FLOW_CTRL 2
> -#define FTDI_SET_BAUD3
> -#define FTDI_SET_DATA4
> -#define FTDI_GET_MDM_ST 5
> -#define FTDI_SET_EVENT_CHR 6
> -#define FTDI_SET_ERROR_CHR 7
> -#define FTDI_SET_LATENCY 9
> -#define FTDI_GET_LATENCY 10
> -
> -#define DeviceOutVendor
> ((USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
> -#define DeviceInVendor ((USB_DIR_IN
> |USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
> +#define FTDI_RESET 0
> +#define FTDI_SET_MDM_CTRL 1
> +#define FTDI_SET_FLOW_CTRL 2
> +#define FTDI_SET_BAUD 3
> +#define FTDI_SET_DATA 4
> +#define FTDI_GET_MDM_ST5
> +#define FTDI_SET_EVENT_CHR 6
> +#define FTDI_SET_ERROR_CHR 7
> +#define FTDI_SET_LATENCY 9
> +#define FTDI_GET_LATENCY 10
> +
> +#define DeviceOutVendor \
> + ((USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> +#define DeviceInVendor \
> + ((USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
>
> /* RESET */
>
> -#define FTDI_RESET_SIO 0
> -#define FTDI_RESET_RX1
> -#define FTDI_RESET_TX2
> +#define FTDI_RESET_SIO 0
> +#define FTDI_RESET_RX 1
> +#define FTDI_RESET_TX 2
>
> /* SET_MDM_CTRL */
>
> -#define FTDI_DTR 1
> -#define FTDI_SET_DTR (FTDI_DTR << 8)
> -#define FTDI_RTS 2
> -#define FTDI_SET_RTS (FTDI_RTS << 8)
> +#define FTDI_DTR 1
> +#define FTDI_SET_DTR (FTDI_DTR << 8)
> +#define FTDI_RTS 2
> +#define FTDI_SET_RTS (FTDI_RTS << 8)
>
> /* SET_FLOW_CTRL */
>
> -#define FTDI_RTS_CTS_HS 1
> -#define FTDI_DTR_DSR_HS 2
> -#define FTDI_XON_XOFF_HS 4
> +#define FTDI_RTS_CTS_HS1
> +#define FTDI_DTR_DSR_HS2
> +#define FTDI_XON_XOFF_HS 4
>
> /* SET_DATA */
>
> -#define FTDI_PARITY (0x7 << 8)
> -#define FTDI_ODD (0x1 << 8)
> -#define FTDI_EVEN(0x2 << 8)
> -#define FTDI_MARK(0x3 << 8)
> -#define FTDI_SPACE (0x4 << 8)
> +#define FTDI_PARITY(0x7 << 8)
> +#define FTDI_ODD (0x1 << 8)
> +#define FTDI_EVEN (0x2 << 8)
> +#define FTDI_MARK (0x3 << 8)
> +#define FTDI_SPACE (0x4 << 8)
>
> -#define FTDI_STOP(0x3 << 11)
> -#define FTDI_STOP1 (0x0 << 11)
> -#define FTDI_STOP15 (0x1 << 11)
> -#define FTDI_STOP2 (0x2 << 11)
> +#define FTDI_STOP (0x3 << 11)
> +#define FTDI_STOP1 (0x0 << 11)
> +#define FTDI_STOP15(0x1 << 11)
> +#define FTDI_STOP2 (0x2 << 11)
>
> /* GET_MDM_ST */
> /* TODO: should be sent every 40ms */
> -#define FTDI_CTS (1<<4)// CTS line status
> -#define FTDI_DSR (1<<5)// DSR line status
> -#define FTDI_RI (1<<6)// RI line status
> -#define FTDI_RLSD (1<<7)// Receive Line Signal Detect
> +#define FTDI_CTS (1 << 4)/* CTS line status */
> +#define FTDI_DSR (1 << 5)/* DSR line status */
> +#define FTDI_RI(1 << 6)/* RI line status */
> +#define FTDI_RLSD (1 << 7)/* Receive Line Signal Detect */
>
> /* Status */
>
> -#define FTDI_DR (1<<0)// Data Ready
> -#define FTDI_OE (1<<1)// Overrun Err
> -#define FTDI_PE (1<<2)// Parity Err
> -#define FTDI_FE (1<<3)// Framing Err
> -#define FTDI_BI (1<<4)// Break Interrupt
> -#define FTDI_THRE (1<<5)// Transmitter Holding Register
> -#define FTDI_TEMT (1<<6)// Transmitter Empty
> -#define FTDI_FIFO (1<<7)// Error in FIFO
> +#define FTDI_DR(1 << 0)/* Data Ready */
> +#define FTDI_OE(1 << 1)/* Overrun Err */
> +#define FTDI_PE(1 << 2)/* Parity Err */
> +#define FTDI_FE(1 << 3)/* Framing Err */
> +#define FTDI_BI(1 << 4)/* B
[PATCH] ui: Fix default window_id value
./chardev/baum.c expects the default window_id value to be -1, and not 0
which could be confused with a proper window id (when numbered from 0 by
the ui backend).
This fixes getting Braille output with the curses and gtk frontends.
Signed-off-by: Samuel Thibault
Fixes: f29b3431f62 ("console: move window ID code from baum to sdl")
Reviewed-by: Philippe Mathieu-Daudé
---
It would be useful to backport this to stable trees.
diff --git a/ui/console.c b/ui/console.c
index f8d7643fe4..beb733c833 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1310,6 +1310,7 @@ static QemuConsole *new_console(DisplayState *ds,
console_type_t console_type,
}
s->ds = ds;
s->console_type = console_type;
+s->window_id = -1;
if (QTAILQ_EMPTY()) {
s->index = 0;
[PATCH] ui: Fix default window_id value
./chardev/baum.c expects the default window_id value to be -1, and not 0
which could be confused with a proper window id (when numbered from 0 by
the ui backend).
This fixes getting Braille output with the curses and gtk frontends.
Signed-off-by: Samuel Thibault
---
It would be useful to backport this to stable trees.
diff --git a/ui/console.c b/ui/console.c
index f8d7643fe4..beb733c833 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1310,6 +1310,7 @@ static QemuConsole *new_console(DisplayState *ds,
console_type_t console_type,
}
s->ds = ds;
s->console_type = console_type;
+s->window_id = -1;
if (QTAILQ_EMPTY()) {
s->index = 0;
Re: Is traceroute supposed to work in user mode networking (slirp) ?
Ottavio Caruso, le dim. 19 juil. 2020 12:07:21 +0100, a ecrit: > On Sun, 19 Jul 2020 at 03:50, Samuel Thibault wrote: > > Ottavio Caruso, le mar. 14 juil. 2020 12:15:48 +0100, a ecrit: > > > I cannot get traceroute to work with standard udp from any of my > > > guests. > > > > > > $ traceroute 8.8.8.8 > > > traceroute to 8.8.8.8 (8.8.8.8), 64 hops max, 40 byte packets > > > 1 * * * > > > > That was because > > > > - libslirp was not forwarding the ttl value, thus always set to 64 hops > > - libslirp was not reporting icmp errors. > > > > I had a try at both, and that indeed seems to be fixing the issue: > > https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48 > > https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/49 > > > > > Any clues? Is this intended behaviour? Any workarounds that don't > > > involve tap/tun/bridges? > > > > Not without updating libslirp with the abovementioned patches. > > Thanks Samuel. I've added a comment on the portal, but for the benefit > of qemu-devel: > > Applying this patch on the latest qemu (5.0.90), Did you also apply https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48 ? Samuel
Re: Is traceroute supposed to work in user mode networking (slirp) ?
Hello, Ottavio Caruso, le mar. 14 juil. 2020 12:15:48 +0100, a ecrit: > I cannot get traceroute to work with standard udp from any of my > guests. > > $ traceroute 8.8.8.8 > traceroute to 8.8.8.8 (8.8.8.8), 64 hops max, 40 byte packets > 1 * * * That was because - libslirp was not forwarding the ttl value, thus always set to 64 hops - libslirp was not reporting icmp errors. I had a try at both, and that indeed seems to be fixing the issue: https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48 https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/49 > Any clues? Is this intended behaviour? Any workarounds that don't > involve tap/tun/bridges? Not without updating libslirp with the abovementioned patches. Samuel
Re: [PATCH v2] slirp: update submodule to v4.2.0 + mingw-fix
Marc-André Lureau, le mar. 17 mars 2020 19:13:36 +0100, a ecrit: > git shortlog > 126c04acbabd7ad32c2b018fe10dfac2a3bc1210..7012a2c62e5b54eab88c119383022ec7ce86e9b2 > > 5eraph (1): > Use specific outbound IP address > > Akihiro Suda (8): > remove confusing comment that exists from ancient slirp > add slirp_new(SlirpConfig *, SlirpCb *, void *) > allow custom MTU > add disable_host_loopback (prohibit connections to 127.0.0.1) > add SlirpConfig version > emu: remove dead code > emu: disable by default > fix a typo in a comment > > Anders Waldenborg (1): > state: fix loading of guestfwd state > > Giuseppe Scrivano (1): > socket: avoid getpeername after shutdown(SHUT_WR) > > Jindrich Novy (1): > Don't leak memory when reallocation fails. > > Jordi Pujol Palomer (1): > fork_exec: correctly parse command lines that contain spaces > > Marc-André Lureau (54): > Merge branch 'AkihiroSuda/libslirp-slirp4netns' > Merge branch 'fix-typo' into 'master' > meson: make it subproject friendly > Merge branch 'meson' into 'master' > misc: fix compilation warnings > Merge branch 'fix-shutdown-wr' into 'master' > sbuf: remove unused and undefined sbcopy() path > sbuf: check more strictly sbcopy() bounds with offset > sbuf: replace a comment with a runtime warning > Replace remaining malloc/free user with glib > tcp_attach() can no longer fail > state: can't ENOMEM > sbuf: use unsigned types > sbuf: simplify sbreserve() > dnssearch: use g_strv_length() > vmstate: silence scan-build warning > gitlab-ci: run scan-build > Merge branch 'mem-cleanups' into 'master' > libslirp.map: bind slirp_new to SLIRP_4.1 version > meson: fix libtool versioning > Release v4.1.0 > Merge branch '4.1.0' into 'master' > CHANGELOG: start unreleased section > Merge branch 'add-unix' into 'master' > util: add G_SIZEOF_MEMBER() macro > Check bootp_filename is not going to be truncated > bootp: remove extra cast > bootp: replace simple snprintf() with strcpy() > tftp: clarify what is actually OACK m_len > tcp_emu: add more fixme/warnings comments > util: add slirp_fmt() helpers > dhcpv6: use slirp_fmt() > misc: use slirp_fmt0() > tftp: use slirp_fmt0() > tcp_ctl: use slirp_fmt() > tcp_emu: fix unsafe snprintf() usages > misc: improve error report > Use g_snprintf() > util: add gnuc format function attribute to slirp_fmt* > Merge branch 'aw-guestfwd-state' into 'master' > Merge branch 'slirp-fmt' into 'master' > socket: remove extra label and variable > socket: factor out sotranslate ipv4/ipv6 handling > socket: remove need for extra scope_id variable > socket: do not fallback on host loopback if get_dns_addr() failed > socket: do not fallback on loopback addr for addresses in our > mask/prefix > Prepare for v4.2.0 release > Merge branch 'translate-fix' into 'master' > Merge branch 'release-v4.2.0' into 'master' > changelog: post-release > changelog: fix link > .gitlab-ci: add --werror, treat CI build warnings as errors > Revert "socket: remove need for extra scope_id variable" > Merge branch 'mingw-fix' into 'master' > > PanNengyuan (1): > libslirp: fix NULL pointer dereference in tcp_sockclosed > > Philippe Mathieu-Daudé (1): > Add a git-publish configuration file > > Prasad J Pandit (4): > slirp: ncsi: compute checksum for valid data length > slirp: use correct size while emulating IRC commands > slirp: use correct size while emulating commands > slirp: tftp: restrict relative path access > > Renzo Davoli (2): > Add slirp_remove_guestfwd() > Add slirp_add_unix() > > Samuel Thibault (14): > ip_reass: explain why we should not always update the q pointer > Merge branch 'comment' into 'master' > Merge branch 'no-emu' into 'master' > Fix bogus indent, no source change > ip_reass: Fix use after free > Merge branch 'reass2' into 'master' > Make host receive broadcast packets > arp: Allow 0.0.0.0 destination address > Merge branch 'warnings' into 'master' > Merge branch 'arp_0' into 'master' > Merge branch 'broadcast' into 'master' > tcp_emu: Fix oob access > Merge branch 'oob' into 'master' > Merge branch 'master' into 'master' > > Cc: Samuel Thibault > Signe
Re: [PATCH 4/4] usb-serial: Fix timeout closing the device
Jason Andryuk, le jeu. 12 mars 2020 08:55:23 -0400, a ecrit: > Linux guests wait ~30 seconds when closing the emulated /dev/ttyUSB0. > During that time, the kernel driver is sending many control URBs > requesting GetModemStat (5). Real hardware returns a status with > FTDI_THRE (Transmitter Holding Register) and FTDI_TEMT (Transmitter > Empty) set. QEMU leaves them clear, and it seems Linux is waiting for > FTDI_TEMT to be set to indicate the tx queue is empty before closing. > > Set the bits when responding to a GetModemStat query and avoid the > shutdown delay. > > Signed-off-by: Jason Andryuk Reviewed-by: Samuel Thibault > --- > Looking at a USB dump for a real FTDI USB adapter, I see these bits set > in all the bulk URBs where QEMU currently has them clear. > --- > hw/usb/dev-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index ef33bcd127..5389235f17 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -332,7 +332,7 @@ static void usb_serial_handle_control(USBDevice *dev, > USBPacket *p, > break; > case DeviceInVendor | FTDI_GET_MDM_ST: > data[0] = usb_get_modem_lines(s) | 1; > -data[1] = 0; > +data[1] = FTDI_THRE | FTDI_TEMT; > p->actual_length = 2; > break; > case DeviceOutVendor | FTDI_SET_EVENT_CHR: > -- > 2.24.1 > -- Samuel c> ah (on trouve fluide glacial sur le net, ou il faut aller dans le monde reel ?) s> dans le monde reel c> zut
Re: [PATCH 3/4] usb-serial: Increase receive buffer to 496
Jason Andryuk, le jeu. 12 mars 2020 08:55:22 -0400, a ecrit:
> A FTDI USB adapter on an xHCI controller can send 512 byte USB packets.
> These are 8 * ( 2 bytes header + 62 bytes data). A 384 byte receive
> buffer is insufficient to fill a 512 byte packet, so bump the receive
> size to 496 ( 512 - 2 * 8 ).
>
> Signed-off-by: Jason Andryuk
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 96b6c34202..ef33bcd127 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -29,7 +29,7 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while
> (0)
> #define DPRINTF(fmt, ...) do {} while(0)
> #endif
>
> -#define RECV_BUF 384
> +#define RECV_BUF (512 - (2 * 8))
>
> /* Commands */
> #define FTDI_RESET 0
> --
> 2.24.1
>
--
Samuel
After watching my newly-retired dad spend two weeks learning how to make a new
folder, it became obvious that "intuitive" mostly means "what the writer or
speaker of intuitive likes".
(Bruce Ediger, bedi...@teal.csn.org, in comp.os.linux.misc, on X the
intuitiveness of a Mac interface.)
Re: [PATCH 2/4] usb-serial: chunk data to wMaxPacketSize
Jason Andryuk, le jeu. 12 mars 2020 08:55:21 -0400, a ecrit:
> usb-serial has issues with xHCI controllers where data is lost in the
> VM. Inspecting the URBs in the guest, EHCI starts every 64 byte boundary
> (wMaxPacketSize) with a header. EHCI hands packets into
> usb_serial_token_in() with size 64, so these cannot cross the 64 byte
> boundary. The xHCI controller has packets of 512 bytes and the usb-serial
> will just write through the 64 byte boundary. In the guest, this means
> data bytes are interpreted as header, so data bytes don't make it out
> the serial interface.
>
> Re-work usb_serial_token_in to chunk data into 64 byte units - 2 byte
> header and 62 bytes data. The Linux driver reads wMaxPacketSize to find
> the chunk size, so we match that.
>
> Real hardware was observed to pass in 512 byte URBs (496 bytes data +
> 8 * 2 byte headers). Since usb-serial only buffers 384 bytes of data,
> usb-serial will pass in 6 64 byte blocks and 1 12 byte partial block for
> 462 bytes max.
>
> Signed-off-by: Jason Andryuk
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 43 +++
> 1 file changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 71fa786bd8..96b6c34202 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -360,15 +360,16 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
>
> static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
> {
> -int first_len, len;
> +const int max_packet_size = desc_iface0.eps[0].wMaxPacketSize;
> +int packet_len;
> uint8_t header[2];
>
> -first_len = RECV_BUF - s->recv_ptr;
> -len = p->iov.size;
> -if (len <= 2) {
> +packet_len = p->iov.size;
> +if (packet_len <= 2) {
> p->status = USB_RET_NAK;
> return;
> }
> +
> header[0] = usb_get_modem_lines(s) | 1;
> /* We do not have the uart details */
> /* handle serial break */
> @@ -380,21 +381,31 @@ static void usb_serial_token_in(USBSerialState *s,
> USBPacket *p)
> } else {
> header[1] = 0;
> }
> -len -= 2;
> -if (len > s->recv_used)
> -len = s->recv_used;
> -if (!len) {
> +
> +if (!s->recv_used) {
> p->status = USB_RET_NAK;
> return;
> }
> -if (first_len > len)
> -first_len = len;
> -usb_packet_copy(p, header, 2);
> -usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> -if (len > first_len)
> -usb_packet_copy(p, s->recv_buf, len - first_len);
> -s->recv_used -= len;
> -s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +
> +while (s->recv_used && packet_len > 2) {
> +int first_len, len;
> +
> +len = MIN(packet_len, max_packet_size);
> +len -= 2;
> +if (len > s->recv_used)
> +len = s->recv_used;
> +
> +first_len = RECV_BUF - s->recv_ptr;
> +if (first_len > len)
> +first_len = len;
> +usb_packet_copy(p, header, 2);
> +usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> +if (len > first_len)
> +usb_packet_copy(p, s->recv_buf, len - first_len);
> +s->recv_used -= len;
> +s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +packet_len -= len + 2;
> +}
>
> return;
> }
> --
> 2.24.1
>
--
Samuel
We are Pentium of Borg. Division is futile. You will be approximated.
(seen in someone's .signature)
Re: [PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function
Jason Andryuk, le jeu. 12 mars 2020 08:55:20 -0400, a ecrit:
> We'll be adding a loop, so move the code into a helper function. breaks
> are replaced with returns.
>
> Signed-off-by: Jason Andryuk
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 77 +
> 1 file changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index daac75b7ae..71fa786bd8 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -358,13 +358,53 @@ static void usb_serial_handle_control(USBDevice *dev,
> USBPacket *p,
> }
> }
>
> +static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
> +{
> +int first_len, len;
> +uint8_t header[2];
> +
> +first_len = RECV_BUF - s->recv_ptr;
> +len = p->iov.size;
> +if (len <= 2) {
> +p->status = USB_RET_NAK;
> +return;
> +}
> +header[0] = usb_get_modem_lines(s) | 1;
> +/* We do not have the uart details */
> +/* handle serial break */
> +if (s->event_trigger && s->event_trigger & FTDI_BI) {
> +s->event_trigger &= ~FTDI_BI;
> +header[1] = FTDI_BI;
> +usb_packet_copy(p, header, 2);
> +return;
> +} else {
> +header[1] = 0;
> +}
> +len -= 2;
> +if (len > s->recv_used)
> +len = s->recv_used;
> +if (!len) {
> +p->status = USB_RET_NAK;
> +return;
> +}
> +if (first_len > len)
> +first_len = len;
> +usb_packet_copy(p, header, 2);
> +usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> +if (len > first_len)
> +usb_packet_copy(p, s->recv_buf, len - first_len);
> +s->recv_used -= len;
> +s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +
> +return;
> +}
> +
> static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
> {
> USBSerialState *s = (USBSerialState *)dev;
> uint8_t devep = p->ep->nr;
> struct iovec *iov;
> -uint8_t header[2];
> -int i, first_len, len;
> +int i;
>
> switch (p->pid) {
> case USB_TOKEN_OUT:
> @@ -382,38 +422,7 @@ static void usb_serial_handle_data(USBDevice *dev,
> USBPacket *p)
> case USB_TOKEN_IN:
> if (devep != 1)
> goto fail;
> -first_len = RECV_BUF - s->recv_ptr;
> -len = p->iov.size;
> -if (len <= 2) {
> -p->status = USB_RET_NAK;
> -break;
> -}
> -header[0] = usb_get_modem_lines(s) | 1;
> -/* We do not have the uart details */
> -/* handle serial break */
> -if (s->event_trigger && s->event_trigger & FTDI_BI) {
> -s->event_trigger &= ~FTDI_BI;
> -header[1] = FTDI_BI;
> -usb_packet_copy(p, header, 2);
> -break;
> -} else {
> -header[1] = 0;
> -}
> -len -= 2;
> -if (len > s->recv_used)
> -len = s->recv_used;
> -if (!len) {
> -p->status = USB_RET_NAK;
> -break;
> -}
> -if (first_len > len)
> -first_len = len;
> -usb_packet_copy(p, header, 2);
> -usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> -if (len > first_len)
> -usb_packet_copy(p, s->recv_buf, len - first_len);
> -s->recv_used -= len;
> -s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +usb_serial_token_in(s, p);
> break;
>
> default:
> --
> 2.24.1
>
--
Samuel
Tu as lu les docs. Tu es devenu un informaticien. Que tu le veuilles
ou non. Lire la doc, c'est le Premier et Unique Commandement de
l'informaticien.
-+- TP in: Guide du Linuxien pervers - "L'évangile selon St Thomas"
Re: [PATCH v2 07/30] qapi/block-core.json: Use literal block for ascii art
Hello, Philippe Mathieu-Daudé, le lun. 17 févr. 2020 01:44:35 +0100, a ecrit: > On Sat, Feb 15, 2020 at 10:01 PM Aleksandar Markovic > wrote: > > 9:56 PM Sub, 15.02.2020. Philippe Mathieu-Daudé > > је написао/ла: > > > On Fri, Feb 14, 2020 at 12:04 AM Aleksandar Markovic > > > wrote: > > > > > > > > 6:59 PM Čet, 13.02.2020. Peter Maydell је > > > > написао/ла: > > > > > > > > > > The ascii-art graph > > > > > > > > Just out of couriousity, are unicode characters allowed in rst files? > > > > > > I remember 2 years ago a blind developer thanked the QEMU community to > > > still restrict commits to 80 characters, because while 4K display are > > > available, he and other visually impaired developers cloud still > > > browse the QEMU codebase with their refreshable Braille display (which > > > was 80 cels). I don't know how many visually impaired developers are > > > following this project. A quick google returns " There is no concept > > > of Unicode in Braille. In that sense Braille is similar to old 8-bit > > > code pages which represented different symbols in different languages > > > for the same symbol code." > > > (https://superuser.com/questions/629443/represent-unicode-characters-in-braille). > > > > > > (I'm Cc'ing Samuel who cares about Braille displays.) Nowadays' screen reader do provide some translations for some unicode fancies. But the analogy with codepage remains true: since there are only 256 braille pattern, there will be ambiguity between representation of plain text and representation of unicode fancies. Using plain ascii avoids this issue, i.e. blind developers will know that '|' and '-' are commonly used for drawing, and their Braille representations are not ambiguous. So that's indeed better to keep them ascii. More generally, ascii art is however hard to catch with a Braille device anyway... Samuel
[Bug 1812451] Re: In windows host, tftp arbitrary file read vulnerability
This is fixed upstream by
https://gitlab.freedesktop.org/slirp/libslirp/commit/14ec36e107a8c9af7d0a80c3571fe39b291ff1d4
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1812451
Title:
In windows host, tftp arbitrary file read vulnerability
Status in QEMU:
Fix Committed
Bug description:
https://github.com/qemu/qemu/blob/master/slirp/tftp.c#L343
if (!strncmp(req_fname, "../", 3) ||
req_fname[strlen(req_fname) - 1] == '/' ||
strstr(req_fname, "/../")) {
tftp_send_error(spt, 2, "Access violation", tp);
return;
}
There is file path check for not allowing escape tftp directory.
But, in windows, file path is separated by "\" backslash.
So, guest can read arbitrary file in Windows host.
This bug is variant of CVE-2019-2553 - Directory traversal
vulnerability.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1812451/+subscriptions
Re: [PATCH] build-sys: do not include Windows SLIRP dependencies in $LIBS
Paolo Bonzini, le mer. 11 déc. 2019 15:23:23 +0100, a ecrit: > When including the internal SLIRP library, we should add all the libraries > that > it needs for the build. Right now they are all included by QEMU, but > -liphlpapi > is not needed without slirp. Move it from LIBS to slirp_libs. > > Based on a patch by Marc-André Lureau. > > Signed-off-by: Paolo Bonzini Acked-by: Samuel Thibault (I don't have a win environment to test this) > --- > configure | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index 6099be1..d16dad2 100755 > --- a/configure > +++ b/configure > @@ -926,7 +926,7 @@ if test "$mingw32" = "yes" ; then >DSOSUF=".dll" ># MinGW needs -mthreads for TLS and macro _MT. >QEMU_CFLAGS="-mthreads $QEMU_CFLAGS" > - LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS" > + LIBS="-lwinmm -lws2_32 $LIBS" >write_c_skeleton; >if compile_prog "" "-liberty" ; then > LIBS="-liberty $LIBS" > @@ -6069,6 +6069,9 @@ case "$slirp" in > mkdir -p slirp > slirp_cflags="-I\$(SRC_PATH)/slirp/src -I\$(BUILD_DIR)/slirp/src" > slirp_libs="-L\$(BUILD_DIR)/slirp -lslirp" > +if test "$mingw32" = "yes" ; then > + slirp_libs="$slirp_libs -lws2_32 -liphlpapi" > +fi > ;; > >system) > -- > 1.8.3.1 > > -- Samuel > Quelqu'un aurait-il une solution pour réinitialiser un MBR Si tu veux qu'il soit complètement blanc (pas souhaitable, à mon avis) : dd if=/dev/zero of=/dev/hda bs=512 count=1 (sous Linux) -+- OT in Guide du linuxien (très) pervers - "Pour les K difficiles" -+-
Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect
Samuel Thibault, le jeu. 14 nov. 2019 14:27:12 +0100, a ecrit: > Samuel Thibault, le jeu. 14 nov. 2019 14:08:41 +0100, a ecrit: > > The way to properly fix it is to add a brlapi channel to spice: > > And that would be workable through a spice agent as well, so that > braille management from orca Or event NVDA or Narrator, actually. > running inside the guest could talk > directly through to brltty running on the host instead of having to go > through the emulation layers. > > Samuel
Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect
Samuel Thibault, le jeu. 14 nov. 2019 14:08:41 +0100, a ecrit: > The way to properly fix it is to add a brlapi channel to spice: And that would be workable through a spice agent as well, so that braille management from orca running inside the guest could talk directly through to brltty running on the host instead of having to go through the emulation layers. Samuel
Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect
Hello, Teemu Kuusisto, le jeu. 14 nov. 2019 14:09:15 +0200, a ecrit: > As a blind developer I would be very happy to use QEMU's baum chardev for a > braille display. Unfortunately, this device fails to detect the tty in which > the spice client is running. Ah indeed that case was never looked at. > The current code calls qemu_console_get_window_id() to get the tty. > The code does not currently consider the fact that the lifetime of the > connected graphical consoles is not the same as the lifetime of the > VM. Indeed, with spice the situation is different. > This function returns zero, which causes the code to skip even the > default behavior of brlapi's brlapi__enterTtyMode() (including > checcking some env variables such as CONTROLVT) Mmm, indeed that should be fixed into letting brlapi try to find it, so that CONTROLVT can work. > window id sounds like something different than a tty number, maybe a > number of X display? Yes, under X you need to provide the X window id. > Is it possible to get callbacks for connect and disconnect of a > graphical console (like spice and vnc)? How? [...] > Such events should lead to calls of brlapi__EnterTtyMode() and > brlapi__leaveTtyMode(). That would seem to be the way to go indeed, but see below. > Is it further possible to get any information of the connected > client to determine the tty, and possibly sub-windows too (see > brlapi__enterTtyModeWithPath), in which the client is running? More precisely you would need to know the X window ID on the front-end side. The VNC and spice protocols don't currently provide this. Also when the VM and the frontend are running on different machines this would not make any sense anyway so I don't think it will get added to spice & vnc anyway. One ugly way to get it to work is to run the spice client and keep it open, get its X window id through xwininfo or equivalent, and only then start qemu with CONTROLVT set to that id. But of course you can't close the client and reopen it later. The way to properly fix it is to add a brlapi channel to spice: in addition to main, display, inputs, cursor, playback, and record channels, we would have a brlapi channel. The brlapi protocol is already packet-based, so that would work nicely. The server in qemu would list the brlapi channel in addition to others, and brlapi packets would flow over. The spice client would just forward brlapi packets over without modification, except for the enterttymode packet, in which it would just prepend its own windowpath and window id. The forwarding implementation could be implemented in libbrlapi so that spice clients don't have to maintain the support. Similarly, we could modify libbrlapi to not necessarily connect to the usual brlapi socket, but let the application provide send/recv functions to exchange the packets. Samuel
Re: [PATCH] MAINTAINERS: slirp: Remove myself as maintainer
Thomas Huth, le lun. 11 nov. 2019 14:15:36 +0100, a ecrit: > On 11/11/2019 08.57, Jan Kiszka wrote: > > May I point out that this one was never merged? > > > > Sorry, I really can't help in this area anymore. > > I'm planning to send a "qtest + misc" PULL request tomorrow ... I can > add the patch there if Samuel does not have anything else pending > related to SLIRP right now... I don't have anything, please add it. Sorry I didn't realize that I should do more than just ack-ing the change. Samuel
Re: [PATCH v4] smb daemon get additional command line parameters from env variable
Jordi Pujol, le sam. 02 nov. 2019 08:41:52 +0100, a ecrit:
> @@ -909,6 +910,12 @@ static int slirp_smb(SlirpState* s, cons
> CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
> g_free(smb_conf);
>
> +options = g_getenv("SMBDOPTIONS");
> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +g_free(options);
> +}
Again, what g_getenv mustn't be freed. I believe you even get a warning
about it: g_getenv returns a const gchar *.
The old value of smb_cmdline, however, has to be freed, otherwise that's
a memory leak.
Samuel
Re: [PATCH v3] smb daemon get additional command line parameters from env variable
Jordi Pujol, le ven. 01 nov. 2019 15:38:19 +0100, a ecrit:
> +options = g_getenv("SMBDOPTIONS");
> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +}
> +g_free(options);
But then do not free it :)
Samuel
Re: [PATCH v2] smb daemon get additional command line parameters from env variable
Hello,
Jordi Pujol, le jeu. 31 oct. 2019 14:33:00 +0100, a ecrit:
> The smbd daemon takes additional command line options
> from environment variable SMBDOPTIONS.
> Set the environment variable SMBDOPTIONS before executing qemu.
>
> Example:
>
> export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"
>
> Signed-off-by: Jordi Pujol Palomer
> ---
> --- qemu-4.1-a/net/slirp.c
> +++ qemu_4.1-b/net/slirp.c
> @@ -909,6 +909,12 @@ static int slirp_smb(SlirpState* s, cons
> CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
> g_free(smb_conf);
>
> +char *options = g_strdup(g_getenv("SMBDOPTIONS"));
Why strduping it? you can just use g_getenv.
> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +}
> +g_free(options);
> +
> if (slirp_add_exec(s->slirp, smb_cmdline, _addr, 139) < 0 ||
> slirp_add_exec(s->slirp, smb_cmdline, _addr, 445) < 0) {
> slirp_smb_cleanup(s);
>
> --- qemu-4.1-a/slirp/src/misc.c 2019-10-29 14:40:15.043120941 +0100
> +++ qemu-4.1-b/slirp/src/misc.c 2019-10-29 14:41:04.440235684 +0100
Please submit this part to https://gitlab.freedesktop.org/slirp/libslirp/
Make sure to note in the changelog that g_shell_parse_argv does only
tokenization, and no replacement.
Samuel
> @@ -168,7 +168,9 @@ g_spawn_async_with_fds_slirp(const gchar
> int fork_exec(struct socket *so, const char *ex)
> {
> GError *err = NULL;
> -char **argv;
> +gint argc = 0;
> +gchar **argv = NULL;
> +gboolean ret;
> int opt, sp[2];
>
> DEBUG_CALL("fork_exec");
> @@ -179,7 +181,13 @@ int fork_exec(struct socket *so, const c
> return 0;
> }
>
> -argv = g_strsplit(ex, " ", -1);
> +ret = g_shell_parse_argv(ex, , , );
> +if (err) {
> +g_critical("fork_exec invalid command: %s", err->message);
> +g_error_free(err);
> +return 0;
> +}
> +
> g_spawn_async_with_fds(NULL /* cwd */, argv, NULL /* env */,
> G_SPAWN_SEARCH_PATH, fork_exec_child_setup,
> NULL /* data */, NULL /* child_pid */, sp[1],
> sp[1],
> **
>
> Thanks,
>
> Jordi Pujol
>
Re: Python 2 and test/vm/netbsd
Eduardo Habkost, le ven. 18 oct. 2019 13:41:43 -0300, a ecrit: > On Fri, Oct 18, 2019 at 06:00:19PM +0200, Samuel Thibault wrote: > > It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps > > NetBSD has a slightly different behavior which makes the implementation > > fail to notice the error. > > If anybody is interested in investigating it, a network traffic > dump generated by `-object filter-dump` is attached. The dump does show the Destination Unreachable icmp message, so it seems it's the guest which does not notice it for some reason. Samuel
Re: Python 2 and test/vm/netbsd
Samuel Thibault, le ven. 18 oct. 2019 18:00:19 +0200, a ecrit: > Philippe Mathieu-Daudé, le ven. 18 oct. 2019 16:58:00 +0200, a ecrit: > > On 10/18/19 4:29 PM, Eduardo Habkost wrote: > > > In addition to that, the connect() error should be generating a > > > ICMP6_UNREACH message, and I'd expect the NetBSD guest to notice > > > it instead of waiting for timeout. > > > > Is this missing in SLiRP? > > It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps > NetBSD has a slightly different behavior which makes the implementation > fail to notice the error. It definitely is there in tcp_input(): on tcp_fconnect() error an ICMP6_UNREACH message is sent. I can confirm that this works with a Linux host and Linux guest. Samuel
Re: Python 2 and test/vm/netbsd
Philippe Mathieu-Daudé, le ven. 18 oct. 2019 16:58:00 +0200, a ecrit: > On 10/18/19 4:29 PM, Eduardo Habkost wrote: > > On Fri, Oct 18, 2019 at 12:44:39PM +0200, Gerd Hoffmann wrote: > > >Hi, > > > > > > > > Running with V=1, I see packages being downloaded at reasonable > > > > > speeds, but > > > > > there's a huge interval (of various minutes) between each package > > > > > download. > > > > > > > > I've found the cause for the slowness I'm seeing: for each file > > > > being downloaded, the guest spents at least 75 seconds trying to > > > > connect to the IPv6 address of ftp.NetBSD.org, before trying > > > > IPv4. > > > > > > Ah, that nicely explains why it worked just fine for me. First, I have > > > a local proxy configured so the installer isn't going to connect to > > > ftp.NetBSD.org directly. Second I have IPv6 connectivity. > > > > > > > I don't know if this is a NetBSD bug, or a slirp bug. > > > > > > Both I'd say ... > > > > > > First, by default slirp should not send IPv6 router announcements > > > to the user network if the host has no IPv6 connectivity. > > > > > > Second, the recommended way to connect is to try ipv4 and ipv6 in > > > parallel, then use whatever connects first. Web browsers typically > > > do it that way. wget and curl don't do that though, they try one > > > address after the other, and I guess this is where the delay comes > > > from ... > > > > In addition to that, the connect() error should be generating a > > ICMP6_UNREACH message, and I'd expect the NetBSD guest to notice > > it instead of waiting for timeout. > > Is this missing in SLiRP? It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps NetBSD has a slightly different behavior which makes the implementation fail to notice the error. Samuel
Re: [PATCH v2 2/2] curses: correctly pass the color pair to setcchar()
Matthew Kilgore, le jeu. 03 oct. 2019 23:53:38 -0400, a ecrit:
> The current code does not correctly pass the color pair information to
> setcchar(), it instead always passes zero. This results in the curses
> output always being in white on black.
>
> This patch fixes this by using PAIR_NUMBER() to retrieve the color pair
> number from the chtype value, and then passes that value as an argument
> to setcchar().
>
> Signed-off-by: Matthew Kilgore
Reviewed-by: Samuel Thibault
Tested-by: Samuel Thibault
Thanks!
> ---
> ui/curses.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ui/curses.c b/ui/curses.c
> index 84003f56a323..3a1b71451c93 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -77,12 +77,14 @@ static void curses_update(DisplayChangeListener *dcl,
> for (x = 0; x < width; x++) {
> chtype ch = line[x] & A_CHARTEXT;
> chtype at = line[x] & A_ATTRIBUTES;
> +short color_pair = PAIR_NUMBER(line[x]);
> +
> ret = getcchar(_to_curses[ch], wch, , , NULL);
> if (ret == ERR || wch[0] == 0) {
> wch[0] = ch;
> wch[1] = 0;
> }
> -setcchar(_line[x], wch, at, 0, NULL);
> +setcchar(_line[x], wch, at, color_pair, NULL);
> }
> mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
> }
> --
> 2.23.0
>
--
Samuel
bash: ls: Computer bought the farm
THAT frightens ppl! :P
id rather see: "bash: ls: Initialization of googol(AWAX)
disengaged in HYPER32/64 mode due to faulty page request at
AX:12A34F84B"
at least that would give me the feeling that the
*programmers* knows what is going on :P
(lovely Hurd...)
Re: [PATCH v2 1/2] curses: use the bit mask constants provided by curses
Matthew Kilgore, le jeu. 03 oct. 2019 23:53:37 -0400, a ecrit:
> The curses API provides the A_ATTRIBUTES and A_CHARTEXT bit masks for
> getting the attributes and character parts of a chtype, respectively. We
> should use provided constants instead of using 0xff.
>
> Signed-off-by: Matthew Kilgore
Reviewed-by: Samuel Thibault
Tested-by: Samuel Thibault
> ---
> ui/curses.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/curses.c b/ui/curses.c
> index ec281125acbd..84003f56a323 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -75,8 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
> line = screen + y * width;
> for (h += y; y < h; y ++, line += width) {
> for (x = 0; x < width; x++) {
> -chtype ch = line[x] & 0xff;
> -chtype at = line[x] & ~0xff;
> +chtype ch = line[x] & A_CHARTEXT;
> +chtype at = line[x] & A_ATTRIBUTES;
> ret = getcchar(_to_curses[ch], wch, , , NULL);
> if (ret == ERR || wch[0] == 0) {
> wch[0] = ch;
> --
> 2.23.0
>
--
Samuel
The nice thing about Windows is - It does not just crash, it displays a
dialog box and lets you press 'OK' first.
(Arno Schaefer's .sig)
[PULL 0/1] slirp update
The following changes since commit 860d9048c78ce59c5903c3d5209df56f38400986: Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190924-pull-request' into staging (2019-09-24 13:51:51 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 120b721f5b590393971673a315f02969ec89cccb: slirp: Allow non-local DNS address when restrict is off (2019-10-01 19:03:08 +0200) slirp: Allow non-local DNS address when restrict is off Samuel Thibault (1): slirp: Allow non-local DNS address when restrict is off net/slirp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
[PULL 1/1] slirp: Allow non-local DNS address when restrict is off
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.
Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
---
net/slirp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
error_setg(errp, "DNS doesn't belong to network");
return -1;
}
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse IPv6 DNS");
return -1;
}
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len))
{
error_setg(errp, "IPv6 DNS doesn't belong to network");
return -1;
}
--
2.23.0
[PATCHv2] slirp: Allow non-local DNS address when restrict is off
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.
Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault
---
Difference from first version:
- handle DNS IPv6 as well
- reference bug with Buglink
net/slirp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
error_setg(errp, "DNS doesn't belong to network");
return -1;
}
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse IPv6 DNS");
return -1;
}
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len))
{
error_setg(errp, "IPv6 DNS doesn't belong to network");
return -1;
}
--
2.23.0
[PATCH] slirp: Allow non-local DNS address when restrict is off
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.
Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault
---
net/slirp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
error_setg(errp, "DNS doesn't belong to network");
return -1;
}
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse IPv6 DNS");
return -1;
}
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len))
{
error_setg(errp, "IPv6 DNS doesn't belong to network");
return -1;
}
--
2.23.0
[PATCH] slirp: Allow non-local DNS address when restrict is off
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.
This fixes LP 1010484.
Signed-off-by: Samuel Thibault
---
net/slirp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..4d158b0542 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
error_setg(errp, "DNS doesn't belong to network");
return -1;
}
--
2.23.0
Re: [Qemu-devel] slirp, incoming packets get truncated
Hello, Philippe Mathieu-Daudé, le lun. 09 sept. 2019 12:51:23 +0200, a ecrit: > Anyhow, if you plan to properly (with your S-o-b) commit your patch to > the libslirp repository, Actually the libslirp repository already has something there. Since IPv4/6 have a maximum of 64KB packets, it just allocates that size on the stack, whîch is simpler in the end. Also it makes mru/mtu configurable, we will be able to plug that into qemu once released. Samuel
