On Fri, Feb 17, 2012 at 6:24 PM, Stefan Hajnoczi
<stefa...@linux.vnet.ibm.com> wrote:
> On Fri, Feb 17, 2012 at 12:20:08PM +0800, zwu.ker...@gmail.com wrote:
>> From: Zhi Yong Wu <wu...@linux.vnet.ibm.com>
>>
>> As you have known, QEMU upstream currently doesn't support for -netdev
>> socket,listen; This patch makes it work now.
>
> This commit description does not give any context. Please explain what
> the bug is so readers know what this patch fixes.
>
> I tried the following test:
>
> $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 \
> -drive if=virtio,file=vm1.img,cache=none \
> -netdev socket,listen=127.0.0.1:1234,id=socket0 \
> -device virtio-net-pci,netdev=socket0
>
> $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 \
> -drive if=virtio,file=vm2.img,cache=none \
> -netdev socket,connect=127.0.0.1:1234,id=socket0 \
> -device virtio-net-pci,netdev=socket0
>
> The first thing I noticed was that the output of "info network" in vm1
> looks wrong. It should show the virtio-net-pci NIC peered with a socket
> fd connection. Instead it shows it peered with a dummy VLANClientState
> and I see two socket fds with no peers.
By the way, Can you see socket file descriptioner? Where and How did
you see them?
>
> Network connectivity between the two QEMUs does not work. I assigned
> static IPs in both VMs, ran tcpdump in vm1, and then tried to ping vm1
> from vm2.
>
>> Signed-off-by: Zhi Yong Wu <wu...@linux.vnet.ibm.com>
>> ---
>> net/socket.c | 17 +++++++++++++++++
>> 1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/socket.c b/net/socket.c
>> index d4c2002..f82e69d 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -43,6 +43,7 @@ typedef struct NetSocketState {
>> } NetSocketState;
>>
>> typedef struct NetSocketListenState {
>> + VLANClientState *nc;
>> VLANState *vlan;
>> char *model;
>> char *name;
>> @@ -389,6 +390,11 @@ static void net_socket_accept(void *opaque)
>> break;
>> }
>> }
>> +
>> + if (s->nc) {
>> + qemu_del_vlan_client(s->nc);
>> + }
>> +
>> s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1);
>
> This has a few issues:
>
> net_socket_fd_init() does not set the peer, only vlan. This means the
> connected socket and NIC are not peered up.
>
> qemu_del_vlan_client() brings the link down...we never bring it back up.
>
> We need to avoid leaking s->nc because it is not freed in
> qemu_del_vlan_client(). Once peering is fixed s->nc will not be freed
> during NIC deletion anymore!
>
> It leaves a dangling pointer to s->nc in the qdev netdev property and
> NICInfo nd_table[]. Not sure if this is a problem but it's messy.
>
> I suggest using the real NetSocketState instead - do not create a dummy
> VLANClientState. This means we create the socket fd NetSocketState
> right away and never need to update the peer. Simply bring the link up
> once the socket file descriptor is connected.
>
> Stefan
>
--
Regards,
Zhi Yong Wu
