diff mbox

[RFC,0/3] tests/pxe-testt: add testcase using vhost-user-bridge

Message ID 20170712211305.ajtizorj4yq6jble@localhost.localdomain
State New
Headers show

Commit Message

Jens Freimann July 12, 2017, 9:13 p.m. UTC 
On Wed, Jul 12, 2017 at 06:10:05PM +0300, Michael S. Tsirkin wrote:
>On Wed, Jul 12, 2017 at 11:41:46AM +0200, Jens Freimann wrote:
>> This implements a testcase for pxe-test using the vhost-user interface. Spawn a
>> vhost-user-bridge process and connect it to the qemu process.
>>
>> It is send as an RFC because:
>>  - Patch 3/3: there must be cleaner way to do this.
>>  - Does Patch 1/3 make sense or should I just redirect all output to /dev/null?
>>  - don't hardcode port numbers in qemu cmdline, create socket and pass
>>     fd to -netdev (need to figure out how to do this)
>
>Doesn't this work?
>
>-netdev socket,id=str[,fd=h]

It should. But I get this:

        dded sock 5 for watching. max_sock: 5
        Sock 3 removed from dispatcher watch.
        qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not contain a multicast address
        qemu-system-x86_64: -netdev socket,id=n1,fd=3: Device 'socket' could not be initialized
        Broken pipe
 
 
     tmpfs = mkdtemp(template);
     if (!tmpfs) {
@@ -109,9 +135,10 @@ static void test_pxe_vhost_user(void)
         root = tmpfs2;
     }
 
+    sock = vubr_create_socket(&si_remote, RPORT);
+
     qemu_args = g_strdup_printf(QEMU_CMD, MEMSZ, MEMSZ, (root),
-                                "char0", vubr_args[2], "char0", disk,
-                                RPORT, LPORT);
+            "char0", vubr_args[2], "char0", disk, sock);



regards,
Jens

Comments

Michael S. Tsirkin July 12, 2017, 10:33 p.m. UTC | #1 
On Wed, Jul 12, 2017 at 11:13:05PM +0200, Jens Freimann wrote:
> On Wed, Jul 12, 2017 at 06:10:05PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Jul 12, 2017 at 11:41:46AM +0200, Jens Freimann wrote:
> > > This implements a testcase for pxe-test using the vhost-user interface. Spawn a
> > > vhost-user-bridge process and connect it to the qemu process.
> > > 
> > > It is send as an RFC because:
> > >  - Patch 3/3: there must be cleaner way to do this.
> > >  - Does Patch 1/3 make sense or should I just redirect all output to /dev/null?
> > >  - don't hardcode port numbers in qemu cmdline, create socket and pass
> > >     fd to -netdev (need to figure out how to do this)
> > 
> > Doesn't this work?
> > 
> > -netdev socket,id=str[,fd=h]
> 
> It should. But I get this:
> 
>        dded sock 5 for watching. max_sock: 5
>        Sock 3 removed from dispatcher watch.
>        qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not contain a multicast address
>        qemu-system-x86_64: -netdev socket,id=n1,fd=3: Device 'socket' could not be initialized
>        Broken pipe

Not sure what's wrong with it, should not be too hard to debug.

Or maybe we should add unix domain socket support.
Sounds like a reasonable feature to me.

> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index 5a0d182..a14c1d9 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -29,7 +29,7 @@
> #define QEMU_CMD_NETDEV " -device virtio-net-pci,netdev=net0 "\
>                         " -netdev vhost-user,id=net0,chardev=%s,vhostforce "\
>                         " -netdev user,id=n0,tftp=./,bootfile=%s "\
> -                        " -netdev socket,id=n1,udp=localhost:%d,localaddr=localhost:%d"
> +                        " -netdev socket,id=n1,fd=%d"
> #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=n0 "\
>                         " -device virtio-net-pci,netdev=n1 "
> 
> @@ -72,16 +72,42 @@ static const char *init_hugepagefs(const char *path)
>     return path;
> }
> 
> +static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
> +{
> +    int sock =  -1;
> +
> +    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr) == 0) {
> +        g_test_message("inet_aton failed\n");
> +        return -1;
> +    }
> +
> +    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +    if (sock == -1) {
> +        g_test_message("socket creation failed\n");
> +    }
> +    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
> +        printf("connect, %d", errno);
> +        return -1;
> +    }
> +
> +    return sock;
> +}
> +
> static void test_pxe_vhost_user(void)
> {
>     char template[] = "/tmp/vhost-user-bridge-XXXXXX";
>     char template2[] = "/tmp/hugepages-XXXXXX";
>     gchar *vubr_args[] = {NULL, NULL, NULL, NULL};
> +    struct sockaddr_in si_remote = {
> +        .sin_family = AF_INET,
> +        .sin_port = htons(RPORT),
> +    };
>     const char *hugefs;
>     GError *error = NULL;
>     char *vubr_binary;
>     char *qemu_args;
>     GPid vubr_pid;
> +    int sock = -1;
> 
>     tmpfs = mkdtemp(template);
>     if (!tmpfs) {
> 
>     tmpfs = mkdtemp(template);
>     if (!tmpfs) {
> @@ -109,9 +135,10 @@ static void test_pxe_vhost_user(void)
>         root = tmpfs2;
>     }
> 
> +    sock = vubr_create_socket(&si_remote, RPORT);
> +
>     qemu_args = g_strdup_printf(QEMU_CMD, MEMSZ, MEMSZ, (root),
> -                                "char0", vubr_args[2], "char0", disk,
> -                                RPORT, LPORT);
> +            "char0", vubr_args[2], "char0", disk, sock);
> 
> 
> 
> regards,
> Jens
Stefan Hajnoczi July 19, 2017, 8:44 a.m. UTC | #2 
On Thu, Jul 13, 2017 at 01:33:43AM +0300, Michael S. Tsirkin wrote:
> On Wed, Jul 12, 2017 at 11:13:05PM +0200, Jens Freimann wrote:
> > On Wed, Jul 12, 2017 at 06:10:05PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Jul 12, 2017 at 11:41:46AM +0200, Jens Freimann wrote:
> > > > This implements a testcase for pxe-test using the vhost-user interface. Spawn a
> > > > vhost-user-bridge process and connect it to the qemu process.
> > > > 
> > > > It is send as an RFC because:
> > > >  - Patch 3/3: there must be cleaner way to do this.
> > > >  - Does Patch 1/3 make sense or should I just redirect all output to /dev/null?
> > > >  - don't hardcode port numbers in qemu cmdline, create socket and pass
> > > >     fd to -netdev (need to figure out how to do this)
> > > 
> > > Doesn't this work?
> > > 
> > > -netdev socket,id=str[,fd=h]
> > 
> > It should. But I get this:
> > 
> >        dded sock 5 for watching. max_sock: 5
> >        Sock 3 removed from dispatcher watch.
> >        qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not contain a multicast address
> >        qemu-system-x86_64: -netdev socket,id=n1,fd=3: Device 'socket' could not be initialized
> >        Broken pipe
> 
> Not sure what's wrong with it, should not be too hard to debug.

Jens asked me this question on IRC.  I didn't know the answer either but
git-log(1) shows what happened:

-net socket,fd= was broken for SOCK_DGRAM from the moment mcast was
introduced by commit 3d830459b1eccdb61b75e2712fd364012ce5a115 ("'-net
socket,mcast=' option support (initial patch by Juan Jose Ciarlante)").

The issue is probably that the fd init code doesn't know whether the
user wanted mcast or not.  Right now it's hardcoded to assume that mcast
is desired.  It may be necessary to extend the command-line syntax to
specify mcast explicitly with fd=.

Stefan
Jens Freimann July 19, 2017, 3:51 p.m. UTC | #3 
On Wed, Jul 19, 2017 at 09:44:02AM +0100, Stefan Hajnoczi wrote:
>On Thu, Jul 13, 2017 at 01:33:43AM +0300, Michael S. Tsirkin wrote:
>> On Wed, Jul 12, 2017 at 11:13:05PM +0200, Jens Freimann wrote:
>> > On Wed, Jul 12, 2017 at 06:10:05PM +0300, Michael S. Tsirkin wrote:
>> > > On Wed, Jul 12, 2017 at 11:41:46AM +0200, Jens Freimann wrote:
>> > > > This implements a testcase for pxe-test using the vhost-user interface. Spawn a
>> > > > vhost-user-bridge process and connect it to the qemu process.
>> > > >
>> > > > It is send as an RFC because:
>> > > >  - Patch 3/3: there must be cleaner way to do this.
>> > > >  - Does Patch 1/3 make sense or should I just redirect all output to /dev/null?
>> > > >  - don't hardcode port numbers in qemu cmdline, create socket and pass
>> > > >     fd to -netdev (need to figure out how to do this)
>> > >
>> > > Doesn't this work?
>> > >
>> > > -netdev socket,id=str[,fd=h]
>> >
>> > It should. But I get this:
>> >
>> >        dded sock 5 for watching. max_sock: 5
>> >        Sock 3 removed from dispatcher watch.
>> >        qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not contain a multicast address
>> >        qemu-system-x86_64: -netdev socket,id=n1,fd=3: Device 'socket' could not be initialized
>> >        Broken pipe
>>
>> Not sure what's wrong with it, should not be too hard to debug.
>
>Jens asked me this question on IRC.  I didn't know the answer either but
>git-log(1) shows what happened:
>
>-net socket,fd= was broken for SOCK_DGRAM from the moment mcast was
>introduced by commit 3d830459b1eccdb61b75e2712fd364012ce5a115 ("'-net
>socket,mcast=' option support (initial patch by Juan Jose Ciarlante)").
>
>The issue is probably that the fd init code doesn't know whether the
>user wanted mcast or not.  Right now it's hardcoded to assume that mcast
>is desired.  It may be necessary to extend the command-line syntax to
>specify mcast explicitly with fd=.

Thanks for confirming this Stefan! I'll try to come up with an extension to the
command-line syntax and send a proposal to the list.

regards,
Jens
diff mbox

Patch

diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 5a0d182..a14c1d9 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -29,7 +29,7 @@ 
 #define QEMU_CMD_NETDEV " -device virtio-net-pci,netdev=net0 "\
                         " -netdev vhost-user,id=net0,chardev=%s,vhostforce "\
                         " -netdev user,id=n0,tftp=./,bootfile=%s "\
-                        " -netdev socket,id=n1,udp=localhost:%d,localaddr=localhost:%d"
+                        " -netdev socket,id=n1,fd=%d"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=n0 "\
                         " -device virtio-net-pci,netdev=n1 "
 
@@ -72,16 +72,42 @@  static const char *init_hugepagefs(const char *path)
     return path;
 }
 
+static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
+{
+    int sock =  -1;
+
+    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr) == 0) {
+        g_test_message("inet_aton failed\n");
+        return -1;
+    }
+
+    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    if (sock == -1) {
+        g_test_message("socket creation failed\n");
+    }
+    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
+        printf("connect, %d", errno);
+        return -1;
+    }
+
+    return sock;
+}
+
 static void test_pxe_vhost_user(void)
 {
     char template[] = "/tmp/vhost-user-bridge-XXXXXX";
     char template2[] = "/tmp/hugepages-XXXXXX";
     gchar *vubr_args[] = {NULL, NULL, NULL, NULL};
+    struct sockaddr_in si_remote = {
+        .sin_family = AF_INET,
+        .sin_port = htons(RPORT),
+    };
     const char *hugefs;
     GError *error = NULL;
     char *vubr_binary;
     char *qemu_args;
     GPid vubr_pid;
+    int sock = -1;
 
     tmpfs = mkdtemp(template);
     if (!tmpfs) {